[clang] [clang-repl] fix vtable symbol duplication error (closes #141039) (PR #185648)
Emery Conrad via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 11 09:29:08 PDT 2026
https://github.com/conrade-ctc updated https://github.com/llvm/llvm-project/pull/185648
>From 05c2c79948547d0629f727111fe98502280f612f Mon Sep 17 00:00:00 2001
From: Emery Conrad <emery.conrad at chicagotrading.com>
Date: Tue, 10 Mar 2026 07:22:18 -0500
Subject: [PATCH 1/2] [clang-repl] fix vtable symbol duplication error
In incremental mode, emit by ExternalLinkage causes duplicate
symbol error. A single targeted change in getVTableLinkage()
fixes the issue by returning LinkOnceODRLinkage when
IncrementalExtensions is active. The JIT linker then keeps the first
definition a silently discards subsequent ones.
(Claude Sonnet 4.6 helped find this minimal delta)
---
clang/lib/CodeGen/CGVTables.cpp | 8 ++++++++
clang/test/Interpreter/virtualdef-outside.cpp | 19 +++++++++++++++++++
2 files changed, 27 insertions(+)
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..cadc98169226a 100644
--- a/clang/lib/CodeGen/CGVTables.cpp
+++ b/clang/lib/CodeGen/CGVTables.cpp
@@ -1151,6 +1151,14 @@ CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD) {
? llvm::GlobalVariable::LinkOnceODRLinkage
: llvm::Function::InternalLinkage;
+ // In incremental compilation, each partial translation unit is a separate
+ // LLVM module. Multiple PTUs may independently emit the vtable (once the
+ // key function's body is visible), causing duplicate symbol errors in the
+ // JIT. Use linkonce_odr so the JIT linker keeps the first definition and
+ // silently discards the rest, matching how template vtables work.
+ if (Context.getLangOpts().IncrementalExtensions)
+ return llvm::GlobalVariable::LinkOnceODRLinkage;
+
return llvm::GlobalVariable::ExternalLinkage;
case TSK_ImplicitInstantiation:
diff --git a/clang/test/Interpreter/virtualdef-outside.cpp b/clang/test/Interpreter/virtualdef-outside.cpp
new file mode 100644
index 0000000000000..a9198b3a22da5
--- /dev/null
+++ b/clang/test/Interpreter/virtualdef-outside.cpp
@@ -0,0 +1,19 @@
+// 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 One { virtual void print() { printf("ONE\n"); } };
+One().print();
+// CHECK: ONE
+
+struct Two { virtual void print(); };
+void Two::print() { printf("TWO\n"); }
+Two().print();
+// CHECK: TWO
+
+%quit
>From dcf3c835426ce6e127d3d9e2f058c9eda419b34f Mon Sep 17 00:00:00 2001
From: Emery Conrad <emery.conrad at chicagotrading.com>
Date: Wed, 11 Mar 2026 10:43:08 -0500
Subject: [PATCH 2/2] [clang-repl] fix vtable double-emission via
DefinedVTables tracking
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.
Because this fix prevents the double emission at the root, the
LinkOnceODRLinkage workaround added in the previous commit is reverted;
vtables with non-inline key functions correctly use ExternalLinkage again.
---
clang/lib/CodeGen/CGVTables.cpp | 16 +++++++---------
clang/lib/CodeGen/CodeGenModule.cpp | 4 ++++
clang/lib/CodeGen/CodeGenModule.h | 9 +++++++++
clang/lib/CodeGen/ItaniumCXXABI.cpp | 9 +++++++++
4 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp
index cadc98169226a..61778534ac06d 100644
--- a/clang/lib/CodeGen/CGVTables.cpp
+++ b/clang/lib/CodeGen/CGVTables.cpp
@@ -1151,14 +1151,6 @@ CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD) {
? llvm::GlobalVariable::LinkOnceODRLinkage
: llvm::Function::InternalLinkage;
- // In incremental compilation, each partial translation unit is a separate
- // LLVM module. Multiple PTUs may independently emit the vtable (once the
- // key function's body is visible), causing duplicate symbol errors in the
- // JIT. Use linkonce_odr so the JIT linker keeps the first definition and
- // silently discards the rest, matching how template vtables work.
- if (Context.getLangOpts().IncrementalExtensions)
- return llvm::GlobalVariable::LinkOnceODRLinkage;
-
return llvm::GlobalVariable::ExternalLinkage;
case TSK_ImplicitInstantiation:
@@ -1308,11 +1300,17 @@ void CodeGenModule::EmitDeferredVTables() {
size_t savedSize = DeferredVTables.size();
#endif
- for (const CXXRecordDecl *RD : DeferredVTables)
+ for (const CXXRecordDecl *RD : DeferredVTables) {
+ // In incremental compilation, the vtable may have been defined in a
+ // previous PTU's module. The GlobalVariable in this module is just an
+ // external declaration; the JIT resolves it from the earlier module.
+ if (DefinedVTables.count(RD))
+ 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 3b64be7a477d6..4988604c3c8b3 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -8506,6 +8506,10 @@ void CodeGenModule::moveLazyEmissionStates(CodeGenModule *NewBuilder) {
"Newly created module should not have deferred vtables");
NewBuilder->DeferredVTables = std::move(DeferredVTables);
+ assert(NewBuilder->DefinedVTables.empty() &&
+ "Newly created module should not have defined vtables");
+ NewBuilder->DefinedVTables = std::move(DefinedVTables);
+
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..7fafe137a2ea6 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> DefinedVTables;
+
/// A queue of (optional) vtables that may be emitted opportunistically.
std::vector<const CXXRecordDecl *> OpportunisticVTables;
@@ -1574,6 +1579,10 @@ class CodeGenModule : public CodeGenTypeCache {
DeferredVTables.push_back(RD);
}
+ void markVTableDefined(const CXXRecordDecl *RD) {
+ DefinedVTables.insert(RD);
+ }
+
/// Emit code for a single global function or var decl. Forward declarations
/// are emitted lazily.
void EmitGlobal(GlobalDecl D);
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index 8a06051a1c730..1b4f8e1d11f8a 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2097,6 +2097,15 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables &CGVT,
// Set the correct linkage.
VTable->setLinkage(Linkage);
+ // Track ExternalLinkage vtable definitions so moveLazyEmissionStates() can
+ // tell the next PTU not to re-emit them. We only do this for ExternalLinkage
+ // because the JIT reliably exports strong symbols cross-module.
+ // WeakODR/LinkOnceODR vtables (e.g. inline key functions, template
+ // instantiations) are not reliably cross-module resolvable in the JIT, so
+ // later PTUs must re-emit their own copy.
+ if (Linkage == llvm::GlobalValue::ExternalLinkage)
+ CGM.markVTableDefined(RD);
+
if (CGM.supportsCOMDAT() && VTable->isWeakForLinker())
VTable->setComdat(CGM.getModule().getOrInsertComdat(VTable->getName()));
More information about the cfe-commits
mailing list