This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Do not change binding of symbols when creating relocatable output.
ClosedPublic

Authored by grimar on Aug 15 2016, 8:55 AM.

Details

Summary

Spec says "A hidden symbol contained in a relocatable object must be either
removed or converted to STB_LOCAL binding by the link-editor when the
relocatable object is included in an executable file or shared object".
But we previously converted symbols to STB_LOCAL even when -r was specified.

Broken binary was produced, this is PR28967, patch fixes the issue.

Diff Detail

Event Timeline

grimar updated this revision to Diff 68042.Aug 15 2016, 8:55 AM
grimar retitled this revision from to [ELF] - Do not change binding for hidden symbols when creating relocatable output..
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added subscribers: llvm-commits, grimar, evgeny777.
ruiu added inline comments.Aug 15 2016, 10:58 AM
ELF/OutputSections.cpp
1319

I'm not sure if this would fix the problem entirely. My gut is that we want to keep the original symbol type if -r is specified. If that's the case, we want to add

if (Config->Relocatable)
  return S->Binding;

here. But I don't know if that's correct.

grimar added inline comments.Aug 15 2016, 11:02 PM
ELF/OutputSections.cpp
1319

gold allows GNU_UNIQUE->STB_GLOBAL transformation for relocatable output,
so I tried to be consistent with it here.
That is the only difference with your suggestion I think. And looks gnu ld does not support
--no-gnu-unique at all, so probably we can go with any solution here.

ruiu edited edge metadata.Aug 17 2016, 1:15 PM

I don't think that the current gold behavior you described was intended one. I cannot describe why they are doing it. On the other hand, the semantics that "all symbols are copied as-is to the output if -r is given" is very easy to understand and justify. I believe we should do that way.

grimar updated this revision to Diff 68490.Aug 18 2016, 12:40 AM
grimar retitled this revision from [ELF] - Do not change binding for hidden symbols when creating relocatable output. to [ELF] - Do not change binding of symbols when creating relocatable output..
grimar edited edge metadata.
  • Do not change binding of symbols when creating relocatable output.
emaste added a subscriber: emaste.Aug 18 2016, 5:03 AM
ruiu accepted this revision.Aug 18 2016, 1:29 PM
ruiu edited edge metadata.

LGTM!

This revision is now accepted and ready to land.Aug 18 2016, 1:29 PM
This revision was automatically updated to reflect the committed changes.