This is an archive of the discontinued LLVM Phabricator instance.

scudo: Only use the Android reserved TLS slot when building libc's copy of the allocator.
ClosedPublic

Authored by pcc on Nov 19 2019, 3:55 PM.

Details

Summary

When we're not building libc's allocator, just use a regular TLS variable. This
lets the unit tests pass on Android devices whose libc uses Scudo. Otherwise
libc's copy of Scudo and the unit tests' copy will both try to use the same
TLS slot, in likely incompatible ways.

This requires using ELF TLS, so start passing -fno-emulated-tls when building
the library and the unit tests on Android.

Diff Detail

Event Timeline

pcc created this revision.Nov 19 2019, 3:55 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptNov 19 2019, 3:55 PM
Herald added subscribers: Restricted Project, mgorny, srhines. · View Herald Transcript
cryptoad accepted this revision.Nov 20 2019, 8:07 AM

Does this incur a minimal toolchain revision or something since I think ELF TLS was only added recently?

This revision is now accepted and ready to land.Nov 20 2019, 8:07 AM
pcc added a comment.Nov 20 2019, 10:35 AM

Does this incur a minimal toolchain revision or something since I think ELF TLS was only added recently?

Yes, it requires NDK r21, which was the first revision to include https://android-review.googlesource.com/c/platform/bionic/+/967181 which made Android's ELF TLS compatible with current lld.

I'm not sure if we document the minimum NDK revision anywhere though, and different components have different requirements. It's probably fine to require r21 for scudo since not many folks aside from platform developers will be using/testing it on Android.

This revision was automatically updated to reflect the committed changes.