[PATCH] D71794: [ELF] Don't special case weak symbols for pie with no shared objects

Tue Jan 7 10:16:52 PST 2020

MaskRay added a comment.

In D71794#1807562 <https://reviews.llvm.org/D71794#1807562>, @peter.smith wrote:

> Apologies for the delay in responding, the original came in when I was on vacation.
>
> - Am I correct in thinking that the extra dynamic symbols that will result with -pie and no shared libraries are just wasted space, but won't cause anything to happen at run time as there are no dynamic relocations generated against them?

Yes, it just wastes some space on -static-pie programs, like ~5 symbols. It doesn't cause anything at run time. A statically linked executable does not use shared objects, so what is left in .dynsym does not matter. Well, glibc supports dlopen from a statically linked executable, and the loaded shared object may use symbols from the executable. In that case, the link should use `--export-dynamic`, instead of relying on the ~5 weak symbols in .dynsym.

> - The main argument for removing is that this our ld.bfd is ad-hoc and this part doesn't help much, removing it would simplify here and permit a simplification in D71795 <https://reviews.llvm.org/D71795>. Is there a plan beyond D71795 <https://reviews.llvm.org/D71795> to more closely follow ld.bfd behaviour, or is this just an area where we expect cosmetic inconsistencies?
> 
>   At the moment I'm happy to go ahead with this if we've got a good idea of what our desired end-goal is.

It does not help. It can get in the away in certain places due to the different behaviors between -no-pie and -pie. Yes, it permits a simplification in D71795 <https://reviews.llvm.org/D71795>. We will expect cosmetic inconsistencies with ld.bfd in certain undefined weak scenarios. ld.bfd treats undefined weak symbols in an architecture dependent manner, and not many people understand the differences well. lld should not follow its rules. We may need to abide by psABI in some cases, but make our reasonable decisions in others.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71794/new/

https://reviews.llvm.org/D71794