This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objcopy] Fix --keep-global-symbol/--globalize-symbol for undefined symbols.
ClosedPublic

Authored by rupprecht on Oct 25 2018, 2:46 PM.

Details

Summary

--keep-global-symbol and --globalize-symbol don't make sense for undefined symbols, so it should be ignored for those symbols. This matches GNU objcopy behavior.

Diff Detail

Event Timeline

rupprecht created this revision.Oct 25 2018, 2:46 PM

My feeling is that a single separate test is not the right way to test this. I think it makes more sense for an undefined symbol to be added to globalize.test and --keep-global-symbols.test. What do you think?

test/tools/llvm-objcopy/globalize-undefined-sym.test
11

Are you sure this is Foo? That's a symbol that doesn't even exist. I assume you meant it to be Weak or Global?

rupprecht added inline comments.Oct 29 2018, 11:07 AM
test/tools/llvm-objcopy/globalize-undefined-sym.test
11

Foo is intentional, sorry for not explaining in a test comment why -- fixed.

  • Update test
jakehehrlich accepted this revision.Oct 29 2018, 11:14 AM

This seems correct to me. LGTM.

This revision is now accepted and ready to land.Oct 29 2018, 11:14 AM
rupprecht updated this revision to Diff 171600.Oct 29 2018, 3:45 PM
  • Move test cases to already existing tests

My feeling is that a single separate test is not the right way to test this. I think it makes more sense for an undefined symbol to be added to globalize.test and --keep-global-symbols.test. What do you think?

Makes sense to me -- done.

This revision was automatically updated to reflect the committed changes.