sberg created this revision.
sberg added a reviewer: serge-sans-paille.
sberg added a project: clang.
Herald added a project: All.
sberg requested review of this revision.
...even if the size resulted from a macro expansion. This reverts back to the
behavior prior to
https://github.com/llvm/llvm-project/commit/886715af962de2c92fac4bd37104450345711e4a
"[clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible
arrays". The new behavior caused false out-of-bounds-index reports from e.g.
HarfBuzz built with `-fsanitize=array-bounds`: HarfBuzz has various "fake"
flexible array members of the form
TypearrayZ[HB_VAR_ARRAY];
in https://github.com/harfbuzz/harfbuzz/blob/main/src/hb-open-type.hh, where
`HB_VAR_ARRAY` is defined as
#ifndef HB_VAR_ARRAY
#define HB_VAR_ARRAY 1
#endif
in https://github.com/harfbuzz/harfbuzz/blob/main/src/hb-machinery.hh.
Also added a test.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D128643
Files:
clang/include/clang/AST/Expr.h
clang/lib/AST/Expr.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/Sema/SemaChecking.cpp
clang/test/CodeGen/bounds-checking.c
Index: clang/test/CodeGen/bounds-checking.c
===
--- clang/test/CodeGen/bounds-checking.c
+++ clang/test/CodeGen/bounds-checking.c
@@ -58,3 +58,14 @@
// CHECK-NOT: cont:
return b[i];
}
+
+#define FLEXIBLE 1
+struct Macro {
+ int a[FLEXIBLE];
+};
+
+// CHECK-LABEL: define {{.*}} @f7
+int f7(struct Macro *m, int i) {
+ // CHECK-NOT: call {{.*}} @llvm.ubsantrap
+ return m->a[i];
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -15869,8 +15869,8 @@
return;
// Also don't warn for flexible array members.
-if (BaseExpr->isFlexibleArrayMember(Context,
-getLangOpts().StrictFlexArrays))
+if (BaseExpr->isFlexibleArrayMember(Context,
getLangOpts().StrictFlexArrays,
+true))
return;
// Suppress the warning if the subscript expression (as identified by the
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -932,7 +932,7 @@
if (const auto *CE = dyn_cast(Base)) {
if (CE->getCastKind() == CK_ArrayToPointerDecay &&
!CE->getSubExpr()->IgnoreParens()->isFlexibleArrayMember(
-Context, std::max(StrictFlexArraysLevel, 1))) {
+Context, std::max(StrictFlexArraysLevel, 1), false)) {
IndexedType = CE->getSubExpr()->getType();
const ArrayType *AT = IndexedType->castAsArrayTypeUnsafe();
if (const auto *CAT = dyn_cast(AT))
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -203,8 +203,8 @@
return false;
}
-bool Expr::isFlexibleArrayMember(ASTContext ,
- int StrictFlexArraysLevel) const {
+bool Expr::isFlexibleArrayMember(ASTContext , int StrictFlexArraysLevel,
+ bool IgnoreSizeFromMacro) const {
const NamedDecl *ND = nullptr;
if (const DeclRefExpr *DRE = dyn_cast(this))
ND = DRE->getDecl();
@@ -260,7 +260,8 @@
}
if (ConstantArrayTypeLoc CTL = TL.getAs()) {
const Expr *SizeExpr = dyn_cast(CTL.getSizeExpr());
- if (!SizeExpr || SizeExpr->getExprLoc().isMacroID())
+ if (!SizeExpr ||
+ (IgnoreSizeFromMacro && SizeExpr->getExprLoc().isMacroID()))
return false;
}
break;
Index: clang/include/clang/AST/Expr.h
===
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -451,7 +451,8 @@
/// - 1 => [0], [1], [ ]
/// - 2 => [0], [ ]
/// - 3 => [ ]
- bool isFlexibleArrayMember(ASTContext , int StrictFlexArraysLevel) const;
+ bool isFlexibleArrayMember(ASTContext , int StrictFlexArraysLevel,
+ bool IgnoreSizeFromMacro) const;
/// setValueKind - Set the value kind produced by this expression.
void setValueKind(ExprValueKind Cat) { ExprBits.ValueKind = Cat; }
Index: clang/test/CodeGen/bounds-checking.c
===
--- clang/test/CodeGen/bounds-checking.c
+++ clang/test/CodeGen/bounds-checking.c
@@ -58,3 +58,14 @@
// CHECK-NOT: cont:
return b[i];
}
+
+#define FLEXIBLE 1
+struct Macro {
+ int a[FLEXIBLE];
+};
+
+// CHECK-LABEL: define {{.*}} @f7
+int f7(struct Macro *m, int i) {
+ // CHECK-NOT: call {{.*}} @llvm.ubsantrap
+ return m->a[i];
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
---
