[PATCH] D22034: [MSVC][DLL] use local vftable mangling only exported classes with virtual destructor

[Dmitry Polukhin via cfe-commits]

DmitryPolukhin added a comment.

In http://reviews.llvm.org/D22034#474985, @majnemer wrote:

> In http://reviews.llvm.org/D22034#474937, @DmitryPolukhin wrote:
>
> > David, do you know real programs that relay on constexpr and dllexport semantic that doesn't work on MSVC?
>
>
> Yes, Chrome relied on these semantics.


Hm, does it mean that Chrome has some workaround to bypass that MSVC doesn't support it?

> > Anyway current implementation is not compatible with MSVC in much more common case without constexp.

> 

> 

> I don't see how it is not compatible.  The address of the vftable is not observable as far as I know...


You can easily observe the difference if vftable is exported from DLL but some virtual functions are not (for example, private). MSVC will uses imported vftable and Clang will try to rebuild fvtable that will fail on link time.

> > At the moment my patch works with your example just because it only changes mangling but don't use imported vftable that seems to be wrong.

> 

> 

> Why is it //wrong//?  It should obey the ABI in all practical matters but it might be less efficient space-wise.


I think it is not ABI compatible use local vftable when MSVC uses imported. The easy way to observe the difference I already described but it might be other side effect if user start unloading DLL from address space. So user may expect that it is safe to unload library #1 that created object from library #2 because it doesn't have references to library #1 addresses but due to local fvtable it might result in crash.


http://reviews.llvm.org/D22034