Support bounds widening for conditionals with while loops (#803)

Mandeep Singh Grang · web-flow · commit c69e3767e262 · 2020-03-19T18:26:11.000-07:00
Added support for widenening bounds for nt_array_ptr dereferences in while
loops. For example, "while (*p) {}" would widen the bounds of p upon
entry to the loop.

To compute In[B] we compute the intersection of Out[B*-&gt;B], where B* are all
preds of B. When there is a back edge from block B' to B (for example in
loops), the Out set for block B' will be empty when we first enter B. As a
result, the intersection operation would always result in an empty In set for
B.

So to handle this, we consider the In and Out sets for all blocks to have a
default value of "Top" which indicates a set of all members of the Gen set. In
this way we ensure that the intersection does not result in an empty set even
if the Out set for a block is actually empty.

But we also need to handle the case where there is an unconditional jump into a
block (for example, as a result of a goto). In this case, we cannot widen the
bounds because we would not have checked for the ptr dereference. So in this
case we want the intersection to result in an empty set.

So we initialize the In and Out sets of all blocks, except the Entry block, as
"Top".

Top represents the union of the Gen sets of all edges. We have chosen the
offsets of ptr variables in Top to be the max unsigned int. The reason behind
this is that in order to compute the actual In sets for blocks we are going to
intersect the Out sets on all the incoming edges of the block. And in that case
we would always pick the ptr with the smaller offset. Chosing max unsigned int
also makes handling Top much easier as we do not need to explicitly store edge
info.
diff --git a/clang/include/clang/Sema/BoundsAnalysis.h b/clang/include/clang/Sema/BoundsAnalysis.h
@@ -132,6 +132,27 @@ namespace clang {
     // defined in the function.
     DeclSetTy NtPtrsInScope;
 
+    // To compute In[B] we compute the intersection of Out[B*->B], where B* are
+    // all preds of B. When there is a back edge from block B' to B (for
+    // example in loops), the Out set for block B' will be empty when we first
+    // enter B. As a result, the intersection operation would always result in
+    // an empty In set for B.
+
+    // So to handle this, we consider the In and Out sets for all blocks to
+    // have a default value of "Top" which indicates a set of all members of
+    // the Gen set. In this way we ensure that the intersection does not result
+    // in an empty set even if the Out set for a block is actually empty.
+
+    // But we also need to handle the case where there is an unconditional jump
+    // into a block (for example, as a result of a goto). In this case, we
+    // cannot widen the bounds because we would not have checked for the ptr
+    // dereference. So in this case we want the intersection to result in an
+    // empty set.
+
+    // So we initialize the In and Out sets of all blocks, except the Entry
+    // block, as "Top".
+    BoundsMapTy Top;
+
   public:
     BoundsAnalysis(Sema &S, CFG *Cfg) :
       S(S), Cfg(Cfg), Ctx(S.Context),
@@ -234,8 +255,8 @@ namespace clang {
     // @return E with casts stripped off.
     Expr *IgnoreCasts(const Expr *E);
 
-    // We do not want to run dataflow analysis on null, entry or exit blocks.
-    // So we skip them.
+    // We do not want to run dataflow analysis on null or exit blocks. So we
+    // skip them.
     // @param[in] B is the block which may need to the skipped from dataflow
     // analysis.
     // @return Whether B should be skipped.
@@ -274,6 +295,10 @@ namespace clang {
     // @param[in] S is the current Stmt in the block.
     void FillKillSet(ElevatedCFGBlock *EB, const Stmt *S);
 
+    // Initialize the In and Out sets for all blocks, except the Entry block,
+    // as Top.
+    void InitInOutSets();
+
     // Compute the intersection of sets A and B.
     // @param[in] A is a set.
     // @param[in] B is a set.
diff --git a/clang/lib/Sema/BoundsAnalysis.cpp b/clang/lib/Sema/BoundsAnalysis.cpp
@@ -26,9 +26,9 @@ void BoundsAnalysis::WidenBounds(FunctionDecl *FD) {
   // Note: By default, PostOrderCFGView iterates in reverse order. So we always
   // get a reverse post order when we iterate PostOrderCFGView.
   for (const CFGBlock *B : PostOrderCFGView(Cfg)) {
-    // SkipBlock will skip all null, entry and exit blocks. PostOrderCFGView
-    // does not contain any unreachable blocks. So at the end of this loop
-    // BlockMap only contains reachable blocks.
+    // SkipBlock will skip all null and exit blocks. PostOrderCFGView does not
+    // contain any unreachable blocks. So at the end of this loop BlockMap only
+    // contains reachable blocks.
     if (SkipBlock(B))
       continue;
 
@@ -57,6 +57,9 @@ void BoundsAnalysis::WidenBounds(FunctionDecl *FD) {
   ComputeGenSets();
   ComputeKillSets();
 
+  // Initialize the In and Out sets to Top.
+  InitInOutSets();
+
   // Compute In and Out sets.
   while (!WorkList.empty()) {
     ElevatedCFGBlock *EB = WorkList.next();
@@ -67,6 +70,19 @@ void BoundsAnalysis::WidenBounds(FunctionDecl *FD) {
   }
 }
 
+void BoundsAnalysis::InitInOutSets() {
+  for (const auto item : BlockMap) {
+    ElevatedCFGBlock *EB = item.second;
+
+    if (EB->Block == &Cfg->getEntry())
+      continue;
+
+    EB->In = Top;
+    for (const CFGBlock *succ : EB->Block->succs())
+      EB->Out[succ] = Top;
+  }
+}
+
 void BoundsAnalysis::ComputeGenSets() {
   // If there is an edge B1->B2 and the edge condition is of the form
   // "if (*(p + i))" then Gen[B1] = {B2, p:i} .
@@ -307,7 +323,17 @@ void BoundsAnalysis::FillGenSetAndGetBoundsVars(const Expr *E,
     UpperOffset = DerefOffset.ssub_ov(UpperOffset, Overflow);
     if (Overflow)
       continue;
+
     EB->Gen[SuccEB->Block][V] = UpperOffset.getLimitedValue();
+
+    // Top represents the union of the Gen sets of all edges. We have chosen
+    // the offsets of ptr variables in Top to be the max unsigned int. The
+    // reason behind this is that in order to compute the actual In sets for
+    // blocks we are going to intersect the Out sets on all the incoming edges
+    // of the block. And in that case we would always pick the ptr with the
+    // smaller offset. Chosing max unsigned int also makes handling Top much
+    // easier as we do not need to explicitly store edge info.
+    Top[V] = std::numeric_limits<unsigned>::max();
   }
 }
 
@@ -560,17 +586,17 @@ void BoundsAnalysis::ComputeInSets(ElevatedCFGBlock *EB) {
   // In[B1] = n Out[B*->B1], where B* are all preds of B1.
 
   BoundsMapTy Intersections;
-  bool ItersectionEmpty = true;
+  bool FirstIntersection = true;
 
   for (const CFGBlock *pred : EB->Block->preds()) {
     if (SkipBlock(pred))
-      continue;
+      return;
 
     ElevatedCFGBlock *PredEB = BlockMap[pred];
 
-    if (ItersectionEmpty) {
+    if (FirstIntersection) {
       Intersections = PredEB->Out[EB->Block];
-      ItersectionEmpty = false;
+      FirstIntersection = false;
     } else
       Intersections = Intersect(Intersections, PredEB->Out[EB->Block]);
   }
@@ -591,6 +617,7 @@ void BoundsAnalysis::ComputeOutSets(ElevatedCFGBlock *EB,
   }
 
   BoundsMapTy Diff = Difference(EB->In, KilledVars);
+
   for (const CFGBlock *succ : EB->Block->succs()) {
     if (SkipBlock(succ))
       continue;
@@ -635,6 +662,8 @@ Expr *BoundsAnalysis::GetTerminatorCondition(const CFGBlock *B) const {
   if (const Stmt *S = B->getTerminatorStmt()) {
     if (const auto *IfS = dyn_cast<IfStmt>(S))
       return const_cast<Expr *>(IfS->getCond());
+    if (const auto *WhileS = dyn_cast<WhileStmt>(S))
+      return const_cast<Expr *>(WhileS->getCond());
   }
   return nullptr;
 }
@@ -649,7 +678,7 @@ bool BoundsAnalysis::IsNtArrayType(const VarDecl *V) const {
 }
 
 bool BoundsAnalysis::SkipBlock(const CFGBlock *B) const {
-  return !B || B == &Cfg->getEntry() || B == &Cfg->getExit();
+  return !B || B == &Cfg->getExit();
 }
 
 template<class T>
@@ -715,8 +744,21 @@ template<class T>
 bool BoundsAnalysis::Differ(T &A, T &B) const {
   if (A.size() != B.size())
     return true;
-  auto Ret = Intersect(A, B);
-  return Ret.size() != A.size();
+
+  for (const auto item : A) {
+    if (!B.count(item.first))
+      return true;
+    if (B[item.first] != item.second)
+      return true;
+  }
+
+  for (const auto item : B) {
+    if (!A.count(item.first))
+      return true;
+    if (A[item.first] != item.second)
+      return true;
+  }
+  return false;
 }
 
 OrderedBlocksTy BoundsAnalysis::GetOrderedBlocks() {
diff --git a/clang/test/CheckedC/inferred-bounds/widened-bounds.c b/clang/test/CheckedC/inferred-bounds/widened-bounds.c
@@ -534,3 +534,133 @@ void f20() {
 // CHECK:  [B1]
 // CHECK-NOT: upper_bound(u)
 }
+
+void f21() {
+  char p _Nt_checked[] : count(0) = "abc";
+
+  while (p[0])
+    while (p[1])   // expected-error {{out-of-bounds memory access}}
+      while (p[2]) // expected-error {{out-of-bounds memory access}}
+  {}
+
+// CHECK: In function: f21
+// CHECK:  [B6]
+// CHECK:    1: p[0]
+// CHECK:  [B5]
+// CHECK:    1: p[1]
+// CHECK: upper_bound(p) = 1
+// CHECK:  [B4]
+// CHECK:    1: p[2]
+// CHECK: upper_bound(p) = 2
+// CHECK:  [B3]
+// CHECK: upper_bound(p) = 3
+// CHECK:  [B2]
+// CHECK: upper_bound(p) = 2
+// CHECK:  [B1]
+// CHECK: upper_bound(p) = 1
+}
+
+void f22() {
+  _Nt_array_ptr<char> p : count(0) = "a";
+
+  if (*p)
+    while (*(p + 1))   // expected-error {{out-of-bounds memory access}}
+      if (*(p + 2)) // expected-error {{out-of-bounds memory access}}
+  {}
+
+// CHECK: In function: f22
+// CHECK:  [B5]
+// CHECK:    2: *p
+// CHECK:  [B4]
+// CHECK:    1: *(p + 1)
+// CHECK: upper_bound(p) = 1
+// CHECK:  [B3]
+// CHECK:    1: *(p + 2)
+// CHECK: upper_bound(p) = 2
+// CHECK:  [B2]
+// CHECK: upper_bound(p) = 3
+// CHECK:  [B1]
+// CHECK: upper_bound(p) = 2
+}
+
+void f23() {
+  _Nt_array_ptr<char> p : count(0) = "";
+
+// CHECK: In function: f23
+
+  goto B;
+  while (*p) {
+B:  p;
+    while (*(p + 1)) { // expected-error {{out-of-bounds memory access}}
+      p;
+    }
+  }
+
+// CHECK:  [B14]
+// CHECK:    T: goto B;
+// CHECK:  [B13]
+// CHECK:    1: *p
+// CHECK:    T: while
+// CHECK:  [B12]
+// CHECK:   B:
+// CHECK:    1: p
+// CHECK-NOT: upper_bound(p)
+// CHECK:  [B11]
+// CHECK:    1: *(p + 1)
+// CHECK:    T: while
+// CHECK:  [B10]
+// CHECK:    1: p
+// CHECK-NOT: upper_bound(p)
+
+  while (*p) {
+    p;
+    while (*(p + 1)) { // expected-error {{out-of-bounds memory access}}
+C:    p;
+    }
+  }
+  goto C;
+
+// CHECK:  [B7]
+// CHECK:    1: *p
+// CHECK:    T: while
+// CHECK:  [B6]
+// CHECK:    1: p
+// CHECK: upper_bound(p) = 1
+// CHECK:  [B5]
+// CHECK:    1: *(p + 1)
+// CHECK:    T: while
+// CHECK-NOT: upper_bound(p)
+// CHECK:  [B4]
+// CHECK:   C:
+// CHECK:    1: p
+// CHECK-NOT: upper_bound(p)
+// CHECK:  [B2]
+// CHECK-NOT: upper_bound(p)
+// CHECK:  [B1]
+// CHECK:    T: goto C;
+}
+
+void f24() {
+  _Nt_array_ptr<char> p : count(0) = "";
+
+  while (*p) {
+    p++;
+    while (*(p+1)) { // expected-error {{out-of-bounds memory access}}
+      p;
+    }
+  }
+
+// CHECK: In function: f24
+// CHECK:  [B6]
+// CHECK:    1: *p
+// CHECK:    T: while
+// CHECK:  [B5]
+// CHECK:    1: p++
+// CHECK: upper_bound(p) = 1
+// CHECK:  [B4]
+// CHECK:    1: *(p + 1)
+// CHECK:    T: while
+// CHECK:  [B3]
+// CHECK:    1: p
+// CHECK-NOT: upper_bound(p)
+}
