This is an archive of the discontinued LLVM Phabricator instance.

Revert "Also demote lazy symbols."
ClosedPublic

Authored by chh on Sep 10 2018, 3:22 PM.

Diff Detail

Event Timeline

chh created this revision.Sep 10 2018, 3:22 PM
ruiu added a comment.Sep 10 2018, 3:23 PM

Can you add a test that failed without this revert?

chh added a comment.Sep 10 2018, 3:35 PM

Rui, could you help me creating a test?
All I have now is Android's link command for dex2oatds.
I don't know how to reduce that to an lld unit test.

ruiu added a comment.Sep 10 2018, 3:50 PM

I guess if you just pass a single file that contains undefined symbol __i686.get_pc_thunk.bx, you can reproduce the issue, no?

chh updated this revision to Diff 164952.Sep 11 2018, 12:50 PM
chh edited the summary of this revision. (Show Details)

Add a unit test case.

ruiu added a comment.Sep 11 2018, 1:38 PM

We generally don't check in a binary file unless there's no way to generate a file from text. Looks like we already have a test that creates a .gnu.linkonce section, see test/ELF/comdat-linkonce.s. Could you create a test based on it?

test/ELF/i386-linkonce.s
3 ↗(On Diff #164952)

Please write a test that is expected to succeed to make it more reliable.

chh updated this revision to Diff 164989.Sep 11 2018, 3:34 PM

Make it a positive unit test. Create library to test from .s file.

chh marked an inline comment as done.Sep 11 2018, 3:34 PM
ruiu accepted this revision.Sep 11 2018, 3:51 PM

LGTM

Thank you for doing this!

test/ELF/i386-linkonce.s
9 ↗(On Diff #164989)

nit: calll -> call for consistency with other tests.

This revision is now accepted and ready to land.Sep 11 2018, 3:51 PM
This revision was automatically updated to reflect the committed changes.
chh marked an inline comment as done.Sep 11 2018, 4:02 PM