Remove brainTransplant (#463)

* split variable adder and constraint adder passes

* populate typedef map during variable adder phase

* avoid numparam crash

* Add ReasonFailed to brainTransplant; Refactor insertNewFVConstraint

* another assert

* add tests proto vs body

* formatting, clarity

* remove xfail from tests, add grep

* wording: 'new'->'seen'

* more mergefailure reasons

* remove calls to braintransplant (regression fail: 80)

* split add variable phase at higher level

* only save merged FVC (regression fail: 24)

* make special case match description

* remove code for braintransplant

* restore the safety of PragramVariableAdder

* first pass comments; assert for backwards merge

* second comment pass - usage and defn

* add test for #427, now solved

Author: kyleheadley

clang/include/clang/3C/3C.h

@@ -106,6 +106,9 @@ class _3CInterface {
 
   // Constraint Building.
 
+  // Create ConstraintVariables to hold constraints
+  bool addVariables();
+
   // Build initial constraints.
   bool buildInitialConstraints();
 

clang/include/clang/3C/AVarBoundsInfo.h

@@ -214,10 +214,6 @@ class AVarBoundsInfo {
   // Get the ProgramVar for the provided VarKey.
   ProgramVar *getProgramVar(BoundsKey VK);
 
-  // Function that does brain transplant of the provided bounds key (NewBK)
-  // with existing bounds key (OldBK).
-  void brainTransplant(BoundsKey NewBK, BoundsKey OldBK);
-
   // Propagate the array bounds information for all array ptrs.
   bool performFlowAnalysis(ProgramInfo *PI);
 

clang/include/clang/3C/ConstraintBuilder.h

@@ -15,6 +15,22 @@
 #include "clang/3C/TypeVariableAnalysis.h"
 #include "clang/AST/ASTConsumer.h"
 
+// First step in generating initial constraints is to collect functions
+// and variables the need to be analysed. This will also merge
+// function definitions together.
+class VariableAdderConsumer : public clang::ASTConsumer {
+public:
+  explicit VariableAdderConsumer(ProgramInfo &I, clang::ASTContext *C)
+      : Info(I) {}
+
+  virtual void HandleTranslationUnit(clang::ASTContext &);
+
+private:
+  ProgramInfo &Info;
+};
+
+// Final step in generating initial constraints is to scan type variables and
+// function bodies for relationships that generate the constraints.
 class ConstraintBuilderConsumer : public clang::ASTConsumer {
 public:
   explicit ConstraintBuilderConsumer(ProgramInfo &I, clang::ASTContext *C)

clang/include/clang/3C/ConstraintVariables.h

@@ -136,8 +136,9 @@ class ConstraintVariable {
   // Force use of equality constraints in function calls for this CV
   virtual void equateArgumentConstraints(ProgramInfo &I) = 0;
 
-  // Update this CV with information from duplicate declaration CVs
-  virtual void brainTransplant(ConstraintVariable *, ProgramInfo &) = 0;
+  // Internally combine the constraints and other data from the first parameter
+  // with this constraint variable. Used with redeclarations, especially of
+  // functions declared in multiple files.
   virtual void mergeDeclaration(ConstraintVariable *, ProgramInfo &,
                                 std::string &ReasonFailed) = 0;
 
@@ -378,7 +379,7 @@ class PointerVariableConstraint : public ConstraintVariable {
 
   const CAtoms &getCvars() const { return Vars; }
 
-  void brainTransplant(ConstraintVariable *From, ProgramInfo &I) override;
+  // Include new ConstAtoms, supplemental info, and merge function pointers
   void mergeDeclaration(ConstraintVariable *From, ProgramInfo &I,
                         std::string &ReasonFailed) override;
 
@@ -459,8 +460,6 @@ class FVComponentVariable {
 
   void mergeDeclaration(FVComponentVariable *From, ProgramInfo &I,
                         std::string &ReasonFailed);
-  void brainTransplant(FVComponentVariable *From, ProgramInfo &I);
-
   std::string mkItypeStr(const EnvironmentMap &E) const;
   std::string mkTypeStr(const EnvironmentMap &E) const;
   std::string mkString(const EnvironmentMap &E) const;
@@ -530,7 +529,7 @@ class FunctionVariableConstraint : public ConstraintVariable {
     return S->getKind() == FunctionVariable;
   }
 
-  void brainTransplant(ConstraintVariable *From, ProgramInfo &I) override;
+  // Merge return value and all params
   void mergeDeclaration(ConstraintVariable *FromCV, ProgramInfo &I,
                         std::string &ReasonFailed) override;
 

clang/include/clang/3C/ProgramInfo.h

@@ -31,6 +31,11 @@ class ProgramVariableAdder {
     getABoundsInfo().insertVariable(D);
   }
 
+  virtual bool seenTypedef(PersistentSourceLoc PSL) = 0;
+
+  virtual void addTypedef(PersistentSourceLoc PSL, bool ShouldCheck) = 0;
+
+
 protected:
   virtual AVarBoundsInfo &getABoundsInfo() = 0;
 };
@@ -166,49 +171,33 @@ class ProgramInfo : public ProgramVariableAdder {
   // For each call to a generic function, remember how the type parameters were
   // instantiated so they can be inserted during rewriting.
   TypeParamBindingsT TypeParamBindings;
-
-  // Insert the given FVConstraint* set into the provided Map.
-  void insertIntoExternalFunctionMap(ExternalFunctionMapType &Map,
-                                     const std::string &FuncName,
-                                     FVConstraint *NewC, FunctionDecl *FD,
-                                     ASTContext *C);
-
-  // Inserts the given FVConstraint* set into the provided static map.
-  void insertIntoStaticFunctionMap(StaticFunctionMapType &Map,
-                                   const std::string &FuncName,
-                                   const std::string &FileName,
-                                   FVConstraint *ToIns, FunctionDecl *FD,
-                                   ASTContext *C);
-
+  
   // Special-case handling for decl introductions. For the moment this covers:
   //  * void-typed variables
   //  * va_list-typed variables
   void specialCaseVarIntros(ValueDecl *D, ASTContext *Context);
 
   // Inserts the given FVConstraint set into the extern or static function map.
-  // Note: This can trigger a brainTransplant from an existing FVConstraint into
-  // the argument FVConstraint. The brainTransplant copies the atoms of the
-  // existing FVConstraint into the argument. This effectively throws out any
-  // constraints that may been applied to the argument FVConstraint, so do not
-  // call this function any time other than immediately after constructing an
-  // FVConstraint.
-  void insertNewFVConstraint(FunctionDecl *FD, FVConstraint *FVCon,
-                             ASTContext *C);
+  // Returns the merged version if it was a redeclaration, or the constraint
+  // parameter if it was new.
+  FunctionVariableConstraint *
+  insertNewFVConstraint(FunctionDecl *FD, FVConstraint *FVCon, ASTContext *C);
 
   // Retrieves a FVConstraint* from a Decl (which could be static, or global)
   FVConstraint *getFuncFVConstraint(FunctionDecl *FD, ASTContext *C);
 
-  // For each pointer type in the declaration of D, add a variable to the
-  // constraint system for that pointer type.
-  void addVariable(clang::DeclaratorDecl *D, clang::ASTContext *AstContext);
-
   void insertIntoPtrSourceMap(const PersistentSourceLoc *PSL,
                               ConstraintVariable *CV);
 
   void computePtrLevelStats();
 
   void insertCVAtoms(ConstraintVariable *CV,
                      std::map<ConstraintKey, ConstraintVariable *> &AtomMap);
+
+  // For each pointer type in the declaration of D, add a variable to the
+  // constraint system for that pointer type.
+  void addVariable(clang::DeclaratorDecl *D, clang::ASTContext *AstContext);
+
 };
 
 #endif

clang/lib/3C/3C.cpp

@@ -345,13 +345,33 @@ _3CInterface::_3CInterface(const struct _3COptions &CCopt,
   CurrCompDB = CompDB;
 }
 
+bool _3CInterface::addVariables() {
+
+  std::lock_guard<std::mutex> Lock(InterfaceMutex);
+
+  ClangTool &Tool = getGlobalClangTool();
+
+  // 1a. Add Variables.
+  std::unique_ptr<ToolAction> AdderTool = newFrontendActionFactoryA<
+      GenericAction<VariableAdderConsumer, ProgramInfo>>(GlobalProgramInfo);
+
+  if (AdderTool) {
+    int ToolExitCode = Tool.run(AdderTool.get());
+    if (ToolExitCode != 0)
+      return false;
+  } else
+    llvm_unreachable("No action");
+
+  return true;
+}
+
 bool _3CInterface::buildInitialConstraints() {
 
   std::lock_guard<std::mutex> Lock(InterfaceMutex);
 
   ClangTool &Tool = getGlobalClangTool();
 
-  // 1. Gather constraints.
+  // 1b. Gather constraints.
   std::unique_ptr<ToolAction> ConstraintTool = newFrontendActionFactoryA<
       GenericAction<ConstraintBuilderConsumer, ProgramInfo>>(GlobalProgramInfo);
 

clang/lib/3C/AVarBoundsInfo.cpp

@@ -481,14 +481,6 @@ ProgramVar *AVarBoundsInfo::getProgramVar(BoundsKey VK) {
   return Ret;
 }
 
-void AVarBoundsInfo::brainTransplant(BoundsKey NewBK, BoundsKey OldBK) {
-  // Here, we use the ProgramVar of NewBK and use it for OldBK.
-  if (NewBK != OldBK) {
-    ProgramVar *NewPVar = getProgramVar(NewBK);
-    insertProgramVar(OldBK, NewPVar);
-  }
-}
-
 bool AVarBoundsInfo::hasVarKey(PersistentSourceLoc &PSL) {
   return DeclVarMap.left().find(PSL) != DeclVarMap.left().end();
 }

clang/lib/3C/ConstraintBuilder.cpp

@@ -269,7 +269,7 @@ class FunctionVisitor : public RecursiveASTVisitor<FunctionVisitor> {
               constrainConsVarGeq(ParameterDC, ArgumentConstraints, CS, &PL,
                                   Wild_to_Safe, false, &Info, false);
 
-              if (AllTypes && TFD != nullptr) {
+              if (AllTypes && TFD != nullptr && I < TFD->getNumParams()) {
                 auto *PVD = TFD->getParamDecl(I);
                 auto &ABI = Info.getABoundsInfo();
                 // Here, we need to handle context-sensitive assignment.
@@ -479,20 +479,6 @@ class ConstraintGenVisitor : public RecursiveASTVisitor<ConstraintGenVisitor> {
                                 TypeVarInfo &TVI)
       : Context(Context), Info(I), CB(Info, Context), TVInfo(TVI), ISD() {}
 
-  bool VisitTypedefDecl(TypedefDecl *TD) {
-    CVarSet Empty;
-    auto PSL = PersistentSourceLoc::mkPSL(TD, *Context);
-    // If we haven't seen this typedef before, initialize it's entry in the
-    // typedef map. If we have seen it before, and we need to preserve the
-    // constraints contained within it.
-    if (!Info.seenTypedef(PSL))
-      // Add this typedef to the program info, if it contains a ptr to
-      // an anonymous struct we mark as not being rewritable.
-      Info.addTypedef(PSL, !PtrToStructDef::containsPtrToStructDef(TD));
-
-    return true;
-  }
-
   bool VisitVarDecl(VarDecl *G) {
 
     if (G->hasGlobalStorage() && isPtrOrArrayType(G->getType())) {
@@ -564,7 +550,22 @@ class ConstraintGenVisitor : public RecursiveASTVisitor<ConstraintGenVisitor> {
 class VariableAdderVisitor : public RecursiveASTVisitor<VariableAdderVisitor> {
 public:
   explicit VariableAdderVisitor(ASTContext *Context, ProgramVariableAdder &VA)
-      : Context(Context), VarAdder(VA) {}
+    : Context(Context), VarAdder(VA) {}
+
+
+  bool VisitTypedefDecl(TypedefDecl* TD) {
+    CVarSet empty;
+    auto PSL = PersistentSourceLoc::mkPSL(TD, *Context);
+    // If we haven't seen this typedef before, initialize it's entry in the
+    // typedef map. If we have seen it before, and we need to preserve the
+    // constraints contained within it
+    if (!VarAdder.seenTypedef(PSL))
+      // Add this typedef to the program info, if it contains a ptr to
+      // an anonymous struct we mark as not being rewritable
+      VarAdder.addTypedef(PSL, !PtrToStructDef::containsPtrToStructDef(TD));
+
+    return true;
+  }
 
   bool VisitVarDecl(VarDecl *D) {
     FullSourceLoc FL = Context->getFullLoc(D->getBeginLoc());
@@ -604,7 +605,7 @@ class VariableAdderVisitor : public RecursiveASTVisitor<VariableAdderVisitor> {
   }
 };
 
-void ConstraintBuilderConsumer::HandleTranslationUnit(ASTContext &C) {
+void VariableAdderConsumer::HandleTranslationUnit(ASTContext &C) {
   Info.enterCompilationUnit(C);
   if (Verbose) {
     SourceManager &SM = C.getSourceManager();
@@ -617,19 +618,45 @@ void ConstraintBuilderConsumer::HandleTranslationUnit(ASTContext &C) {
   }
 
   VariableAdderVisitor VAV = VariableAdderVisitor(&C, Info);
+  TranslationUnitDecl *TUD = C.getTranslationUnitDecl();
+  // Collect Variables.
+  for (const auto &D : TUD->decls()) {
+    VAV.TraverseDecl(D);
+  }
+
+  if (Verbose)
+    errs() << "Done analyzing\n";
+
+  Info.exitCompilationUnit();
+  return;
+}
+
+void ConstraintBuilderConsumer::HandleTranslationUnit(ASTContext &C) {
+  Info.enterCompilationUnit(C);
+  if (Verbose) {
+    SourceManager &SM = C.getSourceManager();
+    FileID MainFileId = SM.getMainFileID();
+    const FileEntry *FE = SM.getFileEntryForID(MainFileId);
+    if (FE != nullptr)
+      errs() << "Analyzing file " << FE->getName() << "\n";
+    else
+      errs() << "Analyzing\n";
+  }
+
+
   TypeVarVisitor TV = TypeVarVisitor(&C, Info);
   ConstraintResolver CSResolver(Info, &C);
   ContextSensitiveBoundsKeyVisitor CSBV =
       ContextSensitiveBoundsKeyVisitor(&C, Info);
   ConstraintGenVisitor GV = ConstraintGenVisitor(&C, Info, TV);
   TranslationUnitDecl *TUD = C.getTranslationUnitDecl();
+
   // Generate constraints.
   for (const auto &D : TUD->decls()) {
-    // The order of these traversals CANNOT be changed because both the type
-    // variable and constraint gen visitor require that variables have been
-    // added to ProgramInfo, and the constraint gen visitor requires the type
-    // variable information gathered in the type variable traversal.
-    VAV.TraverseDecl(D);
+    // The order of these traversals CANNOT be changed because the constraint
+    // gen visitor requires the type variable information gathered in the type
+    // variable traversal.
+
     CSBV.TraverseDecl(D);
     TV.TraverseDecl(D);
     GV.TraverseDecl(D);