diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 9f2bcf9af367..260d85b19802 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5667,7 +5667,8 @@ class Sema final { BoundsExpr *MakeMemberBoundsConcrete(Expr *MemberBase, bool IsArrow, BoundsExpr *Bounds); BoundsExpr *ConcretizeFromFunctionTypeWithArgs(BoundsExpr *Bounds, ArrayRef Args, - NonModifyingContext ErrorKind); + NonModifyingContext ErrorKind, + NonModifyingMessage Message); /// ConvertToFullyCheckedType: convert an expression E to a fully checked type. This /// is used to retype declrefs and member exprs in checked scopes with bounds-safe diff --git a/clang/lib/Sema/SemaBounds.cpp b/clang/lib/Sema/SemaBounds.cpp index 374692def0de..7fb2d991258a 100644 --- a/clang/lib/Sema/SemaBounds.cpp +++ b/clang/lib/Sema/SemaBounds.cpp @@ -259,14 +259,17 @@ namespace { const ArrayRef Arguments; llvm::SmallBitVector VisitedArgs; Sema::NonModifyingContext ErrorKind; + Sema::NonModifyingMessage Message; bool ModifyingArg; public: CheckForModifyingArgs(Sema &SemaRef, ArrayRef Args, - Sema::NonModifyingContext ErrorKind) : + Sema::NonModifyingContext ErrorKind, + Sema::NonModifyingMessage Message) : SemaRef(SemaRef), Arguments(Args), VisitedArgs(Args.size()), ErrorKind(ErrorKind), + Message(Message), ModifyingArg(false) {} bool FoundModifyingArg() { @@ -277,8 +280,7 @@ namespace { unsigned index = E->getIndex(); if (index < Arguments.size() && !VisitedArgs[index]) { VisitedArgs.set(index); - if (!SemaRef.CheckIsNonModifying(Arguments[index], ErrorKind, - Sema::NonModifyingMessage::NMM_Error)) { + if (!SemaRef.CheckIsNonModifying(Arguments[index], ErrorKind, Message)) { ModifyingArg = true; } } @@ -313,11 +315,11 @@ namespace { BoundsExpr *Sema::ConcretizeFromFunctionTypeWithArgs( BoundsExpr *Bounds, ArrayRef Args, - NonModifyingContext ErrorKind) { + NonModifyingContext ErrorKind, NonModifyingMessage Message) { if (!Bounds || Bounds->isInvalid()) return Bounds; - auto CheckArgs = CheckForModifyingArgs(*this, Args, ErrorKind); + auto CheckArgs = CheckForModifyingArgs(*this, Args, ErrorKind, Message); CheckArgs.TraverseStmt(Bounds); if (CheckArgs.FoundModifyingArg()) return nullptr; @@ -3279,7 +3281,7 @@ namespace { BoundsExpr *CheckCallExpr(CallExpr *E, CheckedScopeSpecifier CSS, CheckingState &State, CHKCBindTemporaryExpr *Binding = nullptr) { - BoundsExpr *ResultBounds = CallExprBounds(E, Binding); + BoundsExpr *ResultBounds = CallExprBounds(E, Binding, CSS); QualType CalleeType = E->getCallee()->getType(); // Extract the pointee type. The caller type could be a regular pointer @@ -3367,7 +3369,8 @@ namespace { S.ConcretizeFromFunctionTypeWithArgs( const_cast(ParamBounds), ArgExprs, - Sema::NonModifyingContext::NMC_Function_Parameter); + Sema::NonModifyingContext::NMC_Function_Parameter, + Sema::NonModifyingMessage::NMM_Error); if (!SubstParamBounds) continue; @@ -5936,7 +5939,8 @@ namespace { // If ResultName is non-null, it is a temporary variable where the result // of the call expression is stored immediately upon return from the call. BoundsExpr *CallExprBounds(const CallExpr *CE, - CHKCBindTemporaryExpr *ResultName) { + CHKCBindTemporaryExpr *ResultName, + CheckedScopeSpecifier CSS) { BoundsExpr *ReturnBounds = nullptr; if (CE->getType()->isCheckedPointerPtrType()) { if (CE->getType()->isVoidPointerType()) @@ -5963,7 +5967,7 @@ namespace { BoundsExpr *FunBounds = FunReturnAnnots.getBoundsExpr(); InteropTypeExpr *IType =FunReturnAnnots.getInteropTypeExpr(); // If there is no return bounds and there is an interop type - // annotation, use the bounds impied by the interop type + // annotation, use the bounds implied by the interop type // annotation. if (!FunBounds && IType) FunBounds = CreateTypeBasedBounds(nullptr, IType->getType(), @@ -5978,10 +5982,22 @@ namespace { CE->getNumArgs()); // Concretize Call Bounds with argument expressions. - // We can only do this if the argument expressions are non-modifying + // We can only do this if the argument expressions are non-modifying. + // For argument expressions that are modifying, we issue an error + // message only in checked scope because the argument expressions may + // be re-evaluated during bounds validation. + // TODO: The long-term solution is to introduce temporaries for + // modifying argument expressions whose corresponding formals are used + // in return or parameter bounds expressions. + // Equality between the value computed by an argument expression and its + // associated temporary would also need to be recorded. + Sema::NonModifyingMessage Message = + (CSS == CheckedScopeSpecifier::CSS_Unchecked) ? + Sema::NonModifyingMessage::NMM_None : + Sema::NonModifyingMessage::NMM_Error; ReturnBounds = S.ConcretizeFromFunctionTypeWithArgs(FunBounds, ArgExprs, - Sema::NonModifyingContext::NMC_Function_Return); + Sema::NonModifyingContext::NMC_Function_Return, Message); // If concretization failed, this means we tried to substitute with // a non-modifying expression, which is not allowed by the // specification. diff --git a/clang/test/CheckedC/inferred-bounds/calls.c b/clang/test/CheckedC/inferred-bounds/calls.c index 44afe6601421..8073558ee97c 100644 --- a/clang/test/CheckedC/inferred-bounds/calls.c +++ b/clang/test/CheckedC/inferred-bounds/calls.c @@ -169,7 +169,6 @@ void f10(_Array_ptr a, _Array_ptr b) { void f11(_Array_ptr a, _Array_ptr b) { _Array_ptr c : bounds(a, a+1) = f_bounds(b++, a); // \ - // expected-error {{expression not allowed in argument for parameter used in function return bounds}} \ // expected-error {{inferred bounds for 'c' are unknown after initialization}} \ // expected-note {{(expanded) declared bounds are 'bounds(a, a + 1)'}} } @@ -230,7 +229,6 @@ void f12(int i, int j) { void f13(int i, int j) { _Array_ptr b : count(i) = f_count(j++, i); // \ - // expected-error {{expression not allowed in argument for parameter used in function return bounds}} \ // expected-error {{inferred bounds for 'b' are unknown after initialization}} \ // expected-note {{(expanded) declared bounds are 'bounds(b, b + i)'}} } @@ -285,7 +283,6 @@ void f14(int i, int j) { void f15(int i, int j) { _Array_ptr b : byte_count(i) = f_byte(j++, i); // \ - // expected-error {{expression not allowed in argument for parameter used in function return bounds}} \ // expected-error {{inferred bounds for 'b' are unknown after initialization}} \ // expected-note {{(expanded) declared bounds are 'bounds((_Array_ptr)b, (_Array_ptr)b + i)'}} } @@ -349,7 +346,6 @@ void f20(int* a, int* b) { void f21(int* a, int* b) { _Array_ptr c : bounds(a, a+1) = f_boundsi(b++, a); // \ - // expected-error {{expression not allowed in argument for parameter used in function return bounds}} \ // expected-error {{inferred bounds for 'c' are unknown after initialization}} \ // expected-note {{(expanded) declared bounds are 'bounds(a, a + 1)'}} } @@ -412,7 +408,6 @@ void f22(int i, int j) { void f23(int i, int j) { _Array_ptr b : count(i) = f_counti(j++, i); // \ - // expected-error {{expression not allowed in argument for parameter used in function return bounds}} \ // expected-error {{inferred bounds for 'b' are unknown after initialization}} \ // expected-note {{(expanded) declared bounds are 'bounds(b, b + i)'}} } @@ -469,7 +464,6 @@ void f24(int i, int j) { void f25(int i, int j) { _Array_ptr b : byte_count(i) = f_bytei(j++, i); // \ - // expected-error {{expression not allowed in argument for parameter used in function return bounds}} \ // expected-error {{inferred bounds for 'b' are unknown after initialization}} \ // expected-note {{(expanded) declared bounds are 'bounds((_Array_ptr)b, (_Array_ptr)b + i)'}} } diff --git a/clang/test/CheckedC/static-checking/bounds-side-effects.c b/clang/test/CheckedC/static-checking/bounds-side-effects.c index a1a33f3c6ceb..1d2e10694285 100644 --- a/clang/test/CheckedC/static-checking/bounds-side-effects.c +++ b/clang/test/CheckedC/static-checking/bounds-side-effects.c @@ -227,3 +227,87 @@ void f105(int len, _Array_ptr p : count(len), int i) { len += 1, p = alloc(len * sizeof(int)); } +extern int memcmp_test(const void *src1 : byte_count(n), + const void *src2 :byte_count(n), unsigned int n); +extern void *memchr_test(const void *s : byte_count(n), int c, unsigned int n) : + bounds(s, (_Array_ptr) s + n); +extern _Itype_for_any(T) void *malloc_test(unsigned int size) : + itype(_Array_ptr) byte_count(size); + +// Checked pointers in checked scope +void f106() +_Checked +{ + int len = 4; + _Nt_array_ptr p : count(5) = "hello"; + int i = memcmp_test(p, "hello", ++len); // expected-error {{increment expression not allowed in argument for parameter used in function parameter bounds expression}} \ + // expected-error {{increment expression not allowed in argument for parameter used in function parameter bounds expression}} +} + +// Checked pointers in unchecked scope +void f106_u1() +{ + int len = 4; + _Nt_array_ptr p : count(5) = "hello"; + int i = memcmp_test(p, "hello", ++len); // expected-error {{increment expression not allowed in argument for parameter used in function parameter bounds expression}} +} + +// Unchecked pointers in unchecked scope +void f106_u2() +{ + int len = 4; + char *p = "hello"; + int i = memcmp_test(p, "hello", ++len); +} + +// Checked pointers in checked scope +void f107() +_Checked +{ + int len = 4; + int p _Checked [5] = {'h', 'e', 'l', 'l', 'o'}; + _Array_ptr pos = memchr_test(p, 'l', ++len); // expected-error {{increment expression not allowed in argument for parameter used in function return bounds expression}} \ + // expected-error {{increment expression not allowed in argument for parameter used in function parameter bounds expression}} +} + +// Checked pointers in unchecked scope +void f107_u1() +{ + int len = 4; + int p _Checked [5] = {'h', 'e', 'l', 'l', 'o'}; + // Ideally, there should be an error for modifying expressions + // used in the return bounds expression also. + _Array_ptr pos = memchr_test(p, 'l', ++len); // expected-error {{increment expression not allowed in argument for parameter used in function parameter bounds expression}} +} + +// Unchecked pointers in unchecked scope +void f107_u2() +{ + int len = 4; + int p[5] = {'h', 'e', 'l', 'l', 'o'}; + int *pos = memchr_test(p, 'l', ++len); +} + +// Checked pointers in checked scope +void f108() +_Checked +{ + int len = 5; + _Array_ptr p = malloc_test(++len); // expected-error {{increment expression not allowed in argument for parameter used in function return bounds expression}} +} + +// Checked pointers in unchecked scope +void f108_u1() +{ + int len = 5; + // Ideally, there should be an error for modifying expressions + // used in the return bounds expression also. + _Array_ptr p = malloc_test(++len); +} + +// Unchecked pointers in unchecked scope +void f108_u2() +{ + int len = 5; + char *p = malloc_test(++len); +}