[TSan] Remove side effects from ThreadTid

[yln]

Summary

ThreadTid and FindThreadByUid should be free of side effects.

Before this commit two consecutive calls to ThreadTid (with the same
arguments) would return different results.

[yln]

Is this an oversight or on purpose?
The functions are duplicated in lsan_thread.cpp, but the LSan version does not have the tctx->user_id = 0; part.

[yln]

If I add a second, dummy call to ThreadTid in one of the places where it is used (e.g., pthread_join interceptor) many tests start to fail.

[dvyukov]

Hi Julian,

Why ThreadTid and FindThreadByUid should be free of side effects? What is the problem you are trying to solve?
I have very worrying feelings about this piece of code. One does not simply shuffle lines here :)
I think it had something to do with races between join and user_id reuse. Are all linux tests reliably pass with this change? Please track history regarding why this line was added.

[yln]

Thanks for pointing this out, Dmitry! I have found your explanation:
https://github.com/llvm-mirror/compiler-rt/commit/411b2c9d6787b64939fc15fdeec65e9d65ba1a51
https://reviews.llvm.org/D54521#1305021

What is the problem you are trying to solve?

I am trying to look up a thread by its user_id (pthread_t).

More context:
The normal way we make TSan aware of threads is CreateThread/StartThread in pthread_create interceptor/__tsan_thread_start_func on parent/child thread with some synchronization between them. On Darwin, we have some special threads that are not created via pthread_create and I can't intercept their creation. Is there a way to ask (from the context of the running thread) "Hey TSan, do you know about me?" Thanks!

[dvyukov]

You may need another function, or add a consume_uid flag to the current one. But if you will do any changes in this area, please add some comment here as to why we reset user_id, it's indeed tricky.

compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp

if (thr->tls_addr && thr->tls_size)
     DontNeedShadowFor(thr->tls_addr, thr->tls_size);
   thr->is_dead = true;
   ctx->thread_registry->FinishThread(thr->tid);
 }
 
 static bool FindThreadByUid(ThreadContextBase *tctx, void *arg) {
   uptr uid = (uptr)arg;
   if (tctx->user_id == uid && tctx->status != ThreadStatusInvalid) {
-    tctx->user_id = 0;
     return true;
   }
   return false;
 }
 
 int ThreadTid(ThreadState *thr, uptr pc, uptr uid) {
   int res = ctx->thread_registry->FindThread(FindThreadByUid, (void*)uid);
   DPrintf("#%d: ThreadTid uid=%zu tid=%d\n", thr->tid, uid, res);