[BoundsWidening] Determine checked scope specifier per statement (#1139)

We use the CheckedCAnalysesPrepass.cpp to gather the checked scopes for
statements. We store a map of statements to their checked scope specifiers. An
entry in this map is only made for the following statements:
1. For the first non-compound statement of a compound statement.
2. For the first statement that follows a compound statement.

We then use this info in the bounds widening analysis to determine the checked
scope specifiers for each statement.

clang/include/clang/Sema/BoundsWideningAnalysis.h

@@ -82,14 +82,17 @@ namespace clang {
     Lexicographic Lex;
     BoundsVarsTy &BoundsVarsLower;
     BoundsVarsTy &BoundsVarsUpper;
+    CheckedScopeMapTy &CheckedScopeMap;
 
   public:
     BoundsWideningUtil(Sema &SemaRef, CFG *Cfg,
                        ASTContext &Ctx, Lexicographic Lex,
                        BoundsVarsTy &BoundsVarsLower,
-                       BoundsVarsTy &BoundsVarsUpper) :
+                       BoundsVarsTy &BoundsVarsUpper,
+                       CheckedScopeMapTy &CheckedScopeMap) :
       SemaRef(SemaRef), Cfg(Cfg), Ctx(Ctx), Lex(Lex),
-      BoundsVarsLower(BoundsVarsLower), BoundsVarsUpper(BoundsVarsUpper) {}
+      BoundsVarsLower(BoundsVarsLower), BoundsVarsUpper(BoundsVarsUpper),
+      CheckedScopeMap(CheckedScopeMap) {}
 
     // Check if B2 is a subrange of B1.
     // @param[in] B1 is the first range.
@@ -172,6 +175,14 @@ namespace clang {
     // expression.
     const VarDecl *GetNullTermPtrInExpr(Expr *E) const;
 
+    // Update the checked scope specifier for the current statement if it has
+    // changed from that of the previous statement.
+    // @param[in] CurrStmt is the current statement.
+    // @param[out] CSS is updated with the checked scope specifier for the
+    // current statement if it has changed from that of the previous statement.
+    void UpdateCheckedScopeSpecifier(const Stmt *CurrStmt,
+                                     CheckedScopeSpecifier &CSS) const;
+
     // Invoke IgnoreValuePreservingOperations to strip off casts.
     // @param[in] E is the expression whose casts must be stripped.
     // @return E with casts stripped off.
@@ -332,11 +343,13 @@ namespace clang {
 
     BoundsWideningAnalysis(Sema &SemaRef, CFG *Cfg,
                            BoundsVarsTy &BoundsVarsLower,
-                           BoundsVarsTy &BoundsVarsUpper) :
+                           BoundsVarsTy &BoundsVarsUpper,
+                           CheckedScopeMapTy &CheckedScopeMap) :
       SemaRef(SemaRef), Cfg(Cfg), Ctx(SemaRef.Context),
       Lex(Lexicographic(Ctx, nullptr)), OS(llvm::outs()),
       BWUtil(BoundsWideningUtil(SemaRef, Cfg, Ctx, Lex,
-                                BoundsVarsLower, BoundsVarsUpper)) {}
+                                BoundsVarsLower, BoundsVarsUpper,
+                                CheckedScopeMap)) {}
 
     // Run the dataflow analysis to widen bounds for null-terminated arrays.
     // @param[in] FD is the current function.

clang/include/clang/Sema/CheckedCAnalysesPrepass.h

@@ -38,6 +38,9 @@ namespace clang {
   // of F in whose declared bounds expressions F occurs.
   using BoundsSiblingFieldsTy = llvm::DenseMap<const FieldDecl *, FieldSetTy>;
 
+  // CheckedScopeMapTy maps a statement to its checked scope specifier.
+  using CheckedScopeMapTy = llvm::DenseMap<const Stmt *, CheckedScopeSpecifier>;
+
   struct PrepassInfo {
     // VarUses maps each VarDecl V in a function to the DeclRefExpr (if any)
     // that is the first use of V, if V fulfills the following conditions:
@@ -82,6 +85,10 @@ namespace clang {
     // a deterministic iteration order we must remember to sort the keys as
     // well as the values.
     BoundsSiblingFieldsTy BoundsSiblingFields;
+
+    // A map of statements to their checked scope specifiers. An entry in this
+    // map is made only when the checked scope specifier changes.
+    CheckedScopeMapTy CheckedScopeMap;
   };
 } // end namespace clang
 #endif

clang/lib/Sema/BoundsWideningAnalysis.cpp

@@ -82,6 +82,7 @@ BoundsMapTy BoundsWideningAnalysis::GetOutOfLastStmt(
   // statement and return the StmtOut for the last statement of the block.
 
   BoundsMapTy StmtOut = EB->In;
+  CheckedScopeSpecifier CSS = CheckedScopeSpecifier::CSS_None;
 
   for (CFGBlock::const_iterator I = EB->Block->begin(),
                                 E = EB->Block->end();
@@ -93,6 +94,9 @@ BoundsMapTy BoundsWideningAnalysis::GetOutOfLastStmt(
     if (!CurrStmt)
       continue;
 
+    // Update the checked scope specifier for the current statement.
+    BWUtil.UpdateCheckedScopeSpecifier(CurrStmt, CSS);
+
     // The In of the current statement is the value of StmtOut computed so far.
     BoundsMapTy InOfCurrStmt = StmtOut;
 
@@ -121,9 +125,6 @@ BoundsMapTy BoundsWideningAnalysis::GetOutOfLastStmt(
     Expr *OriginalLValue = std::get<1>(ValuesToReplaceInBounds);
     VarSetTy PtrsWithAffectedBounds = std::get<2>(ValuesToReplaceInBounds);
 
-    // TODO: Determine the Checked scope for each statement.
-    CheckedScopeSpecifier CSS = CheckedScopeSpecifier::CSS_None;
-
     for (const VarDecl *V : PtrsWithAffectedBounds) {
       auto StmtInIt = InOfCurrStmt.find(V);
       if (StmtInIt == InOfCurrStmt.end())
@@ -1419,6 +1420,14 @@ const VarDecl *BoundsWideningUtil::GetNullTermPtrInExpr(Expr *E) const {
   return nullptr;
 }
 
+void BoundsWideningUtil::UpdateCheckedScopeSpecifier(
+  const Stmt *CurrStmt, CheckedScopeSpecifier &CSS) const {
+
+  auto ScopeIt = CheckedScopeMap.find(CurrStmt);
+  if (ScopeIt != CheckedScopeMap.end())
+    CSS = ScopeIt->second;
+}
+
 Expr *BoundsWideningUtil::IgnoreCasts(const Expr *E) const {
   return Lex.IgnoreValuePreservingOperations(Ctx, const_cast<Expr *>(E));
 }

clang/lib/Sema/CheckedCAnalysesPrepass.cpp

@@ -277,6 +277,52 @@ class PrepassHelper : public RecursiveASTVisitor<PrepassHelper> {
       return ProcessWhereClause(S->getWhereClause());
     }
 
+    bool VisitCompoundStmt(CompoundStmt *CS) {
+      if (!CS)
+        return true;
+
+      // Get the checked scope specifier for the current compound statement.
+      CheckedScopeSpecifier CSS = CS->getCheckedSpecifier();
+
+      bool FirstStmt = true;
+
+      // A compound statement is of the form {stmt stmt}. Iterate through the
+      // nested statements of the compound statement.
+      for (auto I = CS->body_begin(), E = CS->body_end(); I != E; ++I) {
+        const Stmt *CurrStmt = *I;
+
+        // If this is a compound statement the RecursiveASTVisitor will visit
+        // it later. So we don't need to process it now.
+        if (isa<CompoundStmt>(CurrStmt)) {
+          continue;
+
+        // Else if this is a label statement. Basic blocks may begin at label
+        // statements. We need to be able to update the CSS value correctly at
+        // the beginning of a basic block as we are assured of ordered lookup
+        // only within a basic block (i.e. the first statement of a basic block
+	// may be accessed out of statement-order). The AST only stores the
+	// sub-statement of a label statement. So accordingly we store the CSS
+	// for the sub-statement.
+	} else if (auto *L = dyn_cast<LabelStmt>(CurrStmt)) {
+          Info.CheckedScopeMap[L->getSubStmt()] = CSS;
+
+        // Else if this is the first statement in a compound statement store
+        // its checked scope specifier.
+        } else if (FirstStmt) {
+          Info.CheckedScopeMap[CurrStmt] = CSS;
+
+        // Else if the previous statement was a compound statement store the
+        // checked scope specifier of the current statement.
+        } else if (isa<CompoundStmt>(*(I - 1))) {
+          Info.CheckedScopeMap[CurrStmt] = CSS;
+        }
+
+        FirstStmt = false;
+      }
+
+      return true;
+    }
+
     void DumpBoundsVars(FunctionDecl *FD) {
       PrintDeclMap<const VarDecl *>(FD, "BoundsVars Lower",
                                     Info.BoundsVarsLower);

clang/lib/Sema/SemaBounds.cpp

@@ -2576,7 +2576,8 @@ namespace {
       Facts(Facts),
       BoundsWideningAnalyzer(BoundsWideningAnalysis(SemaRef, Cfg,
                                                     Info.BoundsVarsLower,
-                                                    Info.BoundsVarsUpper)),
+                                                    Info.BoundsVarsUpper,
+                                                    Info.CheckedScopeMap)),
       AbstractSetMgr(AbstractSetManager(SemaRef, Info.VarUses)),
       BoundsSiblingFields(Info.BoundsSiblingFields),
       IncludeNullTerminator(false) {}
@@ -2593,7 +2594,8 @@ namespace {
       Facts(Facts),
       BoundsWideningAnalyzer(BoundsWideningAnalysis(SemaRef, nullptr,
                                                     Info.BoundsVarsLower,
-                                                    Info.BoundsVarsUpper)),
+                                                    Info.BoundsVarsUpper,
+                                                    Info.CheckedScopeMap)),
       AbstractSetMgr(AbstractSetManager(SemaRef, Info.VarUses)),
       BoundsSiblingFields(Info.BoundsSiblingFields),
       IncludeNullTerminator(false) {}