Page MenuHomePhabricator

[builtins] Try to ensure single copy of emulated TLS state
ClosedPublic

Authored by smeenai on Jul 29 2021, 6:46 PM.

Details

Summary

Multiple copies of emulated TLS state means inconsistent results when
accessing the same thread-local variable from different shared objects
(https://github.com/android/ndk/issues/1551). Making __emutls_get_address
be a weak default visibility symbol should make the dynamic linker
ensure only a single copy gets used at runtime. This is best-effort, but
the more robust approach of putting emulated TLS into its own shared
object would (a) be a much bigger change, and (b) shared objects are
pretty heavyweight, and adding a new one to a space-constrained
environment isn't an easy sell. Given the expected rarity of direct
accesses to emulated TLS variables across different shared objects, the
best-effort approach should suffice.

Diff Detail

Event Timeline

smeenai requested review of this revision.Jul 29 2021, 6:46 PM
smeenai created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJul 29 2021, 6:46 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
danalbert accepted this revision.Jul 30 2021, 1:42 PM

LGTM, but @rprichard knows this area better than I do so he should probably weigh in before submitting :)

This revision is now accepted and ready to land.Jul 30 2021, 1:42 PM
rprichard accepted this revision.Jul 30 2021, 2:02 PM

Yeah, this looks fine to me.

Weak symbol is a purely link-time concept, AFAIK. It is ignored in the dynamic loader, so, unless we link multiple copies of this file into the same DSO, "weak" attribute here should not have any effect.

Weak symbol is a purely link-time concept, AFAIK. It is ignored in the dynamic loader, so, unless we link multiple copies of this file into the same DSO, "weak" attribute here should not have any effect.

There's a new -Bsymbolic-non-weak-functions flag (https://reviews.llvm.org/D102570), where marking something weak has an important effect on the linker.

Oh, that's a cool feature.

LGTM then.

Weak symbol is a purely link-time concept, AFAIK. It is ignored in the dynamic loader, so, unless we link multiple copies of this file into the same DSO, "weak" attribute here should not have any effect.

Doesn't the dynamic linker coalesce multiple copies of weak symbols at runtime, to make things like vague linkage for typeinfos work? Or does that coalescing happen regardless of the symbol being strong or weak?

This revision was landed with ongoing or failed builds.Jul 30 2021, 2:54 PM
This revision was automatically updated to reflect the committed changes.

Weak symbol is a purely link-time concept, AFAIK. It is ignored in the dynamic loader, so, unless we link multiple copies of this file into the same DSO, "weak" attribute here should not have any effect.

Doesn't the dynamic linker coalesce multiple copies of weak symbols at runtime, to make things like vague linkage for typeinfos work? Or does that coalescing happen regardless of the symbol being strong or weak?

It resolves everything to the first definition in the lookup order. The only special handling of weak symbols that I've found is that missing definition is not an error.

It resolves everything to the first definition in the lookup order. The only special handling of weak symbols that I've found is that missing definition is not an error.

FWIW: I think FreeBSD's loader keeps going after finding a weak definition, looking for a non-weak definition. This is probably where it's implemented.

It resolves everything to the first definition in the lookup order. The only special handling of weak symbols that I've found is that missing definition is not an error.

FWIW: I think FreeBSD's loader keeps going after finding a weak definition, looking for a non-weak definition. This is probably where it's implemented.

glibc does this as well if you specify the LD_DYNAMIC_WEAK environment variable.

It resolves everything to the first definition in the lookup order. The only special handling of weak symbols that I've found is that missing definition is not an error.

Yes.

FWIW: I think FreeBSD's loader keeps going after finding a weak definition, looking for a non-weak definition. This is probably where it's implemented.

FreeBSD's behavior (glibc pre-2.2 behavior or LD_DYNAMIC_WEAK=1) does not conform to the ELF specification. https://reviews.freebsd.org/D26352

Mach-O's weak definition coalesce behavior is indeed similar to FreeBSD/glibc pre-2.2's.

The patch does make emutls work with the new ld.lld -Bsymbolic-non-weak-functions.

ELF doesn't use the term "coalescing". ELF uses symbol preemption or symbol interposition.