[PATCH] D54889: Fiber support for thread sanitizer

[Dmitry Vyukov via Phabricator via llvm-commits]

dvyukov added a comment.

In D54889#1325232 <https://reviews.llvm.org/D54889#1325232>, @yuri wrote:

>




>> 3. I think instead of _tsan_get_current_fiber we could use pointer 0. It prevents things like switching to context of another thread, which won't end well. And just simplifies API surface.
> 
> For us possibility to use context of another thread is required feature. We use it to call into library which use fibers from external finber-unaware code.
>  It is on-par with Windows API for fibers - ConvertThreadToFiber().

Tell me more.
You have context for all fibers, __tsan_create_fiber returns it.
_tsan_get_current_fiber is only useful for the physical thread. Do you use __tsan_switch_to_fiber to switch from a fiber to a non-current physical thread? I don't see how this can work. Or what do you do?

>> 4. What if a fiber calls pthread_exit?
> 
> I think, it's enough to add check and abort until someone will really need support for it

Let's add a check and a test then. These things tend to lead to very obscure failure modes that take days to debug on large user programs.

>> 5. Can we combine _tsan_set_fiber_name with _tsan_create_fiber?
> 
> It will not solve our requirements. It is convenient to set fiber name from the fiber itself and even change it sometimes, analogously to pthread_setname_np().

I see. Let's leave it as separate function. I guess it's more flexible and will cover all possible use cases.

>> 7. Do we want to support nested fibers/tasks? Does GCD have nested tasks? I think TBB/OpenMP does. Won't begin/end fiber/task be a more generic API for this? E.g. fiber switching can be expressed as stop running the prev fiber (which switches back to the physical thread context), then start running new fiber.
> 
> GCD has no nested task. GCD tasks can start other tasks, but can't switch or wait.
> 
> For me current API looks more generic then what you suggest.
>  Also, switch to physical thread context may be problematic in some case (see answer to question 3).

Let's leave tasks aside for now. It's quite complex without tasks already.

>> 8. How does this play with setjmp/longjmp? In particular with fiber switching based on setjmp/longjmp? jmp_bufs is part of ThreadState, won't we always try to do longjmp from a wrong ThreadState in such case?
> 
> My idea is when setjmp/longjmp is used for fiber switch, client should disable interceptors before call and enable them after it.

What do you mean by "disable interceptors"? Do we have such thing?

> This procedure can be simplified by flag of _tsan_switch_to_fiber().

How? setjmp/longjmp happen outside of __tsan_switch_to_fiber.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D54889