Re: [PING] [PATCH] Fix parameters of __tsan_vptr_update

[Konstantin Serebryany <konstantin dot s dot serebryany at gmail dot com>]

On Mon, Jan 19, 2015 at 11:09 PM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
>
> Hi,
>
> On Mon, 19 Jan 2015 18:49:21, Konstantin Serebryany wrote:
>>
>> [text-only]
>>
>> On Mon, Jan 19, 2015 at 7:42 AM, Mike Stump <mikestump@comcast.net> wrote:
>>> On Jan 19, 2015, at 12:43 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>>> I can't really make my mind on this. I would mildly prefer sleep's (if
>>>> they work reliably!).
>>>
>>> Let me state it more forcefully.
>> You don't have to convince us here.
>> I'd love to get rid of sleep calls in the tsan test suite -- they are
>> a minor but a constant annoyance.
>> But I also want to keep the tests *very simple*, i.e.
>> 1. Single file w/o any non-system includes, no linking of extra
>> libraries/objects
>> 2. Not too much extra code. (ideally, 1 line for init, 1 line for
>> "signal", 1 line for "wait")
>> 3. Strictly posix or c++11 (unless we are testing something specific)
>>
>> Your idea with barrier_wait/dlsym sounds interesting, but I can't see
>> the code in this mail thread.
>> What do I miss?
>>
>
> We discussed two alternatives to sleep:
>
> 1. step function, optionally with sched_yield to make it somewhat less busy waiting:
> __attribute__((no_sanitize_thread))
> void step (int i)
> {
>    while (__atomic_load_n (&serial, __ATOMIC_ACQUIRE) != i - 1)
>      sched_yield();
>    __atomic_store_n (&serial, i, __ATOMIC_RELEASE);
> }

This will not work: tsan will still  instrument atomics in a function
with __attribute__((no_sanitize_thread))
(This applies to both clang and gcc variants)

> 2. tsan-invisible barriers:
>
> cat tsan_barrier.h
> /* TSAN-invisible barriers.  Link with -ldl.  */
> #include <pthread.h>
> #include <dlfcn.h>
>
> static __typeof(pthread_barrier_wait) *barrier_wait;
>
> static
> void barrier_init (pthread_barrier_t *barrier, unsigned count)
> {
>   void *h = dlopen ("libpthread.so.0", RTLD_LAZY);
>   barrier_wait = (__typeof (pthread_barrier_wait) *)
>           dlsym (h, "pthread_barrier_wait");
>   pthread_barrier_init (barrier, NULL, count);
> }

So, we will have a single extra header file, but no extra .c files.
This sounds tolerable.
I am not sure how portable that is, but today's tsan works only on
modern Linux anyway.

If you want to contribute the code, please send a patch to upstream LLVM:
we may not be able to take the code from gcc source tree to LLVM, the
other direction is easy.
Or please give us some time to fix the tests in upstream ourselves and
than port them to the gcc test suite.

Dmitry, wdyt?


>
>
> We preferred the second alternative, because it does not do busy waiting.
> We include this header file in every positive test case and link with -ldl.

-ldl is not required, since -fsanitize=thread already adds that.

--kcc

>
> Bernd.
>
>