libstdc++: Fix testsuite for skipping gdb tests on remote/non-native target

libstdc++: Fix testsuite for skipping gdb tests on remote/non-native target

[Marc Poulhies]

[-- Attachment #1: Type: text/plain, Size: 357 bytes --]

This fixes an incorrect invocation of gdb on remote targets where DejaGNU would try to run host's gdb in remote target simulator.
gdb-test skips the testing when target is remote or non native but the gdb version check function does not.

libstdc++-v3/ChangeLog:
        * testsuite/lib/gdb-test.exp (gdb_batch_check): Exit if non native or remote target.


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3724 bytes --]

commit 0c4ae4ff46b1d7633f1e06f57d348b5817b8f640
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jul 20 12:35:37 2021

    libstdc++: Add more tests for filesystem::create_directory [PR101510]
    
    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/101510
            * src/c++17/fs_ops.cc (create_dir): Adjust whitespace.
            * testsuite/27_io/filesystem/operations/create_directory.cc:
            Test creating directory with name of existing symlink to
            directory.
            * testsuite/experimental/filesystem/operations/create_directory.cc:
            Likewise.

diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
index 66207ae5e44..cec76446f06 100644
--- a/libstdc++-v3/src/c++17/fs_ops.cc
+++ b/libstdc++-v3/src/c++17/fs_ops.cc
@@ -577,8 +577,7 @@ namespace
   {
     bool created = false;
 #ifdef _GLIBCXX_HAVE_SYS_STAT_H
-    posix::mode_t mode
-      = static_cast<std::underlying_type_t<fs::perms>>(perm);
+    posix::mode_t mode = static_cast<std::underlying_type_t<fs::perms>>(perm);
     if (posix::mkdir(p.c_str(), mode))
       {
 	const int err = errno;
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/create_directory.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/create_directory.cc
index a0e50471275..256621481d7 100644
--- a/libstdc++-v3/testsuite/27_io/filesystem/operations/create_directory.cc
+++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/create_directory.cc
@@ -54,6 +54,33 @@ test01()
   b = create_directory(p);
   VERIFY( !b );
 
+  auto f = p/"file";
+  std::ofstream{f} << "create file";
+  b = create_directory(f, ec);
+  VERIFY( ec == std::errc::file_exists );
+  VERIFY( !b );
+  try
+  {
+    create_directory(f);
+    VERIFY( false );
+  }
+  catch (const fs::filesystem_error& e)
+  {
+    VERIFY( e.code() == std::errc::file_exists );
+    VERIFY( e.path1() == f );
+  }
+
+  // PR libstdc++/101510 create_directory on an existing symlink to a directory
+  fs::create_directory(p/"dir");
+  auto link = p/"link";
+  fs::create_directory_symlink("dir", link);
+  ec = bad_ec;
+  b = fs::create_directory(link, ec);
+  VERIFY( !b );
+  VERIFY( !ec );
+  b = fs::create_directory(link);
+  VERIFY( !b );
+
   remove_all(p, ec);
 }
 
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/create_directory.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/create_directory.cc
index ee2a74b8803..39f95b61a45 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/operations/create_directory.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/create_directory.cc
@@ -46,12 +46,40 @@ test01()
   VERIFY( exists(p) );
 
   // Test existing path (libstdc++/71036).
+  ec = make_error_code(std::errc::invalid_argument);
   b = create_directory(p, ec);
   VERIFY( !ec );
   VERIFY( !b );
   b = create_directory(p);
   VERIFY( !b );
 
+  auto f = p/"file";
+  std::ofstream{f} << "create file";
+  b = create_directory(f, ec);
+  VERIFY( ec == std::errc::file_exists );
+  VERIFY( !b );
+  try
+  {
+    create_directory(f);
+    VERIFY( false );
+  }
+  catch (const fs::filesystem_error& e)
+  {
+    VERIFY( e.code() == std::errc::file_exists );
+    VERIFY( e.path1() == f );
+  }
+
+  // PR libstdc++/101510 create_directory on an existing symlink to a directory
+  fs::create_directory(p/"dir");
+  auto link = p/"link";
+  fs::create_directory_symlink("dir", link);
+  ec = make_error_code(std::errc::invalid_argument);
+  b = fs::create_directory(link, ec);
+  VERIFY( !b );
+  VERIFY( !ec );
+  b = fs::create_directory(link);
+  VERIFY( !b );
+
   remove_all(p, ec);
 }
 

Re: [NEWS] libstdc++: Fix testsuite for skipping gdb tests on remote/non-native target

[Marc Poulhies]

[-- Attachment #1: Type: text/plain, Size: 797 bytes --]

With the correct patch attached, sorry for the incorrect previous one !

Marc

----- Original Message -----
> From: "gcc-patches" <gcc-patches@gcc.gnu.org>
> To: "gcc-patches" <gcc-patches@gcc.gnu.org>, "libstdc++" <libstdc++@gcc.gnu.org>
> Cc: "Luc Michel" <lmichel@kalray.eu>
> Sent: Tuesday, July 20, 2021 4:12:16 PM
> Subject: [NEWS]  libstdc++: Fix testsuite for skipping gdb tests on remote/non-native target

> This fixes an incorrect invocation of gdb on remote targets where DejaGNU would
> try to run host's gdb in remote target simulator.
> gdb-test skips the testing when target is remote or non native but the gdb
> version check function does not.
> 
> libstdc++-v3/ChangeLog:
>        * testsuite/lib/gdb-test.exp (gdb_batch_check): Exit if non native or remote
>         target.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch; name=patch.txt, Size: 608 bytes --]

diff --git a/libstdc++-v3/testsuite/lib/gdb-test.exp b/libstdc++-v3/testsuite/lib/gdb-test.exp
index af20c85e5a0..0ec9ac46c68 100644
--- a/libstdc++-v3/testsuite/lib/gdb-test.exp
+++ b/libstdc++-v3/testsuite/lib/gdb-test.exp
@@ -244,6 +244,8 @@ proc gdb-test { marker {selector {}} {load_xmethods 0} } {
 
 # Invoke gdb with a command and pattern-match the output.
 proc gdb_batch_check {command pattern} {
+    if { ![isnative] || [is_remote target] } { return 0 }
+
     set gdb_name $::env(GUALITY_GDB_NAME)
     set cmd "$gdb_name -nw -nx -quiet -batch -ex \"$command\""
     send_log "Spawning: $cmd\n"

Re: [NEWS] libstdc++: Fix testsuite for skipping gdb tests on remote/non-native target

[Jonathan Wakely]

On Wed, 21 Jul 2021 at 16:02, Marc Poulhies via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> With the correct patch attached, sorry for the incorrect previous one !

Thanks for the patch. I agree we should skip the version checks, not
only the actual tests. But I wonder whether we want to do that in
xmethods.exp and prettyprinters.exp rather than in the gdb_batch_check
proc. Or maybe like this instead:

--- a/libstdc++-v3/testsuite/lib/gdb-test.exp
+++ b/libstdc++-v3/testsuite/lib/gdb-test.exp
@@ -280,6 +280,8 @@ proc gdb_batch_check {command pattern} {
# but not earlier versions.
# Return 1 if the version is ok, 0 otherwise.
proc gdb_version_check {} {
+    if { ![isnative] || [is_remote target] } { return 0 }
+
    return [gdb_batch_check "python print(gdb.lookup_global_symbol)" \
             "<built-in function lookup_global_symbol>"]
}
@@ -288,6 +290,8 @@ proc gdb_version_check {} {
# in a manner similar to the check for a version of gdb which supports the
# pretty-printer tests below.
proc gdb_version_check_xmethods {} {
+    if { ![isnative] || [is_remote target] } { return 0 }
+
    return [gdb_batch_check \
             "python import gdb.xmethod; print(gdb.xmethod.XMethod)" \
             "<class 'gdb\\.xmethod\\.XMethod'>"]



I don't think it really makes much difference, I'm just unsure what is
"cleaner" and more consistent with DG conventions and/or the rest of
the gdb-test.exp file.

Re: [NEWS] libstdc++: Fix testsuite for skipping gdb tests on remote/non-native target

[Marc Poulhies]

----- Original Message -----
> From: "Jonathan Wakely" <jwakely@redhat.com>
> To: "Marc Poulhies" <mpoulhies@kalrayinc.com>
> Cc: "libstdc++" <libstdc++@gcc.gnu.org>, "gcc-patches" <gcc-patches@gcc.gnu.org>, "Luc Michel" <lmichel@kalray.eu>
> Sent: Wednesday, July 21, 2021 5:53:52 PM
> Subject: Re: [NEWS] libstdc++: Fix testsuite for skipping gdb tests on remote/non-native target

> Thanks for the patch. I agree we should skip the version checks, not
> only the actual tests. But I wonder whether we want to do that in
> xmethods.exp and prettyprinters.exp rather than in the gdb_batch_check
> proc. Or maybe like this instead:
> 
> I don't think it really makes much difference, I'm just unsure what is
> "cleaner" and more consistent with DG conventions and/or the rest of
> the gdb-test.exp file.

Here are a bit more information on the issue we are having.
While trying to understand why the testsuite is taking a long time to execute, we (Luc in Cc) discovered that the logs contain:

Spawning: gdb -nw -nx -quiet -batch -ex "python print(gdb.lookup_global_symbol)"
spawn -ignore SIGHUP kvx-mppa --unnamed-log --bootcluster=node0 --no-trap --no-gdb-attach --dcache-no-check -- gdb -nw -nx -quiet -batch -ex python print(gdb.lookup_global_symbol)
UNSUPPORTED: prettyprinters.exp
testcase /work1/mpoulhies/tools-csw/gcc/libstdc++-v3/testsuite/libstdc++-prettyprinters/prettyprinters.exp completed in 613 seconds

kvx-mppa being a simulator and gdb the host (x86) binary.
The strangest part being that running the command by hand will fail immediately, but when DG is running the tests, it waits until the timeout is reached: we don't understand this behavior, but we get several <defunct> processes probably waiting to be joined..

I don't have a strong opinion here as I'm really no DG expert. I find the filtering in gdb-test maybe more robust as it prevents any error like the above. Having the test in prettyprinters/xmethod allows for quicker exit (saves 15s here), but I don't see that as a strong argument.

Thanks for the review,

Marc