Author: Timm Baeder Date: 2024-09-07T06:36:21+02:00 New Revision: eef8116be169de2a7a55b2a9b003efd2c7ee0be0
URL: https://github.com/llvm/llvm-project/commit/eef8116be169de2a7a55b2a9b003efd2c7ee0be0 DIFF: https://github.com/llvm/llvm-project/commit/eef8116be169de2a7a55b2a9b003efd2c7ee0be0.diff LOG: [clang][bytecode] Only visit local variables if they have constant init (#107576) See the comment I added for why this is weird. We might want to have a different mechanism for this in the future. Fixes https://github.com/llvm/llvm-project/issues/101801 Added: Modified: clang/lib/AST/ByteCode/Compiler.cpp clang/lib/AST/ByteCode/Context.cpp clang/lib/AST/ByteCode/InterpStack.cpp clang/lib/AST/ByteCode/InterpStack.h Removed: ################################################################################ diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index e96bafd89c7eca..7d812301b6d36d 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -5615,11 +5615,18 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) { if (isa<DecompositionDecl>(VD)) return revisit(VD); - // Visit local const variables like normal. - if ((VD->hasGlobalStorage() || VD->isLocalVarDecl() || - VD->isStaticDataMember()) && + if ((VD->hasGlobalStorage() || VD->isStaticDataMember()) && typeShouldBeVisited(VD->getType())) return revisit(VD); + + // FIXME: The evaluateValue() check here is a little ridiculous, since + // it will ultimately call into Context::evaluateAsInitializer(). In + // other words, we're evaluating the initializer, just to know if we can + // evaluate the initializer. + if (VD->isLocalVarDecl() && typeShouldBeVisited(VD->getType()) && + VD->getInit() && !VD->getInit()->isValueDependent() && + VD->evaluateValue()) + return revisit(VD); } } else { if (const auto *VD = dyn_cast<VarDecl>(D); diff --git a/clang/lib/AST/ByteCode/Context.cpp b/clang/lib/AST/ByteCode/Context.cpp index e682d87b703ad7..594c74cd3796d6 100644 --- a/clang/lib/AST/ByteCode/Context.cpp +++ b/clang/lib/AST/ByteCode/Context.cpp @@ -44,13 +44,14 @@ bool Context::isPotentialConstantExpr(State &Parent, const FunctionDecl *FD) { bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) { ++EvalID; bool Recursing = !Stk.empty(); + size_t StackSizeBefore = Stk.size(); Compiler<EvalEmitter> C(*this, *P, Parent, Stk); auto Res = C.interpretExpr(E, /*ConvertResultToRValue=*/E->isGLValue()); if (Res.isInvalid()) { C.cleanup(); - Stk.clear(); + Stk.clearTo(StackSizeBefore); return false; } @@ -60,7 +61,7 @@ bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) { #ifndef NDEBUG // Make sure we don't rely on some value being still alive in // InterpStack memory. - Stk.clear(); + Stk.clearTo(StackSizeBefore); #endif } @@ -72,12 +73,13 @@ bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) { bool Context::evaluate(State &Parent, const Expr *E, APValue &Result) { ++EvalID; bool Recursing = !Stk.empty(); + size_t StackSizeBefore = Stk.size(); Compiler<EvalEmitter> C(*this, *P, Parent, Stk); auto Res = C.interpretExpr(E); if (Res.isInvalid()) { C.cleanup(); - Stk.clear(); + Stk.clearTo(StackSizeBefore); return false; } @@ -87,7 +89,7 @@ bool Context::evaluate(State &Parent, const Expr *E, APValue &Result) { #ifndef NDEBUG // Make sure we don't rely on some value being still alive in // InterpStack memory. - Stk.clear(); + Stk.clearTo(StackSizeBefore); #endif } @@ -99,6 +101,7 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD, APValue &Result) { ++EvalID; bool Recursing = !Stk.empty(); + size_t StackSizeBefore = Stk.size(); Compiler<EvalEmitter> C(*this, *P, Parent, Stk); bool CheckGlobalInitialized = @@ -107,7 +110,8 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD, auto Res = C.interpretDecl(VD, CheckGlobalInitialized); if (Res.isInvalid()) { C.cleanup(); - Stk.clear(); + Stk.clearTo(StackSizeBefore); + return false; } @@ -117,7 +121,7 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD, #ifndef NDEBUG // Make sure we don't rely on some value being still alive in // InterpStack memory. - Stk.clear(); + Stk.clearTo(StackSizeBefore); #endif } diff --git a/clang/lib/AST/ByteCode/InterpStack.cpp b/clang/lib/AST/ByteCode/InterpStack.cpp index b8cdaeee72166c..ae3721e983741d 100644 --- a/clang/lib/AST/ByteCode/InterpStack.cpp +++ b/clang/lib/AST/ByteCode/InterpStack.cpp @@ -32,6 +32,16 @@ void InterpStack::clear() { #endif } +void InterpStack::clearTo(size_t NewSize) { + assert(NewSize <= size()); + size_t ToShrink = size() - NewSize; + if (ToShrink == 0) + return; + + shrink(ToShrink); + assert(size() == NewSize); +} + void *InterpStack::grow(size_t Size) { assert(Size < ChunkSize - sizeof(StackChunk) && "Object too large"); @@ -81,6 +91,21 @@ void InterpStack::shrink(size_t Size) { Chunk->End -= Size; StackSize -= Size; + +#ifndef NDEBUG + size_t TypesSize = 0; + for (PrimType T : ItemTypes) + TYPE_SWITCH(T, { TypesSize += aligned_size<T>(); }); + + size_t StackSize = size(); + while (TypesSize > StackSize) { + TYPE_SWITCH(ItemTypes.back(), { + TypesSize -= aligned_size<T>(); + ItemTypes.pop_back(); + }); + } + assert(TypesSize == StackSize); +#endif } void InterpStack::dump() const { diff --git a/clang/lib/AST/ByteCode/InterpStack.h b/clang/lib/AST/ByteCode/InterpStack.h index 153d17f0d70e1b..43988bb680d1c6 100644 --- a/clang/lib/AST/ByteCode/InterpStack.h +++ b/clang/lib/AST/ByteCode/InterpStack.h @@ -86,6 +86,7 @@ class InterpStack final { /// Clears the stack without calling any destructors. void clear(); + void clearTo(size_t NewSize); /// Returns whether the stack is empty. bool empty() const { return StackSize == 0; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
