This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho] Add support for -alias
ClosedPublic

Authored by keith on Jul 16 2022, 11:52 AM.

Details

Reviewers
int3
oontvoo
Group Reviewers
Restricted Project
Commits
rG0bc100986c25: [lld-macho] Add support for -alias
Summary

This creates a symbol alias similar to --defsym in the elf linker. This
is used by swiftpm for all executables, so it's useful to support. This
doesn't implement -alias_list but that could be done pretty easily as
needed.

Diff Detail

Event Timeline

keith created this revision.Jul 16 2022, 11:52 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 16 2022, 11:52 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
keith requested review of this revision.Jul 16 2022, 11:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 16 2022, 11:52 AM
keith updated this revision to Diff 445271.Jul 16 2022, 3:31 PM

Switch to vector

int3 added a subscriber: int3.Jul 17 2022, 2:50 PM
int3 added inline comments.
lld/MachO/Driver.cpp
1566

const auto &

1569

why not just pair.second

lld/test/MachO/aliases.s
27–28

would be nice to check that the addresses are the same too

keith updated this revision to Diff 445360.Jul 17 2022, 4:38 PM
keith marked 3 inline comments as done.

Add addresses to tests

int3 accepted this revision.Jul 17 2022, 6:17 PM

Thanks!

lld/MachO/Driver.cpp
1566

I think you did the next two lines but forgot this one?

lld/test/MachO/aliases.s
17

nit: would be nice to align the 2nd column

This revision is now accepted and ready to land.Jul 17 2022, 6:17 PM
keith updated this revision to Diff 445384.Jul 17 2022, 9:30 PM
keith marked 2 inline comments as done.

Formatting

oontvoo added inline comments.
lld/MachO/Driver.cpp
1569–1573

i wonder if it's worth adding a Symtab::copy(<src>, <target>) rather than listing all the symbol's internals here. (reason being that occasionally we do add a new field, which means going to hunt down all these instances and update them - kind of bug-prone)

lld/test/MachO/aliases.s
8

nit: this shouldn't be needed (ie., the tests should be in the OPT framework, not LLD-macho since it's not implementing argument parsing code)

keith updated this revision to Diff 445560.Jul 18 2022, 10:24 AM
keith marked 2 inline comments as done.

Move alias logic into SymbolTable.cpp

lld/MachO/Driver.cpp
1569–1573

Yea good idea, I moved it. I didn't see any other similar logic today so I made it specific to this use case, but it still seems nicer to live with the other symtab logic. lmk what you think!

oontvoo accepted this revision.Jul 18 2022, 10:36 AM

LG - thanks!

This revision was automatically updated to reflect the committed changes.
thakis added a subscriber: thakis.Oct 2 2022, 6:34 AM

Is it time to do the 2nd FIXME in MarkLive.cpp now?

// FIXME: When we implement these flags, make symbols from them GC
// roots:
// * -reexported_symbol(s_list)
// * -alias(-list)
// * -init
keith added a comment.Oct 3 2022, 9:14 AM

Ah thanks, submitted https://reviews.llvm.org/D135082. I also ran that test against ld64 and it passed, otherwise I didn't look at the ld64 behavior