Author: lebedevri Date: Tue Jul 31 23:06:16 2018 New Revision: 338489 URL: http://llvm.org/viewvc/llvm-project?rev=338489&view=rev Log: [AST] CastExpr: BasePathSize is not large enough.
Summary: rC337815 / D49508 had to cannibalize one bit of `CastExprBitfields::BasePathSize` in order to squeeze `PartOfExplicitCast` boolean. That reduced the maximal value of `PartOfExplicitCast` from 9 bits (~512) down to 8 bits (~256). Apparently, that mattered. Too bad there weren't any tests. It caused [[ https://bugs.llvm.org/show_bug.cgi?id=38356 | PR38356 ]]. So we need to increase `PartOfExplicitCast` back at least to 9 bits, or a bit more. For obvious reasons, we can't do that in `CastExprBitfields` - that would blow up the size of every `Expr`. So we need to either just add a variable into the `CastExpr` (as done here), or use `llvm::TrailingObjects`. The latter does not seem to be straight-forward. Perhaps, that needs to be done not for the `CastExpr` itself, but for all of it's `final` children. Reviewers: rjmccall, rsmith, erichkeane Reviewed By: rjmccall Subscribers: bricci, hans, cfe-commits, waddlesplash Differential Revision: https://reviews.llvm.org/D50050 Added: cfe/trunk/test/CodeGenCXX/castexpr-basepathsize-threshold.cpp Modified: cfe/trunk/include/clang/AST/Expr.h cfe/trunk/include/clang/AST/ExprCXX.h cfe/trunk/include/clang/AST/ExprObjC.h cfe/trunk/include/clang/AST/Stmt.h cfe/trunk/lib/AST/Expr.cpp cfe/trunk/lib/AST/ExprCXX.cpp Modified: cfe/trunk/include/clang/AST/Expr.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=338489&r1=338488&r2=338489&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/Expr.h (original) +++ cfe/trunk/include/clang/AST/Expr.h Tue Jul 31 23:06:16 2018 @@ -2787,20 +2787,26 @@ public: /// representation in the source code (ExplicitCastExpr's derived /// classes). class CastExpr : public Expr { +public: + using BasePathSizeTy = unsigned int; + static_assert(std::numeric_limits<BasePathSizeTy>::max() >= 16384, + "[implimits] Direct and indirect base classes [16384]."); + private: Stmt *Op; bool CastConsistency() const; + BasePathSizeTy *BasePathSize(); + const CXXBaseSpecifier * const *path_buffer() const { return const_cast<CastExpr*>(this)->path_buffer(); } CXXBaseSpecifier **path_buffer(); - void setBasePathSize(unsigned basePathSize) { - CastExprBits.BasePathSize = basePathSize; - assert(CastExprBits.BasePathSize == basePathSize && - "basePathSize doesn't fit in bits of CastExprBits.BasePathSize!"); + void setBasePathSize(BasePathSizeTy basePathSize) { + assert(!path_empty() && basePathSize != 0); + *(BasePathSize()) = basePathSize; } protected: @@ -2823,7 +2829,9 @@ protected: Op(op) { CastExprBits.Kind = kind; CastExprBits.PartOfExplicitCast = false; - setBasePathSize(BasePathSize); + CastExprBits.BasePathIsEmpty = BasePathSize == 0; + if (!path_empty()) + setBasePathSize(BasePathSize); assert(CastConsistency()); } @@ -2831,7 +2839,9 @@ protected: CastExpr(StmtClass SC, EmptyShell Empty, unsigned BasePathSize) : Expr(SC, Empty) { CastExprBits.PartOfExplicitCast = false; - setBasePathSize(BasePathSize); + CastExprBits.BasePathIsEmpty = BasePathSize == 0; + if (!path_empty()) + setBasePathSize(BasePathSize); } public: @@ -2859,8 +2869,12 @@ public: typedef CXXBaseSpecifier **path_iterator; typedef const CXXBaseSpecifier * const *path_const_iterator; - bool path_empty() const { return CastExprBits.BasePathSize == 0; } - unsigned path_size() const { return CastExprBits.BasePathSize; } + bool path_empty() const { return CastExprBits.BasePathIsEmpty; } + unsigned path_size() const { + if (path_empty()) + return 0U; + return *(const_cast<CastExpr *>(this)->BasePathSize()); + } path_iterator path_begin() { return path_buffer(); } path_iterator path_end() { return path_buffer() + path_size(); } path_const_iterator path_begin() const { return path_buffer(); } @@ -2908,7 +2922,12 @@ public: /// @endcode class ImplicitCastExpr final : public CastExpr, - private llvm::TrailingObjects<ImplicitCastExpr, CXXBaseSpecifier *> { + private llvm::TrailingObjects<ImplicitCastExpr, CastExpr::BasePathSizeTy, + CXXBaseSpecifier *> { + size_t numTrailingObjects(OverloadToken<CastExpr::BasePathSizeTy>) const { + return path_empty() ? 0 : 1; + } + private: ImplicitCastExpr(QualType ty, CastKind kind, Expr *op, unsigned BasePathLength, ExprValueKind VK) @@ -3013,7 +3032,8 @@ public: /// (Type)expr. For example: @c (int)f. class CStyleCastExpr final : public ExplicitCastExpr, - private llvm::TrailingObjects<CStyleCastExpr, CXXBaseSpecifier *> { + private llvm::TrailingObjects<CStyleCastExpr, CastExpr::BasePathSizeTy, + CXXBaseSpecifier *> { SourceLocation LPLoc; // the location of the left paren SourceLocation RPLoc; // the location of the right paren @@ -3027,6 +3047,10 @@ class CStyleCastExpr final explicit CStyleCastExpr(EmptyShell Shell, unsigned PathSize) : ExplicitCastExpr(CStyleCastExprClass, Shell, PathSize) { } + size_t numTrailingObjects(OverloadToken<CastExpr::BasePathSizeTy>) const { + return path_empty() ? 0 : 1; + } + public: static CStyleCastExpr *Create(const ASTContext &Context, QualType T, ExprValueKind VK, CastKind K, Modified: cfe/trunk/include/clang/AST/ExprCXX.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=338489&r1=338488&r2=338489&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/ExprCXX.h (original) +++ cfe/trunk/include/clang/AST/ExprCXX.h Tue Jul 31 23:06:16 2018 @@ -301,7 +301,8 @@ public: /// \c static_cast<int>(1.0). class CXXStaticCastExpr final : public CXXNamedCastExpr, - private llvm::TrailingObjects<CXXStaticCastExpr, CXXBaseSpecifier *> { + private llvm::TrailingObjects<CXXStaticCastExpr, CastExpr::BasePathSizeTy, + CXXBaseSpecifier *> { CXXStaticCastExpr(QualType ty, ExprValueKind vk, CastKind kind, Expr *op, unsigned pathSize, TypeSourceInfo *writtenTy, SourceLocation l, SourceLocation RParenLoc, @@ -312,6 +313,10 @@ class CXXStaticCastExpr final explicit CXXStaticCastExpr(EmptyShell Empty, unsigned PathSize) : CXXNamedCastExpr(CXXStaticCastExprClass, Empty, PathSize) {} + size_t numTrailingObjects(OverloadToken<CastExpr::BasePathSizeTy>) const { + return path_empty() ? 0 : 1; + } + public: friend class CastExpr; friend TrailingObjects; @@ -337,7 +342,8 @@ public: /// check to determine how to perform the type conversion. class CXXDynamicCastExpr final : public CXXNamedCastExpr, - private llvm::TrailingObjects<CXXDynamicCastExpr, CXXBaseSpecifier *> { + private llvm::TrailingObjects< + CXXDynamicCastExpr, CastExpr::BasePathSizeTy, CXXBaseSpecifier *> { CXXDynamicCastExpr(QualType ty, ExprValueKind VK, CastKind kind, Expr *op, unsigned pathSize, TypeSourceInfo *writtenTy, SourceLocation l, SourceLocation RParenLoc, @@ -348,6 +354,10 @@ class CXXDynamicCastExpr final explicit CXXDynamicCastExpr(EmptyShell Empty, unsigned pathSize) : CXXNamedCastExpr(CXXDynamicCastExprClass, Empty, pathSize) {} + size_t numTrailingObjects(OverloadToken<CastExpr::BasePathSizeTy>) const { + return path_empty() ? 0 : 1; + } + public: friend class CastExpr; friend TrailingObjects; @@ -380,6 +390,7 @@ public: class CXXReinterpretCastExpr final : public CXXNamedCastExpr, private llvm::TrailingObjects<CXXReinterpretCastExpr, + CastExpr::BasePathSizeTy, CXXBaseSpecifier *> { CXXReinterpretCastExpr(QualType ty, ExprValueKind vk, CastKind kind, Expr *op, unsigned pathSize, @@ -392,6 +403,10 @@ class CXXReinterpretCastExpr final CXXReinterpretCastExpr(EmptyShell Empty, unsigned pathSize) : CXXNamedCastExpr(CXXReinterpretCastExprClass, Empty, pathSize) {} + size_t numTrailingObjects(OverloadToken<CastExpr::BasePathSizeTy>) const { + return path_empty() ? 0 : 1; + } + public: friend class CastExpr; friend TrailingObjects; @@ -419,7 +434,8 @@ public: /// value. class CXXConstCastExpr final : public CXXNamedCastExpr, - private llvm::TrailingObjects<CXXConstCastExpr, CXXBaseSpecifier *> { + private llvm::TrailingObjects<CXXConstCastExpr, CastExpr::BasePathSizeTy, + CXXBaseSpecifier *> { CXXConstCastExpr(QualType ty, ExprValueKind VK, Expr *op, TypeSourceInfo *writtenTy, SourceLocation l, SourceLocation RParenLoc, SourceRange AngleBrackets) @@ -429,6 +445,10 @@ class CXXConstCastExpr final explicit CXXConstCastExpr(EmptyShell Empty) : CXXNamedCastExpr(CXXConstCastExprClass, Empty, 0) {} + size_t numTrailingObjects(OverloadToken<CastExpr::BasePathSizeTy>) const { + return path_empty() ? 0 : 1; + } + public: friend class CastExpr; friend TrailingObjects; @@ -1471,7 +1491,8 @@ public: /// \endcode class CXXFunctionalCastExpr final : public ExplicitCastExpr, - private llvm::TrailingObjects<CXXFunctionalCastExpr, CXXBaseSpecifier *> { + private llvm::TrailingObjects< + CXXFunctionalCastExpr, CastExpr::BasePathSizeTy, CXXBaseSpecifier *> { SourceLocation LParenLoc; SourceLocation RParenLoc; @@ -1486,6 +1507,10 @@ class CXXFunctionalCastExpr final explicit CXXFunctionalCastExpr(EmptyShell Shell, unsigned PathSize) : ExplicitCastExpr(CXXFunctionalCastExprClass, Shell, PathSize) {} + size_t numTrailingObjects(OverloadToken<CastExpr::BasePathSizeTy>) const { + return path_empty() ? 0 : 1; + } + public: friend class CastExpr; friend TrailingObjects; Modified: cfe/trunk/include/clang/AST/ExprObjC.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprObjC.h?rev=338489&r1=338488&r2=338489&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/ExprObjC.h (original) +++ cfe/trunk/include/clang/AST/ExprObjC.h Tue Jul 31 23:06:16 2018 @@ -1571,7 +1571,8 @@ public: /// \endcode class ObjCBridgedCastExpr final : public ExplicitCastExpr, - private llvm::TrailingObjects<ObjCBridgedCastExpr, CXXBaseSpecifier *> { + private llvm::TrailingObjects< + ObjCBridgedCastExpr, CastExpr::BasePathSizeTy, CXXBaseSpecifier *> { friend class ASTStmtReader; friend class ASTStmtWriter; friend class CastExpr; @@ -1581,6 +1582,10 @@ class ObjCBridgedCastExpr final SourceLocation BridgeKeywordLoc; unsigned Kind : 2; + size_t numTrailingObjects(OverloadToken<CastExpr::BasePathSizeTy>) const { + return path_empty() ? 0 : 1; + } + public: ObjCBridgedCastExpr(SourceLocation LParenLoc, ObjCBridgeCastKind Kind, CastKind CK, SourceLocation BridgeKeywordLoc, Modified: cfe/trunk/include/clang/AST/Stmt.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=338489&r1=338488&r2=338489&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/Stmt.h (original) +++ cfe/trunk/include/clang/AST/Stmt.h Tue Jul 31 23:06:16 2018 @@ -204,7 +204,7 @@ protected: unsigned Kind : 6; unsigned PartOfExplicitCast : 1; // Only set for ImplicitCastExpr. - unsigned BasePathSize : 32 - 6 - 1 - NumExprBits; + unsigned BasePathIsEmpty : 1; }; class CallExprBitfields { Modified: cfe/trunk/lib/AST/Expr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=338489&r1=338488&r2=338489&view=diff ============================================================================== --- cfe/trunk/lib/AST/Expr.cpp (original) +++ cfe/trunk/lib/AST/Expr.cpp Tue Jul 31 23:06:16 2018 @@ -1734,6 +1734,21 @@ NamedDecl *CastExpr::getConversionFuncti return nullptr; } +CastExpr::BasePathSizeTy *CastExpr::BasePathSize() { + assert(!path_empty()); + switch (getStmtClass()) { +#define ABSTRACT_STMT(x) +#define CASTEXPR(Type, Base) \ + case Stmt::Type##Class: \ + return static_cast<Type *>(this) \ + ->getTrailingObjects<CastExpr::BasePathSizeTy>(); +#define STMT(Type, Base) +#include "clang/AST/StmtNodes.inc" + default: + llvm_unreachable("non-cast expressions not possible here"); + } +} + CXXBaseSpecifier **CastExpr::path_buffer() { switch (getStmtClass()) { #define ABSTRACT_STMT(x) @@ -1772,7 +1787,9 @@ ImplicitCastExpr *ImplicitCastExpr::Crea const CXXCastPath *BasePath, ExprValueKind VK) { unsigned PathSize = (BasePath ? BasePath->size() : 0); - void *Buffer = C.Allocate(totalSizeToAlloc<CXXBaseSpecifier *>(PathSize)); + void *Buffer = + C.Allocate(totalSizeToAlloc<CastExpr::BasePathSizeTy, CXXBaseSpecifier *>( + PathSize ? 1 : 0, PathSize)); ImplicitCastExpr *E = new (Buffer) ImplicitCastExpr(T, Kind, Operand, PathSize, VK); if (PathSize) @@ -1783,7 +1800,9 @@ ImplicitCastExpr *ImplicitCastExpr::Crea ImplicitCastExpr *ImplicitCastExpr::CreateEmpty(const ASTContext &C, unsigned PathSize) { - void *Buffer = C.Allocate(totalSizeToAlloc<CXXBaseSpecifier *>(PathSize)); + void *Buffer = + C.Allocate(totalSizeToAlloc<CastExpr::BasePathSizeTy, CXXBaseSpecifier *>( + PathSize ? 1 : 0, PathSize)); return new (Buffer) ImplicitCastExpr(EmptyShell(), PathSize); } @@ -1794,7 +1813,9 @@ CStyleCastExpr *CStyleCastExpr::Create(c TypeSourceInfo *WrittenTy, SourceLocation L, SourceLocation R) { unsigned PathSize = (BasePath ? BasePath->size() : 0); - void *Buffer = C.Allocate(totalSizeToAlloc<CXXBaseSpecifier *>(PathSize)); + void *Buffer = + C.Allocate(totalSizeToAlloc<CastExpr::BasePathSizeTy, CXXBaseSpecifier *>( + PathSize ? 1 : 0, PathSize)); CStyleCastExpr *E = new (Buffer) CStyleCastExpr(T, VK, K, Op, PathSize, WrittenTy, L, R); if (PathSize) @@ -1805,7 +1826,9 @@ CStyleCastExpr *CStyleCastExpr::Create(c CStyleCastExpr *CStyleCastExpr::CreateEmpty(const ASTContext &C, unsigned PathSize) { - void *Buffer = C.Allocate(totalSizeToAlloc<CXXBaseSpecifier *>(PathSize)); + void *Buffer = + C.Allocate(totalSizeToAlloc<CastExpr::BasePathSizeTy, CXXBaseSpecifier *>( + PathSize ? 1 : 0, PathSize)); return new (Buffer) CStyleCastExpr(EmptyShell(), PathSize); } Modified: cfe/trunk/lib/AST/ExprCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprCXX.cpp?rev=338489&r1=338488&r2=338489&view=diff ============================================================================== --- cfe/trunk/lib/AST/ExprCXX.cpp (original) +++ cfe/trunk/lib/AST/ExprCXX.cpp Tue Jul 31 23:06:16 2018 @@ -559,7 +559,9 @@ CXXStaticCastExpr *CXXStaticCastExpr::Cr SourceLocation RParenLoc, SourceRange AngleBrackets) { unsigned PathSize = (BasePath ? BasePath->size() : 0); - void *Buffer = C.Allocate(totalSizeToAlloc<CXXBaseSpecifier *>(PathSize)); + void *Buffer = + C.Allocate(totalSizeToAlloc<CastExpr::BasePathSizeTy, CXXBaseSpecifier *>( + PathSize ? 1 : 0, PathSize)); auto *E = new (Buffer) CXXStaticCastExpr(T, VK, K, Op, PathSize, WrittenTy, L, RParenLoc, AngleBrackets); @@ -571,7 +573,9 @@ CXXStaticCastExpr *CXXStaticCastExpr::Cr CXXStaticCastExpr *CXXStaticCastExpr::CreateEmpty(const ASTContext &C, unsigned PathSize) { - void *Buffer = C.Allocate(totalSizeToAlloc<CXXBaseSpecifier *>(PathSize)); + void *Buffer = + C.Allocate(totalSizeToAlloc<CastExpr::BasePathSizeTy, CXXBaseSpecifier *>( + PathSize ? 1 : 0, PathSize)); return new (Buffer) CXXStaticCastExpr(EmptyShell(), PathSize); } @@ -584,7 +588,9 @@ CXXDynamicCastExpr *CXXDynamicCastExpr:: SourceLocation RParenLoc, SourceRange AngleBrackets) { unsigned PathSize = (BasePath ? BasePath->size() : 0); - void *Buffer = C.Allocate(totalSizeToAlloc<CXXBaseSpecifier *>(PathSize)); + void *Buffer = + C.Allocate(totalSizeToAlloc<CastExpr::BasePathSizeTy, CXXBaseSpecifier *>( + PathSize ? 1 : 0, PathSize)); auto *E = new (Buffer) CXXDynamicCastExpr(T, VK, K, Op, PathSize, WrittenTy, L, RParenLoc, AngleBrackets); @@ -596,7 +602,9 @@ CXXDynamicCastExpr *CXXDynamicCastExpr:: CXXDynamicCastExpr *CXXDynamicCastExpr::CreateEmpty(const ASTContext &C, unsigned PathSize) { - void *Buffer = C.Allocate(totalSizeToAlloc<CXXBaseSpecifier *>(PathSize)); + void *Buffer = + C.Allocate(totalSizeToAlloc<CastExpr::BasePathSizeTy, CXXBaseSpecifier *>( + PathSize ? 1 : 0, PathSize)); return new (Buffer) CXXDynamicCastExpr(EmptyShell(), PathSize); } @@ -641,7 +649,9 @@ CXXReinterpretCastExpr::Create(const AST SourceLocation RParenLoc, SourceRange AngleBrackets) { unsigned PathSize = (BasePath ? BasePath->size() : 0); - void *Buffer = C.Allocate(totalSizeToAlloc<CXXBaseSpecifier *>(PathSize)); + void *Buffer = + C.Allocate(totalSizeToAlloc<CastExpr::BasePathSizeTy, CXXBaseSpecifier *>( + PathSize ? 1 : 0, PathSize)); auto *E = new (Buffer) CXXReinterpretCastExpr(T, VK, K, Op, PathSize, WrittenTy, L, RParenLoc, AngleBrackets); @@ -653,7 +663,9 @@ CXXReinterpretCastExpr::Create(const AST CXXReinterpretCastExpr * CXXReinterpretCastExpr::CreateEmpty(const ASTContext &C, unsigned PathSize) { - void *Buffer = C.Allocate(totalSizeToAlloc<CXXBaseSpecifier *>(PathSize)); + void *Buffer = + C.Allocate(totalSizeToAlloc<CastExpr::BasePathSizeTy, CXXBaseSpecifier *>( + PathSize ? 1 : 0, PathSize)); return new (Buffer) CXXReinterpretCastExpr(EmptyShell(), PathSize); } @@ -676,7 +688,9 @@ CXXFunctionalCastExpr::Create(const ASTC const CXXCastPath *BasePath, SourceLocation L, SourceLocation R) { unsigned PathSize = (BasePath ? BasePath->size() : 0); - void *Buffer = C.Allocate(totalSizeToAlloc<CXXBaseSpecifier *>(PathSize)); + void *Buffer = + C.Allocate(totalSizeToAlloc<CastExpr::BasePathSizeTy, CXXBaseSpecifier *>( + PathSize ? 1 : 0, PathSize)); auto *E = new (Buffer) CXXFunctionalCastExpr(T, VK, Written, K, Op, PathSize, L, R); if (PathSize) @@ -687,7 +701,9 @@ CXXFunctionalCastExpr::Create(const ASTC CXXFunctionalCastExpr * CXXFunctionalCastExpr::CreateEmpty(const ASTContext &C, unsigned PathSize) { - void *Buffer = C.Allocate(totalSizeToAlloc<CXXBaseSpecifier *>(PathSize)); + void *Buffer = + C.Allocate(totalSizeToAlloc<CastExpr::BasePathSizeTy, CXXBaseSpecifier *>( + PathSize ? 1 : 0, PathSize)); return new (Buffer) CXXFunctionalCastExpr(EmptyShell(), PathSize); } Added: cfe/trunk/test/CodeGenCXX/castexpr-basepathsize-threshold.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/castexpr-basepathsize-threshold.cpp?rev=338489&view=auto ============================================================================== --- cfe/trunk/test/CodeGenCXX/castexpr-basepathsize-threshold.cpp (added) +++ cfe/trunk/test/CodeGenCXX/castexpr-basepathsize-threshold.cpp Tue Jul 31 23:06:16 2018 @@ -0,0 +1,27 @@ +// RUN: %clang_cc1 %s -emit-llvm -o - + +// https://bugs.llvm.org/show_bug.cgi?id=38356 +// We only check that we do not crash. + +template <typename a, a b(unsigned), int c, unsigned...> +struct d : d<a, b, c - 1> {}; +template <typename a, a b(unsigned), unsigned... e> +struct d<a, b, 0, e...> { + a f[0]; +}; +struct g { + static g h(unsigned); +}; +struct i { + void j() const; + // Current maximum depth of recursive template instantiation is 1024, + // thus, this \/ threshold value is used here. BasePathSize in CastExpr might + // not fit it, so we are testing that we do fit it. + // If -ftemplate-depth= is provided, larger values (4096 and up) cause crashes + // elsewhere. + d<g, g::h, (1U << 10U) - 2U> f; +}; +void i::j() const { + const void *k{f.f}; + (void)k; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
