[PATCH] D128643: [clang] -fsanitize=array-bounds: treat all trailing size-1 arrays as flexible

[Stephan Bergmann via Phabricator via cfe-commits]

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=<n> 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

  Type                arrayZ[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<CastExpr>(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<ConstantArrayType>(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 &Ctx,
-                                 int StrictFlexArraysLevel) const {
+bool Expr::isFlexibleArrayMember(ASTContext &Ctx, int StrictFlexArraysLevel,
+                                 bool IgnoreSizeFromMacro) const {
   const NamedDecl *ND = nullptr;
   if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(this))
     ND = DRE->getDecl();
@@ -260,7 +260,8 @@
     }
     if (ConstantArrayTypeLoc CTL = TL.getAs<ConstantArrayTypeLoc>()) {
       const Expr *SizeExpr = dyn_cast<IntegerLiteral>(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 &Ctx, int StrictFlexArraysLevel) const;
+  bool isFlexibleArrayMember(ASTContext &Ctx, int StrictFlexArraysLevel,
+                             bool IgnoreSizeFromMacro) const;
 
   /// setValueKind - Set the value kind produced by this expression.
   void setValueKind(ExprValueKind Cat) { ExprBits.ValueKind = Cat; }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D128643.440206.patch
Type: text/x-patch
Size: 3326 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220627/4434c439/attachment.bin>