diff --git a/clang/lib/Sema/BoundsUtils.cpp b/clang/lib/Sema/BoundsUtils.cpp index 5854f858384e..e2d45674af06 100644 --- a/clang/lib/Sema/BoundsUtils.cpp +++ b/clang/lib/Sema/BoundsUtils.cpp @@ -186,6 +186,8 @@ BoundsExpr *BoundsUtil::ExpandToRange(Sema &S, VarDecl *D, BoundsExpr *B) { BoundsExpr *BoundsUtil::ReplaceLValueInBounds(Sema &S, BoundsExpr *Bounds, Expr *LValue, Expr *OriginalValue, CheckedScopeSpecifier CSS) { + if (Bounds->isUnknown() || Bounds->isAny()) + return Bounds; Expr *Replaced = ReplaceLValue(S, Bounds, LValue, OriginalValue, CSS); if (!Replaced) return CreateBoundsUnknown(S); diff --git a/clang/lib/Sema/SemaBounds.cpp b/clang/lib/Sema/SemaBounds.cpp index 1dc9660d3579..59edebff6eda 100644 --- a/clang/lib/Sema/SemaBounds.cpp +++ b/clang/lib/Sema/SemaBounds.cpp @@ -549,12 +549,12 @@ namespace { llvm::DenseMap> LostLValues; // UnknownSrcBounds maps an AbstractSet A whose observed bounds are - // unknown to a set of expressions with unknown bounds that have been - // assigned to A. + // unknown to the first expression with unknown bounds (if any) that + // has been assigned to an lvalue expression in A. // // UnknownSrcBounds is used to emit notes to provide more context to the // user when diagnosing unknown bounds errors. - llvm::DenseMap> UnknownSrcBounds; + llvm::DenseMap UnknownSrcBounds; // BlameAssignments maps an AbstractSet A to an expression in a top-level // CFG statement that last updates any variable used in the declared @@ -4521,14 +4521,14 @@ namespace { // The observed bounds of A are unknown because at least one expression // e with unknown bounds was assigned to an lvalue expression in A. + // Emit a note for the first expression with unknown bounds that was + // assigned to A (this expression is the only one that is tracked in + // State.UnknownSrcBounds). auto BlameSrcIt = State.UnknownSrcBounds.find(A); if (BlameSrcIt != State.UnknownSrcBounds.end()) { - SmallVector UnknownSources = BlameSrcIt->second; - for (auto I = UnknownSources.begin(); I != UnknownSources.end(); ++I) { - Expr *Src = *I; - S.Diag(Src->getBeginLoc(), diag::note_unknown_source_bounds) + Expr *Src = BlameSrcIt->second; + S.Diag(Src->getBeginLoc(), diag::note_unknown_source_bounds) << Src << A->GetRepresentative() << Src->getSourceRange(); - } } } @@ -4860,8 +4860,10 @@ namespace { // If LValue has target bounds, the initial observed bounds of LValue // are SrcBounds. These bounds will be updated to account for any uses // of LValue below. + BoundsExpr *PrevLValueBounds = nullptr; if (HasTargetBounds) { LValueAbstractSet = AbstractSetMgr.GetOrCreateAbstractSet(LValue); + PrevLValueBounds = State.ObservedBounds[LValueAbstractSet]; State.ObservedBounds[LValueAbstractSet] = SrcBounds; // In an unchecked scope, if an expression with checked pointer type @@ -4880,14 +4882,6 @@ namespace { } } - // If Src initially has unknown bounds (before making any lvalue - // replacements), use Src to explain bounds checking errors that - // can occur when validating the bounds context. - if (HasTargetBounds) { - if (SrcBounds->isUnknown()) - State.UnknownSrcBounds[LValueAbstractSet].push_back(Src); - } - // Adjust ObservedBounds to account for any uses of LValue in the bounds. for (auto const &Pair : State.ObservedBounds) { const AbstractSet *A = Pair.first; @@ -4921,10 +4915,22 @@ namespace { BoundsUtil::ReplaceLValueInBounds(S, SrcBounds, LValue, OriginalValue, CSS); - // Record that E updates the observed bounds of LValue. - if (HasTargetBounds) + // If the updated observed bounds of LValue are different than the + // previous observed bounds of LValue, record that E updates the + // observed bounds of LValue. + // We can check this cheaply because ReplaceLValueInBounds returns + // PrevLValueBounds as AdjustedSrcBounds if the previous observed + // bounds of LValue were not adjusted. + if (HasTargetBounds && PrevLValueBounds != AdjustedSrcBounds) { State.BlameAssignments[LValueAbstractSet] = E; + // If the original bounds of Src (before replacing LValue) were + // unknown, record that the expression Src with unknown bounds was + // assigned to LValue. + if (SrcBounds->isUnknown()) + State.UnknownSrcBounds[LValueAbstractSet] = Src; + } + // If the initial source bounds were not unknown, but they are unknown // after replacing uses of LValue, then the assignment to LValue caused // the source bounds (which are the observed bounds for LValue) to be diff --git a/clang/test/CheckedC/inferred-bounds/bounds-context.c b/clang/test/CheckedC/inferred-bounds/bounds-context.c index bcbc674c9402..e36bb17631d3 100644 --- a/clang/test/CheckedC/inferred-bounds/bounds-context.c +++ b/clang/test/CheckedC/inferred-bounds/bounds-context.c @@ -1138,7 +1138,6 @@ void multiple_assign1( // Observed bounds context after assignments: { a => bounds(unknown), b => bounds(unknown) } len = 0, a = b; // expected-error {{inferred bounds for 'a' are unknown after assignment}} \ // expected-note {{lost the value of the expression 'len' which is used in the (expanded) inferred bounds 'bounds(a, a + len)' of 'a'}} \ - // expected-note {{assigned expression 'b' with unknown bounds to 'a'}} \ // expected-error {{inferred bounds for 'b' are unknown after assignment}} \ // expected-note {{lost the value of the expression 'len' which is used in the (expanded) inferred bounds 'bounds(b, b + len)' of 'b'}} // CHECK: Statement S: diff --git a/clang/test/CheckedC/static-checking/bounds-decl-checking.c b/clang/test/CheckedC/static-checking/bounds-decl-checking.c index 4bdb94ab3765..ccf173fbd48e 100644 --- a/clang/test/CheckedC/static-checking/bounds-decl-checking.c +++ b/clang/test/CheckedC/static-checking/bounds-decl-checking.c @@ -817,6 +817,35 @@ void f96(_Nt_array_ptr p : bounds(p, (p + (len + 4)) + 4), int len) { // expected-note {{(expanded) inferred bounds are 'bounds(p, (p + (len + 4)) + 4)'}} } +// +// Test diagnostic behavior for bounds checking across multiple assignments. +// + +void f97(_Array_ptr p : count(i), // expected-note 4 {{(expanded) declared bounds are 'bounds(p, p + i)'}} + unsigned int i, + _Array_ptr unknwn1, + _Array_ptr unknwn2) { + p = unknwn1, p = p; // expected-error {{inferred bounds for 'p' are unknown after assignment}} \ + // expected-note {{assigned expression 'unknwn1' with unknown bounds to 'p'}} + + p = unknwn1, p++; // expected-error {{inferred bounds for 'p' are unknown after assignment}} \ + // expected-note {{assigned expression 'unknwn1' with unknown bounds to 'p'}} + + p = unknwn1, p = unknwn2; // expected-error {{inferred bounds for 'p' are unknown after assignment}} \ + // expected-note {{assigned expression 'unknwn1' with unknown bounds to 'p'}} + + p++, i = 0, p = unknwn1; // 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 - 1, p - 1 + i)' of 'p'}} +} + +void f98(_Array_ptr p : count(i), unsigned int i) { // expected-note 2 {{(expanded) declared bounds are 'bounds(p, p + i)'}} + i++, p += 2; // expected-warning {{cannot prove declared bounds for 'p' are valid after assignment}} \ + // expected-note {{(expanded) inferred bounds are 'bounds(p - 2, p - 2 + i)'}} + + p += 2, i++; // expected-warning {{cannot prove declared bounds for 'p' are valid after increment}} \ + // expected-note {{(expanded) inferred bounds are 'bounds(p - 2, p - 2 + i - 1U)'}} +} + // // Test validating bounds for unchecked pointer with bounds-safe interfaces // in unchecked and checked scopes. @@ -825,7 +854,7 @@ void f96(_Nt_array_ptr p : bounds(p, (p + (len + 4)) + 4), int len) { _Unchecked extern int *get_unchecked(_Array_ptr a); extern _Array_ptr 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)'}} +_Unchecked void f99(int *p : count(i), // expected-note 8 {{(expanded) declared bounds are 'bounds(p, p + i)'}} int *q : bounds(unknown), _Array_ptr r, unsigned int i) {