[llvm-dev] Linker visibility merging for weak and strong symbols
Peter Smith via llvm-dev
llvm-dev at lists.llvm.org
Thu Mar 19 01:39:57 PDT 2020
Reading the ELF spec on visibility in the latest version of the specification. https://www.sco.com/developers/gabi/latest/ch4.symtab.html#visibility
There is an interesting line before the rules on merging visibility:
None of the visibility attributes affects resolution of symbols within an executable or shared object during link-editing -- such resolution is controlled by the binding type. Once the link-editor has chosen its resolution, these attributes impose two requirements, both based on the fact that references in the code being linked may have been optimized to take advantage of the attributes.
Given the "Once the link-editor has chosen its resolution" which would prefer the non-weak definition of S over the definition of S, then apply the rules for combining visibility this would imply that we ignore the visibility of the weak definition in favour of the non-weak definition.
My guess is that many linkers have not implemented it that way, instead choosing the most restrictive visibility when processing the symbol.
If my reading is correct, then it will be worth raising PRs on the linkers that get this wrong. LLD looks like one of them, the early version of ld.bfd I have on my laptop also does but it could have been fixed in a later version.
As it will take some time for fixes to percolate through the ecosystem, it would, as you say, be worth avoiding generating code with weak and non-weak definitions where the weak definition has a more restrictive visibility.
From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of Shoaib Meenai via llvm-dev <llvm-dev at lists.llvm.org>
Sent: 19 March 2020 01:55
To: llvm-dev at lists.llvm.org
Cc: Chris Sarbora
Subject: [llvm-dev] Linker visibility merging for weak and strong symbols
We ran into an interesting visibility merging issue internally. We have a static library libA that’s built with RTTI and exceptions, and defines and exports a class Foo and has a key function for it (and therefore also defines the typeinfo for class Foo with default visibility). We have another static library libB that’s built with exceptions but without RTTI, and throws and catches objects of type Foo. Because libB is built without RTTI, the compiler generates a weak symbol for the typeinfo for Foo (instead of assuming that the key function means the typeinfo would be provided externally). libB is also built with hidden visibility, so this weak typeinfo for Foo symbol also gets hidden visibility. Finally, libA and libB are linked into the same shared library. LLD sets the visibility of a symbol to the lowest visibility of any instance of that symbol (https://github.com/llvm/llvm-project/blob/64a5e57a61bb6f6080236bd1e21afdf044ae810a/lld/ELF/Symbols.cpp#L386), so the typeinfo for class Foo ends up with hidden visibility, which means references to it from other shared libraries can’t be resolved and cause link errors.
I believe the proper solution here would be to compile libB with both exceptions and RTTI (and in general I think enabling exceptions but not enabling RTTI is a pretty big footgun, and I’d be supportive of a compiler option to flag that). However, one of my coworkers suggested that LLD should take weak vs. strong symbols into account when merging visibilities. The proposal is that a weak definition’s visibility shouldn’t be able to reduce the final symbol’s visibility below that of the lowest strong symbol definition. In this situation, since the strong definition of the typeinfo for Foo has default visibility and the weak definition has hidden visibility, the final symbol would have default visibility. (On the other hand, if both definitions were strong or both were weak, the final symbol would have hidden visibility.) I’m not familiar enough with visibility and visibility merging to have a good sense of the upsides and downsides of this proposal, so I figured I’d sent it to the list to see what people thought!
More information about the llvm-dev