This is an archive of the discontinued LLVM Phabricator instance.

Fix COMPILER_RT_DEBUG build for targets that don't support thread local storage.
ClosedPublic

Authored by delcypher on Aug 4 2021, 7:36 PM.

Details

Summary

022439931f5be77efaf80b44d587666b0c9b13b5 added code that is only enabled
when COMPILER_RT_DEBUG is enabled. This code doesn't build on targets
that don't support thread local storage because the code added uses the
THREADLOCAL macro. Consequently the COMPILER_RT_DEBUG build broke for
some Apple targets (e.g. 32-bit iOS simulators).

/Volumes/user_data/dev/llvm/llvm.org/main/src/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_mutex.cpp:216:8: error: thread-local storage is not supported for the current target
static THREADLOCAL InternalDeadlockDetector deadlock_detector;
       ^
/Volumes/user_data/dev/llvm/llvm.org/main/src/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h:227:24: note: expanded from macro 'THREADLOCAL'
 # define THREADLOCAL   __thread
                        ^
1 error generated.

To fix this, this patch introduces a SANITIZER_SUPPORTS_THREADLOCAL
macro that is 1 iff thread local storage is supported by the current
target. That condition is then added to SANITIZER_CHECK_DEADLOCKS to
ensure the code is only enabled when thread local storage is available.

The implementation of SANITIZER_SUPPORTS_THREADLOCAL currently assumes
Clang. See llvm-project/clang/include/clang/Basic/Features.def for the
definition of the tls feature.

rdar://81543007

Diff Detail

Event Timeline

delcypher requested review of this revision.Aug 4 2021, 7:36 PM
delcypher created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2021, 7:36 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
delcypher edited the summary of this revision. (Show Details)Aug 4 2021, 7:41 PM
delcypher edited the summary of this revision. (Show Details)Aug 4 2021, 7:54 PM
dvyukov accepted this revision.Aug 5 2021, 2:12 AM
This revision is now accepted and ready to land.Aug 5 2021, 2:12 AM