Author: Timm Baeder Date: 2026-09-20T13:31:04+02:00 New Revision: aa6b03ed5a9ffe5ba6f0d945fac9961884b5c6ed
URL: https://github.com/llvm/llvm-project/commit/aa6b03ed5a9ffe5ba6f0d945fac9961884b5c6ed DIFF: https://github.com/llvm/llvm-project/commit/aa6b03ed5a9ffe5ba6f0d945fac9961884b5c6ed.diff LOG: [clang][bytecode] Notify bytecode interpreter after deserializing constant global declarations (#198062) The problem looks something like this: ```c++ extern const int m; constexpr int getm() { return m; } const int m = 12; static_assert(getm() == 12); ``` The generated bytecode for `getm()` references a global variable stored in `interp::Program`. This variable is uninitialized when the bytecode is generated, but since it's extern, that's not diagnosed (because it might be initialized later). When we parse the redeclaration of `m` on line 4, we evaluate its initializer via `Expr::EvaluateAsInitializer`, which makes the bytecode interpreter aware that `m` has been redeclared. It will create a new global for the new declaration and make the previous global point to the new one. When the bytecode is then evaluated in the `static_assert`, there is no problem. However, when the redeclaration isn't part of the TU but gets de-serialized via a module (which is the case in `test/Modules/redecl-add-after-load.cpp`), we don't call `EvaluateAsInitializer`, we don't even necessarily de-serialize the initializer expression at all. So the bytecode interpreter doesn't know about the new declaration and the static_assert ultimately fails because `m` is not initialized. Marking this as a draft PR since I'm not sure this is the best way to do this and if bytecode-specific things in other parts of the code base are tolerated. Added: clang/test/AST/ByteCode/Inputs/module.modulemap clang/test/AST/ByteCode/Inputs/redecl1.h clang/test/AST/ByteCode/module-redecl1.cpp Modified: clang/lib/AST/ByteCode/Compiler.cpp clang/lib/AST/ByteCode/Compiler.h clang/lib/AST/ByteCode/Context.cpp clang/lib/AST/ByteCode/Context.h clang/lib/AST/ByteCode/EvalEmitter.cpp clang/lib/AST/ByteCode/EvalEmitter.h clang/lib/AST/ByteCode/InterpState.cpp clang/lib/AST/ByteCode/InterpState.h clang/lib/AST/ByteCode/Program.cpp clang/lib/Serialization/ASTReaderDecl.cpp clang/test/Modules/pr102360.cppm clang/test/Modules/redecl-add-after-load.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index 2d5dbdfda003c..38a095f8a1164 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -6076,6 +6076,25 @@ bool Compiler<Emitter>::visitAPValueInitializer(const APValue &Val, return false; } +template <class Emitter> +bool Compiler<Emitter>::registerRedecl(const VarDecl *VD, const APValue &Val) { + if (P.getGlobal(VD)) + return true; + + UnsignedOrNone GlobalIndex = P.createGlobal(VD, /*Init=*/nullptr); + if (!GlobalIndex) { + llvm_unreachable("Why didn't that work?"); + } + + assert(canClassify(VD->getType()) && + "registerRedecl should only be called with primitive values"); + + PrimType T = classifyPrim(VD->getType()); + if (!visitAPValue(Val, T, VD)) + return false; + return this->emitInitGlobal(T, *GlobalIndex, {}); +} + template <class Emitter> bool Compiler<Emitter>::VisitBuiltinCallExpr(const CallExpr *E, unsigned BuiltinID) { diff --git a/clang/lib/AST/ByteCode/Compiler.h b/clang/lib/AST/ByteCode/Compiler.h index 884ff9336de2b..56a4d098640c3 100644 --- a/clang/lib/AST/ByteCode/Compiler.h +++ b/clang/lib/AST/ByteCode/Compiler.h @@ -260,6 +260,8 @@ class Compiler final : public ConstStmtVisitor<Compiler<Emitter>, bool>, bool visitCXXExpansionStmtInstantiation(const CXXExpansionStmtInstantiation *S); + bool registerRedecl(const VarDecl *VD, const APValue &V); + protected: bool visitStmt(const Stmt *S); bool visitExpr(const Expr *E, bool DestroyToplevelScope) override; diff --git a/clang/lib/AST/ByteCode/Context.cpp b/clang/lib/AST/ByteCode/Context.cpp index d96bc68217576..61ee2d255e905 100644 --- a/clang/lib/AST/ByteCode/Context.cpp +++ b/clang/lib/AST/ByteCode/Context.cpp @@ -179,6 +179,13 @@ bool Context::evaluateDestruction(State &Parent, const VarDecl *VD, return true; } +void Context::registerRedecl(const VarDecl *VD, const APValue &V) { + Expr::EvalStatus Status; + Compiler<EvalEmitter> C(*this, *P, Status, Stk, FrameAlloc); + + C.registerRedecl(VD, V); +} + template <typename ResultT> bool Context::evaluateStringRepr(State &Parent, const Expr *SizeExpr, const Expr *PtrExpr, ResultT &Result) { diff --git a/clang/lib/AST/ByteCode/Context.h b/clang/lib/AST/ByteCode/Context.h index a555896a822a5..1390bc280602e 100644 --- a/clang/lib/AST/ByteCode/Context.h +++ b/clang/lib/AST/ByteCode/Context.h @@ -68,6 +68,7 @@ class Context final { /// Evaluates a toplevel initializer. bool evaluateAsInitializer(State &Parent, const VarDecl *VD, const Expr *Init, APValue &Result); + void registerRedecl(const VarDecl *VD, const APValue &V); /// Evaluates the destruction of a variable. bool evaluateDestruction(State &Parent, const VarDecl *VD, APValue Value); diff --git a/clang/lib/AST/ByteCode/EvalEmitter.cpp b/clang/lib/AST/ByteCode/EvalEmitter.cpp index d0f41ec9fd075..0e8b6f9dce56f 100644 --- a/clang/lib/AST/ByteCode/EvalEmitter.cpp +++ b/clang/lib/AST/ByteCode/EvalEmitter.cpp @@ -21,6 +21,10 @@ EvalEmitter::EvalEmitter(Context &Ctx, Program &P, State &Parent, InterpStack &Stk, FrameAllocator &FA) : Ctx(Ctx), P(P), S(Parent, P, Stk, FA, Ctx, this), EvalResult(&Ctx) {} +EvalEmitter::EvalEmitter(Context &Ctx, Program &P, Expr::EvalStatus &Status, + InterpStack &Stk, FrameAllocator &FA) + : Ctx(Ctx), P(P), S(Status, P, Stk, FA, Ctx, this), EvalResult(&Ctx) {} + /// Clean up all our resources. This needs to done in failed evaluations before /// we call InterpStack::clear(), because there might be a Pointer on the stack /// pointing into a Block in the EvalEmitter. diff --git a/clang/lib/AST/ByteCode/EvalEmitter.h b/clang/lib/AST/ByteCode/EvalEmitter.h index 3e2b0b3bfcc93..069758615012d 100644 --- a/clang/lib/AST/ByteCode/EvalEmitter.h +++ b/clang/lib/AST/ByteCode/EvalEmitter.h @@ -65,6 +65,9 @@ class EvalEmitter : public SourceMapper { EvalEmitter(Context &Ctx, Program &P, State &Parent, InterpStack &Stk, FrameAllocator &FrameAlloc); + EvalEmitter(Context &Ctx, Program &P, Expr::EvalStatus &Status, + InterpStack &Stk, FrameAllocator &FrameAlloc); + /// Define a label. void emitLabel(LabelTy Label); /// Create a label. diff --git a/clang/lib/AST/ByteCode/InterpState.cpp b/clang/lib/AST/ByteCode/InterpState.cpp index fa59b52408611..7d0364fa7d10d 100644 --- a/clang/lib/AST/ByteCode/InterpState.cpp +++ b/clang/lib/AST/ByteCode/InterpState.cpp @@ -32,9 +32,8 @@ InterpState::InterpState(const State &Parent, Program &P, InterpStack &Stk, } InterpState::InterpState(const State &Parent, Program &P, InterpStack &Stk, - FrameAllocator &FrameAlloc, - - Context &Ctx, const Function *Func) + FrameAllocator &FrameAlloc, Context &Ctx, + const Function *Func) : State(Ctx.getASTContext(), Parent.getEvalStatus()), M(nullptr), FrameAlloc(FrameAlloc), P(P), Stk(Stk), Ctx(Ctx), BottomFrame(*this), Current(&BottomFrame), StepsLeft(Ctx.getLangOpts().ConstexprStepLimit), @@ -46,6 +45,19 @@ InterpState::InterpState(const State &Parent, Program &P, InterpStack &Stk, EvalMode = Parent.EvalMode; } +InterpState::InterpState(Expr::EvalStatus &Status, Program &P, InterpStack &Stk, + FrameAllocator &FrameAlloc, Context &Ctx, + SourceMapper *M) + : State(Ctx.getASTContext(), Status), M(M), FrameAlloc(FrameAlloc), P(P), + Stk(Stk), Ctx(Ctx), BottomFrame(*this), Current(&BottomFrame), + StepsLeft(Ctx.getLangOpts().ConstexprStepLimit), + InfiniteSteps(StepsLeft == 0), EvalID(Ctx.getEvalID()) { + InConstantContext = true; + CheckingPotentialConstantExpression = false; + CheckingForUndefinedBehavior = true; + EvalMode = EvaluationMode::ConstantExpression; +} + bool InterpState::inConstantContext() const { if (ConstantContextOverride) return *ConstantContextOverride; diff --git a/clang/lib/AST/ByteCode/InterpState.h b/clang/lib/AST/ByteCode/InterpState.h index 91d0c5be2bb0c..b4f89e9583d97 100644 --- a/clang/lib/AST/ByteCode/InterpState.h +++ b/clang/lib/AST/ByteCode/InterpState.h @@ -48,8 +48,12 @@ class InterpState final : public State { InterpState(const State &Parent, Program &P, InterpStack &Stk, FrameAllocator &FrameAlloc, Context &Ctx, SourceMapper *M = nullptr); + InterpState(const State &Parent, Program &P, InterpStack &Stk, - FrameAllocator &FrameAlloc, Context &Ctx, const Function *Func); + FrameAllocator &FA, Context &Ctx, const Function *Func); + + InterpState(Expr::EvalStatus &Status, Program &P, InterpStack &Stk, + FrameAllocator &FA, Context &Ctx, SourceMapper *M); ~InterpState(); diff --git a/clang/lib/AST/ByteCode/Program.cpp b/clang/lib/AST/ByteCode/Program.cpp index cf6b7e1682121..0ab9ee0440bf8 100644 --- a/clang/lib/AST/ByteCode/Program.cpp +++ b/clang/lib/AST/ByteCode/Program.cpp @@ -19,7 +19,26 @@ using namespace clang::interp; Pointer Program::getPtrGlobal(unsigned Idx) const { assert(Idx < Globals.size()); - return Pointer(Globals[Idx]->block()); + + Block *B = Globals[Idx]->block(); + + // Force de-serialization of a redeclaration that might initialize this + // global. + if (B->getMetadataSize() != 0 && + B->getBlockDesc<GlobalInlineDescriptor>().InitState != + GlobalInitState::Initialized) { + if (const VarDecl *VD = B->getDescriptor()->asVarDecl()) { + const VarDecl *MD = VD->getMostRecentDecl(); + if (MD != VD && MD->hasInit() && !MD->getInit()->isValueDependent()) { + MD->evaluateValue(); + // Note that we need to get Globals[Idx] here again since the code block + // above might've actually changed what global Idx points to. + return Pointer(Globals[Idx]->block()); + } + } + } + + return Pointer(B); } UnsignedOrNone Program::getGlobal(const ValueDecl *VD) { diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 7fc585b12153b..d7d650ef02192 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -11,6 +11,7 @@ // //===----------------------------------------------------------------------===// +#include "../AST/ByteCode/Context.h" #include "ASTCommon.h" #include "ASTReaderInternals.h" #include "clang/AST/ASTConcept.h" @@ -1705,6 +1706,7 @@ RedeclarableResult ASTDeclReader::VisitVarDeclImpl(VarDecl *VD) { void ASTDeclReader::ReadVarDeclInit(VarDecl *VD) { if (uint64_t Val = Record.readInt()) { + ASTContext &Context = Reader.getContext(); EvaluatedStmt *Eval = VD->ensureEvaluatedStmt(); Eval->HasConstantInitialization = (Val & 2) != 0; Eval->HasConstantDestruction = (Val & 4) != 0; @@ -1714,7 +1716,15 @@ void ASTDeclReader::ReadVarDeclInit(VarDecl *VD) { if (Eval->WasEvaluated) { Eval->Evaluated = Record.readAPValue(); if (Eval->Evaluated.needsCleanup()) - Reader.getContext().addDestruction(&Eval->Evaluated); + Context.addDestruction(&Eval->Evaluated); + + // The bytecode interpreter has its own internal representation of global + // variables. Notify it that we just deserialized one and what its value + // is. This is important because this declaration might initialize a + // previously declared global (e.g. because that one is extern). + if (Context.getLangOpts().EnableNewConstInterp && + !VD->getType().isNull() && VD->getPreviousDecl() != nullptr) + Context.getInterpContext().registerRedecl(VD, Eval->Evaluated); } // Store the offset of the initializer. Don't deserialize it yet: it might diff --git a/clang/test/AST/ByteCode/Inputs/module.modulemap b/clang/test/AST/ByteCode/Inputs/module.modulemap new file mode 100644 index 0000000000000..6ba8d2e8a184b --- /dev/null +++ b/clang/test/AST/ByteCode/Inputs/module.modulemap @@ -0,0 +1 @@ +module redecl1 { header "redecl1.h" } diff --git a/clang/test/AST/ByteCode/Inputs/redecl1.h b/clang/test/AST/ByteCode/Inputs/redecl1.h new file mode 100644 index 0000000000000..6e02cab6b3133 --- /dev/null +++ b/clang/test/AST/ByteCode/Inputs/redecl1.h @@ -0,0 +1,6 @@ + +extern const int variable = 120; + + +struct S { int a; }; +extern constexpr S vars = {12}; diff --git a/clang/test/AST/ByteCode/module-redecl1.cpp b/clang/test/AST/ByteCode/module-redecl1.cpp new file mode 100644 index 0000000000000..90961eeae4aff --- /dev/null +++ b/clang/test/AST/ByteCode/module-redecl1.cpp @@ -0,0 +1,25 @@ +// RUN: %clang_cc1 -cc1 -xobjective-c++ %s -fmodules -fimplicit-module-maps -fmodules-cache-path=Inputs/ -I %S/Inputs -verify -std=c++11 +// RUN: %clang_cc1 -cc1 -xobjective-c++ %s -fmodules -fimplicit-module-maps -fmodules-cache-path=Inputs/ -I %S/Inputs -verify -std=c++11 -fexperimental-new-constant-interpreter + +// expected-no-diagnostics + +struct S { int a; }; + +extern const int variable; +extern const S vars; + +constexpr int test() { return variable; } +constexpr int test2() { return vars.a; } + +struct C { + static const int variable; + static const S vars; +}; + + + +/// The module contains a definition for 'variable', so the function call below +/// should work and return the correct value. +@import redecl1; +static_assert(test() == 120, ""); +static_assert(test2() == 12, ""); diff --git a/clang/test/Modules/pr102360.cppm b/clang/test/Modules/pr102360.cppm index e0dab1a031801..31109d6959512 100644 --- a/clang/test/Modules/pr102360.cppm +++ b/clang/test/Modules/pr102360.cppm @@ -10,6 +10,14 @@ // RUN: %clang_cc1 -std=c++20 %t/d.cpp -fsyntax-only -verify \ // RUN: -fprebuilt-module-path=%t +// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-module-interface -o %t/a.pcm -fexperimental-new-constant-interpreter +// RUN: %clang_cc1 -std=c++20 %t/b.cppm -emit-module-interface -o %t/b.pcm \ +// RUN: -fprebuilt-module-path=%t -fexperimental-new-constant-interpreter +// RUN: %clang_cc1 -std=c++20 %t/c.cppm -emit-module-interface -o %t/c.pcm \ +// RUN: -fprebuilt-module-path=%t -fexperimental-new-constant-interpreter +// RUN: %clang_cc1 -std=c++20 %t/d.cpp -fsyntax-only -verify \ +// RUN: -fprebuilt-module-path=%t -fexperimental-new-constant-interpreter + //--- a.cppm export module a; diff --git a/clang/test/Modules/redecl-add-after-load.cpp b/clang/test/Modules/redecl-add-after-load.cpp index f888460f297e7..101be0b1fde4e 100644 --- a/clang/test/Modules/redecl-add-after-load.cpp +++ b/clang/test/Modules/redecl-add-after-load.cpp @@ -2,6 +2,10 @@ // RUN: %clang_cc1 -x objective-c++ -fmodules -fimplicit-module-maps -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -verify -std=c++11 // RUN: %clang_cc1 -x objective-c++ -fmodules -fimplicit-module-maps -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -verify -std=c++11 -DIMPORT_DECLS +// RUN: %clang_cc1 -x objective-c++ -fmodules -fimplicit-module-maps -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -verify -std=c++11 -fexperimental-new-constant-interpreter +// RUN: %clang_cc1 -x objective-c++ -fmodules -fimplicit-module-maps -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -verify -std=c++11 -DIMPORT_DECLS -fexperimental-new-constant-interpreter + + // expected-no-diagnostics #ifdef IMPORT_DECLS _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
