From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14675 invoked by alias); 5 Jan 2015 22:02:09 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 14657 invoked by uid 89); 5 Jan 2015 22:02:08 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: resqmta-ch2-04v.sys.comcast.net Received: from resqmta-ch2-04v.sys.comcast.net (HELO resqmta-ch2-04v.sys.comcast.net) (69.252.207.36) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 05 Jan 2015 22:02:07 +0000 Received: from resomta-ch2-10v.sys.comcast.net ([69.252.207.106]) by resqmta-ch2-04v.sys.comcast.net with comcast id cN1Z1p00C2JGN3p01N25CV; Mon, 05 Jan 2015 22:02:05 +0000 Received: from [IPv6:2001:558:6045:a4:40c6:7199:cd03:b02d] ([IPv6:2001:558:6045:a4:40c6:7199:cd03:b02d]) by resomta-ch2-10v.sys.comcast.net with comcast id cN221p00U2ztT3H01N23Av; Mon, 05 Jan 2015 22:02:05 +0000 Content-Type: text/plain; charset=windows-1252 Mime-Version: 1.0 (Mac OS X Mail 7.3 \(1878.6\)) Subject: Re: [PATCH] Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C From: Mike Stump In-Reply-To: Date: Mon, 05 Jan 2015 22:02:00 -0000 Cc: Jakub Jelinek , "H.J. Lu" , "gcc-patches@gcc.gnu.org" , Dmitry Vyukov Content-Transfer-Encoding: quoted-printable Message-Id: References: ,<3C12133C-DABF-40FA-94F7-9DB785F6E914@comcast.net>,,,<623A5348-6FC9-4F7B-A9BC-B2B098AF7D37@comcast.net>,<20150104191658.GK1667@tucnak.redhat.com> ,<8E43F8AA-96BA-47A3-A886-C058459B4108@comcast.net> To: Bernd Edlinger X-IsSubscribed: yes X-SW-Source: 2015-01/txt/msg00171.txt.bz2 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 c= ase. I ran it 20 times, and got 11 fails with: $ i=3D20; while let i--; do make RUNTESTFLAGS=3Dtsan.exp=3Daligned_vs_unali= gned_race.C check-g++ | grep FAIL; done FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O2 execution test FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O0 execution test FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O2 execution test FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O2 execution test FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O2 execution test FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O0 execution test FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O0 execution test FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O2 execution test FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O2 execution test FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O2 execution test FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C -O0 execution test $=20 When I fixed the problem, I ran it 20 times: $ i=3D20; while let i--; do make RUNTESTFLAGS=3Dtsan.exp=3Daligned_vs_unali= gned_race.C check-g++ | grep FAIL; done $=20 and got 0 failures. So, it seems to work. I=92d like a tsan person to review it and we can go = from there. The only thing I would add to it would be a huge comment that explains that= the tsan run time isn=92t thread safe and they should use a lock free upda= te to the shadow bits, but, they don=92t. We introduce the step primitive = to work around that bug. Ideally, the the problem should be filed into a b= ug database for the tsan code gen and when closed as not to be fixed, we ca= n then change the word bug to design, but leave the bug reference so that o= thers that want to completely understand the issue can go read up on it. I= f they actually fix the codegen to be thread safe, then we can simply remov= e all the step code. To make this clang friendly, if one turns off inlining and obscures the sem= antics with weak from the optimizer and puts it into a header files and the= n #includes that header files, I think it would work. I=92ll leave this to= someone that might want to do that. If not, I=92m fine with #ifdef clang/= gcc and have the gcc test cases use step and the clang test cases, well, be= unreliable. $ svn diff Index: g++.dg/tsan/aligned_vs_unaligned_race.C =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- g++.dg/tsan/aligned_vs_unaligned_race.C (revision 219198) +++ g++.dg/tsan/aligned_vs_unaligned_race.C (working copy) @@ -1,11 +1,17 @@ +/* { dg-additional-sources "lib.c" } */ /* { dg-shouldfail "tsan" } */ + #include #include #include +#include + +void step(int); =20 uint64_t Global[2]; =20 void *Thread1(void *x) { + step (2); Global[1]++; return NULL; } @@ -15,6 +21,7 @@ void *Thread2(void *x) { struct __attribute__((packed, aligned(1))) u_uint64_t { uint64_t val; }; u_uint64_t *p4 =3D reinterpret_cast(p1 + 1); (*p4).val++; + step (1); return NULL; } =20 $ cat g++.dg/tsan/lib.c=20 /* { dg-do compile } */ #include volatile int serial; __attribute__((no_sanitize_address)) void step(int i) { while (serial !=3D i-1) ; while (1) { if (++serial =3D=3D i) return; --serial; sleep (1); } }