https://github.com/conrade-ctc updated https://github.com/llvm/llvm-project/pull/185648
>From 965c25f6f6fb556d8fd4968ad0f9d621a08b7dfc Mon Sep 17 00:00:00 2001 From: Emery Conrad <[email protected]> Date: Tue, 10 Mar 2026 07:22:18 -0500 Subject: [PATCH] [clang-repl] fix vtable double-emission via DefinedVTables tracking clang/test/Interpreter/virtualdef-outside.cpp exposes bug, which has the following root cause: moveLazyEmissionStates() carries DeferredVTables across PTU boundaries but not the ItaniumCXXABI::VTables DenseMap, which is the per-module cache that guards against re-emission via hasInitializer(). Each new PTU's fresh ItaniumCXXABI doesn't know a previous PTU already defined the vtable, so EmitDeferredVTables() re-emits it. Fix: add CodeGenModule::DefinedVTables (SmallPtrSet) to track classes whose vtable was defined with ExternalLinkage in the current module. moveLazyEmissionStates() transfers the set to the next PTU's module. EmitDeferredVTables() skips any class already in DefinedVTables, leaving its GlobalVariable as an external declaration that the JIT resolves from the defining PTU's already-loaded code. Only ExternalLinkage vtables are tracked: the JIT reliably exports strong symbols cross-module. WeakODR/LinkOnceODR vtables (inline key functions, template instantiations) are not reliably cross-module resolvable in the JIT, so later PTUs must re-emit their own copy via linkonce deduplication. closes #141039 --- clang/lib/CodeGen/CGVTables.cpp | 10 ++++- clang/lib/CodeGen/CodeGenModule.cpp | 4 ++ clang/lib/CodeGen/CodeGenModule.h | 5 +++ clang/test/Interpreter/virtualdef-outside.cpp | 40 +++++++++++++++++++ 4 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 clang/test/Interpreter/virtualdef-outside.cpp diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp index 3891697a986e4..016a7de54ff2f 100644 --- a/clang/lib/CodeGen/CGVTables.cpp +++ b/clang/lib/CodeGen/CGVTables.cpp @@ -1218,6 +1218,7 @@ CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD) { /// emits them as-needed. void CodeGenModule::EmitVTable(CXXRecordDecl *theClass) { VTables.GenerateClassData(theClass); + EmittedVTables.insert(theClass); } void @@ -1300,11 +1301,18 @@ void CodeGenModule::EmitDeferredVTables() { size_t savedSize = DeferredVTables.size(); #endif - for (const CXXRecordDecl *RD : DeferredVTables) + for (const CXXRecordDecl *RD : DeferredVTables) { + // if a table has been emitted in an earlier PTU, but was also marked + // deferred, we should skip if the linkage is external + if (EmittedVTables.count(RD) && + getVTableLinkage(RD) == llvm::GlobalValue::ExternalLinkage) + continue; + if (shouldEmitVTableAtEndOfTranslationUnit(*this, RD)) VTables.GenerateClassData(RD); else if (shouldOpportunisticallyEmitVTables()) OpportunisticVTables.push_back(RD); + } assert(savedSize == DeferredVTables.size() && "deferred extra vtables during vtable emission?"); diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index d08fe0feb692e..6e3c11286c7eb 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -8519,6 +8519,10 @@ void CodeGenModule::moveLazyEmissionStates(CodeGenModule *NewBuilder) { "Newly created module should not have deferred vtables"); NewBuilder->DeferredVTables = std::move(DeferredVTables); + assert(NewBuilder->EmittedVTables.empty() && + "Newly created module should not have defined vtables"); + NewBuilder->EmittedVTables = std::move(EmittedVTables); + assert(NewBuilder->MangledDeclNames.empty() && "Newly created module should not have mangled decl names"); assert(NewBuilder->Manglings.empty() && diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index 0081bf5c4cf5f..86dcb0a7b4d22 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -456,6 +456,11 @@ class CodeGenModule : public CodeGenTypeCache { /// A queue of (optional) vtables to consider emitting. std::vector<const CXXRecordDecl*> DeferredVTables; + /// In incremental compilation, the set of vtable classes whose vtable + /// definitions were emitted into a previous PTU's module. Carried forward + /// by moveLazyEmissionStates() so later PTUs skip re-defining them. + llvm::SmallPtrSet<const CXXRecordDecl *, 8> EmittedVTables; + /// A queue of (optional) vtables that may be emitted opportunistically. std::vector<const CXXRecordDecl *> OpportunisticVTables; diff --git a/clang/test/Interpreter/virtualdef-outside.cpp b/clang/test/Interpreter/virtualdef-outside.cpp new file mode 100644 index 0000000000000..8c2fc4891182b --- /dev/null +++ b/clang/test/Interpreter/virtualdef-outside.cpp @@ -0,0 +1,40 @@ +// REQUIRES: host-supports-jit +// RUN: cat %s | clang-repl | FileCheck %s +// virtual functions defined outside of class had duplicate symbols: +// duplicate definition of symbol '__ZTV3Two' (i.e., vtable for Two) +// see https://github.com/llvm/llvm-project/issues/141039. +// fixed in PR #185648 + +extern "C" int printf(const char *, ...); + +struct X1 { virtual void vi() { printf("1vi\n"); } }; +X1().vi(); +// CHECK: 1vi + +struct X2 { virtual void vo(); }; +void X2::vo() { printf("2vo\n"); } +X2().vo(); +// CHECK: 2vo + +struct X3 { \ + void ni() { printf("3ni\n"); } \ + void no(); \ + virtual void vi() { printf("3vi\n"); } \ + virtual void vo(); \ + virtual ~X3() { printf("3d\n"); } \ +}; +void X3::no() { printf("3no\n"); } +void X3::vo() { printf("3vo\n"); } +auto x3 = new X3; +x3->ni(); +// CHECK: 3ni +x3->no(); +// CHECK: 3no +x3->vi(); +// CHECK: 3vi +x3->vo(); +// CHECK: 3vo +delete x3; +// CHECK: 3d + +%quit _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
