serge-sans-paille updated this revision to Diff 435520.
serge-sans-paille added a comment.
Herald added a subscriber: martong.
Take into account @efriedma and @kees review, plus adding a bunch of extra test
case
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,101 @@
+// 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
+
+#ifndef DYNAMIC
+#define OBJECT_SIZE_BUILTIN __builtin_object_size
+#else
+#define OBJECT_SIZE_BUILTIN __builtin_dynamic_object_size
+#endif
+
+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,11 @@
Randomizing structure layout is a C-only feature.
+- Clang now supports the ``-fstrict-flex-arrays`` to only consider trailing
+ arrays with unknown size arrays as flexible arrays. This breaks compatibility
+ with some legacy code but allows for better folding of
+ ``__builtin_object_size``.
+
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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
