This is an archive of the discontinued LLVM Phabricator instance.

[lld/mac] Give range extension thunks for local symbols local visibility
ClosedPublic

Authored by thakis on Mar 28 2022, 5:18 PM.

Details

Summary

When two local symbols (think: file-scope static functions, or functions in
unnamed namespaces) with the same name in two different translation units
both needed thunks, ld64.lld previously created external thunks for both
of them. These thunks ended up with the same name, leading to a duplicate
symbol error for the thunk symbols.

Instead, give thunks for local symbols local visibility.

(Hitting this requires a jump to a local symbol from over 128 MiB away.
It's unlikely that a single .o file is 128 MiB large, but with ICF
you can end up with a situation where the local symbol is ICF'd with
a symbol in a separate translation unit. And that can introduce a
large enough jump to require a thunk.)

Fixes PR54599.

Diff Detail

Event Timeline

thakis created this revision.Mar 28 2022, 5:18 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 28 2022, 5:18 PM
thakis requested review of this revision.Mar 28 2022, 5:18 PM
thakis updated this revision to Diff 418743.Mar 28 2022, 5:22 PM

update more comments

int3 accepted this revision.Mar 28 2022, 5:57 PM
int3 added a subscriber: int3.

Nice!

Can you include a bit more info in the commit message? Would be nice if people didn't have to click through to the PR

lld/MachO/ConcatOutputSection.cpp
92–93

nit: reflow paragraph?

lld/test/MachO/arm64-thunk-visibility.s
4

ultra nit: not sure if you intended to use one dot or three :p

This revision is now accepted and ready to land.Mar 28 2022, 5:57 PM
This revision was landed with ongoing or failed builds.Mar 30 2022, 1:49 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 30 2022, 1:49 PM
thakis edited the summary of this revision. (Show Details)Mar 30 2022, 1:49 PM

Landed with much more detailed commit message. Thanks!