Bounds checking bounds-safe interfaces in unchecked scopes (#1169)

* Add LValuesAssignedChecked member to CheckedState

LValuesAssignedChecked contains AbstractSets representing lvalues expressions that have unchecked pointer type that were assigned a checked pointer during the current top-level statement (if the statement is in an unchecked scope). AbstractSets in LValuesAssignedChecked should have their bounds validated after checking the current statement.

* Add SkipBoundsValidation method

* Remove expected error for lvalue with a bounds-safe interface in an unchecked scope

* Put declaration of short int with declared bounds in a checked scope so it results in a warning

* Update unchecked pointer inverse test so there is a checked pointer with declared bounds

* Add tests for validating the bounds of an unchecked pointer with a bounds-safe interface

* Add test for assigning a checked array to p

* Add tests for an integer-typed variable with declared bounds

* Fix expected warning to work on both Windows and Linux

* Fix typo in comment

Author: kkjeer

clang/lib/Sema/SemaBounds.cpp

@@ -579,13 +579,25 @@ namespace {
       // then be used to validate the bounds context.
       llvm::DenseMap<Expr *, Expr *> TargetSrcEquality;
 
+      // LValuesAssignedChecked is a set of AbstractSets containing lvalue
+      // expressions with unchecked pointer type that have been assigned an
+      // expression with checked pointer type at some point during the current
+      // top-level statement (if the statement occurs in an unchecked scope).
+      // These AbstractSets should have their bounds validated during
+      // ValidateBoundsContext. In an unchecked scope, AbstractSets containing
+      // lvalue expressions with unchecked pointer type that have not been
+      // assigned an expression with checked pointer type during the current
+      // statement should not have their bounds validated.
+      AbstractSetSetTy LValuesAssignedChecked;
+
       // Resets the checking state after checking a top-level CFG statement.
       void Reset() {
         SameValue.clear();
         LostLValues.clear();
         UnknownSrcBounds.clear();
         BlameAssignments.clear();
         TargetSrcEquality.clear();
+        LValuesAssignedChecked.clear();
       }
   };
 }
@@ -4429,6 +4441,8 @@ namespace {
           this->S.GetLValueDeclaredBounds(A->GetRepresentative(), CSS);
         if (!DeclaredBounds || DeclaredBounds->isUnknown())
           continue;
+        if (SkipBoundsValidation(A, CSS, State))
+          continue;
         if (ObservedBounds->isUnknown())
           DiagnoseUnknownObservedBounds(S, A, DeclaredBounds, State);
         else {
@@ -4447,6 +4461,33 @@ namespace {
       }
     }
 
+    // SkipBoundsValidation returns true if the observed bounds of A should
+    // not be validated after the current top-level statement.
+    //
+    // The bounds of A should not be validated if the current statement is
+    // in an unchecked scope, and A represents lvalue expressions that:
+    // 1. Have unchecked type (i.e. their declared bounds were specified using
+    //    a bounds-safe interface), and:
+    // 2. Were not assigned a checked pointer at any point in the current
+    //    top-level statement.
+    bool SkipBoundsValidation(const AbstractSet *A, CheckedScopeSpecifier CSS,
+                              CheckingState State) {
+      if (CSS != CheckedScopeSpecifier::CSS_Unchecked)
+        return false;
+
+      if (A->GetRepresentative()->getType()->isCheckedPointerType() ||
+          A->GetRepresentative()->getType()->isCheckedArrayType())
+        return false;
+
+      // State.LValuesAssignedChecked contains AbstractSets with unchecked type
+      // that were assigned a checked pointer at some point in the current
+      // statement. If A belongs to this set, we must validate the bounds of A.
+      if (State.LValuesAssignedChecked.contains(A))
+        return false;
+
+      return true;
+    }
+
     // DiagnoseUnknownObservedBounds emits an error message for an AbstractSet
     // A whose observed bounds are unknown after checking the top-level CFG
     // statement St.
@@ -4822,6 +4863,21 @@ namespace {
       if (HasTargetBounds) {
         LValueAbstractSet = AbstractSetMgr.GetOrCreateAbstractSet(LValue);
         State.ObservedBounds[LValueAbstractSet] = SrcBounds;
+
+        // In an unchecked scope, if an expression with checked pointer type
+        // is assigned to an unchecked pointer LValue (whose bounds are
+        // declared via a bounds-safe interface), bounds validation should
+        // validate the bounds of LValue after the current top-level statement.
+        // For unchecked pointer LValues that were not assigned a checked
+        // pointer expression during the current top-level statement, bounds
+        // validation should skip validating the bounds of LValue.
+        if (CSS == CheckedScopeSpecifier::CSS_Unchecked) {
+          if (!LValue->getType()->isCheckedPointerType() &&
+              !LValue->getType()->isCheckedArrayType()) {
+            if (IsBoundsSafeInterfaceAssignment(LValue->getType(), Src))
+              State.LValuesAssignedChecked.insert(LValueAbstractSet);
+          }
+        }
       }
 
       // If Src initially has unknown bounds (before making any lvalue

clang/test/CheckedC/inferred-bounds/member-reference.c

@@ -348,7 +348,7 @@ int global_arr2 _Checked[5];
 void f12(struct Interop_S1 a1) {
   // TODO: need bundled block.
   a1.p = global_arr2;
-  a1.len = 5; // expected-error {{inferred bounds for 'a1.p' are unknown after assignment}}
+  a1.len = 5;
 }
 
 // CHECK: BinaryOperator {{0x[0-9a-f]+}} {{.*}} 'int *' '='

clang/test/CheckedC/static-checking/bounds-decl-checking.c

@@ -760,9 +760,9 @@ void f91(_Array_ptr<int> p : count(1)) _Unchecked { // expected-note {{(expanded
        // expected-note {{(expanded) inferred bounds are 'bounds(p - 1, p - 1 + 1)'}}
 }
 
-void f92(int *p : count(1)) _Unchecked { // expected-note {{(expanded) declared bounds are 'bounds(p, p + 1)'}}
- ++p; // expected-warning {{cannot prove declared bounds for 'p' are valid after increment}} \
-      // expected-note {{(expanded) inferred bounds are 'bounds(p - 1, p - 1 + 1)'}}
+_Unchecked void f92(_Array_ptr<int> p : bounds(q, q + 1), int *q) { // expected-note {{(expanded) declared bounds are 'bounds(q, q + 1)'}}
+ ++q; // expected-warning {{cannot prove declared bounds for 'p' are valid after increment}} \
+      // expected-note {{(expanded) inferred bounds are 'bounds(q - 1, q - 1 + 1)'}}
 }
 
 //
@@ -816,3 +816,73 @@ void f96(_Nt_array_ptr<char> p : bounds(p, (p + (len + 4)) + 4), int len) {
                                                               // expected-note {{(expanded) declared bounds are 'bounds(v, (v + (1 + len)) + 2)'}} \
                                                               // expected-note {{(expanded) inferred bounds are 'bounds(p, (p + (len + 4)) + 4)'}}
 }
+
+//
+// Test validating bounds for unchecked pointer with bounds-safe interfaces
+// in unchecked and checked scopes.
+//
+
+_Unchecked extern int *get_unchecked(_Array_ptr<int> a);
+extern _Array_ptr<int> get_checked(unsigned int i) : count(i);
+
+_Unchecked void f97(int *p : count(i), // expected-note 8 {{(expanded) declared bounds are 'bounds(p, p + i)'}}
+                    int *q : bounds(unknown),
+                    _Array_ptr<int> r,
+                    unsigned int i) {
+  // For statements where a checked pointer is not assigned to p, do not
+  // validate the bounds of p.
+  i = 0;
+  p++;
+  p = q;
+  q = r, p = q;
+  p = get_unchecked(r);
+  p += i;
+
+  // The type of the RHS expression p - (_Array_ptr<int>)q is int *, so a
+  // checked pointer is not assigned to p here.
+  p -= (_Array_ptr<int>)q; // expected-warning {{incompatible integer to pointer conversion assigning to 'int *'}}
+
+  // For statements where a checked pointer is assigned to p, validate the
+  // bounds of p.
+  p = (_Array_ptr<int>)(p - q); // expected-error {{inferred bounds for 'p' are unknown after assignment}} \
+                                // expected-note {{assigned expression '(_Array_ptr<int>)(p - q)' with unknown bounds to 'p'}}
+
+  p = r; // expected-error {{inferred bounds for 'p' are unknown after assignment}} \
+         // expected-note {{assigned expression 'r' with unknown bounds to 'p'}}
+
+  p = (_Array_ptr<int>)q; // expected-error {{inferred bounds for 'p' are unknown after assignment}} \
+                          // expected-note {{assigned expression '(_Array_ptr<int>)q' with unknown bounds to 'p'}}
+
+  p = get_checked(i), i++; // expected-warning {{cannot prove declared bounds for 'p' are valid after increment}} \
+                           // expected-note {{(expanded) inferred bounds are 'bounds(value of get_checked(i), value of get_checked(i) + i - 1U)'}}
+
+  p = (_Array_ptr<int>)p, --p; // expected-warning {{cannot prove declared bounds for 'p' are valid after decrement}} \
+                               // expected-note {{'bounds((int *)p + 1, (int *)p + 1 + i)'}}
+
+  int arr _Checked[3] : count(3) = {0, 1, 2};
+  p = arr; // expected-error {{it is not possible to prove that the inferred bounds of 'p' imply the declared bounds of 'p' after assignment}} \
+           // expected-note {{the declared upper bounds use the variable 'i' and there is no relational information involving 'i' and any of the expressions used by the inferred upper bounds}} \
+           // expected-note {{(expanded) inferred bounds are 'bounds(arr, arr + 3)'}}
+  
+  // In a checked scope, always validate the bounds of p.
+  _Checked {
+    i = 1; // expected-error {{inferred bounds for 'p' are unknown after assignment}} \
+           // expected-note {{lost the value of the expression 'i' which is used in the (expanded) inferred bounds 'bounds(p, p + i)' of 'p'}}
+
+    ++p; // expected-warning {{cannot prove declared bounds for 'p' are valid after increment}} \
+         // expected-note {{(expanded) inferred bounds are 'bounds(p - 1, p - 1 + i)'}}
+  }
+
+  // Non-pointer-typed values with declared bounds do not have their bounds
+  // validated in unchecked scopes.
+  short int t1 : byte_count(2) = (short int)arr; // expected-warning {{cast to smaller integer type 'short' from '_Array_ptr<int>'}}
+  t1 = (short int)r; // expected-warning {{cast to smaller integer type 'short' from '_Array_ptr<int>'}}
+
+  // Non-pointer-typed values with declared bounds have their bounds
+  // validated in checked scopes.
+  _Checked {
+    short int t2 : byte_count(2) = (short int)r; // expected-error {{inferred bounds for 't2' are unknown after initialization}} \
+                                                 // expected-note {{(expanded) declared bounds are 'bounds((_Array_ptr<char>)t2, (_Array_ptr<char>)t2 + 2)'}} \
+                                                 // expected-warning {{cast to smaller integer type 'short' from '_Array_ptr<int>'}}
+  }
+}

clang/test/CheckedC/static-checking/free-variables.c

@@ -133,10 +133,13 @@ void f3(struct S1 a3) {
 void f4(void) {
   int a checked[5];
   // Check that parentheses are correctly ignored.
-  short int t1 : byte_count(5 * sizeof(int)) = ((((short int)(a)))); // expected-warning {{cast to smaller integer type 'short' from '_Array_ptr<int>'}} \
-                                                                     // expected-warning {{cannot prove declared bounds for 't1' are valid after initialization}} \
-                                                                     // expected-note {{(expanded) declared bounds are 'bounds((_Array_ptr<char>)t1, (_Array_ptr<char>)t1 + 5 * sizeof(int))'}} \
-                                                                     // expected-note {{(expanded) inferred bounds are 'bounds(a, a + 5)'}}
+  _Checked {
+    short int t1 : byte_count(5 * sizeof(int)) = ((((short int)(a)))); // expected-warning {{cast to smaller integer type 'short' from '_Array_ptr<int>'}} \
+                                                                       // expected-warning {{cannot prove declared bounds for 't1' are valid after initialization}} \
+                                                                       // expected-note {{(expanded) declared bounds are 'bounds((_Array_ptr<char>)t1, (_Array_ptr<char>)t1 + 5 * sizeof(int))'}} \
+                                                                       // expected-note {{(expanded) inferred bounds are 'bounds(a, a + 5)'}}
+  }
+
   array_ptr<int> t2 : byte_count(5 * sizeof(int)) = (((a)));
   array_ptr<int> t3 : byte_count(5 * sizeof(int)) = (((array_ptr<int>)(((a)))));