This is an archive of the discontinued LLVM Phabricator instance.

[tsan] Add a DCHECK to verify __tsan_read* and __tsan_write function aren't called from ScopedInterceptor
ClosedPublic

Authored by kubamracek on Dec 9 2015, 8:36 AM.

Details

Summary

Interceptors using ScopedInteceptor should never call into user's code before the ScopedInterceptor is out of scope (and its destructor is called). Let's add a DCHECK to enforce that.

Diff Detail

Event Timeline

kubamracek updated this revision to Diff 42305.Dec 9 2015, 8:36 AM
kubamracek retitled this revision from to [tsan] Add a DCHECK to verify __tsan_read* and __tsan_write function aren't called from ScopedInterceptor.
kubamracek updated this object.
kubamracek added reviewers: dvyukov, glider, samsonov, kcc.
kubamracek added subscribers: llvm-commits, zaks.anna.
dvyukov accepted this revision.Dec 11 2015, 5:46 AM
dvyukov edited edge metadata.
This revision is now accepted and ready to land.Dec 11 2015, 5:46 AM

This currently fails several GCD tests (fix is in http://reviews.llvm.org/D15419), plus the sigsuspend.cc test. This fails because a signal handler (do_nothing_signal_handler) is called via this backtrace:

frame #0: __sanitizer::CheckFailed "((thr->in_interceptor_count)) == ((0))" at sanitizer_common.cc:158
frame #1:__tsan_write1(addr=0x0000000100001080) + 146 at tsan_interface_inl.h:39
frame #2: sigsuspend.cc.tmp`::do_nothing_signal_handler() + 47 at sigsuspend.cc:17 [opt]
frame #3: libsystem_platform.dylib`_sigtramp + 26
frame #4: libsystem_kernel.dylib`__sigsuspend + 11
frame #5: wrap_sigsuspend(mask=0x00007fff5fbff9cc) + 142 at tsan_interceptors.cc:2104
frame #6: sigsuspend.cc.tmp`main + 157 at sigsuspend.cc:35 [opt]

...and the wrap_sigsuspend interceptor calls REAL(sigsuspend) while ScopedInterceptor is still in scope:

TSAN_INTERCEPTOR(int, sigsuspend, const __sanitizer_sigset_t *mask) {
  SCOPED_TSAN_INTERCEPTOR(sigsuspend, mask);
  return REAL(sigsuspend)(mask);
}
kubamracek updated this revision to Diff 42979.Dec 16 2015, 3:11 AM
kubamracek edited edge metadata.

Updating patch.

kubamracek requested a review of this revision.Dec 16 2015, 3:11 AM
kubamracek edited edge metadata.
dvyukov accepted this revision.Dec 18 2015, 2:09 AM
dvyukov edited edge metadata.
This revision is now accepted and ready to land.Dec 18 2015, 2:09 AM
This revision was automatically updated to reflect the committed changes.

There are some failures on the Linux buildbot, http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/13046/steps/test%20tsan%20in%20debug%20compiler-rt%20build/logs/stdio:

Failing Tests (3):
ThreadSanitizer :: dl_iterate_phdr.cc
ThreadSanitizer :: signal_sync.cc
ThreadSanitizer :: signal_thread.cc

The "dl_iterate_phdr.cc" one should be easy to fix, we just need to annotate the callback from the interceptor into user code. However, I don't know yet what's the right solution for the signal tests.