This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by MaskRay on Dec 20 2019, 9:59 PM.

Details

Summary

D59275 added the following clause to Symbol::includeInDynsym()

if (isUndefWeak() && Config->Pie && SharedFiles.empty())
  return false;

D59549 explored the possibility to generalize it for -no-pie.

GNU ld's rules are architecture dependent and partly controlled by -z
{,no-}dynamic-undefined-weak. Our attempts to mimic its rules are
actually half-baked and don't provide perceivable benefits (it can save
a few more weak undefined symbols in .dynsym in a -static-pie
executable). Let's just delete the rule for simplicity.

This makes D71795 NFC.

Diff Detail

Event Timeline

MaskRay created this revision.Dec 20 2019, 9:59 PM
Harbormaster completed remote builds in B42870: Diff 235003.
MaskRay marked an inline comment as done.Dec 20 2019, 10:12 PM
MaskRay added inline comments.
lld/test/ELF/ppc32-weak-undef-call.s
15

This behavior change does not affect anything. I added this file just to check our behavior regarding the glibc weak undefined symbol __gmon_start__. The instruction will not be called when it is undefined, so we don't care the target of the branch.

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?
  • 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. Is there a plan beyond 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.

MaskRay added a comment.EditedJan 7 2020, 10:15 AM

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 or --dynamic-list, 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. Is there a plan beyond 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. 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.

peter.smith accepted this revision.Jan 7 2020, 10:33 AM

Ok LGTM. May be worth waiting a day to see if there are any final objections.

This revision is now accepted and ready to land.Jan 7 2020, 10:33 AM
lld/test/ELF/ppc32-weak-undef-call.s