https://github.com/vgvassilev updated https://github.com/llvm/llvm-project/pull/185648
>From 2477eb6a3a89f0db5994738f27034fdcfb3559d8 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 99640f5ce2ad1..1254c708cd8e1 100644 --- a/clang/lib/CodeGen/CGVTables.cpp +++ b/clang/lib/CodeGen/CGVTables.cpp @@ -1210,6 +1210,7 @@ CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD) { /// emits them as-needed. void CodeGenModule::EmitVTable(CXXRecordDecl *theClass) { VTables.GenerateClassData(theClass); + EmittedVTables.insert(theClass); } void @@ -1292,11 +1293,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 416a5beca0de7..91bcec157d0d4 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -8696,6 +8696,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 b7dab4ef583ff..5e5e73d9c4c44 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -478,6 +478,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
