This is an archive of the discontinued LLVM Phabricator instance.

Differential D90808

[lsan] Remove unnecessary elf-tls condition
ClosedPublic

Authored by oontvoo on Nov 4 2020, 6:18 PM.

Details

Reviewers
vitalybuka
Commits
rG796650d9903a: [lsan] Remove unnecessary elf-tls condition

Diff Detail

Event Timeline

oontvoo created this revision.Nov 4 2020, 6:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 4 2020, 6:18 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
oontvoo requested review of this revision.Nov 4 2020, 6:18 PM
oontvoo updated this revision to Diff 303002.Nov 4 2020, 6:24 PM

put back the comment so it doesn't look like we accidentally forgot arm-32 android for leak-detection.

oontvoo updated this revision to Diff 303005.EditedNov 4 2020, 6:30 PM

fix breakages from D89251

oontvoo updated this revision to Diff 303006.Nov 4 2020, 6:33 PM

updated

Herald added subscribers: fedor.sergeev, krytarowski. · View Herald TranscriptNov 4 2020, 6:33 PM
Harbormaster completed remote builds in B77635: Diff 303001.Nov 4 2020, 6:50 PM
oontvoo updated this revision to Diff 303007.Nov 4 2020, 6:52 PM

rebase

Harbormaster completed remote builds in B77636: Diff 303002.Nov 4 2020, 6:58 PM
Harbormaster completed remote builds in B77639: Diff 303005.Nov 4 2020, 7:06 PM
Harbormaster completed remote builds in B77640: Diff 303006.Nov 4 2020, 7:13 PM
vitalybuka accepted this revision.Nov 4 2020, 7:21 PM
vitalybuka added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_linux.h
165–171

this is unrelated?

This revision is now accepted and ready to land.Nov 4 2020, 7:21 PM
oontvoo updated this revision to Diff 303016.Nov 4 2020, 7:22 PM

reverted sanitizer_linux.h (it'll be taken care of in D90811)

This revision was landed with ongoing or failed builds.Nov 4 2020, 7:23 PM
Closed by commit rG796650d9903a: [lsan] Remove unnecessary elf-tls condition (authored by oontvoo). · Explain Why
This revision was automatically updated to reflect the committed changes.
oontvoo added a commit: rG796650d9903a: [lsan] Remove unnecessary elf-tls condition.
Harbormaster completed remote builds in B77641: Diff 303007.Nov 4 2020, 7:23 PM
Harbormaster completed remote builds in B77648: Diff 303016.Nov 4 2020, 7:51 PM

Revision Contents

PathSize
compiler-rt/
lib/
lsan/
14 lines
sanitizer_common/
8 lines

Diff 303006

compiler-rt/lib/lsan/lsan_common.h

Show All 23 Lines
// LeakSanitizer relies on some Glibc's internals (e.g. TLS machinery) on Linux. // LeakSanitizer relies on some Glibc's internals (e.g. TLS machinery) on Linux.
// Also, LSan doesn't like 32 bit architectures // Also, LSan doesn't like 32 bit architectures
// because of "small" (4 bytes) pointer size that leads to high false negative // because of "small" (4 bytes) pointer size that leads to high false negative
// ratio on large leaks. But we still want to have it for some 32 bit arches // ratio on large leaks. But we still want to have it for some 32 bit arches
// (e.g. x86), see https://github.com/google/sanitizers/issues/403. // (e.g. x86), see https://github.com/google/sanitizers/issues/403.
// To enable LeakSanitizer on a new architecture, one needs to implement the // To enable LeakSanitizer on a new architecture, one needs to implement the
// internal_clone function as well as (probably) adjust the TLS machinery for // internal_clone function as well as (probably) adjust the TLS machinery for
// the new architecture inside the sanitizer library. // the new architecture inside the sanitizer library.
#if (SANITIZER_LINUX && \ #if (SANITIZER_LINUX && || SANITIZER_MAC) && (SANITIZER_WORDSIZE == 64) && \
Lint: Pre-merge checks
Inline Actions

clang-tidy: error: invalid token at start of a preprocessor expression [clang-diagnostic-error]
not useful

Lint: Pre-merge checks: clang-tidy: error: invalid token at start of a preprocessor expression [clang-diagnostic-error]…
(!SANITIZER_ANDROID || defined(ANDROID_HAS_ELF_TLS)) || \
SANITIZER_MAC) && \
(SANITIZER_WORDSIZE == 64) && \
(defined(__x86_64__) || defined(__mips64) || defined(__aarch64__) || \ (defined(__x86_64__) || defined(__mips64) || defined(__aarch64__) || \
defined(__powerpc64__) || defined(__s390x__)) defined(__powerpc64__) || defined(__s390x__))
#define CAN_SANITIZE_LEAKS 1 #define CAN_SANITIZE_LEAKS 1
#elif defined(__i386__) && \ #elif defined(__i386__) && (SANITIZER_LINUX || SANITIZER_MAC)
(SANITIZER_LINUX && \
(!SANITIZER_ANDROID || defined(ANDROID_HAS_ELF_TLS)) || \
SANITIZER_MAC)
#define CAN_SANITIZE_LEAKS 1 #define CAN_SANITIZE_LEAKS 1
// Exclude leak-detection on arm32 for Android because `__aeabi_read_tp`
// is missing. This caused a link error.
#elif defined(__arm__) && SANITIZER_LINUX && !SANITIZER_ANDROID #elif defined(__arm__) && SANITIZER_LINUX && !SANITIZER_ANDROID
#define CAN_SANITIZE_LEAKS 1 #define CAN_SANITIZE_LEAKS 1
#elif SANITIZER_NETBSD || SANITIZER_FUCHSIA #elif SANITIZER_NETBSD || SANITIZER_FUCHSIA
#define CAN_SANITIZE_LEAKS 1 #define CAN_SANITIZE_LEAKS 1
#else #else
#define CAN_SANITIZE_LEAKS 0 #define CAN_SANITIZE_LEAKS 0
#endif #endif
▲ Show 20 Lines • Show All 232 Lines • Show Last 20 Lines

compiler-rt/lib/sanitizer_common/sanitizer_linux.h

Show First 20 Lines • Show All 154 Lines • ▼ Show 20 Lines
// Bionic provides this API since S. // Bionic provides this API since S.
extern "C" SANITIZER_WEAK_ATTRIBUTE void __libc_get_static_tls_bounds(void **, extern "C" SANITIZER_WEAK_ATTRIBUTE void __libc_get_static_tls_bounds(void **,
void **); void **);
extern "C" SANITIZER_WEAK_ATTRIBUTE void __libc_iterate_dynamic_tls( extern "C" SANITIZER_WEAK_ATTRIBUTE void __libc_iterate_dynamic_tls(
pid_t, void (*cb)(void *, void *, uptr, void *), void *); pid_t, void (*cb)(void *, void *, uptr, void *), void *);
#define HAS_ANDROID_THREAD_PROPERTIES_API (&__libc_iterate_dynamic_tls != 0) #define HAS_ANDROID_THREAD_PROPERTIES_API (&__libc_iterate_dynamic_tls != 0)
#else
#define HAS_ANDROID_THREAD_PROPERTIES_API (0)
#endif // SANITIZER_ANDROID #endif // SANITIZER_ANDROID
} // namespace __sanitizer } // namespace __sanitizer
#else
#define HAS_ANDROID_THREAD_PROPERTIES_API (0)
#endif // SANITIZER_FREEBSD || SANITIZER_LINUX || SANITIZER_NETBSD ||
// SANITIZER_SOLARIS
#endif
#endif // SANITIZER_LINUX_H #endif // SANITIZER_LINUX_H
vitalybukaUnsubmitted
Not Done
ReplyInline Actions

this is unrelated?

vitalybuka: this is unrelated?