[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays=<n> for stricter handling of flexible arrays
Fangrui Song via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 27 23:30:48 PDT 2022
MaskRay added a comment.
In D126864#3614235 <https://reviews.llvm.org/D126864#3614235>, @sberg wrote:
> In D126864#3612149 <https://reviews.llvm.org/D126864#3612149>, @sberg wrote:
>
>> see https://reviews.llvm.org/D128643 "[clang] -fsanitize=array-bounds: treat all trailing size-1 arrays as flexible" for another regression
>
> For the record (now that the original commit has been reverted anyway for now):
>
> Besides the above regression for `-fsanitize=array-bounds` and macro'ized array size in HarfBuzz, I also ran into yet another regression when Firebird is built with `-fsanitize=array-bounds`: That project, in https://github.com/FirebirdSQL/firebird/blob/master/src/lock/lock_proto.h uses a trailing member
>
> srq lhb_hash[1]; // Hash table
>
> as a flexible array, but declared in a
>
> struct lhb : public Firebird::MemoryHeader
>
> that is not a standard-layout class (because the `Firebird::MemoryHeader` base class also declares non-static data members). And `isFlexibleArrayMember` returns `false` if the surrounding class is not a standard-layout class.
>
> In the end, what brought back a successful `-fsanitize=array-bounds` LibreOffice (which bundles those HarfBuzz and Firebird, among others) for me was to exclude the whole set of blocks
>
> // Don't consider sizes resulting from macro expansions or template argument
> // substitution to form C89 tail-padded arrays.
>
> TypeSourceInfo *TInfo = FD->getTypeSourceInfo();
> while (TInfo) {
> TypeLoc TL = TInfo->getTypeLoc();
> // Look through typedefs.
> if (TypedefTypeLoc TTL = TL.getAs<TypedefTypeLoc>()) {
> const TypedefNameDecl *TDL = TTL.getTypedefNameDecl();
> TInfo = TDL->getTypeSourceInfo();
> continue;
> }
> if (ConstantArrayTypeLoc CTL = TL.getAs<ConstantArrayTypeLoc>()) {
> const Expr *SizeExpr = dyn_cast<IntegerLiteral>(CTL.getSizeExpr());
> if (!SizeExpr || SizeExpr->getExprLoc().isMacroID())
> return false;
> }
> break;
> }
>
> const ObjCInterfaceDecl *ID =
> dyn_cast<ObjCInterfaceDecl>(FD->getDeclContext());
> const RecordDecl *RD = dyn_cast<RecordDecl>(FD->getDeclContext());
> if (RD) {
> if (RD->isUnion())
> return false;
> if (const CXXRecordDecl *CRD = dyn_cast<CXXRecordDecl>(RD)) {
> if (!CRD->isStandardLayout())
> return false;
> }
> } else if (!ID)
> return false;
>
> (by means of an additional `bool` parameter) when `isFlexibleArrayMember` is called from `getArrayIndexingBound` (in `clang/lib/CodeGen/CGExpr.cpp`).
Oh, I did not see this when pushing a test efd90ffbfc427ad4c4675ac1fcae9d53cc7f1322 <https://reviews.llvm.org/rGefd90ffbfc427ad4c4675ac1fcae9d53cc7f1322> . Consider adding additional tests in `clang/test/CodeGen/bounds-checking-fam.c`.
It's worth bringing up such issue to the project. GCC and Clang have been working around them so far but they should eventually be fixed.
For CPython I try to be loud on https://github.com/python/cpython/issues/84301
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126864/new/
https://reviews.llvm.org/D126864
More information about the cfe-commits
mailing list