Page MenuHomePhabricator

[sanitizer] Make DTLS_on_tls_get_addr signal safer
ClosedPublic

Authored by vitalybuka on Dec 1 2020, 2:03 PM.

Details

Summary

Avoid relocating DTV table and use linked list of mmap-ed pages.

Diff Detail

Event Timeline

vitalybuka created this revision.Dec 1 2020, 2:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 1 2020, 2:03 PM
Herald added subscribers: Restricted Project, jfb. · View Herald Transcript
vitalybuka requested review of this revision.Dec 1 2020, 2:03 PM
vitalybuka retitled this revision from [sanitizer] Make signal safer to [sanitizer] Make DTLS_on_tls_get_addr signal safer.Dec 1 2020, 2:04 PM
vitalybuka updated this revision to Diff 308769.Dec 1 2020, 2:11 PM

remove unneded change

vitalybuka added inline comments.Dec 1 2020, 2:14 PM
compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp
142

unrelated, but on recent GLIBC we end-up here

eugenis added inline comments.Dec 1 2020, 2:51 PM
compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp
46–48

I'd rather remove all these "inline"s.

52–66

Rename it to DTLS_NextBlock or something like that? "Resize" usually takes the desired size value.

63

This does not really matter because the atomics are used to synchronize with a signal handler on the same thread, but to avoid confusion I'd replace this with memory_order_seq_cst because the failure code path needs acquire semantics.

compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.h
43

sizeof(atomic_uintptr_t) ?

compiler-rt/test/sanitizer_common/TestCases/Linux/resize_tls_dynamic.cpp
10

the test needs to be disabled on android, too. It glibc-specific, right?

vitalybuka updated this revision to Diff 308799.Dec 1 2020, 3:38 PM
vitalybuka marked 5 inline comments as done.

update

eugenis accepted this revision.Dec 1 2020, 4:08 PM

LGTM

This revision is now accepted and ready to land.Dec 1 2020, 4:08 PM
This revision was landed with ongoing or failed builds.Dec 1 2020, 4:16 PM
This revision was automatically updated to reflect the committed changes.
saghir added a subscriber: saghir.Dec 2 2020, 5:10 AM

This test case is causing failure on PowerPC buildbot http://lab.llvm.org:8011/#/builders/57/builds/1924/steps/7/logs/XPASS__SanitizerCommon-tsan-powerpc64le-Linux__res:
Unexpectedly Passed Tests (1):

SanitizerCommon-tsan-powerpc64le-Linux :: Linux/resize_tls_dynamic.cpp
saghir added a comment.Dec 2 2020, 7:17 AM

This test case is causing failure on PowerPC buildbot http://lab.llvm.org:8011/#/builders/57/builds/1924/steps/7/logs/XPASS__SanitizerCommon-tsan-powerpc64le-Linux__res:
Unexpectedly Passed Tests (1):

SanitizerCommon-tsan-powerpc64le-Linux :: Linux/resize_tls_dynamic.cpp

I have marked the test case as unsupported (by commit: 5045b831a3b9c364d6ef0f09c7a773ca451ae1cc) on all powerpc64 till this can be investigated since it was failing due to XFAIL on the clang-ppc64le-rhel buildbot. FYI @vitalybuka

This test case is causing failure on PowerPC buildbot http://lab.llvm.org:8011/#/builders/57/builds/1924/steps/7/logs/XPASS__SanitizerCommon-tsan-powerpc64le-Linux__res:
Unexpectedly Passed Tests (1):

SanitizerCommon-tsan-powerpc64le-Linux :: Linux/resize_tls_dynamic.cpp

I have marked the test case as unsupported (by commit: 5045b831a3b9c364d6ef0f09c7a773ca451ae1cc) on all powerpc64 till this can be investigated since it was failing due to XFAIL on the clang-ppc64le-rhel buildbot. FYI @vitalybuka

Please feel free to reach out to us on LLVM on Power <powerllvm@ca.ibm.com> if there is anything we could help you with in your investigation for this test case issue. Thanks!