This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Implemented -Bsymbolic-functions command line option
ClosedPublic

Authored by grimar on Jan 21 2016, 9:23 AM.

Details

Summary

[ELF] Implemented -Bsymbolic-functions command line option.

-Bsymbolic-functions:
When creating a shared library, bind references to global
function symbols to the definition within the shared library, if any.

This patch also fixed behavior of already existent -Bsymbolic:
previously PLT entries were created even if -Bsymbolic was specified.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar updated this revision to Diff 45541.Jan 21 2016, 9:23 AM
grimar retitled this revision from to [ELF] Implemented -Bsymbolic-functions command line option.
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added subscribers: llvm-commits, grimar.
ruiu added inline comments.Jan 21 2016, 10:50 AM
ELF/OutputSections.cpp
1027–1030 ↗(On Diff #45541)

The conditions inside these ifs seem to be arbitrarily grouped into two. Probably it is a bit verbose but slightly better.

if (!Config->Shared)
  return false;
if (Body->getVisibility() != STV_DEFAULT)
  return false;
if (Config->Bsymbolic)
  return false;
if (Config->BsymbolicFunction && Body->isFunction())
  return false;
return true;
ELF/Symbols.cpp
84 ↗(On Diff #45541)

I don't think IsFunction is relevant to undefined symbols. You can remove this parameter and always pass false to the base class.

emaste added a subscriber: emaste.Jan 21 2016, 12:28 PM
grimar updated this revision to Diff 45657.Jan 22 2016, 12:48 AM
grimar marked 2 inline comments as done.

Review comments addressed.

ELF/OutputSections.cpp
1027–1030 ↗(On Diff #45541)

I think we can group the last one, they are very close, what do you think ?

if (Config->Bsymbolic || (Config->BsymbolicFunctions && Body->isFunction()))
   return false;
grimar updated this revision to Diff 46394.Jan 29 2016, 10:16 AM
  • Rebased
ruiu edited edge metadata.Jan 29 2016, 10:20 AM

Let's do s/isFunction/isFunc/g. This is consistent with STT_FUNC.

Please wrap your commit message to 80 columns, and remove "**" from the body text.

grimar updated this revision to Diff 46512.Feb 1 2016, 1:15 AM
grimar updated this object.
grimar edited edge metadata.
  • Addressed Rui's comments.
ruiu accepted this revision.Feb 1 2016, 12:43 PM
ruiu edited edge metadata.

LGTM

This revision is now accepted and ready to land.Feb 1 2016, 12:43 PM
This revision was automatically updated to reflect the committed changes.