This is an archive of the discontinued LLVM Phabricator instance.

[ELF, PR27016] - Do not keep undefined locals in .symtab
ClosedPublic

Authored by grimar on Mar 29 2016, 8:03 AM.

Details

Summary

While looking into PR27016 I found that both gold and bfd just do not
include the undefined locals in symtab.

We can support that for sure, but do anybody see reasons to do that then ?
(I can probably implement that if so).

Now that is pretty simple patch to fix PR27016 and also looks even have benefits,
helps to "get rid of undefined ppc64 "dot-symbols"
(https://sourceware.org/ml/binutils/2015-02/msg00237.html).

Diff Detail

Event Timeline

grimar updated this revision to Diff 51912.Mar 29 2016, 8:03 AM
grimar retitled this revision from to [ELF, PR27016] - Do not keep undefined locals in .symtab.
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added subscribers: llvm-commits, grimar.
grimar updated this revision to Diff 51916.Mar 29 2016, 8:06 AM
  • Removed useless arg for llvm-readobj in testcase
rafael edited edge metadata.Mar 29 2016, 8:42 AM
rafael added a subscriber: rafael.

Lgtm

Wait, won't this also drop global undefined?

Wait, won't this also drop global undefined?

No, shouldKeepInSymtab() is executed only from copyLocalSymbols()

emaste added a subscriber: emaste.Mar 29 2016, 8:49 AM

Should be a continue in copyLocalSymbols instead?

Should be a continue in copyLocalSymbols instead?

I think both ways are ok, but I prefer mine. That is exactly what we do - just do not put them in symtab and for me It is ok check it in shouldKeepInSymtab.

So I guess lets wait for possible other opinions about that then.

for me It is ok check it in shouldKeepInSymtab.

OK, either way is fine with me.

ruiu accepted this revision.Mar 29 2016, 9:44 AM
ruiu edited edge metadata.

LGTM

This revision is now accepted and ready to land.Mar 29 2016, 9:44 AM
This revision was automatically updated to reflect the committed changes.