Fixing issue: correctcomputation#390 (#395)

* Fixing issue: correctcomputation#390

* Adding -verify

Author: Machiry

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

@@ -150,11 +150,13 @@ class DataGraph
     }
   }
 
-  bool getNeighbors(Data D, std::set<Data> &DataSet, bool Succ) {
+  bool getNeighbors(Data D, std::set<Data> &DataSet, bool Succ,
+                    bool Append = false) {
     NodeType *N = this->findNode(D);
     if (N == nullptr)
       return false;
-    DataSet.clear();
+    if (!Append)
+      DataSet.clear();
     llvm::SetVector<EdgeType *> Edges;
     if (Succ)
       Edges = N->getEdges();
@@ -165,12 +167,12 @@ class DataGraph
     return !DataSet.empty();
   }
 
-  bool getSuccessors(Data D, std::set<Data> &DataSet) {
-    return getNeighbors(D, DataSet, true);
+  bool getSuccessors(Data D, std::set<Data> &DataSet, bool Append = false) {
+    return getNeighbors(D, DataSet, true, Append);
   }
 
-  bool getPredecessors(Data D, std::set<Data> &DataSet) {
-    return getNeighbors(D, DataSet, false);
+  bool getPredecessors(Data D, std::set<Data> &DataSet, bool Append = false) {
+    return getNeighbors(D, DataSet, false, Append);
   }
 
   NodeType *findNode(Data D) {

clang/lib/3C/AVarBoundsInfo.cpp

@@ -601,11 +601,13 @@ void AvarBoundsInference::mergeReachableProgramVars(
         if (!TmpB->isNumConstant()) {
           // We give preference to non-constant lengths.
           BVar = TmpB;
-        } else if (!this->BI->areSameProgramVar(BVar->getKey(),
-                                                TmpB->getKey())) {
-          // If both are different constants?
-          BVar = nullptr;
-          break;
+        } else {
+          // If we need to merge two constants? Pick the lesser value.
+          int CVal = std::stoi(BVar->getVarName());
+          int TmpVal = std::stoi(TmpB->getVarName());
+          if (TmpVal < CVal) {
+            BVar = TmpB;
+          }
         }
       } else if (!TmpB->isNumConstant() && BVar->getKey() != TmpB->getKey()) {
         // If they are different variables?
@@ -737,22 +739,6 @@ bool AvarBoundsInference::getRelevantBounds(BoundsKey BK,
   return HasBounds;
 }
 
-// Variable comparison. Comparator implementation: where given two BoundsKey
-// they are checked to see if they correspond to the same program variable.
-struct BVarCmp {
-public:
-  BVarCmp(AVarBoundsInfo *ABI) { this->ABInfo = ABI; }
-  bool operator()(BoundsKey A, BoundsKey B) const {
-    if (this->ABInfo != nullptr && this->ABInfo->areSameProgramVar(A, B)) {
-      return false;
-    }
-    return A < B;
-  };
-
-private:
-  AVarBoundsInfo *ABInfo;
-};
-
 bool AvarBoundsInference::areDeclaredBounds(
     BoundsKey K,
     const std::pair<ABounds::BoundsKind, std::set<BoundsKey>> &Bnds) {
@@ -827,7 +813,7 @@ bool AvarBoundsInference::predictBounds(BoundsKey K,
   if (!InferredNBnds.empty()) {
     // All the possible inferred bounds for K
     InferredKBnds.clear();
-    std::set<BoundsKey> TmpBKeys;
+    std::set<BoundsKey> TmpBKeys, AllKeys;
     // TODO: Figure out if there is a discrepency and try to implement
     // root-cause analysis.
 
@@ -838,16 +824,23 @@ bool AvarBoundsInference::predictBounds(BoundsKey K,
           InferredKBnds[INB.first] = INB.second;
         } else {
           TmpBKeys.clear();
-          // Here, we should use intersection by taking care of comparing
-          // bounds key that correspond to the same constant.
-          // Note, DO NOT use findIntersection here, as we need to take
-          // care of comparing bounds key that correspond to the same
-          // constant.
+          AllKeys.clear();
+          // Find intersection between the current bounds and the
+          // bounds propagated from current neighbour, i.e., INB.first.
           auto &S1 = InferredKBnds[INB.first];
           auto &S2 = INB.second;
-          std::set_intersection(S1.begin(), S1.end(), S2.begin(), S2.end(),
-                                std::inserter(TmpBKeys, TmpBKeys.begin()),
-                                BVarCmp(this->BI));
+          // Find intersection of bounds propagated from all neighbours.
+          findIntersection(S1, S2, TmpBKeys);
+
+          AllKeys = S1;
+          AllKeys.insert(S2.begin(), S2.end());
+          // Also, add all constants as potential bounds so that we can pick
+          // a constant with least value later.
+          for (auto CK: AllKeys) {
+            auto *CKVar = this->BI->getProgramVar(CK);
+            if (CKVar != nullptr && CKVar->isNumConstant())
+              TmpBKeys.insert(CK);
+          }
           InferredKBnds[INB.first] = TmpBKeys;
         }
       }
@@ -1132,20 +1125,18 @@ void AVarBoundsInfo::computerArrPointers(ProgramInfo *PI,
   // counterparts.
   std::set<BoundsKey> CtxSensBKeys;
   CtxSensBKeys.clear();
-  std::set<BoundsKey> TmpBKeys, TmpBKeysF;
+  std::set<BoundsKey> TmpBKeys;
   for (auto BK : ArrPointers) {
-    TmpBKeys.clear();
-    ProgVarGraph.getPredecessors(BK, TmpBKeys);
-    TmpBKeysF.insert(TmpBKeys.begin(), TmpBKeys.end());
-    TmpBKeys.clear();
-    ProgVarGraph.getSuccessors(BK, TmpBKeys);
-    TmpBKeysF.insert(TmpBKeys.begin(), TmpBKeys.end());
-    for (auto TBK : TmpBKeysF) {
-      ProgramVar *TmpPVar = getProgramVar(TBK);
-      if (TmpPVar != nullptr) {
-        if (isa<CtxFunctionArgScope>(TmpPVar->getScope())) {
-          CtxSensBKeys.insert(TBK);
-        }
+    CtxSensProgVarGraph.getSuccessors(BK, TmpBKeys, true);
+    CtxSensProgVarGraph.getPredecessors(BK, TmpBKeys, true);
+    RevCtxSensProgVarGraph.getSuccessors(BK, TmpBKeys, true);
+    RevCtxSensProgVarGraph.getPredecessors(BK, TmpBKeys, true);
+  }
+  for (auto TBK : TmpBKeys) {
+    ProgramVar *TmpPVar = getProgramVar(TBK);
+    if (TmpPVar != nullptr) {
+      if (isa<CtxFunctionArgScope>(TmpPVar->getScope())) {
+        CtxSensBKeys.insert(TBK);
       }
     }
   }
@@ -1193,6 +1184,10 @@ bool AVarBoundsInfo::performFlowAnalysis(ProgramInfo *PI) {
   // Repopulate array bounds key.
   ArrPointerBoundsKey.clear();
   ArrPointerBoundsKey.insert(ArrPointers.begin(), ArrPointers.end());
+  // All BoundsKey that has bounds are also array pointers.
+  for (auto &T : this->BInfo) {
+    ArrPointerBoundsKey.insert(T.first);
+  }
 
   // Keep only highest priority bounds.
   // Any thing changed? which means bounds of a variable changed

clang/lib/3C/ConstraintResolver.cpp

@@ -190,6 +190,8 @@ CVarSet ConstraintResolver::getInvalidCastPVCons(CastExpr *E) {
 // ignore parts of it that do not contribute to the final result
 CVarSet ConstraintResolver::getExprConstraintVars(Expr *E) {
   CVarSet EmptyCSet;
+  auto &ABInfo = Info.getABoundsInfo();
+
   if (E != nullptr) {
     auto &CS = Info.getConstraints();
     QualType TypE = E->getType();
@@ -535,7 +537,6 @@ CVarSet ConstraintResolver::getExprConstraintVars(Expr *E) {
 
         // Make the bounds key context sensitive.
         if (NewCV->hasBoundsKey()) {
-          auto &ABInfo = Info.getABoundsInfo();
           auto CSensBKey =
               ABInfo.getContextSensitiveBoundsKey(CE, NewCV->getBoundsKey());
           NewCV->setBoundsKey(CSensBKey);
@@ -600,6 +601,15 @@ CVarSet ConstraintResolver::getExprConstraintVars(Expr *E) {
           new PVConstraint(Str->getType(), nullptr, Str->getStmtClassName(),
                            Info, *Context, nullptr);
       P->constrainOuterTo(CS, CS.getNTArr()); // NB: ARR already there
+
+      BoundsKey TmpKey = ABInfo.getRandomBKey();
+      P->setBoundsKey(TmpKey);
+
+      BoundsKey CBKey = ABInfo.getConstKey(Str->getByteLength());
+      ABounds *NB = new CountBound(CBKey);
+      ABInfo.replaceBounds(TmpKey, Declared, NB);
+
+
       T = {P};
 
       Ret = T;

clang/test/3C/multiconstant_bounds.c

@@ -0,0 +1,19 @@
+// Tests for the 3C.
+//
+// When multiple constants are potential bounds, pick the lower constant.
+// Issue: https://github.com/correctcomputation/checkedc-clang/issues/390
+//
+// RUN: 3c -alltypes %s | FileCheck -match-full-lines %s
+// RUN: 3c -alltypes %s | %clang -c -fcheckedc-extension -Xclang -verify -x c -o %t1.unused -
+// expected-no-diagnostics
+//
+int a(char * : itype(_Nt_array_ptr<char>));
+//CHECK: int a(char * : itype(_Nt_array_ptr<char>));
+void b(char *p) { a(p); }
+//CHECK: void b(_Nt_array_ptr<char> p : count(4)) { a(p); }
+void c(void) {
+  char foo[256];
+  b(foo);
+  b("test");
+}
+//CHECK: char foo _Nt_checked[256];