[tsan] Mark "responsible frames" in stack traces to show which frame contains the race
Needs ReviewPublic

Authored by kubamracek on Apr 5 2017, 3:16 PM.

Details

Reviewers
kcc
dvyukov
Summary

For reports that involve interceptors (e.g. races against free() or memcpy races), TSan displays reports like this:

==================
WARNING: ThreadSanitizer: data race (pid=67781)
  Write of size 1 at 0x7b0400000045 by thread T2:
    #0 memcpy (libclang_rt.tsan_osx_dynamic.dylib)
    #1 Thread2(void*) (testapp)

  Previous write of size 1 at 0x7b0400000045 by thread T1:
    #0 memcpy (libclang_rt.tsan_osx_dynamic.dylib)
    #1 Thread1(void*) (testapp)

  Location is heap block of size 10 at 0x7b0400000040 allocated by main thread:
    #0 operator new[] ...
  ...
==================

The idea of this patch is to signify which frame is responsible for the bug. Usually, we'd assume that the topmost frame contains the problem that TSan is complaining about, but that's not true in the example above, where memcpy is being called in a racy way. This patch turns the report into:

==================
WARNING: ThreadSanitizer: data race (pid=67781)
  Write of size 1 at 0x7b0400000045 by thread T2:
    #0 memcpy (libclang_rt.tsan_osx_dynamic.dylib)
  * #1 Thread2(void*) (testapp)

  Previous write of size 1 at 0x7b0400000045 by thread T1:
    #0 memcpy (libclang_rt.tsan_osx_dynamic.dylib)
  * #1 Thread1(void*) (testapp)

  Issue is caused by frames marked with "*".

  Location is heap block of size 10 at 0x7b0400000040 allocated by main thread:
    #0 operator new[] ...
  ...
==================

This applies to races on interceptors (memcpy, memcmp, strcpy, ...), races against free, external races (via __tsan_external_read/__tsan_external_write API), and mutex issues like double unlock (where the top frame is the pthread_mutex API and not the caller of it).

Diff Detail

kubamracek created this revision.Apr 5 2017, 3:16 PM

I am not completely follow motivation for this. Is it meant for user? Or for any automated tools?
For automated tools we already have SUMMARY line. User must be proficient enough to understand what happens. And in the end this is not specific to libc, and the guilty frame is not necessary the one just above libc. Consider a race in std::vector, it's probably not std::vector code that is guilty. And this extends just to any library, consider you have race in libfoo, but it's actually your code that misuses libfoo. We don't have enough info to provide precise signal. What we provide here is a weak, potentially incorrect signal.