serge-sans-paille updated this revision to Diff 435900.
serge-sans-paille added a comment.

Cleanup testcase


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

https://reviews.llvm.org/D126864

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/test/CodeGen/object-size-flex-array.c
  clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp

Index: clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -verify -fstrict-flex-arrays %s
+
+// We cannot know for sure the size of a flexible array.
+void test() {
+  struct {
+    int f;
+    int a[];
+  } s2;
+  s2.a[2] = 0; // no-warning
+}
+
+// Under -fstrict-flex-arrays `a` is not a flexible array.
+void test1() {
+  struct {
+    int f;
+    int a[1]; // expected-note {{declared here}}
+  } s2;
+  s2.a[2] = 0; // expected-warning 1 {{array index 2 is past the end of the array (which contains 1 element)}}
+}
Index: clang/test/CodeGen/object-size-flex-array.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/object-size-flex-array.c
@@ -0,0 +1,97 @@
+// RUN: %clang -fstrict-flex-arrays    -target x86_64-apple-darwin -S -emit-llvm %s -o - 2>&1 | FileCheck --check-prefix CHECK-STRICT %s
+// RUN: %clang -fno-strict-flex-arrays -target x86_64-apple-darwin -S -emit-llvm %s -o - 2>&1 | FileCheck %s
+
+#define OBJECT_SIZE_BUILTIN __builtin_object_size
+
+typedef struct {
+  float f;
+  double c[];
+} foo_t;
+
+typedef struct {
+  float f;
+  double c[0];
+} foo0_t;
+
+typedef struct {
+  float f;
+  double c[1];
+} foo1_t;
+
+typedef struct {
+  float f;
+  double c[2];
+} foo2_t;
+
+// CHECK-LABEL: @bar
+// CHECK-STRICT-LABEL: @bar
+unsigned bar(foo_t *f) {
+  // CHECK: ret i32 %
+  // CHECK-STRICT: ret i32 %
+  return OBJECT_SIZE_BUILTIN(f->c, 1);
+}
+
+// CHECK-LABEL: @bar0
+// CHECK-STRICT-LABEL: @bar0
+unsigned bar0(foo0_t *f) {
+  // CHECK: ret i32 %
+  // CHECK-STRICT: ret i32 0
+  return OBJECT_SIZE_BUILTIN(f->c, 1);
+}
+
+// CHECK-LABEL: @bar1
+// CHECK-STRICT-LABEL: @bar1
+unsigned bar1(foo1_t *f) {
+  // CHECK: ret i32 %
+  // CHECK-STRICT: ret i32 8
+  return OBJECT_SIZE_BUILTIN(f->c, 1);
+}
+
+// CHECK-LABEL: @bar2
+// CHECK-STRICT-LABEL: @bar2
+unsigned bar2(foo2_t *f) {
+  // CHECK: ret i32 %
+  // CHECK-STRICT: ret i32 16
+  return OBJECT_SIZE_BUILTIN(f->c, 1);
+}
+
+// Also checks for non-trailing flex-array like members
+
+typedef struct {
+  double c[0];
+  float f;
+} foofoo0_t;
+
+typedef struct {
+  double c[1];
+  float f;
+} foofoo1_t;
+
+typedef struct {
+  double c[2];
+  float f;
+} foofoo2_t;
+
+// CHECK-LABEL: @babar0
+// CHECK-STRICT-LABEL: @babar0
+unsigned babar0(foofoo0_t *f) {
+  // CHECK: ret i32 0
+  // CHECK-STRICT: ret i32 0
+  return OBJECT_SIZE_BUILTIN(f->c, 1);
+}
+
+// CHECK-LABEL: @babar1
+// CHECK-STRICT-LABEL: @babar1
+unsigned babar1(foofoo1_t *f) {
+  // CHECK: ret i32 8
+  // CHECK-STRICT: ret i32 8
+  return OBJECT_SIZE_BUILTIN(f->c, 1);
+}
+
+// CHECK-LABEL: @babar2
+// CHECK-STRICT-LABEL: @babar2
+unsigned babar2(foofoo2_t *f) {
+  // CHECK: ret i32 16
+  // CHECK-STRICT: ret i32 16
+  return OBJECT_SIZE_BUILTIN(f->c, 1);
+}
Index: clang/lib/StaticAnalyzer/Core/MemRegion.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -789,6 +789,9 @@
       if (isa<IncompleteArrayType>(AT))
         return true;
 
+      if (getContext().getLangOpts().StrictFlexArrays)
+        return false;
+
       if (const auto *CAT = dyn_cast<ConstantArrayType>(AT)) {
         const llvm::APInt &Size = CAT->getSize();
         if (Size.isZero())
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -15722,6 +15722,9 @@
 /// commonly used to emulate flexible arrays in C89 code.
 static bool IsTailPaddedMemberArray(Sema &S, const llvm::APInt &Size,
                                     const NamedDecl *ND) {
+  if (S.getLangOpts().StrictFlexArrays)
+    return false;
+
   if (Size != 1 || !ND) return false;
 
   const FieldDecl *FD = dyn_cast<FieldDecl>(ND);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6214,6 +6214,9 @@
   Args.AddLastArg(CmdArgs, options::OPT_funroll_loops,
                   options::OPT_fno_unroll_loops);
 
+  Args.addOptInFlag(CmdArgs, options::OPT_fstrict_flex_arrays,
+                    options::OPT_fno_strict_flex_arrays);
+
   Args.AddLastArg(CmdArgs, options::OPT_pthread);
 
   if (Args.hasFlag(options::OPT_mspeculative_load_hardening,
Index: clang/lib/CodeGen/CGExpr.cpp
===================================================================
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -880,13 +880,15 @@
 
 /// Determine whether this expression refers to a flexible array member in a
 /// struct. We disable array bounds checks for such members.
-static bool isFlexibleArrayMemberExpr(const Expr *E) {
+static bool isFlexibleArrayMemberExpr(const Expr *E, bool StrictFlexArrays) {
   // For compatibility with existing code, we treat arrays of length 0 or
   // 1 as flexible array members.
   // FIXME: This is inconsistent with the warning code in SemaChecking. Unify
   // the two mechanisms.
   const ArrayType *AT = E->getType()->castAsArrayTypeUnsafe();
   if (const auto *CAT = dyn_cast<ConstantArrayType>(AT)) {
+    if (StrictFlexArrays)
+      return false;
     // FIXME: Sema doesn't treat [1] as a flexible array member if the bound
     // was produced by macro expansion.
     if (CAT->getSize().ugt(1))
@@ -957,8 +959,10 @@
 
 /// If Base is known to point to the start of an array, return the length of
 /// that array. Return 0 if the length cannot be determined.
-static llvm::Value *getArrayIndexingBound(
-    CodeGenFunction &CGF, const Expr *Base, QualType &IndexedType) {
+static llvm::Value *getArrayIndexingBound(CodeGenFunction &CGF,
+                                          const Expr *Base,
+                                          QualType &IndexedType,
+                                          bool StrictFlexArrays) {
   // For the vector indexing extension, the bound is the number of elements.
   if (const VectorType *VT = Base->getType()->getAs<VectorType>()) {
     IndexedType = Base->getType();
@@ -969,7 +973,7 @@
 
   if (const auto *CE = dyn_cast<CastExpr>(Base)) {
     if (CE->getCastKind() == CK_ArrayToPointerDecay &&
-        !isFlexibleArrayMemberExpr(CE->getSubExpr())) {
+        !isFlexibleArrayMemberExpr(CE->getSubExpr(), StrictFlexArrays)) {
       IndexedType = CE->getSubExpr()->getType();
       const ArrayType *AT = IndexedType->castAsArrayTypeUnsafe();
       if (const auto *CAT = dyn_cast<ConstantArrayType>(AT))
@@ -997,7 +1001,8 @@
   SanitizerScope SanScope(this);
 
   QualType IndexedType;
-  llvm::Value *Bound = getArrayIndexingBound(*this, Base, IndexedType);
+  llvm::Value *Bound = getArrayIndexingBound(*this, Base, IndexedType,
+                                             getLangOpts().StrictFlexArrays);
   if (!Bound)
     return;
 
Index: clang/lib/AST/ExprConstant.cpp
===================================================================
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11600,6 +11600,8 @@
   return LVal.InvalidBase &&
          Designator.Entries.size() == Designator.MostDerivedPathLength &&
          Designator.MostDerivedIsArrayElement &&
+         (Designator.isMostDerivedAnUnsizedArray() ||
+          !Ctx.getLangOpts().StrictFlexArrays) &&
          isDesignatorAtObjectEnd(Ctx, LVal);
 }
 
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -1364,6 +1364,7 @@
   ~MemRegionManager();
 
   ASTContext &getContext() { return Ctx; }
+  const ASTContext &getContext() const { return Ctx; }
 
   llvm::BumpPtrAllocator &getAllocator() { return A; }
 
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1132,6 +1132,10 @@
 def fapple_kext : Flag<["-"], "fapple-kext">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Use Apple's kernel extensions ABI">,
   MarshallingInfoFlag<LangOpts<"AppleKext">>;
+defm strict_flex_arrays : BoolFOption<"strict-flex-arrays",
+  LangOpts<"StrictFlexArrays">, DefaultFalse,
+  PosFlag<SetTrue, [CC1Option], "Enable optimizations based on the strict definition of flexible arrays">,
+  NegFlag<SetFalse>>;
 defm apple_pragma_pack : BoolFOption<"apple-pragma-pack",
   LangOpts<"ApplePragmaPack">, DefaultFalse,
   PosFlag<SetTrue, [CC1Option], "Enable Apple gcc-compatible #pragma pack handling">,
Index: clang/include/clang/Basic/LangOptions.def
===================================================================
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -422,6 +422,7 @@
 LANGOPT(RegisterStaticDestructors, 1, 1, "Register C++ static destructors")
 
 LANGOPT(MatrixTypes, 1, 0, "Enable or disable the builtin matrix type")
+LANGOPT(StrictFlexArrays, 1, 0, "Rely on strict definition of flexible arrays")
 
 COMPATIBLE_VALUE_LANGOPT(MaxTokens, 32, 0, "Max number of tokens per TU or 0")
 
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -68,6 +68,12 @@
 
       Randomizing structure layout is a C-only feature.
 
+- Clang now supports the ``-fstrict-flex-arrays`` option to avoid treating
+  trailing array members with a specified bound as flexible array members. The
+  option yields more accurate ``__builtin_object_size`` and
+  ``__builtin_dynamic_object_size`` results in most cases but may be overly
+  conservative for some legacy code.
+
 Bug Fixes
 ---------
 - ``CXXNewExpr::getArraySize()`` previously returned a ``llvm::Optional``
Index: clang/docs/ClangCommandLineReference.rst
===================================================================
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -2621,6 +2621,10 @@
 
 .. option:: -fuse-init-array, -fno-use-init-array
 
+.. option:: -fstrict-flex-arrays
+
+Do not consider sized arrays at the end of a struct as flexible arrays.
+
 .. option:: -fuse-ld=<arg>
 
 .. option:: -fuse-line-directives, -fno-use-line-directives
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to