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; }
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; }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits