Check that variables and member expressions used in return bounds are unmodified

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -11619,6 +11619,12 @@ def err_bounds_type_annotation_lost_checking : Error<
   def error_static_cast_bounds_invalid : Error<
     "cast source bounds are too narrow for %0">;
 
+  def error_modified_return_bounds : Error<
+    "modified expression '%0' used in the declared return bounds for %1">;
+
+  def note_declared_return_bounds : Note<
+    "(expanded) declared return bounds are '%0'">;
+
   def error_out_of_bounds_access : Error<
     "out-of-bounds %select{||memory access|base value}0">;
 

clang/lib/Sema/SemaBounds.cpp

@@ -652,8 +652,20 @@ namespace {
     uint64_t PointerWidth;
     Stmt *Body;
     CFG *Cfg;
-    BoundsExpr *ReturnBounds; // return bounds expression for enclosing
-                              // function, if any.
+
+    // Declaration for enclosing function. Having this here allows us to emit
+    // the name of the function in any diagnostic message when checking return
+    // bounds.
+    FunctionDecl *FunctionDeclaration;
+    // Return value expression for enclosing function, if any. Having this
+    // here allows us to avoid reconstructing a return value for each
+    // return statement.
+    BoundsValueExpr *ReturnVal;
+    // Expanded declared return bounds expression for enclosing function, if
+    // any. Having this here allows us to avoid re-expanding the return bounds
+    // for each return statement.
+    BoundsExpr *ReturnBounds;
+
     ASTContext &Context;
     std::pair<ComparisonSet, ComparisonSet> &Facts;
 
@@ -2569,22 +2581,33 @@ namespace {
 
 
   public:
-    CheckBoundsDeclarations(Sema &SemaRef, PrepassInfo &Info, Stmt *Body, CFG *Cfg, BoundsExpr *ReturnBounds, std::pair<ComparisonSet, ComparisonSet> &Facts) : S(SemaRef),
+    CheckBoundsDeclarations(Sema &SemaRef, PrepassInfo &Info, Stmt *Body, CFG *Cfg, FunctionDecl *FD, std::pair<ComparisonSet, ComparisonSet> &Facts) : S(SemaRef),
       DumpBounds(SemaRef.getLangOpts().DumpInferredBounds),
       DumpState(SemaRef.getLangOpts().DumpCheckingState),
       DumpSynthesizedMembers(SemaRef.getLangOpts().DumpSynthesizedMembers),
       PointerWidth(SemaRef.Context.getTargetInfo().getPointerWidth(0)),
       Body(Body),
       Cfg(Cfg),
-      ReturnBounds(ReturnBounds),
+      FunctionDeclaration(FD),
+      ReturnVal(nullptr),
+      ReturnBounds(nullptr),
       Context(SemaRef.Context),
       Facts(Facts),
       BoundsWideningAnalyzer(BoundsWideningAnalysis(SemaRef, Cfg,
                                                     Info.BoundsVarsLower,
                                                     Info.BoundsVarsUpper)),
       AbstractSetMgr(AbstractSetManager(SemaRef, Info.VarUses)),
       BoundsSiblingFields(Info.BoundsSiblingFields),
-      IncludeNullTerminator(false) {}
+      IncludeNullTerminator(false) {
+        if (FD) {
+          ReturnVal =
+            new (S.Context) BoundsValueExpr(SourceLocation(),
+                                            FD->getReturnType(),
+                                            BoundsValueExpr::Kind::Return);
+          ReturnBounds =
+            BoundsUtil::ExpandToRange(S, ReturnVal, FD->getBoundsExpr());
+        }
+      }
 
     CheckBoundsDeclarations(Sema &SemaRef, PrepassInfo &Info, std::pair<ComparisonSet, ComparisonSet> &Facts) : S(SemaRef),
       DumpBounds(SemaRef.getLangOpts().DumpInferredBounds),
@@ -2593,6 +2616,8 @@ namespace {
       PointerWidth(SemaRef.Context.getTargetInfo().getPointerWidth(0)),
       Body(nullptr),
       Cfg(nullptr),
+      FunctionDeclaration(nullptr),
+      ReturnVal(nullptr),
       ReturnBounds(nullptr),
       Context(SemaRef.Context),
       Facts(Facts),
@@ -4526,6 +4551,10 @@ namespace {
         return SrcBounds;
       }
 
+      // Account for uses of LValue in the declared return bounds (if any)
+      // for the enclosing function.
+      UpdateReturnBoundsAfterAssignment(LValue, E, Src, CSS);
+
       // Get the original value (if any) of LValue before the assignment,
       // and determine whether the original value uses the value of LValue.
       // OriginalValue is named OV in the Checked C spec.
@@ -4552,6 +4581,47 @@ namespace {
       return ResultBounds;
     }
 
+    // UpdateReturnBoundsAfterAssignment computes the observed bounds for the
+    // return value of the enclosing function (if any) by replacing all
+    // uses of LValue within the declared return bounds with null. If the
+    // declared return bounds use the value of LValue, the observed return
+    // bounds will be bounds(unknown) and an error will be emitted.
+    //
+    // The current implementation does not use invertibility of lvalue
+    // expressions. This simplifies the implementation so that the updated
+    // return bounds can be computed locally at each assignment statement,
+    // rather than keeping track of a return bounds expression across the
+    // entire function body.
+    // TODO: track an observed return bounds expression as a global property
+    // of the function body so that invertibility of lvalue expressions can
+    // be taken into account.
+    void UpdateReturnBoundsAfterAssignment(Expr *LValue, Expr *E, Expr *Src,
+                                           CheckedScopeSpecifier CSS) {
+      if (!ReturnBounds || ReturnBounds->isUnknown() || ReturnBounds->isAny())
+        return;
+
+      // In unchecked scopes, if the enclosing function has its bounds declared
+      // via a bounds-safe interface, we do not check that expressions used in
+      // the declared function bounds are not modified.
+      if (CSS == CheckedScopeSpecifier::CSS_Unchecked) {
+        if (FunctionDeclaration->hasBoundsSafeInterface(Context))
+          return;
+      }
+
+      BoundsExpr *UpdatedReturnBounds =
+        BoundsUtil::ReplaceLValueInBounds(S, ReturnBounds, LValue,
+                                          nullptr, CSS);
+      if (!UpdatedReturnBounds->isUnknown())
+        return;
+
+      S.Diag(LValue->getBeginLoc(), diag::error_modified_return_bounds)
+          << LValue << FunctionDeclaration << E->getSourceRange();
+
+      SourceLocation DeclaredLoc = FunctionDeclaration->getLocation();
+      S.Diag(DeclaredLoc, diag::note_declared_return_bounds)
+        << ReturnBounds << ReturnBounds->getSourceRange();
+    }
+
     // UpdateBoundsAfterAssignment updates the observed bounds context after
     // an lvalue expression LValue is modified via an assignment E of the form
     // LValue = Src, based on the state before the assignment.
@@ -6167,7 +6237,7 @@ void Sema::CheckFunctionBodyBoundsDecls(FunctionDecl *FD, Stmt *Body) {
   BO.AddLifetime = true;
   BO.AddNullStmt = true;
   std::unique_ptr<CFG> Cfg = CFG::buildCFG(nullptr, Body, &getASTContext(), BO);
-  CheckBoundsDeclarations Checker(*this, Info, Body, Cfg.get(), FD->getBoundsExpr(), EmptyFacts);
+  CheckBoundsDeclarations Checker(*this, Info, Body, Cfg.get(), FD, EmptyFacts);
   if (Cfg != nullptr) {
     AvailableFactsAnalysis Collector(*this, Cfg.get());
     Collector.Analyze();