Skip to content

Commit d8696ac

Browse files
[wasm][IRGen] Stop attaching COMDATs to dead stub methods
When generating dead stub methods for vtable entries, we should not attach COMDATs to their definitions. This is because such stubs may be referenced only through aliases, and the presence of a COMDAT on the stub definition would cause the aliased symbol, we want to keep, to be discarded from the symbol table when other object files also have a dead stub method. Given the following two object files generated: ```mermaid graph TD subgraph O0[A.swift.o] subgraph C0[COMDAT Group swift_dead_method_stub] D0_0["[Def] swift_dead_method_stub"] end S0_0["[Symbol] swift_dead_method_stub"] --> D0_0 S1["[Symbol] C1.dead"] --alias--> D0_0 end subgraph O1[B.swift.o] subgraph C1[COMDAT Group swift_dead_method_stub] D0_1["[Def] swift_dead_method_stub"] end S0_1["[Symbol] swift_dead_method_stub"] --> D0_1 S2["[Symbol] C2.beef"] --alias--> D0_1 end ``` When linking these two object files, the linker will pick one of the COMDAT groups of `swift_dead_method_stub`. The other COMDAT group will be discarded, along with all symbols aliased to the discarded definition, even though those aliased symbols (`C1.dead` and `C2.beef`) are the ones we want to keep to construct vtables. This change stops attaching COMDATs to dead stub method definitions. This effectively only affects WebAssembly targets because MachO's `supportsCOMDAT` returns false, and COFF targets don't use `linkonce_odr` for dead stub methods. The COMDAT was added in 7e8f782
1 parent 25543a5 commit d8696ac

File tree

3 files changed

+51
-5
lines changed

3 files changed

+51
-5
lines changed

include/swift/IRGen/Linking.h

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1886,7 +1886,7 @@ class ApplyIRLinkage {
18861886
IRLinkage IRL;
18871887
public:
18881888
ApplyIRLinkage(IRLinkage IRL) : IRL(IRL) {}
1889-
void to(llvm::GlobalValue *GV, bool definition = true) const {
1889+
void to(llvm::GlobalValue *GV, bool nonAliasedDefinition = true) const {
18901890
llvm::Module *M = GV->getParent();
18911891
const llvm::Triple Triple(M->getTargetTriple());
18921892

@@ -1899,9 +1899,16 @@ class ApplyIRLinkage {
18991899
if (Triple.isOSBinFormatELF())
19001900
return;
19011901

1902-
// COMDATs cannot be applied to declarations. If we have a definition,
1903-
// apply the COMDAT.
1904-
if (definition)
1902+
// COMDATs cannot be applied to declarations. Also, definitions that are
1903+
// exported through aliases should not have COMDATs, because the alias
1904+
// itself might represent an externally visible symbol but such symbols
1905+
// are discarded from the symtab when other object files have a COMDAT
1906+
// group with the same signature.
1907+
//
1908+
// If we have a non-aliased definition with ODR-based linkage, attach it
1909+
// to a COMDAT group so that duplicate definitions across object files
1910+
// can be merged by the linker.
1911+
if (nonAliasedDefinition)
19051912
if (IRL.Linkage == llvm::GlobalValue::LinkOnceODRLinkage ||
19061913
IRL.Linkage == llvm::GlobalValue::WeakODRLinkage)
19071914
if (Triple.supportsCOMDAT())

lib/IRGen/GenDecl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2201,7 +2201,7 @@ void IRGenModule::emitVTableStubs() {
22012201
&Module);
22022202
ApplyIRLinkage(canLinkOnce ? IRLinkage::InternalLinkOnceODR
22032203
: IRLinkage::Internal)
2204-
.to(stub);
2204+
.to(stub, /* nonAliasedDefinition */ false);
22052205
stub->setAttributes(constructInitialAttributes());
22062206
stub->setCallingConv(DefaultCC);
22072207
auto *entry = llvm::BasicBlock::Create(getLLVMContext(), "entry", stub);
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file --leading-lines %s %t
3+
4+
// Ensure that swift_dead_method_stub is emitted without comdat linkage
5+
// RUN: %target-swift-frontend -target %target-swift-5.1-abi-triple -parse-as-library -O -module-name A -c -primary-file %t/A.swift %t/B.swift -emit-ir -o - | %FileCheck %s -check-prefix CHECK
6+
// RUN: %target-swift-frontend -target %target-swift-5.1-abi-triple -parse-as-library -O -module-name A -c %t/A.swift -primary-file %t/B.swift -emit-ir -o - | %FileCheck %s -check-prefix CHECK
7+
8+
// CHECK-LABEL: define {{(linkonce_odr )?}}hidden void @_swift_dead_method_stub(
9+
// CHECK-NOT: comdat
10+
// CHECK: {
11+
12+
13+
// Ensure that link-time deduplication for swift_dead_method_stub works correctly
14+
// RUN: %target-swift-frontend -target %target-swift-5.1-abi-triple -parse-as-library -O -module-name A -c -primary-file %t/A.swift %t/B.swift -o %t/A.swift.o
15+
// RUN: %target-swift-frontend -target %target-swift-5.1-abi-triple -parse-as-library -O -module-name A -c %t/A.swift -primary-file %t/B.swift -o %t/B.swift.o
16+
// RUN: %target-build-swift -target %target-swift-5.1-abi-triple %t/B.swift.o %t/A.swift.o -o %t/a.out
17+
18+
//--- A.swift
19+
20+
// Define an open class with a dead vtable entry
21+
class C1 {
22+
private func dead() {}
23+
}
24+
25+
//--- B.swift
26+
27+
class C2: C1 {
28+
// Define another dead vtable entry to ensure that this object file
29+
// also should have dead vtable stub definition
30+
private func beef() {}
31+
}
32+
33+
@main
34+
struct Entry {
35+
static func main() {
36+
_ = C2()
37+
}
38+
}
39+

0 commit comments

Comments
 (0)