[RFC, PR68580] Handle pthread_create error in tsan testsuite
Mon Feb 15 12:05:00 GMT 2016
On Mon, Feb 15, 2016 at 12:29 PM, Bernd Edlinger
> 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_deVries@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
>>> 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
>>>> 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?
I don't know whether it will fire or not, but 4-byte variables that
are 8-byte aligned can still be collocated with something else. Making
vars 8-byte should be safer.
More information about the Gcc-patches