[PATCH] D43184: [WIP] [ItaniunCXXABI] Add an option for loading the type info vtable with dllimport

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In https://reviews.llvm.org/D43184#1009355, @rnk wrote:

> The Qt bug is interesting, but I think we're doing the right thing here. C++ doesn't define any semantics for partially initialized objects. It's unclear if clang or GCC's way of doing things is better or not. With clang, you get to put the global in .bss, so it's not clear what makes for bigger code size.


Fair enough. And I managed to work around it for now, and for a future release, they're planning on redefining this struct for windows, to avoid this issue altogether. (On the other hand, I guess the C++ standard doesn't concern itself about dllimport at all, so on the standard level, there's nothing that would say that this even is a partially initialized object. Not sure in which way it would affect an argument anyway.)

> In https://reviews.llvm.org/D43184#1008003, @mstorsjo wrote:
> 
>> In https://reviews.llvm.org/D43184#1005258, @rnk wrote:
>>
>> > Do you think we should do something like local vftables for itanium instead? Can we assume all methods in the vtable will be exported by the DLL exporting the class?
>>
>>
>> Will this actually ever be needed for other vtables than cxxabi::class_type_info and the others from ItaniumRTTIBuilder::BuildVTablePointer? In what other cases are the vtables referred to from a statically initialized global? And for these vtables - there's no declaration of them at all within most translation units, so that'd require hardcoding all the content of these vtables in ItaniumRTTIBuilder.
> 
> 
> Here's a case where a local vtable would help:
> 
>   struct __declspec(dllimport) A { virtual void a(); };
>   struct __declspec(dllimport) B { virtual void b(); };
>   struct __declspec(dllimport) C : A, B {
>     void a() override;
>     void b() override;
>   };
>   constexpr C c;
> 
> 
> I'm suggesting we can make a DSO local copy of _ZTV1C, so that we can refer to it from `c`. It's OK if the vtable refers to the imported thunks, since the address of virtual methods isn't user visible.
> 
> Either that, or we need to make something like this change here, right?

Ok, I see. For this case I guess it should be mostly straightforward to just tweak a few decisions in ItaniumCXXABI to choose emitting new vtables as linkonce_odr - that would sound like a nice solution.

But for the case of `__cxxabiv1::__class_type_info`, there's no declaration visible in scope at all, and the actual contents of the vtable of this class (and the other similar classes) doesn't seem to be defined in the ABI (it seems to differ significantly between libc++abi and libsupc++). Or do you have any idea of how to work around that as well? We could emit an empty local vtable and try to initialize that at runtime by copying from the dllimported real vtable - but I guess we don't even know the size of it, when its members are defined by the c++abi implementation.


Repository:
  rC Clang

https://reviews.llvm.org/D43184