Page MenuHomePhabricator

[tsan] Properly describe GCD worker threads in reports
ClosedPublic

Authored by kubamracek on Jan 24 2017, 4:46 PM.

Details

Summary

When dealing with GCD worker threads, TSan currently prints weird things like "created by thread T-1" and "[failed to restore the stack]" in reports:

==================
WARNING: ThreadSanitizer: data race (pid=18771)
  Write of size 8 at 0x00010ac221a0 by thread T3:
    ...

  Previous write of size 8 at 0x00010ac221a0 by thread T1:
    ...

  Location is global 'global' at 0x00010ac221a0 (workerthreads.mm.tmp+0x0001000021a0)

  Thread T3 (tid=5981759, running) created by thread T-1
    [failed to restore the stack]

  Thread T1 (tid=5981757, running) created by thread T-1
    [failed to restore the stack]

SUMMARY: ThreadSanitizer: data race workerthreads.mm:23 in __main_block_invoke_2
==================

This patch tries to avoid that and instead print:

==================
WARNING: ThreadSanitizer: data race (pid=18843)
  Write of size 8 at 0x0001044431a0 by thread T3:
    ...

  Previous write of size 8 at 0x0001044431a0 by thread T1:
    ...

  Location is global 'global' at 0x0001044431a0 (workerthreads.mm.tmp+0x0001000021a0)

  Thread T3 (tid=5989400, running) is a GCD worker thread

  Thread T1 (tid=5989398, running) is a GCD worker thread

SUMMARY: ThreadSanitizer: data race workerthreads.mm:23 in __main_block_invoke_2
==================

Diff Detail

Repository
rL LLVM

Event Timeline

kubamracek created this revision.Jan 24 2017, 4:46 PM
dvyukov added inline comments.Jan 25 2017, 7:09 AM
lib/tsan/rtl/tsan_report.cc
239 ↗(On Diff #85655)

This feels like an abuse of kInvalidTid.
Currently we just say that we don't know who is the parent of GCD threads on creation. Which looks legit. But now you are saying: if we don't know a parent of any thread, then it must be a GCD worker thread.

Please mark GCD worker threads more explicitly (either by introducing another special tid, or by marking them with a new flag).
tsan_report.cc is meant to be as dump as possible and just format what it receives. So preferably this should also be a new flag in ReportThread.

kubamracek updated this revision to Diff 86495.Jan 31 2017, 2:05 PM

Updating the patch to introduce a "workerthread" field in ThreadContextBase.

dvyukov accepted this revision.Feb 1 2017, 1:28 AM

Thanks!
Now this also improves asan.

This revision is now accepted and ready to land.Feb 1 2017, 1:28 AM
This revision was automatically updated to reflect the committed changes.