This is an archive of the discontinued LLVM Phabricator instance.

Keep symbols passed by -init and -fini
ClosedPublic

Authored by ruiu on Nov 7 2019, 9:43 PM.

Details

Summary

Previously, symbols passed by -init and -fini look as if they are
not referenced by anyone, and the LTO might eliminate them.
This patch fixes the issue.

Fixes a bug reported in https://bugs.llvm.org/show_bug.cgi?id=43927

Diff Detail

Event Timeline

ruiu created this revision.Nov 7 2019, 9:43 PM
grimar added inline comments.Nov 8 2019, 12:28 AM
lld/ELF/Driver.cpp
1790

Perhaps a stupid question, but still: can these symbols be lazy? i.e. should handleUndefined be used here, like for config->entry?

--gc-sections retains --init= and --fini= symbols. Adding the rule to LTO seems to improve consistency.

lld/test/ELF/lto/init-fini.ll
8

llvm-objdump -t -> llvm-nm

13

Enhance the test to check llvm-readelf -d output for DT_INIT and DT_FINI as well? It will need -pie (or -shared or --export-dynamic).

MaskRay added inline comments.Nov 8 2019, 12:44 AM
lld/ELF/Driver.cpp
1790

Calling handleUndefined will fetch the lazy symbol. However, ld.bfd and gold do not fetch lazy --init or --fini. I think this is a very minor edge case, being incompatible with GNU linkers should be fine.

Calling handleUndefined LGTM because it makes code prettier.

ruiu updated this revision to Diff 228385.Nov 8 2019, 1:56 AM
ruiu marked 2 inline comments as done.
  • address review comments
ruiu marked an inline comment as done.Nov 8 2019, 1:56 AM
ruiu added inline comments.
lld/ELF/Driver.cpp
1790

I don't worry too much about compatibility in this edge case, but I don't think there's a reason to intentionally diverge from GNU too in this case, so I prefer keeping this as is.

lld/test/ELF/lto/init-fini.ll
13

Good suggestion. Done.

grimar accepted this revision.Nov 8 2019, 2:03 AM

LGTM

This revision is now accepted and ready to land.Nov 8 2019, 2:03 AM
grimar added inline comments.Nov 8 2019, 2:04 AM
lld/test/ELF/lto/init-fini.ll
27

nit: you can use --implicit-check-not=DT_INIT --implicit-check-not=DT_FINI

ruiu updated this revision to Diff 228386.Nov 8 2019, 2:07 AM
  • simplify test
This revision was automatically updated to reflect the committed changes.