[PATCH] D33325: [sanitizer] Avoid possible deadlock in child process after fork

[Maxim Ostapenko via Phabricator via llvm-commits]

m.ostapenko added a comment.

In https://reviews.llvm.org/D33325#773332, @eugenis wrote:

> In https://reviews.llvm.org/D33325#773322, @m.ostapenko wrote:
>
> > Oh, sorry.
> >
> > In https://reviews.llvm.org/D33325#773286, @eugenis wrote:
> >
> > > This change is causing problems on Linux. The immediate issue is Tcl, which creates new threads from a pthread_atfork() child handler. The handler is called from libc's fork(), and pthread_create tries to acquire the StackDepot lock before it is released in the fork interceptor.
> >
> >
> > Won't this deadlock with MSan as well? AFAIK it also locks StackDepot in fork interceptor...
>
>
> Maybe it does not use StackDepot in pthread_create?
>
> > 
> > 
> >> Tcl is broken, of course:
> >> 
> >>   If a fork() call in a multi-threaded process leads to a child fork handler calling any function that is not async-signal-safe, the behavior is undefined.
> >>    
> >> 
> >> But in general, even malloc() called from pthread_atfork() will deadlock with this change.
> >>  It looks like the right way to do this is call pthread_atfork() as early as possible to setup lock/unlock handlers for the allocator and the stack depot.
> > 
> > Perhaps we can do it from asan_init? From man:
> >  "The order of calls to pthread_atfork() is significant. The parent and child fork handlers shall be called in the order in which they were established by calls to pthread_atfork(). The prepare fork handlers shall be called in the opposite order."
> > 
> > So in this case our unlock routines will be called before others. This still can deadlock when preloading asan to noinstrumented binary though.
>
> Sounds like that's the best we can do. If we are lucky, sanitizer initialization will happen early enough due to ENSURE_ASAN_INIT in one of the interceptors.


Hm, perhaps we can just intercept pthread_atfork and force ENSURE_ASAN_INITED? And install allocator hooks in AsanInitFromRtl?

> 
> 
>> 
>> 
>>> I'll revert this change in the meantime.


Repository:
  rL LLVM

https://reviews.llvm.org/D33325