This revision was automatically updated to reflect the committed changes.
Closed by commit rGdad36245a5c2: [clang] Rework IsTailPaddedMemberArray into 
isFlexibleArrayMemberExpr (authored by serge-sans-paille).

Changed prior to commit:
  https://reviews.llvm.org/D133108?vs=460899&id=462140#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133108/new/

https://reviews.llvm.org/D133108

Files:
  clang/docs/ReleaseNotes.rst
  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
@@ -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<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;
@@ -15991,12 +16008,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;
@@ -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.
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ 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
 -------------------------------------------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to