[testsuite,rfa] Add gthreads dependency to some failing libstdc++ tests

Message ID 945fd73f-3b7c-e7c3-8735-60bfb569944f@codesourcery.com
State New
Headers show
Series
  • [testsuite,rfa] Add gthreads dependency to some failing libstdc++ tests
Related show

Commit Message

Sandra Loosemore Jan. 23, 2019, 7:50 p.m.
I ran libstdc++ tests on nios2-elf target.  I observed several new tests 
failing with

error: 'mutex' in namespace 'std' does not name a type

The definition of class mutex in include/bits/std_mutex.h is guarded 
with "#ifdef _GLIBCXX_HAS_GTHREADS" so I assume these tests are not 
supposed to work on this target.  This patch adds the equivalent 
dependency to the failing tests.

OK to commit?  (I guess it is possible that this is actually a bug in 
the code instead, and the tests are supposed to pass....)

-Sandra

Comments

Jonathan Wakely Jan. 24, 2019, 10:46 a.m. | #1
On 23/01/19 12:50 -0700, Sandra Loosemore wrote:
>I ran libstdc++ tests on nios2-elf target.  I observed several new 

>tests failing with

>

>error: 'mutex' in namespace 'std' does not name a type

>

>The definition of class mutex in include/bits/std_mutex.h is guarded 

>with "#ifdef _GLIBCXX_HAS_GTHREADS" so I assume these tests are not 

>supposed to work on this target.  This patch adds the equivalent 

>dependency to the failing tests.


Those features *should* work without threads (if you don't have
multiple threads, they don't need any synchronisation), but currently
they use mutexes unconditionally.

The proper fix is to make them work without gthreads, something like
https://gcc.gnu.org/ml/libstdc++/2018-12/msg00010.html

>OK to commit?  (I guess it is possible that this is actually a bug in 

>the code instead, and the tests are supposed to pass....)


I'd prefer to XFAIL them, so that we remember to un-XFAIL them if/when
I make them stop using mutexes. But we don't have a gthreads
effective-target that would allow that, only the dg-require-gthreads
directive.

Also this patch will skip the tests on AIX, which could run these
tests if we added -pthread to the dg-options.

But these features are half-baked and experimental, and not going to
get more changes in time for GCC 9, so OK for trunk.
Sandra Loosemore Jan. 24, 2019, 6:53 p.m. | #2
On 1/24/19 3:46 AM, Jonathan Wakely wrote:
> On 23/01/19 12:50 -0700, Sandra Loosemore wrote:

>> I ran libstdc++ tests on nios2-elf target.  I observed several new 

>> tests failing with

>>

>> error: 'mutex' in namespace 'std' does not name a type

>>

>> The definition of class mutex in include/bits/std_mutex.h is guarded 

>> with "#ifdef _GLIBCXX_HAS_GTHREADS" so I assume these tests are not 

>> supposed to work on this target.  This patch adds the equivalent 

>> dependency to the failing tests.

> 

> Those features *should* work without threads (if you don't have

> multiple threads, they don't need any synchronisation), but currently

> they use mutexes unconditionally.

> 

> The proper fix is to make them work without gthreads, something like

> https://gcc.gnu.org/ml/libstdc++/2018-12/msg00010.html

> 

>> OK to commit?  (I guess it is possible that this is actually a bug in 

>> the code instead, and the tests are supposed to pass....)

> 

> I'd prefer to XFAIL them, so that we remember to un-XFAIL them if/when

> I make them stop using mutexes. But we don't have a gthreads

> effective-target that would allow that, only the dg-require-gthreads

> directive.

> 

> Also this patch will skip the tests on AIX, which could run these

> tests if we added -pthread to the dg-options.

> 

> But these features are half-baked and experimental, and not going to

> get more changes in time for GCC 9, so OK for trunk.


Well, if this testsuite patch would indeed be papering over a bug, I 
think it's probably a bad idea to commit it.  For purposes of nios2-elf 
testing I can just track these as known failures for now, and not worry 
about them.

BTW, I'm more worried about the link errors introduced by the patch for 
PR 86756.  Those are regressions and apparently a problem that could 
affect user code, not just broken test cases for half-baked new features.

-Sandra
Jonathan Wakely Jan. 24, 2019, 8:20 p.m. | #3
On 24/01/19 11:53 -0700, Sandra Loosemore wrote:
>On 1/24/19 3:46 AM, Jonathan Wakely wrote:

>>On 23/01/19 12:50 -0700, Sandra Loosemore wrote:

>>>I ran libstdc++ tests on nios2-elf target.  I observed several new 

>>>tests failing with

>>>

>>>error: 'mutex' in namespace 'std' does not name a type

>>>

>>>The definition of class mutex in include/bits/std_mutex.h is 

>>>guarded with "#ifdef _GLIBCXX_HAS_GTHREADS" so I assume these 

>>>tests are not supposed to work on this target.  This patch adds 

>>>the equivalent dependency to the failing tests.

>>

>>Those features *should* work without threads (if you don't have

>>multiple threads, they don't need any synchronisation), but currently

>>they use mutexes unconditionally.

>>

>>The proper fix is to make them work without gthreads, something like

>>https://gcc.gnu.org/ml/libstdc++/2018-12/msg00010.html

>>

>>>OK to commit?  (I guess it is possible that this is actually a bug 

>>>in the code instead, and the tests are supposed to pass....)

>>

>>I'd prefer to XFAIL them, so that we remember to un-XFAIL them if/when

>>I make them stop using mutexes. But we don't have a gthreads

>>effective-target that would allow that, only the dg-require-gthreads

>>directive.

>>

>>Also this patch will skip the tests on AIX, which could run these

>>tests if we added -pthread to the dg-options.

>>

>>But these features are half-baked and experimental, and not going to

>>get more changes in time for GCC 9, so OK for trunk.

>

>Well, if this testsuite patch would indeed be papering over a bug, I 

>think it's probably a bad idea to commit it.  For purposes of 

>nios2-elf testing I can just track these as known failures for now, 

>and not worry about them.


If that's OK with you (and David can live with the same situation on
AIX for a bit longer) then I'd prefer to leave them FAILing.

>BTW, I'm more worried about the link errors introduced by the patch 

>for PR 86756.  Those are regressions and apparently a problem that 

>could affect user code, not just broken test cases for half-baked new 

>features.


If I understand correctly(*) it can only affect user code that uses
the std::filesystem library, which was new in GCC 8.1, and if those
tests are failing for nios2-elf then it was never usable anyway. The
tests run by default now, but previously they were only run if gcc was
configured with --enable-libstdcxx-filesystem-ts, so this isn't really
a regression in what users can do with the library. I agree they're
more important than the experimental/net/ tests though.

(*) only a few of the 27_io/filesystem/path tests are showing these
linker errors, right? Or are tests outside 27_io/filesystem also
affected?
Jonathan Wakely Jan. 24, 2019, 8:39 p.m. | #4
On 24/01/19 20:20 +0000, Jonathan Wakely wrote:
>On 24/01/19 11:53 -0700, Sandra Loosemore wrote:

>>On 1/24/19 3:46 AM, Jonathan Wakely wrote:

>>>On 23/01/19 12:50 -0700, Sandra Loosemore wrote:

>>>>I ran libstdc++ tests on nios2-elf target.  I observed several 

>>>>new tests failing with

>>>>

>>>>error: 'mutex' in namespace 'std' does not name a type

>>>>

>>>>The definition of class mutex in include/bits/std_mutex.h is 

>>>>guarded with "#ifdef _GLIBCXX_HAS_GTHREADS" so I assume these 

>>>>tests are not supposed to work on this target.  This patch adds 

>>>>the equivalent dependency to the failing tests.

>>>

>>>Those features *should* work without threads (if you don't have

>>>multiple threads, they don't need any synchronisation), but currently

>>>they use mutexes unconditionally.

>>>

>>>The proper fix is to make them work without gthreads, something like

>>>https://gcc.gnu.org/ml/libstdc++/2018-12/msg00010.html

>>>

>>>>OK to commit?  (I guess it is possible that this is actually a 

>>>>bug in the code instead, and the tests are supposed to pass....)

>>>

>>>I'd prefer to XFAIL them, so that we remember to un-XFAIL them if/when

>>>I make them stop using mutexes. But we don't have a gthreads

>>>effective-target that would allow that, only the dg-require-gthreads

>>>directive.

>>>

>>>Also this patch will skip the tests on AIX, which could run these

>>>tests if we added -pthread to the dg-options.

>>>

>>>But these features are half-baked and experimental, and not going to

>>>get more changes in time for GCC 9, so OK for trunk.

>>

>>Well, if this testsuite patch would indeed be papering over a bug, I 

>>think it's probably a bad idea to commit it.  For purposes of 

>>nios2-elf testing I can just track these as known failures for now, 

>>and not worry about them.

>

>If that's OK with you (and David can live with the same situation on

>AIX for a bit longer) then I'd prefer to leave them FAILing.


But feel free to report them to Bugzilla so I can't pretend I don't
know about them ;-)

>>BTW, I'm more worried about the link errors introduced by the patch 

>>for PR 86756.  Those are regressions and apparently a problem that 

>>could affect user code, not just broken test cases for half-baked 

>>new features.

>

>If I understand correctly(*) it can only affect user code that uses

>the std::filesystem library, which was new in GCC 8.1, and if those

>tests are failing for nios2-elf then it was never usable anyway. The

>tests run by default now, but previously they were only run if gcc was

>configured with --enable-libstdcxx-filesystem-ts, so this isn't really

>a regression in what users can do with the library. I agree they're

>more important than the experimental/net/ tests though.

>

>(*) only a few of the 27_io/filesystem/path tests are showing these

>linker errors, right? Or are tests outside 27_io/filesystem also

>affected?


Do you have any idea how to detect the root cause, i.e. chdir, getcwd
etc. being declared in the headers but not at link-time?
Cross-compilers can't do link tests to check for them.

I can easily make the library throw an exception if
std::filesystem::current_path() is called and getcwd isn't available,
as long as I can detect when it's not available.
Sandra Loosemore Jan. 25, 2019, 12:14 a.m. | #5
On 1/24/19 1:20 PM, Jonathan Wakely wrote:
> On 24/01/19 11:53 -0700, Sandra Loosemore wrote:

> 

>> BTW, I'm more worried about the link errors introduced by the patch 

>> for PR 86756.  Those are regressions and apparently a problem that 

>> could affect user code, not just broken test cases for half-baked new 

>> features.

> 

> If I understand correctly(*) it can only affect user code that uses

> the std::filesystem library, which was new in GCC 8.1, and if those

> tests are failing for nios2-elf then it was never usable anyway. The

> tests run by default now, but previously they were only run if gcc was

> configured with --enable-libstdcxx-filesystem-ts, so this isn't really

> a regression in what users can do with the library. I agree they're

> more important than the experimental/net/ tests though.

> 

> (*) only a few of the 27_io/filesystem/path tests are showing these

> linker errors, right? Or are tests outside 27_io/filesystem also

> affected?


Other tests are affected.  This appears to be the full list:

FAIL: 19_diagnostics/error_code/cons/39882.cc (test for excess errors)
FAIL: 19_diagnostics/error_code/modifiers/39882.cc (test for excess errors)
FAIL: 19_diagnostics/error_condition/cons/39881.cc (test for excess errors)
FAIL: 19_diagnostics/error_condition/modifiers/39881.cc (test for excess 
errors)
FAIL: 19_diagnostics/system_error/cons_virtual_derivation.cc (test for 
excess errors)
FAIL: 20_util/hash/operators/size_t.cc (test for excess errors)
FAIL: 27_io/filesystem/operations/all.cc (test for excess errors)
FAIL: 27_io/filesystem/operations/resize_file.cc (test for excess errors)
FAIL: 27_io/filesystem/path/generation/normal2.cc (test for excess errors)
FAIL: experimental/net/buffer/arithmetic.cc (test for excess errors)
FAIL: experimental/net/buffer/const.cc (test for excess errors)
FAIL: experimental/net/buffer/creation.cc (test for excess errors)
FAIL: experimental/net/buffer/mutable.cc (test for excess errors)
FAIL: experimental/net/buffer/size.cc (test for excess errors)

In GCC 8 the 19_diagnostics and 20_util tests in that list PASSed.  The 
other tests are new in GCC 9, but in GCC 8 all the other 
27_io/filesystem tests showed up as UNSUPPORTED on this target.

BTW, here's a snippet from the link map for the first FAIL on that list. 
  AFAICT there is nothing that references the libstdc++ functions that 
call mkdir, etc; those functions are only getting sucked into the link 
because of a reference to some template function in that compilation unit.

/scratch/sandra/nios2-elf-fsf-gcc/install/opt/codesourcery/bin/../lib/gcc/nios2-elf/9.0.0/../../../../nios2-elf/lib/libstdc++.a(fs_ops.o)
 
/scratch/sandra/nios2-elf-fsf-gcc/install/opt/codesourcery/bin/../lib/gcc/nios2-elf/9.0.0/../../../../nios2-elf/lib/libstdc++.a(system_error.o) 
(void std::__cxx11::basic_string<char, std::char_traits<char>, 
std::allocator<char> >::_M_construct<char const*>(char const*, char 
const*, std::forward_iterator_tag))

-Sandra
Jonathan Wakely Jan. 25, 2019, 12:21 p.m. | #6
On 24/01/19 17:14 -0700, Sandra Loosemore wrote:
>On 1/24/19 1:20 PM, Jonathan Wakely wrote:

>>On 24/01/19 11:53 -0700, Sandra Loosemore wrote:

>>

>>>BTW, I'm more worried about the link errors introduced by the 

>>>patch for PR 86756.  Those are regressions and apparently a 

>>>problem that could affect user code, not just broken test cases 

>>>for half-baked new features.

>>

>>If I understand correctly(*) it can only affect user code that uses

>>the std::filesystem library, which was new in GCC 8.1, and if those

>>tests are failing for nios2-elf then it was never usable anyway. The

>>tests run by default now, but previously they were only run if gcc was

>>configured with --enable-libstdcxx-filesystem-ts, so this isn't really

>>a regression in what users can do with the library. I agree they're

>>more important than the experimental/net/ tests though.

>>

>>(*) only a few of the 27_io/filesystem/path tests are showing these

>>linker errors, right? Or are tests outside 27_io/filesystem also

>>affected?

>

>Other tests are affected.  This appears to be the full list:

>

>FAIL: 19_diagnostics/error_code/cons/39882.cc (test for excess errors)

>FAIL: 19_diagnostics/error_code/modifiers/39882.cc (test for excess errors)

>FAIL: 19_diagnostics/error_condition/cons/39881.cc (test for excess errors)

>FAIL: 19_diagnostics/error_condition/modifiers/39881.cc (test for 

>excess errors)

>FAIL: 19_diagnostics/system_error/cons_virtual_derivation.cc (test for 

>excess errors)

>FAIL: 20_util/hash/operators/size_t.cc (test for excess errors)

>FAIL: 27_io/filesystem/operations/all.cc (test for excess errors)

>FAIL: 27_io/filesystem/operations/resize_file.cc (test for excess errors)

>FAIL: 27_io/filesystem/path/generation/normal2.cc (test for excess errors)

>FAIL: experimental/net/buffer/arithmetic.cc (test for excess errors)

>FAIL: experimental/net/buffer/const.cc (test for excess errors)

>FAIL: experimental/net/buffer/creation.cc (test for excess errors)

>FAIL: experimental/net/buffer/mutable.cc (test for excess errors)

>FAIL: experimental/net/buffer/size.cc (test for excess errors)

>

>In GCC 8 the 19_diagnostics and 20_util tests in that list PASSed.  

>The other tests are new in GCC 9, but in GCC 8 all the other 

>27_io/filesystem tests showed up as UNSUPPORTED on this target.

>

>BTW, here's a snippet from the link map for the first FAIL on that 

>list.  AFAICT there is nothing that references the libstdc++ functions 

>that call mkdir, etc; those functions are only getting sucked into the 

>link because of a reference to some template function in that 

>compilation unit.

>

>/scratch/sandra/nios2-elf-fsf-gcc/install/opt/codesourcery/bin/../lib/gcc/nios2-elf/9.0.0/../../../../nios2-elf/lib/libstdc++.a(fs_ops.o)

>

>/scratch/sandra/nios2-elf-fsf-gcc/install/opt/codesourcery/bin/../lib/gcc/nios2-elf/9.0.0/../../../../nios2-elf/lib/libstdc++.a(system_error.o) 

>(void std::__cxx11::basic_string<char, std::char_traits<char>, 

>std::allocator<char> >::_M_construct<char const*>(char const*, char 

>const*, std::forward_iterator_tag))


Thanks. I wonder if this is caused by using -fimplicit-templates in
src/c++17/Makefile.

Patch

diff --git a/libstdc++-v3/testsuite/experimental/net/execution_context/use_service.cc b/libstdc++-v3/testsuite/experimental/net/execution_context/use_service.cc
index 5b30870..67929a4 100644
--- a/libstdc++-v3/testsuite/experimental/net/execution_context/use_service.cc
+++ b/libstdc++-v3/testsuite/experimental/net/execution_context/use_service.cc
@@ -16,6 +16,7 @@ 
 // <http://www.gnu.org/licenses/>.
 
 // { dg-do run { target c++14 } }
+// { dg-require-gthreads "" }
 
 #include <experimental/executor>
 #include <testsuite_hooks.h>
diff --git a/libstdc++-v3/testsuite/experimental/net/headers.cc b/libstdc++-v3/testsuite/experimental/net/headers.cc
index 1705d2d..959ce0d 100644
--- a/libstdc++-v3/testsuite/experimental/net/headers.cc
+++ b/libstdc++-v3/testsuite/experimental/net/headers.cc
@@ -16,6 +16,7 @@ 
 // <http://www.gnu.org/licenses/>.
 
 // { dg-do compile }
+// { dg-require-gthreads "" }
 
 #include <experimental/net>
 
diff --git a/libstdc++-v3/testsuite/experimental/net/internet/address/v4/comparisons.cc b/libstdc++-v3/testsuite/experimental/net/internet/address/v4/comparisons.cc
index 83a8bab..83359ec 100644
--- a/libstdc++-v3/testsuite/experimental/net/internet/address/v4/comparisons.cc
+++ b/libstdc++-v3/testsuite/experimental/net/internet/address/v4/comparisons.cc
@@ -17,6 +17,7 @@ 
 
 // { dg-do run { target c++14 } }
 // { dg-add-options net_ts }
+// { dg-require-gthreads "" }
 
 #include <experimental/internet>
 #include <testsuite_hooks.h>
diff --git a/libstdc++-v3/testsuite/experimental/net/internet/address/v4/cons.cc b/libstdc++-v3/testsuite/experimental/net/internet/address/v4/cons.cc
index 1577480..65f3100 100644
--- a/libstdc++-v3/testsuite/experimental/net/internet/address/v4/cons.cc
+++ b/libstdc++-v3/testsuite/experimental/net/internet/address/v4/cons.cc
@@ -17,6 +17,7 @@ 
 
 // { dg-do run { target c++14 } }
 // { dg-add-options net_ts }
+// { dg-require-gthreads "" }
 
 #include <experimental/internet>
 #include <testsuite_hooks.h>
diff --git a/libstdc++-v3/testsuite/experimental/net/internet/address/v4/creation.cc b/libstdc++-v3/testsuite/experimental/net/internet/address/v4/creation.cc
index 5919845..1a933b9 100644
--- a/libstdc++-v3/testsuite/experimental/net/internet/address/v4/creation.cc
+++ b/libstdc++-v3/testsuite/experimental/net/internet/address/v4/creation.cc
@@ -17,6 +17,7 @@ 
 
 // { dg-do run { target c++14 } }
 // { dg-add-options net_ts }
+// { dg-require-gthreads "" }
 
 #include <experimental/internet>
 #include <testsuite_hooks.h>
diff --git a/libstdc++-v3/testsuite/experimental/net/internet/address/v4/members.cc b/libstdc++-v3/testsuite/experimental/net/internet/address/v4/members.cc
index 37ca8c8..2d71581 100644
--- a/libstdc++-v3/testsuite/experimental/net/internet/address/v4/members.cc
+++ b/libstdc++-v3/testsuite/experimental/net/internet/address/v4/members.cc
@@ -17,6 +17,7 @@ 
 
 // { dg-do run { target c++14 } }
 // { dg-add-options net_ts }
+// { dg-require-gthreads "" }
 
 #include <experimental/internet>
 #include <testsuite_hooks.h>
diff --git a/libstdc++-v3/testsuite/experimental/net/internet/resolver/base.cc b/libstdc++-v3/testsuite/experimental/net/internet/resolver/base.cc
index 746557a..f9bea5f 100644
--- a/libstdc++-v3/testsuite/experimental/net/internet/resolver/base.cc
+++ b/libstdc++-v3/testsuite/experimental/net/internet/resolver/base.cc
@@ -17,6 +17,7 @@ 
 
 // { dg-do run { target c++14 } }
 // { dg-add-options net_ts }
+// { dg-require-gthreads "" }
 
 #include <experimental/internet>
 #include <testsuite_hooks.h>
diff --git a/libstdc++-v3/testsuite/experimental/net/internet/resolver/ops/lookup.cc b/libstdc++-v3/testsuite/experimental/net/internet/resolver/ops/lookup.cc
index 39fb7fd..40cb3db 100644
--- a/libstdc++-v3/testsuite/experimental/net/internet/resolver/ops/lookup.cc
+++ b/libstdc++-v3/testsuite/experimental/net/internet/resolver/ops/lookup.cc
@@ -17,6 +17,7 @@ 
 
 // { dg-do run { target c++14 } }
 // { dg-add-options net_ts }
+// { dg-require-gthreads "" }
 
 #include <experimental/internet>
 #include <testsuite_hooks.h>
diff --git a/libstdc++-v3/testsuite/experimental/net/internet/resolver/ops/reverse.cc b/libstdc++-v3/testsuite/experimental/net/internet/resolver/ops/reverse.cc
index 34ebe5e..831d78c 100644
--- a/libstdc++-v3/testsuite/experimental/net/internet/resolver/ops/reverse.cc
+++ b/libstdc++-v3/testsuite/experimental/net/internet/resolver/ops/reverse.cc
@@ -17,6 +17,7 @@ 
 
 // { dg-do run { target c++14 } }
 // { dg-add-options net_ts }
+// { dg-require-gthreads "" }
 
 #include <experimental/internet>
 #include <testsuite_hooks.h>
diff --git a/libstdc++-v3/testsuite/experimental/net/timer/waitable/cons.cc b/libstdc++-v3/testsuite/experimental/net/timer/waitable/cons.cc
index cd929b3..1f8b99d 100644
--- a/libstdc++-v3/testsuite/experimental/net/timer/waitable/cons.cc
+++ b/libstdc++-v3/testsuite/experimental/net/timer/waitable/cons.cc
@@ -16,6 +16,7 @@ 
 // <http://www.gnu.org/licenses/>.
 
 // { dg-do run { target c++14 } }
+// { dg-require-gthreads "" }
 
 #include <experimental/timer>
 #include <testsuite_hooks.h>
diff --git a/libstdc++-v3/testsuite/experimental/net/timer/waitable/dest.cc b/libstdc++-v3/testsuite/experimental/net/timer/waitable/dest.cc
index dfc3590..6203169 100644
--- a/libstdc++-v3/testsuite/experimental/net/timer/waitable/dest.cc
+++ b/libstdc++-v3/testsuite/experimental/net/timer/waitable/dest.cc
@@ -16,6 +16,7 @@ 
 // <http://www.gnu.org/licenses/>.
 
 // { dg-do run { target c++14 } }
+// { dg-require-gthreads "" }
 
 #include <experimental/timer>
 #include <testsuite_hooks.h>
diff --git a/libstdc++-v3/testsuite/experimental/net/timer/waitable/ops.cc b/libstdc++-v3/testsuite/experimental/net/timer/waitable/ops.cc
index 23c4e34..a31443c 100644
--- a/libstdc++-v3/testsuite/experimental/net/timer/waitable/ops.cc
+++ b/libstdc++-v3/testsuite/experimental/net/timer/waitable/ops.cc
@@ -16,6 +16,7 @@ 
 // <http://www.gnu.org/licenses/>.
 
 // { dg-do run { target c++14 } }
+// { dg-require-gthreads "" }
 
 #include <experimental/timer>
 #include <testsuite_hooks.h>