On Tue, 6 Jan 2015 11:47:30, Mike Stump wrote: > > On Jan 6, 2015, at 9:45 AM, Bernd Edlinger <bernd.edlin...@hotmail.de> wrote: >> I tried your suggestion now, and it seems to work. (on a 4-way core AMD64 >> laptop) >> >> Would you prefer this over adding a sleep in Thread1, which I posted >> previously? > > The problem with the patch is there is nothing in the patch that makes it > work. You merely lower the odds of it failing. The point of addressing the > problem, and the problem is, this test case randomly fails, is to change the > test case, so that it is impossible by design for the test case to ever fail. > In my version of the fix, I think it can't fail.
Yes, I think too that it can't fail under these conditions. Maybe that is just a philosophical problem. If we prepare the test in this way it does not really contain any race condition. (*p4).val++; step (1); happens first, and then: step (2); Global[1]++; but we consider the test passed, when we see a diagnostic? >> Therefore it should not call sleep, > Again, sleep can’t fix race conditions, so therefore tsan can never use it as > an indication that no race exists. If it ever did, that would be a bug. The > only case where that would not be true, would be a hard real time analysis > verifier, and, when we get one, sleep can be so modified. >> because that can be intercepted, even if the code is not instrumented. > I don’t see the relevance. I mention that only, because sleep, usleep and nanosleep are intercepted by tsan_interceptors.cc and calling them alters the printed diagnostics, some tests need to see "as if synchronized by sleep", some don't. As far as I can see, there is no interceptor for sched_yield, so using that in the step function is OK. However the other use of step is a valid test, although one that always fails. Here we see a real race condition with no tsan diagnostic, just because both threads alter the same variable at the same nanosecond, that is remarkable. Just as if tsan had a blind spot here. Once again, with a 3-way handshake in case the Tread1 is scheduled before pthread_create returns: cat tiny-race.c /* { dg-shouldfail "tsan" } */ #include <pthread.h> void step(int); int Global; void *Thread1(void *x) { step (2); Global = 42; return x; } int main() { pthread_t t; pthread_create(&t, 0, Thread1, 0); step (1); step (3); Global = 43; pthread_join(t, 0); return 0; } /* { dg-output "WARNING: ThreadSanitizer: data race.*(\n|\r\n|\r)" } */ >> Unfortunately there is no __attribute__((no_sanitize_thread)) > So, is that any harder to add then: If a new attribute is easier to implement than teaching the tsan.exp tcl script that lib.c is something special, maybe... Bernd.