[cfe-commits] [patch][pr13124] Use strong symbols for functions that go in a strong vtables

[John McCall]

On Jun 27, 2012, at 8:05 PM, Rafael Espíndola wrote:
>> You did not attach a patch, but that's okay, because the behavior it
>> implements is illegal as described.  We cannot emit a strong symbol
>> for something that can emitted as a weak symbol in another translation
>> unit;  I know that the linux linker is quite permissive about this, but (e.g.)
>> the Darwin linker is not, and I would expect that even the Linux linker is
>> generally not going to do The Right Thing with COMDAT.
> 
> The ELF linker keeps the strong symbol. My testing with the darwin
> linker shows the same behavior for both functions and variables. Do
> you know under which circumstances it rejects that? In any case, I
> have attached a patch that makes us produce a weak_odr instead of a
> linkonce_odr (wrote it before getting you second email).

It's not a rejection, it's a lack of coalescing.  In particular, I know that
the Darwin dyld does not coalesce strong symbols with weak ones.

>> In addition, as I read the Itanium ABI, we have no guarantee that the
>> translation unit exporting the key function must also export all the inline
>> virtual functions.  That is, while that translation unit clearly has to emit
>> all the inline virtual functions, it could freely do so with hidden visibility,
>> or even as internal symbols (although that would likely be a
>> pessimization).  That's actually a useful optimization.
>> 
>> The bug here is the translation unit which emits an external reference
>> to the inline method.  That is never allowed under the Itanium ABI;
>> every translation unit which uses the method must define it.  If we're
>> going to emit available_externally vtables, we need to emit all the
>> inline methods they reference.
>> 
>> Obviously, we'd really want a way to do so "on demand", instead of
>> aggressively emitting all the declarations just in case.
> 
> Sorry, why is it a useful optimization? We can always emit the
> functions as available_externally if we think the optimizers might
> want to inline it.

Weak linkage across shared-library boundaries is a serious launch-time
performance problem, because every symbol that's weakly exported from
a dynamic object has to be checked for potential coalescing with symbols
from every other dynamic object.  This is why the Darwin dynamic linker
does not coalesce strong symbols with weak ones, and this is why we would
like to not have to export function symbols at all in the common case that
they're only called (or placed in vtables) within a dynamic library.

Devirtualization, on the other hand, is a fairly minor optimization when
it does not create inlining opportunities.

> Given that both clang and gcc (both 4.6 and 4.7) produce an undefined
> reference to _ZN3barD0Ev in

So a recent optimization has a bug, and it's the same bug in two
different compilers for basically the same reason.  This is not surprising.

> I think the best is to clarify the standard to say that the TU with an
> strong vtable should include symbols for the functions in the vtable
> (even if they are weak). If we really think that this should not be
> the case,  we should at least coordinate with gcc to make sure we
> remain compatible. If only clang implements your suggestion then a gcc
> compiled program might see an undefined reference when linking with a
> clang compiled library.

I agree that we should coordinate with GCC to make sure that both
compilers fix this bug.

John.