[PATCH] D54889: Fiber support for thread sanitizer

Mon Dec 10 05:11:55 PST 2018

yuri marked an inline comment as done.
yuri added inline comments.

================
Comment at: lib/tsan/rtl/tsan_rtl_thread.cc:115
   // TraceAddEvent will reset stack0/mset0 in the new part for us.
-  TraceAddEvent(thr, thr->fast_state, EventTypeMop, 0);
+  TraceAddEvent(thr, thr->fast_state, EventTypeMop, 0, false);

----------------
dvyukov wrote:
> yuri wrote:
> > dvyukov wrote:
> > > I think we should execute start on the fiber itself. This is what we do for normal threads. This should resolve the hacky call problem (right?) and also prevent fibers from breaking in future.
> > > We can do this either by briefly switching to the fiber context and back when we create a new fiber. Or, start it lazily on first switch.
> > Start indeed can be executed on the first call to __tsan_switch_to_fiber(), but this solution is not suitable for destroying fiber, because it is normal for fiber to cycle in infinite loop and be destroyed externally.
> > Do you think it is safe to temporary switch to fiber context for calling ThreadStart and/or ThreadFinish without actually switch to fiber stack?
> Do we use trace in ThreadFinish?
> If not, then it's not a problem. If yes, then switching to the fiber looks fine to me and better then changing non-local code invariants.
> 
> If we do this, do we still need any hacky call changes?
There is no trace in ThreadFinish for detached thread, but it uses proc(), so it is better to switch temporary.
I think, hacky call changed can be reverted. I will check it.

Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54889/new/

https://reviews.llvm.org/D54889