Same problem exists, thread A can still be within REAL(call_once), but after it
ran user code and set the flag to ~0. Roughly, call_once does:
__call_once(flag, arg, func) {
mutex_lock(mut);
if (flag == BEING_INITIALIZED) { wait }
else if (flag == NOT_INITIALIZED_AT_ALL) {
flag = BEING_INITIALIZED;
mutex_unlock(mut);
func(arg); // <=== user code callback
mutex_lock(mut);
atomic_store(&flag, FULLY_INITIALIZED, mo_release); //
"release" store, but within a compiled dylib, thus invisible to TSan
}
mutex_unlock(mut);
}
If thread A is just after the release store, which is invisible to TSan,
__tsan_acquire in thread B will have no effect, and the stores from the callback
to func(arg) will not be synchronized to thread B.
Anyway, I just realized that we can wrap "func" into our own callback,
which will perform the (extra) __tsan_release... Do you think that would work?
E.g.:
void call_once_callback_wrapper(...) {
orig_func(orig_arg);
__tsan_release(flag);
}
INTERCEPTOR(call_once, o, func, arg) {
REAL(call_once)(flag, ..., call_once_callback_wrapper);
}
Kuba
> On 2 Sep 2016, at 13:42, Dmitry Vyukov <dvyukov at google.com> wrote:
>
> INTERCEPTOR(call_once, o) {
> __tsan_release_merge(o);
> REAL(call_once)(o);
> __tsan_acquire(o);
> }
>
> ?
>
>
> On Fri, Sep 2, 2016 at 12:16 PM, Kuba Brecka <kuba.brecka at
gmail.com> wrote:
>>
>>> On 2 Sep 2016, at 12:11, Dmitry Vyukov <dvyukov at
google.com> wrote:
>>>
>>> On Fri, Sep 2, 2016 at 12:09 PM, Kuba Brecka <kuba.brecka at
gmail.com> wrote:
>>>>
>>>>> On 2 Sep 2016, at 11:18, Dmitry Vyukov via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>>>>>
>>>>> On Thu, Sep 1, 2016 at 2:30 PM, Kuba Brecka <kuba.brecka
at gmail.com> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I'm trying to write a TSan interceptor for the
C++11 call_once function. There are currently false positive reports, because
the inner __call_once function is located in the (non-instrumented) libcxx
library, and on macOS we can't expect the users to build their own
instrumented libcxx.
>>>>>>
>>>>>> TSan already supports pthread_once and dispatch_once by
having interceptors that re-implement the logic. However, doing the same for
call_once/__call_once doesn't work, because call_once is explicitly supposed
to be exception-safe, but the sanitizer runtime libraries disallow exception
handling.
>>>>>>
>>>>>> Any ideas how to handle call_once in TSan?
>>>>>
>>>>> Does anybody remember exact reasons we disable exceptions
in sanitizer
>>>>> runtimes? One is that it won't link with C programs.
Are there any
>>>>> other?
>>>>> If C is the only reason: there is already a part of tsan
runtime is
>>>>> linked only to C++ programs (it contains operator
new/delete
>>>>> interceptors). We could add additional files to the cxx
part of
>>>>> runtime and build them with exceptions.
>>>>>
>>>>> Alternatively, the interceptor can handle only
synchronization but
>>>>> forward actual logic to the real function. Along the lines
of:
>>>>>
>>>>> INTERCEPTOR(call_once, o) {
>>>>> __tsan_acquire_release(o);
>>>>> REAL(call_once)(o);
>>>>> }
>>>>>
>>>>> That will have some performance impact. If we hardcode the
"fully
>>>>> initialized" value, then we can eliminate the
additional overhead:
>>>>>
>>>>> INTERCEPTOR(call_once, o) {
>>>>> if (__atomic_load(o, acquire) == FULLY_INITIALIZED) {
>>>>> __tsan_acquire(o);
>>>>> return;
>>>>> }
>>>>> __tsan_acquire_release(o);
>>>>> REAL(call_once)(o);
>>>>> }
>>>>
>>>> Unfortunately, the first fast-path check is inlined and cannot
be intercepted. We can only intercept the inner call to __call_once. But how
would __tsan_acquire_release help here? The issue is that we need to perform
the release *after* user code has run, but before the "o" flag is
changed. Otherwise, TSan will still see a false positive where one thread has
already run user code, and another thread already sees that call_once is
finished, but the acquire has no release to pair with.
>>>
>>>
>>>
>>> Will then the following work?
>>>
>>> INTERCEPTOR(call_once, o) {
>>> REAL(call_once)(o);
>>> __tsan_release_merge(o);
>>> __tsan_acquire(o);
>>> }
>>
>> Still racy. Suppose thread A is still inside REAL(call_once), but
already after it has run user code and updated "o" to ~0. Thread B
load-acquires "o", finds ~0, assumes it's fully initialized, keeps
going, but user code stores hasn't been properly published.
>>
>> Kuba
>>