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