Page MenuHomePhabricator

[lld/mac] Partially implement -export_dynamic
ClosedPublic

Authored by thakis on Jul 6 2021, 7:29 AM.

Details

Reviewers
int3
gkm
Group Reviewers
Restricted Project
Commits
rG3eb2fc4b5051: [lld/mac] Partially implement -export_dynamic
Summary

This implements the part of -export_dynamic that adds external
symbols as dead strip roots even for executables.

It does not yet implement the effect -export_dynamic has for LTO.
I tried just replacing config->outputType != MH_EXECUTE with
(config->outputType != MH_EXECUTE || config->exportDynamic) in
LTO.cpp, but then local symbols make it into the symbol table too,
which is too much (and also doesn't match ld64). So punt on this
for now until I understand it better.
(D91583 may or may not be related too).

Diff Detail

Event Timeline

thakis created this revision.Jul 6 2021, 7:29 AM
Herald added a reviewer: gkm. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
thakis requested review of this revision.Jul 6 2021, 7:29 AM
thakis updated this revision to Diff 356733.Jul 6 2021, 8:12 AM

make test call %lld instead of ld :)

int3 accepted this revision.Jul 6 2021, 8:15 AM

lgtm assuming the test still passes with lld

lld/test/MachO/dead-strip.s
69

this seems to be invoking ld64

This revision is now accepted and ready to land.Jul 6 2021, 8:15 AM

Thanks!

It _only_ passes with %lld :) (But only because ld uses a different symbol order.) I ran it with ld locally to verify that our list of exported symbols matches ld64 (…except for ordering). And then I forgot to undo it before uploading, but luckily the precommit bots caught it.

This revision was landed with ongoing or failed builds.Jul 6 2021, 8:22 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 6 2021, 8:22 AM

Thanks!

It _only_ passes with %lld :) (But only because ld uses a different symbol order.) I ran it with ld locally to verify that our list of exported symbols matches ld64 (…except for ordering). And then I forgot to undo it before uploading, but luckily the precommit bots caught it.

Could you sort the symbols to make it independent of the order, i.e. it works with ld and lld?

Thanks!

It _only_ passes with %lld :) (But only because ld uses a different symbol order.) I ran it with ld locally to verify that our list of exported symbols matches ld64 (…except for ordering). And then I forgot to undo it before uploading, but luckily the precommit bots caught it.

Could you sort the symbols to make it independent of the order, i.e. it works with ld and lld?

There's no good way (I know of) to tell FileCheck "Do CHECK-DAG: for all these lines, but don't allow any other lines to interleave". The motivation for all these -NEXTs is to check that nothing else is in the symbol table other than what's listed in the test.

If there's a way to achieve this with CHECK-DAG, I'd be glad to hear about it.

Many of our tests don't pass as-is with ld though. (Some do, and I often swap out %lld for ld either manually or by editing lld/test/MachO/lit.local.cfg when I'm working on a test, so I think having more tests pass with both linkers is definitely useful when it's easy to do.)

Thanks!

It _only_ passes with %lld :) (But only because ld uses a different symbol order.) I ran it with ld locally to verify that our list of exported symbols matches ld64 (…except for ordering). And then I forgot to undo it before uploading, but luckily the precommit bots caught it.

Could you sort the symbols to make it independent of the order, i.e. it works with ld and lld?

I though of something like llvm-objdump ... | grep ... | sort and then do FileCheck on the result.

There's no good way (I know of) to tell FileCheck "Do CHECK-DAG: for all these lines, but don't allow any other lines to interleave". The motivation for all these -NEXTs is to check that nothing else is in the symbol table other than what's listed in the test.

If there's a way to achieve this with CHECK-DAG, I'd be glad to hear about it.

Many of our tests don't pass as-is with ld though. (Some do, and I often swap out %lld for ld either manually or by editing lld/test/MachO/lit.local.cfg when I'm working on a test, so I think having more tests pass with both linkers is definitely useful when it's easy to do.)

Thanks!

It _only_ passes with %lld :) (But only because ld uses a different symbol order.) I ran it with ld locally to verify that our list of exported symbols matches ld64 (…except for ordering). And then I forgot to undo it before uploading, but luckily the precommit bots caught it.

Could you sort the symbols to make it independent of the order, i.e. it works with ld and lld?

I though of something like llvm-objdump ... | grep ... | sort and then do FileCheck on the result.

That feels like a kludge :) I don't think having the test work with ld is important enough to resort to stuff like that. But maybe someone is motivated to add real support for this use-case to FileCheck.

tschuett added a comment.EditedJul 6 2021, 9:15 AM

Thats fine. But it is odd that you are not testing that you get the right symbols. You are testing that you get the right symbols and in a random order.

The order should be an implementation detail.

Thats fine. But it is odd that you are not testing that you get the right symbols. You are testing that you get the right symbols and in a random order.

The order should be an implementation detail.

Yes, but as said above there's no good way I know of to do an order-independent test that also makes sure that a given list of matches is exhaustive. If I used CHECK-DAG then FileCheck would allow interleaving non-matched lines silently as far as I know.

Then llvm-objdump should canonalize the symbols. llvm-objdump --syms --sorted-syms