[C++20] [Modules] Multiple definitions for type info (virtual tables)

[rnikander]

Here's the example code. There is a base class in one partition, a subclass in another. I think it should work.

https://godbolt.org/z/W4nWj6G7E

The error is currently:

/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/13.0.1/../../../../x86_64-linux-gnu/bin/ld: 
CMakeFiles/mod_bug6.dir/Base.cc.o:(.rodata+0x0): multiple definition of `_ZTSW3Mod4Base'; 
CMakeFiles/mod_bug6.dir/main.cc.o:(.rodata+0x0): first defined here
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

On my laptop it's easier to read:

duplicate symbol 'typeinfo name for Base@Mod' in:
    ModBug6/CMakeFiles/mod_bug6.dir/main.cc.o
    ModBug6/CMakeFiles/mod_bug6.dir/Base.cc.o
duplicate symbol 'typeinfo for Base@Mod' in:
    ModBug6/CMakeFiles/mod_bug6.dir/main.cc.o
    ModBug6/CMakeFiles/mod_bug6.dir/Base.cc.o
ld: 2 duplicate symbols for architecture x86_64

[llvmbot]

@llvm/issue-subscribers-clang-modules

[ChuanqiXu9]

I am not sure if this is a bug. I need to check the ABI standard to be sure.

For the example, it works if we mark the out of line virtual function as inline explicitly: https://godbolt.org/z/xYW4hWvne

[ChuanqiXu9]

The direct cause of the issue is that the symbol of the Base's vtable (_ZTIW3Mod4Base) is strong in both Base.cc and main.cc.

But it looks like the Base's vtable (_ZTI4Base) is strong without modules too: https://godbolt.org/z/zvWEjfWjT .

But I don't feel good if we say this is what it is so this is not a bug. Since this reproducer looks common in module's code. And modules bring new semantics to C++ indeed. So we need to investigate more.

[iains]

https://eel.is/c++draft/basic.link#8
specifically, 6.2 seems to say that we should be merging these?

[ChuanqiXu9]

https://eel.is/c++draft/basic.link#8

But the problem is about the linkage of the virtual table, which is not included in the standard. And for the declarations, we merged it well. Personally I think this is a ABI thing.

[iains]

Ah I see .. it's type info. Indeed we should check in the ABI (I was expecting typeinfo to be a weak def. so mergeable by the linker).

[ChuanqiXu9]

So am I. From https://godbolt.org/z/zvWEjfWjT, the vtable will only be linkonce linkage if every function is inline. This makes sense without modules. But it is no longer workable with modules.

[ChuanqiXu9]

After checking the ABI requirement, I think the compiler's behavior is incorrect and we don't need to touch ABI.

Itanium ABI 5.2.3 (https://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague-vtable) says:

The virtual table for a class is emitted in the same object containing the definition of its key function, i.e. the first non-pure virtual function that is not inline at the point of class definition.

So only the module unit containing the definition should emit the virtual table and all other TU which refers this should emit it as extern.

[ChuanqiXu9]

Sent https://reviews.llvm.org/D150023