diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -208,6 +208,8 @@ underlying type is ``long long`` or ``unsigned long long`` as an extension in C89 mode . Clang previously only diagnosed if the literal had an explicit ``LL`` suffix. +- Clang now correctly diagnoses index that refers past the last possible element + of FAM-like arrays. Non-comprehensive list of changes in this release ------------------------------------------------- diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -15898,17 +15898,24 @@ << TRange << Op->getSourceRange(); } -/// Check whether this array fits the idiom of a size-one tail padded -/// array member of a struct. +/// Check whether this array fits the idiom of a flexible array member, +/// depending on the value of -fstrict-flex-array. /// -/// We avoid emitting out-of-bounds access warnings for such arrays as they are -/// commonly used to emulate flexible arrays in C89 code. -static bool IsTailPaddedMemberArray(Sema &S, const llvm::APInt &Size, - const NamedDecl *ND, - unsigned StrictFlexArraysLevel) { +/// We avoid emitting out-of-bounds access warnings for such arrays. +static bool isFlexibleArrayMemberExpr(Sema &S, const Expr *E, + const NamedDecl *ND, + unsigned StrictFlexArraysLevel) { + if (!ND) return false; + const ConstantArrayType *ArrayTy = + S.Context.getAsConstantArrayType(E->getType()); + llvm::APInt Size = ArrayTy->getSize(); + + if (Size == 0) + return true; + // FIXME: While the default -fstrict-flex-arrays=0 permits Size>1 trailing // arrays to be treated as flexible-array-members, we still emit diagnostics // as if they are not. Pending further discussion... @@ -15974,9 +15981,19 @@ const ConstantArrayType *ArrayTy = Context.getAsConstantArrayType(BaseExpr->getType()); + unsigned StrictFlexArraysLevel = getLangOpts().StrictFlexArrays; + + const NamedDecl *ND = nullptr; + if (const auto *DRE = dyn_cast(BaseExpr)) + ND = DRE->getDecl(); + else if (const auto *ME = dyn_cast(BaseExpr)) + ND = ME->getMemberDecl(); + const Type *BaseType = ArrayTy == nullptr ? nullptr : ArrayTy->getElementType().getTypePtr(); - bool IsUnboundedArray = (BaseType == nullptr); + bool IsUnboundedArray = + BaseType == nullptr || + isFlexibleArrayMemberExpr(*this, BaseExpr, ND, StrictFlexArraysLevel); if (EffectiveType->isDependentType() || (!IsUnboundedArray && BaseType->isDependentType())) return; @@ -15991,12 +16008,6 @@ index = -index; } - const NamedDecl *ND = nullptr; - if (const DeclRefExpr *DRE = dyn_cast(BaseExpr)) - ND = DRE->getDecl(); - if (const MemberExpr *ME = dyn_cast(BaseExpr)) - ND = ME->getMemberDecl(); - if (IsUnboundedArray) { if (EffectiveType->isFunctionType()) return; @@ -16074,17 +16085,10 @@ // example). In this case we have no information about whether the array // access exceeds the array bounds. However we can still diagnose an array // access which precedes the array bounds. - // - // FIXME: this check should be redundant with the IsUnboundedArray check - // above. if (BaseType->isIncompleteType()) return; - // FIXME: this check should be used to set IsUnboundedArray from the - // beginning. llvm::APInt size = ArrayTy->getSize(); - if (!size.isStrictlyPositive()) - return; if (BaseType != EffectiveType) { // Make sure we're comparing apples to apples when comparing index to size @@ -16114,11 +16118,6 @@ if (AllowOnePastEnd ? index.ule(size) : index.ult(size)) return; - // Also don't warn for Flexible Array Member emulation. - const unsigned StrictFlexArraysLevel = getLangOpts().StrictFlexArrays; - if (IsTailPaddedMemberArray(*this, size, ND, StrictFlexArraysLevel)) - return; - // Suppress the warning if the subscript expression (as identified by the // ']' location) and the index expression are both from macro expansions // within a system header. diff --git a/clang/test/Sema/unbounded-array-bounds.c b/clang/test/Sema/unbounded-array-bounds.c --- a/clang/test/Sema/unbounded-array-bounds.c +++ b/clang/test/Sema/unbounded-array-bounds.c @@ -84,3 +84,33 @@ void func() { func + 0xdead000000000000UL; // no crash } + +struct { + int _; + char tail[]; // addr16-note {{declared here}} addr32-note {{declared here}} +} fam; + +struct { + int _; + char tail[0]; // addr16-note {{declared here}} addr32-note {{declared here}} +} fam0; + +struct { + int _; + char tail[1]; // addr16-note {{declared here}} addr32-note {{declared here}} +} fam1; + +void fam_ily() { + ++fam.tail[7073650413200313099]; + // addr16-warning@-1 {{array index 7073650413200313099 refers past the last possible element for an array in 16-bit address space containing 8-bit (1-byte) elements (max possible 65536 elements)}} + // addr32-warning@-2 {{array index 7073650413200313099 refers past the last possible element for an array in 32-bit address space containing 8-bit (1-byte) elements (max possible 4294967296 elements)}} + // No warning for addr64 because the array index is inbound in that case. + ++fam0.tail[7073650413200313099]; + // addr16-warning@-1 {{array index 7073650413200313099 refers past the last possible element for an array in 16-bit address space containing 8-bit (1-byte) elements (max possible 65536 elements)}} + // addr32-warning@-2 {{array index 7073650413200313099 refers past the last possible element for an array in 32-bit address space containing 8-bit (1-byte) elements (max possible 4294967296 elements)}} + // No warning for addr64 because the array index is inbound in that case. + ++fam1.tail[7073650413200313099]; + // addr16-warning@-1 {{array index 7073650413200313099 refers past the last possible element for an array in 16-bit address space containing 8-bit (1-byte) elements (max possible 65536 elements)}} + // addr32-warning@-2 {{array index 7073650413200313099 refers past the last possible element for an array in 32-bit address space containing 8-bit (1-byte) elements (max possible 4294967296 elements)}} + // No warning for addr64 because the array index is inbound in that case. +}