Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-21 Thread Dmitry Vyukov
On Wed, Jan 21, 2015 at 11:34 AM, Jakub Jelinek wrote: > On Wed, Jan 21, 2015 at 12:23:34PM +0400, Dmitry Vyukov wrote: >> Hi Mike, >> >> Yes, I can quantify the cost. Is it very high. >> >> Here is the patch that I used: >> >> --- rtl/tsan_rtl.cc (revision 226644) >> +++ rtl/tsan_rtl.cc (working

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-21 Thread Jakub Jelinek
On Wed, Jan 21, 2015 at 12:23:34PM +0400, Dmitry Vyukov wrote: > Hi Mike, > > Yes, I can quantify the cost. Is it very high. > > Here is the patch that I used: > > --- rtl/tsan_rtl.cc (revision 226644) > +++ rtl/tsan_rtl.cc (working copy) > @@ -709,7 +709,11 @@ > ALWAYS_INLINE USED > void Memo

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-21 Thread Dmitry Vyukov
Hi Mike, Yes, I can quantify the cost. Is it very high. Here is the patch that I used: --- rtl/tsan_rtl.cc (revision 226644) +++ rtl/tsan_rtl.cc (working copy) @@ -709,7 +709,11 @@ ALWAYS_INLINE USED void MemoryAccess(ThreadState *thr, uptr pc, uptr addr, int kAccessSizeLog, bool kAccessI

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-19 Thread Mike Stump
On Jan 19, 2015, at 12:47 AM, Dmitry Vyukov wrote: > Long story short. Tsan has a logical data race the core of data race > detection algorithm. The race is not a bug, but a deliberate design > decision that makes tsan considerably faster. Could you please quantify that for us? Also, what lockle

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-19 Thread Dmitry Vyukov
Long story short. Tsan has a logical data race the core of data race detection algorithm. The race is not a bug, but a deliberate design decision that makes tsan considerably faster. So ironically, if the race memory accesses happen almost simultaneously, tsan can miss the race. Thus we have sleeps

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-09 Thread Jakub Jelinek
On Fri, Jan 09, 2015 at 04:32:47PM +0100, Bernd Edlinger wrote: > Hi, > > On Thu, 8 Jan 2015 22:27:26, Jakub Jelinek wrote: > >> Any objections to approving it now? > > > > LGTM. > > > > Jakub > > would it be OK to apply this patch also to the 4.9 testsuite, > except for c-c++-common/tsan/bitfiel

RE: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-09 Thread Bernd Edlinger
Hi, On Thu, 8 Jan 2015 22:27:26, Jakub Jelinek wrote: >> Any objections to approving it now? > > LGTM. > > Jakub would it be OK to apply this patch also to the 4.9 testsuite, except for c-c++-common/tsan/bitfield_race.c and g++.dg/tsan/aligned_vs_unaligned_race.C of course? Bernd.

RE: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-08 Thread Bernd Edlinger
On Thu, 8 Jan 2015 14:05:32, Mike Stump wrote: > > On Jan 8, 2015, at 1:27 PM, Jakub Jelinek wrote: >>> Any objections to approving it now? >> >> LGTM. > > Patch is Ok. If you could send the clang folks a heads up, I’ve love to see > them adopt the style. Thanks, I am glad that we finally have

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-08 Thread Mike Stump
On Jan 8, 2015, at 1:27 PM, Jakub Jelinek wrote: >> Any objections to approving it now? > > LGTM. Patch is Ok. If you could send the clang folks a heads up, I’ve love to see them adopt the style.

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-08 Thread Jakub Jelinek
On Thu, Jan 08, 2015 at 01:07:02PM -0800, Mike Stump wrote: > On Jan 8, 2015, at 11:29 AM, Jakub Jelinek wrote: > > I disagree. Busy waiting of this kind is not appropriate for the test > > suite, > > What busy waiting, there is none in the last version of the patch? It was still while (... !=

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-08 Thread Mike Stump
On Jan 8, 2015, at 11:29 AM, Jakub Jelinek wrote: > I disagree. Busy waiting of this kind is not appropriate for the test suite, What busy waiting, there is none in the last version of the patch? > tsan can't intercept the calls that you do through dlsym, because you > explicitly bypass tsan in

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-08 Thread Jakub Jelinek
On Thu, Jan 08, 2015 at 11:24:21AM -0800, Mike Stump wrote: > On Jan 7, 2015, at 2:44 PM, Bernd Edlinger wrote: > > Here is a new patch, that uses this method on all tsan tests, > > which seem to depend on sleep(1), and which have unstable results > > because of that. > > > OK for trunk? > > No.

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-08 Thread Mike Stump
On Jan 7, 2015, at 2:44 PM, Bernd Edlinger wrote: > Here is a new patch, that uses this method on all tsan tests, > which seem to depend on sleep(1), and which have unstable results > because of that. > OK for trunk? No. So, I think this is wrong. The problem is that any synchronizing primiti

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-08 Thread Mike Stump
On Jan 7, 2015, at 12:23 AM, Jakub Jelinek wrote: > > I'm fine with adding the no_sanitize_thread attribute, the patch LGTM Thanks, I checked it in, the doc seemed trivial enough. * tsan.c (pass_tsan::gate): Add no_sanitize_thread support. (pass_tsan_O0::gate): Likewise.

RE: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-07 Thread Bernd Edlinger
On Wed, 7 Jan 2015 19:32:16, Jakub Jelinek wrote: >> I am however not sure if I can always use -ldl or have to use some >> target-dependencies here. > > libsanitizer always puts > -lpthread -ldl -lm > into libsanitizer.spec, so I'd say it should be safe and you shouldn't even > need -ldl in dg-ad

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-07 Thread Jakub Jelinek
On Wed, Jan 07, 2015 at 07:21:40PM +0100, Bernd Edlinger wrote: > Ok, just for completeness, I've got the dlopen finally working: > RTLD_NOLOAD is not working, but RTLD_LAZY or RTLD_NOW does. No big deal I guess. > I am however not sure if I can always use -ldl or have to use some > target-depen

RE: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-07 Thread Bernd Edlinger
On Wed, 7 Jan 2015 18:00:27, Jakub Jelinek wrote: > > On Wed, Jan 07, 2015 at 08:58:04AM -0800, Mike Stump wrote: >> On Jan 7, 2015, at 12:23 AM, Jakub Jelinek wrote: >>> But I really don't like the busy waiting. >> >> We’ve already determined that sched_sleep isn’t intercepted and can be used >

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-07 Thread Jakub Jelinek
On Wed, Jan 07, 2015 at 08:58:04AM -0800, Mike Stump wrote: > On Jan 7, 2015, at 12:23 AM, Jakub Jelinek wrote: > > But I really don't like the busy waiting. > > We’ve already determined that sched_sleep isn’t intercepted and can be used > to non-busy wait. Any reason not to use it? > > > As t

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-07 Thread Mike Stump
On Jan 7, 2015, at 12:23 AM, Jakub Jelinek wrote: > But I really don't like the busy waiting. We’ve already determined that sched_sleep isn’t intercepted and can be used to non-busy wait. Any reason not to use it? > As tsan is only supported on x86_64-linux So, I hate hardening the code to be

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-07 Thread Mike Stump
On Jan 6, 2015, at 11:17 PM, Bernd Edlinger wrote: > > On Tue, 6 Jan 2015 16:32:35, Mike Stump wrote: >> >> On Jan 6, 2015, at 3:22 PM, Bernd Edlinger wrote: >>> Yes, I think too that it can't fail under these conditions. >> >> If you mean your version… A lot has been written on how to to make

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-07 Thread Jakub Jelinek
On Wed, Jan 07, 2015 at 03:55:11PM +0100, Bernd Edlinger wrote: > cat barrier.h > #include > > static pthread_barrier_t barrier; > > static int (*barrier_wait)(pthread_barrier_t *); Better static __typeof (pthread_barrier_wait) *barrier_wait; ? > > __attribute__((constructor(101))) I wouldn't

RE: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-07 Thread Bernd Edlinger
On Wed, 7 Jan 2015 09:23:39, Jakub Jelinek wrote: > > But I really don't like the busy waiting. You essentially want something > like pthread_barrier_wait that libtsan isn't aware of, right? > Yes. > As tsan is only supported on x86_64-linux (and in the future could be on > some other 64-bit li

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-07 Thread Jakub Jelinek
On Wed, Jan 07, 2015 at 08:17:34AM +0100, Bernd Edlinger wrote: > > On Tue, 6 Jan 2015 16:32:35, Mike Stump wrote: > > > > On Jan 6, 2015, at 3:22 PM, Bernd Edlinger > > wrote: > >> Yes, I think too that it can't fail under these conditions. > > > > If you mean your version… A lot has been writt

RE: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-06 Thread Bernd Edlinger
On Tue, 6 Jan 2015 16:32:35, Mike Stump wrote: > > On Jan 6, 2015, at 3:22 PM, Bernd Edlinger wrote: >> Yes, I think too that it can't fail under these conditions. > > If you mean your version… A lot has been written on how to to make racy code > non-racy… I’d refer you to the literature on all

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-06 Thread Mike Stump
On Jan 6, 2015, at 3:22 PM, Bernd Edlinger wrote: > Yes, I think too that it can't fail under these conditions. If you mean your version… A lot has been written on how to to make racy code non-racy… I’d refer you to the literature on all the various solutions people have found to date. I don

RE: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-06 Thread Bernd Edlinger
On Tue, 6 Jan 2015 11:47:30, Mike Stump wrote: > > On Jan 6, 2015, at 9:45 AM, Bernd Edlinger 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

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-06 Thread Mike Stump
On Jan 5, 2015, at 5:06 PM, Bernd Edlinger wrote: > > I tried to elaborate your idea a bit, and used proper atomics. > It is necessary that the logic of the step function is completely invisible > to tsan. > Therefore it should not call sleep, Again, sleep can’t fix race conditions, so therefo

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-06 Thread Mike Stump
On Jan 6, 2015, at 9:45 AM, Bernd Edlinger 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

RE: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-06 Thread Bernd Edlinger
> Date: Tue, 6 Jan 2015 10:16:33 +0100 > From: ja...@redhat.com > To: bernd.edlin...@hotmail.de > CC: mikest...@comcast.net; hjl.to...@gmail.com; gcc-patches@gcc.gnu.org; > dvyu...@google.com > Subject: Re: [PATCH] Fix sporadic failure

RE: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-06 Thread Bernd Edlinger
On Tue, 6 Jan 2015 10:16:33, Jakub Jelinek wrote: > > On Tue, Jan 06, 2015 at 10:08:22AM +0100, Bernd Edlinger wrote: >> Hi Mike, >> >> after some hours of sleep I realized that your step function can do >> something very interesting, >> (which you already requested previously): >> >> That is: cr

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-06 Thread Jakub Jelinek
On Tue, Jan 06, 2015 at 10:08:22AM +0100, Bernd Edlinger wrote: > Hi Mike, > > after some hours of sleep I realized that your step function can do something > very interesting, > (which you already requested previously): > > That is: create a race condition that is _always_ at 100% missed by tsa

RE: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-06 Thread Bernd Edlinger
Hi Mike, after some hours of sleep I realized that your step function can do something very interesting, (which you already requested previously): That is: create a race condition that is _always_ at 100% missed by tsan: cat lib.c /* { dg-do compile } */ /* { dg-options "-O2 -fno-sanitize=all"

RE: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-05 Thread Bernd Edlinger
Hi Mike, On Mon, 5 Jan 2015 14:01:42, Mike Stump wrote: > > On Jan 5, 2015, at 12:58 PM, Mike Stump wrote: >> So, to help you out, I tried my hand at wiring up the extra library code: > > So, my tsan build finally finished, and I could try this with a real test > case. I ran it 20 times, and go

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-05 Thread Mike Stump
On Jan 5, 2015, at 12:58 PM, Mike Stump wrote: > So, to help you out, I tried my hand at wiring up the extra library code: So, my tsan build finally finished, and I could try this with a real test case. I ran it 20 times, and got 11 fails with: $ i=20; while let i--; do make RUNTESTFLAGS=tsan

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-05 Thread Mike Stump
On Jan 5, 2015, at 12:49 AM, Bernd Edlinger wrote: > On Sun, 4 Jan 2015 14:18:59, Mike Stump wrote: >> >>> But tsan still gets at least 90% chance to spot that. As a matter of fact, >>> the tsan runtime is just _incredibly_ fast, >>> and catches errors, with a very high reliability. But it is ra

RE: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-05 Thread Bernd Edlinger
Hi, On Sun, 4 Jan 2015 14:18:59, Mike Stump wrote: > >> But tsan still gets at least 90% chance to spot that. As a matter of fact, >> the tsan runtime is just _incredibly_ fast, >> and catches errors, with a very high reliability. But it is racy by design. > > You say by design. I’m skeptical of

RE: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-04 Thread Bernd Edlinger
On Sun, 4 Jan 2015 13:57:38, Mike Stump wrote: > > On Jan 4, 2015, at 1:48 PM, Bernd Edlinger wrote: >> I would need a way to link the test case two helper functions, that are not >> compiled with -fsanitize=thread > > /* { dg-additional-sources “tsan-helper.c" } */ > > might be one step forward

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-04 Thread Mike Stump
On Jan 4, 2015, at 11:44 AM, Bernd Edlinger wrote: > On Sun, 4 Jan 2015 20:16:58, Jakub Jelinek wrote: >> >> On Sun, Jan 04, 2015 at 11:07:31AM -0800, Mike Stump wrote: >>> On Jan 4, 2015, at 9:00 AM, Bernd Edlinger >>> wrote: It is due to a race condition in tsan itself, it cannot decide

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-04 Thread Mike Stump
On Jan 4, 2015, at 1:48 PM, Bernd Edlinger wrote: > I would need a way to link the test case two helper functions, that are not > compiled with -fsanitize=thread /* { dg-additional-sources “tsan-helper.c" } */ might be one step forward to do that. I don’t know off hand about the options. I t

RE: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-04 Thread Bernd Edlinger
On Sun, 4 Jan 2015 11:04:34, Mike Stump wrote: > > Ah, even more curious. So, for testing, it would be best if we had a way to > synchronize the threads so that they reliably can show what you want to show, > and reliably pick which thread will run first and which second and so on. The > proble

RE: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-04 Thread Bernd Edlinger
Hi, On Sun, 4 Jan 2015 20:16:58, Jakub Jelinek wrote: > > On Sun, Jan 04, 2015 at 11:07:31AM -0800, Mike Stump wrote: >> On Jan 4, 2015, at 9:00 AM, Bernd Edlinger wrote: >>> It is due to a race condition in tsan itself, it cannot decide which access >>> was the >>> previous one and which was th

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-04 Thread Jakub Jelinek
On Sun, Jan 04, 2015 at 11:07:31AM -0800, Mike Stump wrote: > On Jan 4, 2015, at 9:00 AM, Bernd Edlinger wrote: > > It is due to a race condition in tsan itself, it cannot decide which access > > was the > > previous one and which was the second one, but our tsan tests are not meant > > as > > a

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-04 Thread Jakub Jelinek
On Sun, Jan 04, 2015 at 11:04:34AM -0800, Mike Stump wrote: > On Jan 3, 2015, at 3:20 AM, Bernd Edlinger wrote: > > Yes, but all other tsan test cases call sleep(1) too. > > Ick. The problem is pushing bugs to obscure them without fixing them, makes > the system inherently less good. > > If th

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-04 Thread Mike Stump
On Jan 4, 2015, at 9:00 AM, Bernd Edlinger wrote: > It is due to a race condition in tsan itself, it cannot decide which access > was the > previous one and which was the second one, but our tsan tests are not meant as > a functional test of the tsan runtime library, they are only meant to test

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-04 Thread Mike Stump
On Jan 3, 2015, at 3:20 AM, Bernd Edlinger wrote: > Yes, but all other tsan test cases call sleep(1) too. Ick. The problem is pushing bugs to obscure them without fixing them, makes the system inherently less good. If there is already one bug that documents the problem that the sleep cures th

RE: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-04 Thread Bernd Edlinger
Aehm, sorry, that's the sporadic failure, I mentioned: https://gcc.gnu.org/ml/gcc-regression/2015-01/msg00041.html New failures: FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O2 execution test New passes: https://gcc.gnu.org/ml/gcc-regression/2015-01/msg00044.html New failures: New passes:

RE: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-03 Thread Bernd Edlinger
On Sat, 3 Jan 2015 01:51:34, Mike Strump wrote: > > On Jan 3, 2015, at 1:01 AM, Bernd Edlinger wrote: >> the test case g++.dg/tsan/aligned_vs_unaligned_race.C >> still fails sporadically (around 1 out of 100 times). >> That is apparently a race condition in the tsan runtime itself. >> To make th

Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-03 Thread Mike Stump
On Jan 3, 2015, at 1:01 AM, Bernd Edlinger wrote: > the test case g++.dg/tsan/aligned_vs_unaligned_race.C > still fails sporadically (around 1 out of 100 times). > That is apparently a race condition in the tsan runtime itself. > To make the test reproducible pass, I need to add a sleep(1) > in on

[PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

2015-01-03 Thread Bernd Edlinger
Hi, the test case g++.dg/tsan/aligned_vs_unaligned_race.C still fails sporadically (around 1 out of 100 times). That is apparently a race condition in the tsan runtime itself. To make the test reproducible pass, I need to add a sleep(1) in one of the two threads. Tested really often with: make