Merge pull request #291 from correctcomputation/MiscRefactorings

jackastner · web-flow · commit 6ec85bcdcdab · 2020-10-15T15:33:13.000-04:00
Refactoring and bug fix changes
diff --git a/clang/include/clang/CConv/ConstraintVariables.h b/clang/include/clang/CConv/ConstraintVariables.h
@@ -112,8 +112,6 @@ class ConstraintVariable {
   virtual bool solutionEqualTo(Constraints &, const ConstraintVariable *) const
     = 0;
 
-  // Constrain all pointers in this ConstraintVariable to be Wild.
-  virtual void constrainToWild(Constraints &CS) const = 0;
   virtual void constrainToWild(Constraints &CS,
                                const std::string &Rsn) const = 0;
   virtual void constrainToWild(Constraints &CS, const std::string &Rsn,
@@ -362,7 +360,7 @@ class PointerVariableConstraint : public ConstraintVariable {
   void print(llvm::raw_ostream &O) const override ;
   void dump() const override { print(llvm::errs()); }
   void dump_json(llvm::raw_ostream &O) const override;
-  void constrainToWild(Constraints &CS) const override;
+
   void constrainToWild(Constraints &CS, const std::string &Rsn) const override;
   void constrainToWild(Constraints &CS, const std::string &Rsn,
                        PersistentSourceLoc *PL) const override;
@@ -477,7 +475,7 @@ class FunctionVariableConstraint : public ConstraintVariable {
   void print(llvm::raw_ostream &O) const override;
   void dump() const override { print(llvm::errs()); }
   void dump_json(llvm::raw_ostream &O) const override;
-  void constrainToWild(Constraints &CS) const override;
+
   void constrainToWild(Constraints &CS, const std::string &Rsn) const override;
   void constrainToWild(Constraints &CS, const std::string &Rsn,
                        PersistentSourceLoc *PL) const override;
diff --git a/clang/include/clang/CConv/DeclRewriter.h b/clang/include/clang/CConv/DeclRewriter.h
@@ -68,7 +68,7 @@ class DeclRewriter {
   void doDeclRewrite(SourceRange &SR, DeclReplacement *N);
 
   void rewriteFunctionDecl(FunctionDeclReplacement *N);
-  void getDeclsOnSameLine(DeclReplacement *N, std::set<Decl *> &Decls);
+  void getDeclsOnSameLine(DeclReplacement *N, std::vector<Decl *> &Decls);
   bool isSingleDeclaration(DeclReplacement *N);
   bool areDeclarationsOnSameLine(DeclReplacement *N1, DeclReplacement *N2);
   SourceRange getNextCommaOrSemicolon(SourceLocation L);
diff --git a/clang/include/clang/CConv/RewriteUtils.h b/clang/include/clang/CConv/RewriteUtils.h
@@ -186,15 +186,15 @@ typedef std::set<DeclReplacement *, DComp> RSet;
 class GlobalVariableGroups {
 public:
   GlobalVariableGroups(SourceManager &SourceMgr) : SM(SourceMgr) { }
-  void addGlobalDecl(Decl *VD, std::set<Decl *> *VDSet = nullptr);
+  void addGlobalDecl(Decl *VD, std::vector<Decl *> *VDVec = nullptr);
 
-  std::set<Decl *> &getVarsOnSameLine(Decl *VD);
+  std::vector<Decl *> &getVarsOnSameLine(Decl *VD);
 
   virtual ~GlobalVariableGroups();
 
 private:
   SourceManager &SM;
-  std::map<Decl *, std::set<Decl *>*> GlobVarGroups;
+  std::map<Decl *, std::vector<Decl *>*> GlobVarGroups;
 };
 
 // Class that handles rewriting bounds information for all the
diff --git a/clang/lib/CConv/ConstraintResolver.cpp b/clang/lib/CConv/ConstraintResolver.cpp
@@ -434,6 +434,7 @@ CVarSet
         QualType ExprType = E->getType();
         Decl *D = CE->getCalleeDecl();
         CVarSet ReallocFlow;
+        bool IsAllocator = false;
         if (D == nullptr) {
           // There are a few reasons that we couldn't get a decl. For example,
           // the call could be done through an array subscript.
@@ -452,6 +453,7 @@ CVarSet
           /* Allocator call */
           if (isFunctionAllocator(FD->getName())) {
             bool didInsert = false;
+            IsAllocator = true;
             if (CE->getNumArgs() > 0) {
               QualType ArgTy;
               std::string FuncName = FD->getNameAsString();
@@ -509,19 +511,24 @@ CVarSet
         for (ConstraintVariable *CV : ReturnCVs) {
           ConstraintVariable *NewCV;
           auto *PCV = dyn_cast<PVConstraint>(CV);
-          if (PCV && PCV->getIsOriginallyChecked()) {
-            // Copying needs to be done differently if the constraint variable
-            // had a checked type in the input program because these constraint
-            // variables contain constant atoms that are reused by the copy
-            // constructor.
-            NewCV = new PVConstraint(CE->getType(), nullptr, PCV->getName(),
-                                     Info, *Context, nullptr,
-                                     PCV->getIsGeneric());
-            if (PCV->hasBoundsKey())
-              NewCV->setBoundsKey(PCV->getBoundsKey());
-              
+          if (!IsAllocator) {
+            if (PCV && PCV->getIsOriginallyChecked()) {
+              // Copying needs to be done differently if the constraint variable
+              // had a checked type in the input program because the constraint
+              // variables contain constant atoms that are reused by the copy
+              // constructor.
+              NewCV = new PVConstraint(CE->getType(), nullptr, PCV->getName(),
+                                       Info, *Context, nullptr,
+                                       PCV->getIsGeneric());
+              if (PCV->hasBoundsKey())
+                NewCV->setBoundsKey(PCV->getBoundsKey());
+            } else {
+              NewCV = CV->getCopy(CS);
+            }
           } else {
-            NewCV = CV->getCopy(CS);
+            // Allocator functions are treated specially, so they do not have
+            // separate parameter and argument return variables.
+            NewCV = CV;
           }
           
           // Make the bounds key context sensitive.
@@ -535,8 +542,9 @@ CVarSet
           
           // Important: Do Safe_to_Wild from returnvar in this copy, which then
           //   might be assigned otherwise (Same_to_Same) to LHS
-          constrainConsVarGeq(NewCV, CV, CS, nullptr, Safe_to_Wild, false,
-                              &Info);
+          if (NewCV != CV)
+            constrainConsVarGeq(NewCV, CV, CS, nullptr, Safe_to_Wild, false,
+                                &Info);
           TmpCVs.insert(NewCV);
           // If this is realloc, constrain the first arg to flow to the return
           if (!ReallocFlow.empty()) {
diff --git a/clang/lib/CConv/ConstraintVariables.cpp b/clang/lib/CConv/ConstraintVariables.cpp
@@ -880,19 +880,9 @@ FunctionVariableConstraint::FunctionVariableConstraint(const Type *Ty,
   ReturnVar = new PVConstraint(RT, D, RETVAR, I, Ctx, &N, IsGeneric);
 }
 
-void FunctionVariableConstraint::constrainToWild(Constraints &CS) const {
-  ReturnVar->constrainToWild(CS);
-
-  for (const auto &V : ParamVars)
-    V->constrainToWild(CS);
-}
-
 void FunctionVariableConstraint::constrainToWild(Constraints &CS,
                                                  const std::string &Rsn) const {
-  ReturnVar->constrainToWild(CS, Rsn);
-
-  for (const auto &V : ParamVars)
-    V->constrainToWild(CS, Rsn);
+  constrainToWild(CS, Rsn, nullptr);
 }
 
 void FunctionVariableConstraint::constrainToWild
@@ -975,41 +965,26 @@ void FunctionVariableConstraint::equateArgumentConstraints(ProgramInfo &Info) {
   }
 }
 
-void PointerVariableConstraint::constrainToWild(Constraints &CS) const {
-  ConstAtom *WA = CS.getWild();
-  for (const auto &V : vars) {
-    if (VarAtom *VA = dyn_cast<VarAtom>(V))
-      CS.addConstraint(CS.createGeq(VA, WA, true));
-  }
-
-  if (FV)
-    FV->constrainToWild(CS);
+void PointerVariableConstraint::constrainToWild(Constraints &CS,
+                                                const std::string &Rsn) const {
+  constrainToWild(CS, Rsn, nullptr);
 }
 
 void PointerVariableConstraint::constrainToWild(Constraints &CS,
                                                 const std::string &Rsn,
                                                 PersistentSourceLoc *PL) const {
-  ConstAtom *WA = CS.getWild();
-  for (const auto &V : vars) {
-    if (VarAtom *VA = dyn_cast<VarAtom>(V))
-      CS.addConstraint(CS.createGeq(VA, WA, Rsn, PL, true));
+  // Constrains the outer pointer level to WILD. Inner pointer levels are
+  // implicitly WILD because of implication constraints.
+  if (!vars.empty()) {
+    Atom *A = *vars.begin();
+    if (auto *VA = dyn_cast<VarAtom>(A))
+      CS.addConstraint(CS.createGeq(VA, CS.getWild(), Rsn, PL, true));
   }
 
   if (FV)
     FV->constrainToWild(CS, Rsn, PL);
 }
 
-void PointerVariableConstraint::constrainToWild(Constraints &CS,
-                                                const std::string &Rsn) const {
-  ConstAtom *WA = CS.getWild();
-  for (const auto &V : vars) {
-    if (VarAtom *VA = dyn_cast<VarAtom>(V))
-      CS.addConstraint(CS.createGeq(VA, WA, Rsn, true));
-  }
-
-  if (FV)
-    FV->constrainToWild(CS, Rsn);
-}
 
 void PointerVariableConstraint::constrainOuterTo(Constraints &CS, ConstAtom *C,
                                                  bool doLB) {
diff --git a/clang/lib/CConv/DeclRewriter.cpp b/clang/lib/CConv/DeclRewriter.cpp
@@ -227,7 +227,7 @@ void DeclRewriter::rewriteMultiDecl(DeclReplacement *N, RSet &ToRewrite) {
   //         original decl was re-written, write that out instead. Existing
   //         initializers are preserved, any declarations that an initializer to
   //         be valid checked-c are given one.
-  std::set<Decl *> SameLineDecls;
+  std::vector<Decl *> SameLineDecls;
   getDeclsOnSameLine(N, SameLineDecls);
 
   bool IsFirst = true;
@@ -408,7 +408,7 @@ bool DeclRewriter::areDeclarationsOnSameLine(DeclReplacement *N1,
     DeclStmt *Stmt2 = N2->getStatement();
     if (Stmt1 == nullptr && Stmt2 == nullptr) {
       auto &DGroup = GP.getVarsOnSameLine(D1);
-      return DGroup.find(D2) != DGroup.end();
+      return llvm::is_contained(DGroup, D2);
     } else if (Stmt1 == nullptr || Stmt2 == nullptr) {
       return false;
     } else {
@@ -430,13 +430,20 @@ bool DeclRewriter::isSingleDeclaration(DeclReplacement *N) {
 }
 
 void DeclRewriter::getDeclsOnSameLine(DeclReplacement *N,
-                                      std::set<Decl *> &Decls) {
-  if (N->getStatement() != nullptr)
-    Decls.insert(N->getStatement()->decls().begin(),
+                                      std::vector<Decl *> &Decls) {
+  if (N->getStatement() != nullptr) {
+    Decls.insert(Decls.begin(), N->getStatement()->decls().begin(),
                  N->getStatement()->decls().end());
-  else
-    Decls.insert(GP.getVarsOnSameLine(N->getDecl()).begin(),
-                 GP.getVarsOnSameLine(N->getDecl()).end());
+  } else {
+    std::vector<Decl *> GlobalLine = GP.getVarsOnSameLine(N->getDecl());
+    Decls.insert(Decls.begin(), GlobalLine.begin(), GlobalLine.end());
+  }
+
+  assert("Invalid ordering in same line decls" &&
+    std::is_sorted(Decls.begin(), Decls.end(), [&](Decl *D0, Decl *D1) {
+      return A.getSourceManager().isBeforeInTranslationUnit(D0->getEndLoc(),
+                                                            D1->getEndLoc());
+  }));
 }
 
 // Note: This is variable declared static in the header file in order to pass
diff --git a/clang/lib/CConv/RewriteUtils.cpp b/clang/lib/CConv/RewriteUtils.cpp
@@ -88,30 +88,33 @@ bool DComp::operator()(DeclReplacement *Lhs,
     return SM.isBeforeInTranslationUnit(X2, Y1);
 }
 
-void GlobalVariableGroups::addGlobalDecl(Decl *VD, std::set<Decl *> *VDSet) {
+void GlobalVariableGroups::addGlobalDecl(Decl *VD, std::vector<Decl *> *VDVec) {
   if (VD && GlobVarGroups.find(VD) == GlobVarGroups.end()) {
-    if (VDSet == nullptr)
-      VDSet = new std::set<Decl *>();
-    VDSet->insert(VD);
-    GlobVarGroups[VD] = VDSet;
+    if (VDVec == nullptr)
+      VDVec = new std::vector<Decl *>();
+    assert("Decls in group are not ordered correctly." && (VDVec->empty() ||
+           SM.isBeforeInTranslationUnit(VDVec->back()->getEndLoc(),
+                                        VD->getEndLoc())));
+    VDVec->push_back(VD);
+    GlobVarGroups[VD] = VDVec;
     // Process the next decl.
     Decl *NDecl = VD->getNextDeclInContext();
     if (isa_and_nonnull<VarDecl>(NDecl) || isa_and_nonnull<FieldDecl>(NDecl))
       if (VD->getBeginLoc() == NDecl->getBeginLoc())
-        addGlobalDecl(dyn_cast<Decl>(NDecl), VDSet);
+        addGlobalDecl(dyn_cast<Decl>(NDecl), VDVec);
   }
 }
 
 
-std::set<Decl *> &GlobalVariableGroups::getVarsOnSameLine(Decl *D) {
+std::vector<Decl *> &GlobalVariableGroups::getVarsOnSameLine(Decl *D) {
   assert (GlobVarGroups.find(D) != GlobVarGroups.end() &&
          "Expected to find the group.");
   return *(GlobVarGroups[D]);
 }
 
 
 GlobalVariableGroups::~GlobalVariableGroups() {
-  std::set<std::set<Decl *> *> VVisited;
+  std::set<std::vector<Decl *> *> VVisited;
   // Free each of the group.
   for (auto &currV : GlobVarGroups) {
     // Avoid double free by caching deleted sets.
