On 15/02/16 08:18, Dmitry Vyukov wrote:
> llvm tsan tests contain test.h file (probably what's called
> test_barrier.h in gcc), you can put the macro there. test.h should
> already be included into all tests.
Hmm.. as the person who introduced test_barrer.h (well before llvm had a test.h
;)
I must say, that although if gcc was first here, we will probably change that
to
match llvm's implementation for gcc-7.
I would not like to add more differences here without a very good reason.
I'd say, if Dmitry sees a reason to improve the error handling in test.h, that
is a good thing, and should go into llvm's tree first.
And independently of that I am looking at using llvm's test.h framework instead
of gcc's test_barrier.h for gcc-7 soon.
On 15/02/16 11:56, Tom de Vries wrote:
> On Mon, Feb 15, 2016 at 11:45 AM, Tom de Vries <tom_devr...@mentor.com> wrote:
>>
>> I've tried to be as clear as possible in the RFC submission that I'm not
>> certain about the cause of the failure, and that the patch is proposing a
>> fix that would make that guessed failure cause explicit.
>>
>>> Sure pthread_create can fail, as malloc and mmap.
>>> But if that is the reason for the failure it would happen
>>> just randomly, everywhere.
>>>
>>> Why do you think that only this test case shows the problem?
>>>
>>
>> As I explained in the RFC submission, my reasoning there was that the test
>> is one of the very few test cases that tests the result of pthread_create
>> and then returns 0, which causes the failure in combination with
>> dg-shouldfail.
>>
>> But thinking about it some more, even if pthread_create would fail, causing
>> the testcase to fail in execution, allowing the execution test to pass due
>> to dg-shouldfail, presumably the dg-output test would still fail in that
>> case, so my reasoning was not sound.
>>
>> So I suppose you're right, indeed the pthread_create fail hypothesis is not
>> the most logical one.
>>
>> Still, the patch is an improvement irrespective of the PR that inspired it,
>> and perhaps a lot more library calls should be checked for errors that just
>> pthread_create.
>>
>>> I think Dmitry's comment may be right on the point.
>>
>>
>> If someone proposes that as a patch for the testcase, great. I'm more that
>> willing to test that in my setup to be able to claim 'bootstrapped and
>> reg-tested on x86_64' in the submission.
>>
No problem. PR65400 was a GCC wrong code bug, so it makes no
sense to have the same test in llvm's tree, thus we are free to fix it on
our own, as we like.
Here is a patch that puts each value on it's own 8-byte aligned memory
location. From my experience with tsan tests, sharing shadow memory
slots between v and q or o is the most likely explanation for the occasional
inability to spot the race condition on v, thus the test case fails, because
the return code is 0, and the expected output is not found.
Boot-strapped/regression tested on x86_64-linux-gnu.
OK for trunk?
Thanks
Bernd.
2016-02-15 Bernd Edlinger <bernd.edlin...@hotmail.de>
* c-c++-common/tsan/pr65400-1.c (v, q, o): Make 8-byte aligned.
--- gcc/testsuite/c-c++-common/tsan/pr65400-1.c.jj 2015-03-19 08:53:38.000000000 +0100
+++ gcc/testsuite/c-c++-common/tsan/pr65400-1.c 2016-02-15 11:09:18.852320827 +0100
@@ -7,9 +7,9 @@
#include "tsan_barrier.h"
static pthread_barrier_t barrier;
-int v;
-int q;
-int o;
+int v __attribute__((aligned(8)));
+int q __attribute__((aligned(8)));
+int o __attribute__((aligned(8)));
extern void baz4 (int *);
__attribute__((noinline, noclone)) int
