Skip to content

Conversation

@kateinoigakukun
Copy link
Member

@kateinoigakukun kateinoigakukun commented Oct 24, 2025

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:

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
Loading

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, and it made swift-format build for Wasm to fail

@kateinoigakukun
Copy link
Member Author

@swift-ci smoke test

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
@kateinoigakukun kateinoigakukun force-pushed the yt/fix-dead-stub-comdat-wasm branch from d8696ac to df41ec0 Compare October 24, 2025 12:30
@kateinoigakukun
Copy link
Member Author

@swift-ci smoke test

Comment on lines +4 to +5
// Note: Windows uses internal linkage for dead stub methods, which doesn't
// use linkonce_odr or COMDAT groups.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain this a bit? Windows definitely uses COMDATs rather aggressively.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I wasn’t clear, in this case, IRGenModule::emitVTableStubs uses internal linkage instead of linkonce_odr only for COFF, so we don’t attach COMDATs in ApplyIRLinkage::to in the first place.

@kateinoigakukun kateinoigakukun marked this pull request as ready for review October 25, 2025 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants