Re: [libstdc++, testsuite] Add dg-require-thread-fence

[Christophe Lyon <christophe dot lyon at linaro dot org>]

On 20 October 2016 at 11:40, Jonathan Wakely <jwakely@redhat.com> wrote:
> Please CC the libstdc++ list for libstdc++ patches, this is a
> requirement for patch submission, see https://gcc.gnu.org/lists.html
>
OK, I thought I wasn't really modifying the lib itself :-)

>
> On 20/10/16 09:55 +0200, Christophe Lyon wrote:
>>
>> Hi,
>>
>> Several times I have noticed/reported test failures because some test
>> cases wouldn't link on arm-none-eabi using the default 'old' cpu
>> target: __sync_synchronize cannot be resolved by the linker.
>
>
> That isn't used by libstdc++ anywhere, so the call to it is being
> emitted by the compiler. It would be good to know where it comes from.
>
>
>
>> The attached long patch adds
>> +// { dg-require-thread-fence "" }
>> to all the tests that require it according to the error messages I get.
>>
>> The change is mechanical:
>> - insert this line below dg-do if present
>> - insert this line at the top of the file otherwise
>>
>> For instance:
>>
>> diff --git a/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
>> b/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
>> index 633175b..a048250 100644
>> --- a/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
>> +++ b/libstdc++-v3/testsuite/18_support/bad_typeid/what.cc
>> @@ -1,3 +1,4 @@
>> +// { dg-require-thread-fence "" }
>> // 2007-01-30  Paolo Carlini  <pcarlini@suse.de>
>>
>> // Copyright (C) 2007-2016 Free Software Foundation, Inc.
>> diff --git a/libstdc++-v3/testsuite/18_support/cxa_vec.cc
>> b/libstdc++-v3/testsuite/18_support/cxa_vec.cc
>> index e712655..f2a6c5a 100644
>> --- a/libstdc++-v3/testsuite/18_support/cxa_vec.cc
>> +++ b/libstdc++-v3/testsuite/18_support/cxa_vec.cc
>> @@ -1,4 +1,5 @@
>> // { dg-do run }
>> +// { dg-require-thread-fence "" }
>> // Avoid use of non-overridable new/delete operators in shared
>> // { dg-options "-static" { target *-*-mingw* } }
>> // Test __cxa_vec routines
>>
>>
>> If that's OK, I'm not sure how to write the ChangeLog entry: it
>> modifies 3287 files.
>>
>> In my testing, it replaces 3287 FAILs by 3287 UNSUPPORTED.
>
>
> Wouldn't it be better to remove the dependency on __sync_synchronize
> rather than declare nearly 50% of the testsuite UNSUPPORTED?
>
> If 3287 tests can't use it is the resulting libstdc++.so actually
> useful to anyone?
>

I thought I would follow the discussion in
https://gcc.gnu.org/ml/gcc-patches/2015-04/msg01196.html
when this directive was introduced.

Furthermore, I saw Ramana's comment in
https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02751.html
and I assumed the use of _sync_synchronize() was on
purpose.

I'm happy to prepare a patch with a better fix, as I'd like
to get rid of these errors.

Thanks,

Christophe