serge-sans-paille updated this revision to Diff 460899.
serge-sans-paille edited the summary of this revision.
serge-sans-paille added a comment.
Address minor nits, plus add a test case that showcases the extra (legitimate!)
warning we now get when accessing a "struct hack" field with an index that
cannot be represented in associated address space.
Kudos to @aaron.ballman for spotting this one!
As a comparison, current behavior for the same test case:
https://godbolt.org/z/86Wcbrz5q (only the flexible array member is flagged with
a warning, the struct hacks are ignored)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133108/new/
https://reviews.llvm.org/D133108
Files:
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/unbounded-array-bounds.c
Index: clang/test/Sema/unbounded-array-bounds.c
===================================================================
--- clang/test/Sema/unbounded-array-bounds.c
+++ 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.
+}
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -15891,17 +15891,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...
@@ -15967,9 +15974,19 @@
const ConstantArrayType *ArrayTy =
Context.getAsConstantArrayType(BaseExpr->getType());
+ unsigned StrictFlexArraysLevel = getLangOpts().StrictFlexArrays;
+
+ const NamedDecl *ND = nullptr;
+ if (const auto *DRE = dyn_cast<DeclRefExpr>(BaseExpr))
+ ND = DRE->getDecl();
+ else if (const auto *ME = dyn_cast<MemberExpr>(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;
@@ -15984,12 +16001,6 @@
index = -index;
}
- const NamedDecl *ND = nullptr;
- if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(BaseExpr))
- ND = DRE->getDecl();
- if (const MemberExpr *ME = dyn_cast<MemberExpr>(BaseExpr))
- ND = ME->getMemberDecl();
-
if (IsUnboundedArray) {
if (EffectiveType->isFunctionType())
return;
@@ -16067,17 +16078,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
@@ -16107,11 +16111,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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
