[PATCH v3 1/2] Add test to check GDB handles dlopen of /proc/self/fd/[num] correctly

[PATCH v3 1/2] Add test to check GDB handles dlopen of /proc/self/fd/[num] correctly

[Asaf Fisher]

---
 gdb/testsuite/gdb.base/solib-proc-self.cc  | 72 ++++++++++++++++++
 gdb/testsuite/gdb.base/solib-proc-self.exp | 86 ++++++++++++++++++++++
 2 files changed, 158 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/solib-proc-self.cc
 create mode 100644 gdb/testsuite/gdb.base/solib-proc-self.exp

diff --git a/gdb/testsuite/gdb.base/solib-proc-self.cc b/gdb/testsuite/gdb.base/solib-proc-self.cc
new file mode 100644
index 00000000000..dc0b446d53c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-proc-self.cc
@@ -0,0 +1,72 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2007-2022 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <sys/mman.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <iostream>
+#include <fstream>
+#include <sstream>
+#include <vector>
+#include <unistd.h>
+
+#ifdef __WIN32__
+#include <windows.h>
+#define dlopen(name, mode) LoadLibrary (name)
+#define dlclose(handle) FreeLibrary (handle)
+#define dlerror() "an error occurred"
+#else
+#include <dlfcn.h>
+#endif
+
+int main()
+{
+  void *handle;
+  /* Read the so's content to a buffer */
+  std::ifstream read_so_file = std::ifstream(SHLIB_NAME);
+  read_so_file.seekg(0, std::ios::end);
+  std::streamsize size = read_so_file.tellg();
+  read_so_file.seekg(0, std::ios::beg);
+  std::vector<char> buffer(size);
+  if (!read_so_file.read(buffer.data(), size))
+  {
+    fprintf (stderr, "Failed to load solib\n");
+    exit(1);
+  }
+
+  int mem_fd = memfd_create("test", 0);
+
+  /* Write the so's data to the memory mapped file. */
+  write(mem_fd, buffer.data(), buffer.size());
+
+  /* Generate the /proc/self/fd/[num] path */
+  std::string prof_self_fd_path; /* break-here */
+  std::stringstream prof_self_fd_path_stream = std::stringstream(prof_self_fd_path);
+  prof_self_fd_path_stream << "/proc/self/fd/" << mem_fd;
+
+  /* Call dlopen on it */
+  handle = dlopen (prof_self_fd_path_stream.str().c_str(), RTLD_LAZY);
+  if (!handle)
+  {
+      fprintf (stderr, "%s\n", dlerror ());
+      exit (1);
+  }
+  /* YAY it worked */
+  dlclose (handle);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/solib-proc-self.exp b/gdb/testsuite/gdb.base/solib-proc-self.exp
new file mode 100644
index 00000000000..b59ba357492
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-proc-self.exp
@@ -0,0 +1,86 @@
+# Copyright 2007-2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# Test connecting and disconnecting at shared library events.
+
+if {[skip_shlib_tests]} {
+    untested "could not run to main"
+    return 0
+}
+
+standard_testfile .cc
+
+# Chose random lib
+set libfile so-disc-shr
+set libsrc "${srcdir}/${subdir}/${libfile}.c"
+set libname "${libfile}.so"
+set libobj [standard_output_file ${libname}]
+
+# Compile the shared lib
+if { [gdb_compile_shlib $libsrc $libobj {debug}] != ""} {
+    return -1
+}
+
+# Compile test
+if [ prepare_for_testing "failed to prepare" $testfile $srcfile "list shlib_load debug c++ additional_flags=-DSHLIB_NAME=\"${libobj}\"" ] {
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+gdb_load_shlib $libobj
+
+if ![runto_main] then {
+    return 0
+}
+
+# Get inferior's PID for later
+set inferior_pid -1
+gdb_test_multiple "info inferior 1" "get inferior pid" {
+    -re "process (\[0-9\]*).*$gdb_prompt $" {
+	set inferior_pid $expect_out(1,string)
+	pass $gdb_test_name
+    }
+}
+
+# Turn on the solib-events so we can see that gdb resolves everything correctly
+gdb_test_no_output "set stop-on-solib-events 1"
+
+# I use this breakpoint to get the memory mapped fd.
+gdb_breakpoint [gdb_get_line_number "break-here"]
+gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+
+set msg "Getting MEMFD"
+set memfd ""
+gdb_test_multiple "p mem_fd" $msg {
+    -re "\\\$$decimal = (\[^\r\n\]*)\r\n$gdb_prompt $" {
+	set memfd $expect_out(1,string)
+	pass $msg
+    }
+}
+
+gdb_test "continue" "Stopped due to shared library event.*" "continue to load"
+
+# Check if inferior resolved the /proc/self/fd/[num] to /proc/[pid]/fd/[num]
+set msg "Inferior's /proc/self resolving $inferior_pid $memfd"
+set inferior_proc_self_path ""
+gdb_test_multiple "continue" $msg {
+    -re "Attempting to replace `self` with inferior's PID. -> (\/proc\/$inferior_pid\/fd\/$memfd\[^\r\n\]*)\r\n.*$gdb_prompt $" {
+	set inferior_proc_self_path $expect_out(1,string)
+	pass $msg
+    }
+}
-- 
2.38.0

[PATCH v3 2/2] Make GDB resolve dlopen of memory mapped shared libraries

[Asaf Fisher]

Introduced `check_proc_self_file` that checks if a path used by
inferior in dlopen is in the form of `/proc/self/...` and if so resolves
it to `/proc/[pid]/...`

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29586
---
 gdb/solib-svr4.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 57 insertions(+), 2 deletions(-)

diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 6acaf87960b..02bd89ef9d6 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -34,6 +34,7 @@
 #include "regcache.h"
 #include "gdbthread.h"
 #include "observable.h"
+#include "gdbsupport/pathstuff.h"
 
 #include "solist.h"
 #include "solib.h"
@@ -48,6 +49,9 @@
 
 #include <map>
 
+#define SLASH_SELF "/self"
+#define PROC_SELF  "/proc" SLASH_SELF
+
 static struct link_map_offsets *svr4_fetch_link_map_offsets (void);
 static int svr4_have_link_map_offsets (void);
 static void svr4_relocate_main_executable (void);
@@ -1259,6 +1263,55 @@ svr4_default_sos (svr4_info *info)
   return newobj;
 }
 
+/* Check and fix a cenerio where the so path that we extract has a path to
+  /proc/self e.g. /proc/self/fd/[fd_num] If inferior dlopen a path that has
+  /proc/self, GDB must not open it directly becuase the files in /proc/self are
+  unique for each process. Instead we resolve /proc/self to
+  /proc/[inferior_pid]. This change will give GDB the correct path */
+
+static size_t
+check_proc_self_file(char *so_name, char *normalized_so_name,
+                                   size_t out_normalized_so_name_len) {
+  /* We dont want a path with /../ yak. */
+  gdb::unique_xmalloc_ptr<char> normalized_path_obj = gdb_realpath(so_name);
+  gdb::string_view normalized_path = gdb::string_view(
+      normalized_path_obj.get(),
+      std::min(strlen(normalized_path_obj.get()), out_normalized_so_name_len));
+
+  /* Is the path really a /proc/self? */
+  if (0 != normalized_path.rfind(PROC_SELF, 0)) return 0;
+
+  /* Lets get the part of the path after /proc/self e.g. /proc/self/fd -> /fd */
+  size_t slash_self_index = normalized_path.rfind(SLASH_SELF);
+  if (std::string::npos == slash_self_index) return 0;
+  size_t after_self_index = slash_self_index + strlen(SLASH_SELF);
+  gdb::string_view after_self_path = normalized_path.substr(after_self_index);
+
+  /* Get inferior path */
+  int inferior_pid = inferior_ptid.pid();
+  std::string inferior_procfs_path = string_printf("/proc/%d", inferior_pid);
+
+  /* Check if there's enoght space in the out buffer for the normalized path. */
+  size_t normalized_so_name_length =
+      inferior_procfs_path.length() + after_self_path.length();
+  if (out_normalized_so_name_len < normalized_so_name_length) return 0;
+
+  /* Build the full path */
+  inferior_procfs_path.append(std::string(after_self_path));
+
+  warning(_("Detected loaded library (%s) from /proc/self.\nAttempting to "
+            "replace `self` with inferior's PID. -> %s"),
+          normalized_path.begin(), inferior_procfs_path.c_str());
+
+  auto out_length =
+      std::min(inferior_procfs_path.length(), out_normalized_so_name_len);
+
+  /* Copy the new path to the out buffer */
+  strncpy(normalized_so_name, inferior_procfs_path.c_str(), out_length);
+
+  return out_length;
+}
+
 /* Read the whole inferior libraries chain starting at address LM.
    Expect the first entry in the chain's previous entry to be PREV_LM.
    Add the entries to the tail referenced by LINK_PTR_PTR.  Ignore the
@@ -1318,8 +1371,10 @@ svr4_read_so_list (svr4_info *info, CORE_ADDR lm, CORE_ADDR prev_lm,
 	    warning (_("Can't read pathname for load map."));
 	  continue;
 	}
-
-      strncpy (newobj->so_name, buffer.get (), SO_NAME_MAX_PATH_SIZE - 1);
+      /* Check if path is in /proc/self */
+      if (0 == check_proc_self_file(buffer.get(), newobj->so_name,
+                              SO_NAME_MAX_PATH_SIZE - 1))
+        strncpy(newobj->so_name, buffer.get(), SO_NAME_MAX_PATH_SIZE - 1);
       newobj->so_name[SO_NAME_MAX_PATH_SIZE - 1] = '\0';
       strcpy (newobj->so_original_name, newobj->so_name);
 
-- 
2.38.0

Re: [PATCH v3 1/2] Add test to check GDB handles dlopen of /proc/self/fd/[num] correctly

[Andrew Burgess]

Asaf Fisher via Gdb-patches <gdb-patches@sourceware.org> writes:

> ---
>  gdb/testsuite/gdb.base/solib-proc-self.cc  | 72 ++++++++++++++++++
>  gdb/testsuite/gdb.base/solib-proc-self.exp | 86 ++++++++++++++++++++++
>  2 files changed, 158 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.base/solib-proc-self.cc
>  create mode 100644 gdb/testsuite/gdb.base/solib-proc-self.exp

Hi!

Thanks for working on this fix.  If you have not contributed to GDB
before then you might want to check out:

  https://sourceware.org/gdb/wiki/ContributionChecklist

I think, given the size of these patches, you would need to complete a
copyright assigment before these patches could be merged.  Details of
the copyright assignment can be found here:

  https://sourceware.org/gdb/wiki/ContributionChecklist#FSF_copyright_Assignment

The best choice for copyright assignment would be this starting point:

  http://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future

which would assign copyright for past and future contributions to GDB
over to the FSF.

Thanks,
Andrew

>
> diff --git a/gdb/testsuite/gdb.base/solib-proc-self.cc b/gdb/testsuite/gdb.base/solib-proc-self.cc
> new file mode 100644
> index 00000000000..dc0b446d53c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/solib-proc-self.cc
> @@ -0,0 +1,72 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2007-2022 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <sys/mman.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <iostream>
> +#include <fstream>
> +#include <sstream>
> +#include <vector>
> +#include <unistd.h>
> +
> +#ifdef __WIN32__
> +#include <windows.h>
> +#define dlopen(name, mode) LoadLibrary (name)
> +#define dlclose(handle) FreeLibrary (handle)
> +#define dlerror() "an error occurred"
> +#else
> +#include <dlfcn.h>
> +#endif
> +
> +int main()
> +{
> +  void *handle;
> +  /* Read the so's content to a buffer */
> +  std::ifstream read_so_file = std::ifstream(SHLIB_NAME);
> +  read_so_file.seekg(0, std::ios::end);
> +  std::streamsize size = read_so_file.tellg();
> +  read_so_file.seekg(0, std::ios::beg);
> +  std::vector<char> buffer(size);
> +  if (!read_so_file.read(buffer.data(), size))
> +  {
> +    fprintf (stderr, "Failed to load solib\n");
> +    exit(1);
> +  }
> +
> +  int mem_fd = memfd_create("test", 0);
> +
> +  /* Write the so's data to the memory mapped file. */
> +  write(mem_fd, buffer.data(), buffer.size());
> +
> +  /* Generate the /proc/self/fd/[num] path */
> +  std::string prof_self_fd_path; /* break-here */
> +  std::stringstream prof_self_fd_path_stream = std::stringstream(prof_self_fd_path);
> +  prof_self_fd_path_stream << "/proc/self/fd/" << mem_fd;
> +
> +  /* Call dlopen on it */
> +  handle = dlopen (prof_self_fd_path_stream.str().c_str(), RTLD_LAZY);
> +  if (!handle)
> +  {
> +      fprintf (stderr, "%s\n", dlerror ());
> +      exit (1);
> +  }
> +  /* YAY it worked */
> +  dlclose (handle);
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/solib-proc-self.exp b/gdb/testsuite/gdb.base/solib-proc-self.exp
> new file mode 100644
> index 00000000000..b59ba357492
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/solib-proc-self.exp
> @@ -0,0 +1,86 @@
> +# Copyright 2007-2022 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +# Test connecting and disconnecting at shared library events.
> +
> +if {[skip_shlib_tests]} {
> +    untested "could not run to main"
> +    return 0
> +}
> +
> +standard_testfile .cc
> +
> +# Chose random lib
> +set libfile so-disc-shr
> +set libsrc "${srcdir}/${subdir}/${libfile}.c"
> +set libname "${libfile}.so"
> +set libobj [standard_output_file ${libname}]
> +
> +# Compile the shared lib
> +if { [gdb_compile_shlib $libsrc $libobj {debug}] != ""} {
> +    return -1
> +}
> +
> +# Compile test
> +if [ prepare_for_testing "failed to prepare" $testfile $srcfile "list shlib_load debug c++ additional_flags=-DSHLIB_NAME=\"${libobj}\"" ] {
> +    return -1
> +}
> +
> +gdb_exit
> +gdb_start
> +gdb_reinitialize_dir $srcdir/$subdir
> +gdb_load ${binfile}
> +gdb_load_shlib $libobj
> +
> +if ![runto_main] then {
> +    return 0
> +}
> +
> +# Get inferior's PID for later
> +set inferior_pid -1
> +gdb_test_multiple "info inferior 1" "get inferior pid" {
> +    -re "process (\[0-9\]*).*$gdb_prompt $" {
> +	set inferior_pid $expect_out(1,string)
> +	pass $gdb_test_name
> +    }
> +}
> +
> +# Turn on the solib-events so we can see that gdb resolves everything correctly
> +gdb_test_no_output "set stop-on-solib-events 1"
> +
> +# I use this breakpoint to get the memory mapped fd.
> +gdb_breakpoint [gdb_get_line_number "break-here"]
> +gdb_continue_to_breakpoint "break-here" ".* break-here .*"
> +
> +set msg "Getting MEMFD"
> +set memfd ""
> +gdb_test_multiple "p mem_fd" $msg {
> +    -re "\\\$$decimal = (\[^\r\n\]*)\r\n$gdb_prompt $" {
> +	set memfd $expect_out(1,string)
> +	pass $msg
> +    }
> +}
> +
> +gdb_test "continue" "Stopped due to shared library event.*" "continue to load"
> +
> +# Check if inferior resolved the /proc/self/fd/[num] to /proc/[pid]/fd/[num]
> +set msg "Inferior's /proc/self resolving $inferior_pid $memfd"
> +set inferior_proc_self_path ""
> +gdb_test_multiple "continue" $msg {
> +    -re "Attempting to replace `self` with inferior's PID. -> (\/proc\/$inferior_pid\/fd\/$memfd\[^\r\n\]*)\r\n.*$gdb_prompt $" {
> +	set inferior_proc_self_path $expect_out(1,string)
> +	pass $msg
> +    }
> +}
> -- 
> 2.38.0

Re: [PATCH v3 2/2] Make GDB resolve dlopen of memory mapped shared libraries

[Andrew Burgess]

Asaf Fisher via Gdb-patches <gdb-patches@sourceware.org> writes:

> Introduced `check_proc_self_file` that checks if a path used by
> inferior in dlopen is in the form of `/proc/self/...` and if so resolves
> it to `/proc/[pid]/...`
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29586

Hi Asaf,

Thanks for providing a fix for this, and thanks for submitting the
copyright assignment paperwork.

I took a look through your patch and ended up tweaking it a little.
I've attached a revised version below, which I'd love to hear your
feedback on.

When making changes my goal was to extend your fix to work with
gdbserver just like it works with native targets.  You can test
different gdbserver setups like this:

  make check-gdb TESTS="gdb.base/solib-proc-self.exp"  \
                 RUNTESTFLAGS="--target_board=native-extended-gdbserver"

This will setup gdbserver and connect GDB to it with 'target
extended-remote', you can also test using the 'native-gdbserver' board,
which will connect to gdbserver as just 'target remote'.

Ideally any new test will pass in all three configurations (the default
plus the two listed above), and initially, your change only fixed the
native target case.  By moving the fix elsewhere in the shared library
loading process I think I new have all three cases working.

I also extended the test to test using a 'target:' sysroot.  By default
when testing with the two gdbserver boards above, we set the sysroot to
"" (the empty string), this tells GDB that remote files can be found on
the local machine, and avoids all files accesses having to go over the
remote protocol.  However, given the nature of this change, I figured it
was worth testing with the 'target:' sysroot too, this means we re-run
the tests bu sending all file accesses over the remote protocol.  That
case is also fixed with the patch below.

Where I'd most appreciate your feedback is for the algorithm by which
the /proc/self path is spotted and converted to /proc/PID.  I've cut the
code back a bit from what you had originally, mostly because I couldn't
find a way to test that the extra complexity was required.  If you have
any additional test cases that could show that the slimmed down code is
not good enough, then that would be great.

Thanks,
Andrew

---

commit 0bf4c98bc225c89a0a1ddcea727eca178ad69710
Author: Asaf Fisher <asaffisher.dev@gmail.com>
Date:   Fri Oct 21 17:42:05 2022 +0000

    gdb: handle loading shared libraries from /proc/self/fd/
    
    Bug PR gdb/29586 describes a situations where a shared library is
    created in memory, then written to a memory mapped file.  The memory
    mapped file will show up as a file descriptor within /proc/self/fd/,
    and this path is then used with dlopen in order to call functions
    within the in-memory shared library.
    
    When attempting to debug this GDB hangs.  The problem is that, GDB
    stops at the shared-library event, reads the shared library path from
    the inferior, which is "/proc/self/fd/<NUM>", and then GDB attempts to
    open this file.
    
    Unfortunately, this means GDB tries to open a file within GDB's
    /proc/self/fd/ directory, not within the inferior's directory.  In the
    case of our hang it turns out that the file descriptor that is opened
    is a pipe, and GDB hangs trying to read from the pipe.
    
    However, the behaviour is really just undefined, depending on which
    file descriptor the inferior tries to open, GDB will open, or fail to
    open, random files within its /proc/self/fd directory.
    
    The solution proposed in this commit is to hook into solib_find_1, and
    spot when GDB is looking for any file in /proc/self/, if this is the
    case, then the filename is rewritten as /proc/<PID>, where <PID> is
    the process-id of the current inferior.
    
    The test passes for the unix, native-gdbserver, and
    native-extended-gdbserver targets.  When testing with either of the
    gdbserver targets, the test is run using the default empty sysroot,
    and also using the 'target:' sysroot.
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29586
    
    Co-authored-by: Andrew Burgess <aburgess@redhat.com>

diff --git a/gdb/solib.c b/gdb/solib.c
index 7cfdd81114c..cf2d0d3bc3a 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -83,6 +83,35 @@ show_solib_search_path (struct ui_file *file, int from_tty,
 #  define DOS_BASED_FILE_SYSTEM 0
 #endif
 
+/* Fix references to files in /proc/self/fd/ when opening a shared library.
+
+   SO_NAME is the name of the shared library being loaded.  This function
+   returns a possibly modified name which should be used as the path to the
+   shared library.
+
+   If SO_NAME starts with /proc/self, then the returned name will be
+   modified to start with /proc/PID where 'PID' is the pid of the current
+   inferior.  */
+
+static gdb::unique_xmalloc_ptr<char>
+filter_proc_self_filenames (gdb::unique_xmalloc_ptr<char> so_name)
+{
+  static const char *proc_self_prefix = "/proc/self";
+
+  /* Is the path really a /proc/self?  */
+  if (!startswith (so_name.get (), proc_self_prefix))
+    return so_name;
+
+  /* Get the part of the path after /proc/self.  For example given
+     '/proc/self/fd' we find the '/fd' part.  */
+  gdb_assert (strlen (so_name.get ()) >= strlen (proc_self_prefix));
+  const char *tail = so_name.get () + strlen (proc_self_prefix);
+
+  /* Build a replacement path.  */
+  int inferior_pid = inferior_ptid.pid ();
+  return xstrprintf ("/proc/%d%s", inferior_pid, tail);
+}
+
 /* Return the full pathname of a binary file (the main executable or a
    shared library file), or NULL if not found.  If FD is non-NULL, *FD
    is set to either -1 or an open file handle for the binary file.
@@ -172,6 +201,12 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
 	}
     }
 
+  /* If the path starts /proc/self then rewrite this as /proc/PID using the
+     current inferior's pid.  Failing to do this will cause GDB to try and
+     open files within its proc directory, rather than the inferiors.  */
+  temp_pathname.reset (xstrdup (in_pathname));
+  temp_pathname = filter_proc_self_filenames (std::move (temp_pathname));
+
   /* Note, we're interested in IS_TARGET_ABSOLUTE_PATH, not
      IS_ABSOLUTE_PATH.  The latter is for host paths only, while
      IN_PATHNAME is a target path.  For example, if we're supposed to
@@ -184,9 +219,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
        3rd attempt, c:/foo/bar.dll ==> /sysroot/foo/bar.dll
   */
 
-  if (!IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) || sysroot == NULL)
-    temp_pathname.reset (xstrdup (in_pathname));
-  else
+  if (IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) && sysroot != NULL)
     {
       bool need_dir_separator;
 
@@ -213,7 +246,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
       /* Cat the prefixed pathname together.  */
       temp_pathname.reset (concat (sysroot,
 				   need_dir_separator ? SLASH_STRING : "",
-				   in_pathname, (char *) NULL));
+				   temp_pathname.get (), (char *) NULL));
     }
 
   /* Handle files to be accessed via the target.  */
diff --git a/gdb/testsuite/gdb.base/solib-proc-self.cc b/gdb/testsuite/gdb.base/solib-proc-self.cc
new file mode 100644
index 00000000000..f2439d738a3
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-proc-self.cc
@@ -0,0 +1,73 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2022 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <sys/mman.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <iostream>
+#include <fstream>
+#include <sstream>
+#include <vector>
+#include <unistd.h>
+
+#ifdef __WIN32__
+#include <windows.h>
+#define dlopen(name, mode) LoadLibrary (name)
+#define dlclose(handle) FreeLibrary (handle)
+#define dlerror() "an error occurred"
+#else
+#include <dlfcn.h>
+#endif
+
+int
+main ()
+{
+  /* Read the shared libraries contents into a buffer.  */
+  std::ifstream read_so_file = std::ifstream (SHLIB_NAME);
+  read_so_file.seekg (0, std::ios::end);
+  std::streamsize size = read_so_file.tellg ();
+  read_so_file.seekg (0, std::ios::beg);
+  std::vector<char> buffer (size);
+  if (!read_so_file.read (buffer.data (), size))
+    {
+      fprintf (stderr, "Failed to load solib\n");
+      exit (1);
+    }
+
+  /* Create a memory mapped file, then write the shared library to that
+     new memory mapped file.  */
+  int mem_fd = memfd_create ("test", 0);
+  write (mem_fd, buffer.data (), buffer.size ());
+
+  /* Generate the /proc/self/fd/[num] path for the memory mapped file.  */
+  std::string proc_self_fd_path;		/* break-here */
+  std::stringstream proc_self_fd_path_stream
+    = std::stringstream (proc_self_fd_path);
+  proc_self_fd_path_stream << "/proc/self/fd/" << mem_fd;
+
+  /* Call dlopen on it.  */
+  void *handle = dlopen (proc_self_fd_path_stream.str ().c_str (), RTLD_LAZY);
+  if (!handle)
+    {
+      fprintf (stderr, "%s\n", dlerror ());
+      exit (1);
+    }
+  /* It worked.  */
+  dlclose (handle);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/solib-proc-self.exp b/gdb/testsuite/gdb.base/solib-proc-self.exp
new file mode 100644
index 00000000000..1c845822490
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-proc-self.exp
@@ -0,0 +1,130 @@
+# Copyright 2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# Test connecting and disconnecting at shared library events.
+
+if {[skip_shlib_tests]} {
+    untested "could not run to main"
+    return 0
+}
+
+standard_testfile .cc
+
+# Reuse an existing library, we don't care about the library contents
+# for this test.
+set libfile so-disc-shr
+set libsrc "${srcdir}/${subdir}/${libfile}.c"
+set libname "${libfile}.so"
+set libobj [standard_output_file ${libname}]
+
+# Compile the shared library.
+if { [gdb_compile_shlib $libsrc $libobj {debug}] != ""} {
+    return -1
+}
+
+# Compile the test executable.
+if [ build_executable "failed to prepare" $testfile $srcfile \
+	 [list shlib_load debug c++ additional_flags=-DSHLIB_NAME="${libobj}"]] {
+    return -1
+}
+
+# Start GDB and run to the point where the test program tries to dlopen a file
+# from within /proc/self/fd/.  Catch the shared library event and check that
+# we actually try to load a file from /proc/<INFERIOR-PID>/fd/.
+#
+# If SYSROOT is not the empty string, then this is set as the value of GDB's
+# sysroot immediately after starting GDB.  The only value that is (currently)
+# supported, other than the empty string, is 'target:'.
+proc do_test { {sysroot ""} } {
+    clean_restart $::binfile
+
+    if {$sysroot != ""} {
+	gdb_test_no_output "set sysroot ${sysroot}"
+    }
+
+    gdb_load_shlib $::libobj
+
+    if ![runto_main] then {
+	return 0
+    }
+
+    # Get inferior's PID for later.
+    set inferior_pid [get_inferior_pid]
+
+    # Turn on the solib-events so we can see that gdb resolves everything
+    # correctly.
+    gdb_test_no_output "set stop-on-solib-events 1"
+
+    # Run to the 'break-here' marker.
+    gdb_breakpoint [gdb_get_line_number "break-here"]
+    gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+
+    set memfd ""
+    gdb_test_multiple "p mem_fd" "Get file descriptor" {
+	-re -wrap "\\\$$::decimal = (\[^\r\n\]*)" {
+	    set memfd $expect_out(1,string)
+	    pass $gdb_test_name
+	}
+    }
+
+    gdb_test "continue" "Stopped due to shared library event.*" \
+	"continue to load"
+
+    # Check if inferior resolved the /proc/self/fd/[num] to /proc/[pid]/fd/[num].
+    gdb_test "continue" \
+	[multi_line \
+	     "Stopped due to shared library event:" \
+	     "  Inferior loaded (?:target:)?/proc/${inferior_pid}/fd/$memfd"]
+}
+
+# First run of the test.
+do_test
+
+# Possible second run of the test.  If we are using a remote target then we
+# should consider setting the sysroot to 'target:' and re-running the test.
+if {[target_info exists gdb_protocol]
+    && ([target_info gdb_protocol] == "remote"
+	|| [target_info gdb_protocol] == "extended-remote")} {
+    # GDB will already be running after the first call to do_test, so we can
+    # take a peek at the current sysroot setting, and decide if we should
+    # repeat the test with a different setting.
+
+    set new_sysroot ""
+    gdb_test_multiple "show sysroot" "" {
+	-wrap -re "The current system root is \"\"\\." {
+	    pass $gdb_test_name
+
+	    # Repeat the test with 'target:' sysroot.
+	    set new_sysroot "target:"
+	}
+	-wrap -re "The current system root is \"target:\"\\." {
+	    pass $gdb_test_name
+
+	    # Nothing else to do, we already tested with target: sysroot.
+	}
+	-re "$gdb_prompt $" {
+	    pass $gdb_test_name
+
+	    # If already testing with any other sysroot, we probably should
+	    # not try to adjust things, so don't do any further testing.
+	}
+    }
+
+    with_test_prefix "sysroot $new_sysroot" {
+	if { $new_sysroot != "" } {
+	    do_test $new_sysroot
+	}
+    }
+}

Re: [PATCH v3 2/2] Make GDB resolve dlopen of memory mapped shared libraries

[Dr Lancelot SIX]

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

Hi Asaf and Andrew,

Thanks for working on this! I have included comments below in the patch.

On 10/11/2022 19:37, Andrew Burgess via Gdb-patches wrote:
> Asaf Fisher via Gdb-patches<gdb-patches@sourceware.org>  writes:
>
>> Introduced `check_proc_self_file` that checks if a path used by
>> inferior in dlopen is in the form of `/proc/self/...` and if so resolves
>> it to `/proc/[pid]/...`
>>
>> Bug:https://sourceware.org/bugzilla/show_bug.cgi?id=29586
> Hi Asaf,
>
> Thanks for providing a fix for this, and thanks for submitting the
> copyright assignment paperwork.
>
> I took a look through your patch and ended up tweaking it a little.
> I've attached a revised version below, which I'd love to hear your
> feedback on.
>
> When making changes my goal was to extend your fix to work with
> gdbserver just like it works with native targets.  You can test
> different gdbserver setups like this:
>
>    make check-gdb TESTS="gdb.base/solib-proc-self.exp"  \
>                   RUNTESTFLAGS="--target_board=native-extended-gdbserver"
>
> This will setup gdbserver and connect GDB to it with 'target
> extended-remote', you can also test using the 'native-gdbserver' board,
> which will connect to gdbserver as just 'target remote'.
>
> Ideally any new test will pass in all three configurations (the default
> plus the two listed above), and initially, your change only fixed the
> native target case.  By moving the fix elsewhere in the shared library
> loading process I think I new have all three cases working.
>
> I also extended the test to test using a 'target:' sysroot.  By default
> when testing with the two gdbserver boards above, we set the sysroot to
> "" (the empty string), this tells GDB that remote files can be found on
> the local machine, and avoids all files accesses having to go over the
> remote protocol.  However, given the nature of this change, I figured it
> was worth testing with the 'target:' sysroot too, this means we re-run
> the tests bu sending all file accesses over the remote protocol.  That
> case is also fixed with the patch below.
>
> Where I'd most appreciate your feedback is for the algorithm by which
> the /proc/self path is spotted and converted to /proc/PID.  I've cut the
> code back a bit from what you had originally, mostly because I couldn't
> find a way to test that the extra complexity was required.  If you have
> any additional test cases that could show that the slimmed down code is
> not good enough, then that would be great.
>
> Thanks,
> Andrew
>
> ---
>
> commit 0bf4c98bc225c89a0a1ddcea727eca178ad69710
> Author: Asaf Fisher<asaffisher.dev@gmail.com>
> Date:   Fri Oct 21 17:42:05 2022 +0000
>
>      gdb: handle loading shared libraries from /proc/self/fd/
>      
>      Bug PR gdb/29586 describes a situations where a shared library is
>      created in memory, then written to a memory mapped file.  The memory
>      mapped file will show up as a file descriptor within /proc/self/fd/,
>      and this path is then used with dlopen in order to call functions
>      within the in-memory shared library.
>      
>      When attempting to debug this GDB hangs.  The problem is that, GDB
>      stops at the shared-library event, reads the shared library path from
>      the inferior, which is "/proc/self/fd/<NUM>", and then GDB attempts to
>      open this file.
>      
>      Unfortunately, this means GDB tries to open a file within GDB's
>      /proc/self/fd/ directory, not within the inferior's directory.  In the
>      case of our hang it turns out that the file descriptor that is opened
>      is a pipe, and GDB hangs trying to read from the pipe.
>      
>      However, the behaviour is really just undefined, depending on which
>      file descriptor the inferior tries to open, GDB will open, or fail to
>      open, random files within its /proc/self/fd directory.
>      
>      The solution proposed in this commit is to hook into solib_find_1, and
>      spot when GDB is looking for any file in /proc/self/, if this is the
>      case, then the filename is rewritten as /proc/<PID>, where <PID> is
>      the process-id of the current inferior.
>      
>      The test passes for the unix, native-gdbserver, and
>      native-extended-gdbserver targets.  When testing with either of the
>      gdbserver targets, the test is run using the default empty sysroot,
>      and also using the 'target:' sysroot.
>      
>      Bug:https://sourceware.org/bugzilla/show_bug.cgi?id=29586
>      gdb.base/solib-proc-self.exp
>      Co-authored-by: Andrew Burgess<aburgess@redhat.com>
>
> diff --git a/gdb/solib.c b/gdb/solib.c
> index 7cfdd81114c..cf2d0d3bc3a 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -83,6 +83,35 @@ show_solib_search_path (struct ui_file *file, int from_tty,
>   #  define DOS_BASED_FILE_SYSTEM 0
>   #endif
>   
> +/* Fix references to files in /proc/self/fd/ when opening a shared library.
> +
> +   SO_NAME is the name of the shared library being loaded.  This function
> +   returns a possibly modified name which should be used as the path to the
> +   shared library.
> +
> +   If SO_NAME starts with /proc/self, then the returned name will be
> +   modified to start with /proc/PID where 'PID' is the pid of the current
> +   inferior.  */
> +
> +static gdb::unique_xmalloc_ptr<char>
> +filter_proc_self_filenames (gdb::unique_xmalloc_ptr<char> so_name)
> +{
> +  static const char *proc_self_prefix = "/proc/self";
> +
> +  /* Is the path really a /proc/self?  */
> +  if (!startswith (so_name.get (), proc_self_prefix))
> +    return so_name;
> +
> +  /* Get the part of the path after /proc/self.  For example given
> +     '/proc/self/fd' we find the '/fd' part.  */
> +  gdb_assert (strlen (so_name.get ()) >= strlen (proc_self_prefix));

I am not sure how this assert can ever fail as the test just before 
checked that so_name starts with proc_self_prefix.  How can it have a 
smaller length?

> +  const char *tail = so_name.get () + strlen (proc_self_prefix);
> +
> +  /* Build a replacement path.  */
> +  int inferior_pid = inferior_ptid.pid ();
> +  return xstrprintf ("/proc/%d%s", inferior_pid, tail);
> +}
> +
>   /* Return the full pathname of a binary file (the main executable or a
>      shared library file), or NULL if not found.  If FD is non-NULL, *FD
>      is set to either -1 or an open file handle for the binary file.
> @@ -172,6 +201,12 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
>   	}
>       }
>   
> +  /* If the path starts /proc/self then rewrite this as /proc/PID using the
> +     current inferior's pid.  Failing to do this will cause GDB to try and
> +     open files within its proc directory, rather than the inferiors.  */
> +  temp_pathname.reset (xstrdup (in_pathname));
> +  temp_pathname = filter_proc_self_filenames (std::move (temp_pathname));
> +
>     /* Note, we're interested in IS_TARGET_ABSOLUTE_PATH, not
>        IS_ABSOLUTE_PATH.  The latter is for host paths only, while
>        IN_PATHNAME is a target path.  For example, if we're supposed to
> @@ -184,9 +219,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
>          3rd attempt, c:/foo/bar.dll ==> /sysroot/foo/bar.dll
>     */
>   
> -  if (!IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) || sysroot == NULL)
> -    temp_pathname.reset (xstrdup (in_pathname));
> -  else
> +  if (IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) && sysroot != NULL)

Just a nit, but you could s/NULL/nullptr while at updating this line

>       {
>         bool need_dir_separator;
>   
> @@ -213,7 +246,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
>         /* Cat the prefixed pathname together.  */
>         temp_pathname.reset (concat (sysroot,
>   				   need_dir_separator ? SLASH_STRING : "",
> -				   in_pathname, (char *) NULL));
> +				   temp_pathname.get (), (char *) NULL));
Same here. Also, could nullptr make it so the "(char *)" cast becomes 
useless?
>       }
>   
>     /* Handle files to be accessed via the target.  */
> diff --git a/gdb/testsuite/gdb.base/solib-proc-self.cc b/gdb/testsuite/gdb.base/solib-proc-self.cc
> new file mode 100644
> index 00000000000..f2439d738a3
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/solib-proc-self.cc
> @@ -0,0 +1,73 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2022 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see<http://www.gnu.org/licenses/>.  */
> +
> +#include <sys/mman.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <iostream>
> +#include <fstream>
> +#include <sstream>
> +#include <vector>
> +#include <unistd.h>
> +
> +#ifdef __WIN32__
> +#include <windows.h>
> +#define dlopen(name, mode) LoadLibrary (name)
> +#define dlclose(handle) FreeLibrary (handle)
> +#define dlerror() "an error occurred"
> +#else
> +#include <dlfcn.h>
> +#endif
> +
> +int
> +main ()
> +{
> +  /* Read the shared libraries contents into a buffer.  */
> +  std::ifstream read_so_file = std::ifstream (SHLIB_NAME);
> +  read_so_file.seekg (0, std::ios::end);
> +  std::streamsize size = read_so_file.tellg ();
> +  read_so_file.seekg (0, std::ios::beg);
> +  std::vector<char> buffer (size);
> +  if (!read_so_file.read (buffer.data (), size))
> +    {
> +      fprintf (stderr, "Failed to load solib\n");
> +      exit (1);
> +    }
> +
> +  /* Create a memory mapped file, then write the shared library to that
> +     new memory mapped file.  */
> +  int mem_fd = memfd_create ("test", 0);
> +  write (mem_fd, buffer.data (), buffer.size ());
> +
> +  /* Generate the /proc/self/fd/[num] path for the memory mapped file.  */
> +  std::string proc_self_fd_path;		/* break-here */
> +  std::stringstream proc_self_fd_path_stream
> +    = std::stringstream (proc_self_fd_path);
> +  proc_self_fd_path_stream << "/proc/self/fd/" << mem_fd;
> +
> +  /* Call dlopen on it.  */
> +  void *handle = dlopen (proc_self_fd_path_stream.str ().c_str (), RTLD_LAZY);
> +  if (!handle)
> +    {
> +      fprintf (stderr, "%s\n", dlerror ());
> +      exit (1);
> +    }
> +  /* It worked.  */
> +  dlclose (handle);
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/solib-proc-self.exp b/gdb/testsuite/gdb.base/solib-proc-self.exp
> new file mode 100644
> index 00000000000..1c845822490
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/solib-proc-self.exp
> @@ -0,0 +1,130 @@
> +# Copyright 2022 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see<http://www.gnu.org/licenses/>.  */
> +
> +# Test connecting and disconnecting at shared library events.
> +
> +if {[skip_shlib_tests]} {
> +    untested "could not run to main"
> +    return 0
> +}
> +
> +standard_testfile .cc
> +
> +# Reuse an existing library, we don't care about the library contents
> +# for this test.
> +set libfile so-disc-shr
> +set libsrc "${srcdir}/${subdir}/${libfile}.c"
> +set libname "${libfile}.so"
> +set libobj [standard_output_file ${libname}]
> +
> +# Compile the shared library.
> +if { [gdb_compile_shlib $libsrc $libobj {debug}] != ""} {
> +    return -1
> +}
> +
> +# Compile the test executable.
> +if [ build_executable "failed to prepare" $testfile $srcfile \
> +	 [list shlib_load debug c++ additional_flags=-DSHLIB_NAME="${libobj}"]] {
> +    return -1
> +}
> +
> +# Start GDB and run to the point where the test program tries to dlopen a file
> +# from within /proc/self/fd/.  Catch the shared library event and check that
> +# we actually try to load a file from /proc/<INFERIOR-PID>/fd/.
> +#
> +# If SYSROOT is not the empty string, then this is set as the value of GDB's
> +# sysroot immediately after starting GDB.  The only value that is (currently)
> +# supported, other than the empty string, is 'target:'.
> +proc do_test { {sysroot ""} } {
> +    clean_restart $::binfile
> +
> +    if {$sysroot != ""} {
> +	gdb_test_no_output "set sysroot ${sysroot}"
> +    }
> +
> +    gdb_load_shlib $::libobj
> +
> +    if ![runto_main] then {
> +	return 0
> +    }
> +
> +    # Get inferior's PID for later.
> +    set inferior_pid [get_inferior_pid]
> +
> +    # Turn on the solib-events so we can see that gdb resolves everything
> +    # correctly.
> +    gdb_test_no_output "set stop-on-solib-events 1"
> +
> +    # Run to the 'break-here' marker.
> +    gdb_breakpoint [gdb_get_line_number "break-here"]
> +    gdb_continue_to_breakpoint "break-here" ".* break-here .*"
> +
> +    set memfd ""
> +    gdb_test_multiple "p mem_fd" "Get file descriptor" {
> +	-re -wrap "\\\$$::decimal = (\[^\r\n\]*)" {
> +	    set memfd $expect_out(1,string)
> +	    pass $gdb_test_name
> +	}
> +    }
> +
> +    gdb_test "continue" "Stopped due to shared library event.*" \
> +	"continue to load"

I've been running this patch (as modified by Andrew) and see a failure 
in the tests.

(gdb) PASS: gdb.base/solib-proc-self.exp: continue to breakpoint: break-here
p mem_fd
$1 = 4
(gdb) PASS: gdb.base/solib-proc-self.exp: Get file descriptor
continue
Continuing.
Stopped due to shared library event:
   Inferior loaded /proc/7331/fd/4
(gdb) PASS: gdb.base/solib-proc-self.exp: continue to load
continue
Continuing.
Stopped due to shared library event (no libraries added or removed)
(gdb) FAIL: gdb.base/solib-proc-self.exp: continue

I believe this "continue" should be removed.  The test passes cleanly 
without it.

Best,
Lancelot.

> +
> +    # Check if inferior resolved the /proc/self/fd/[num] to /proc/[pid]/fd/[num].
> +    gdb_test "continue" \
> +	[multi_line \
> +	     "Stopped due to shared library event:" \
> +	     "  Inferior loaded (?:target:)?/proc/${inferior_pid}/fd/$memfd"]
> +}
> +
> +# First run of the test.
> +do_test
> +
> +# Possible second run of the test.  If we are using a remote target then we
> +# should consider setting the sysroot to 'target:' and re-running the test.
> +if {[target_info exists gdb_protocol]
> +    && ([target_info gdb_protocol] == "remote"
> +	|| [target_info gdb_protocol] == "extended-remote")} {
> +    # GDB will already be running after the first call to do_test, so we can
> +    # take a peek at the current sysroot setting, and decide if we should
> +    # repeat the test with a different setting.
> +
> +    set new_sysroot ""
> +    gdb_test_multiple "show sysroot" "" {
> +	-wrap -re "The current system root is \"\"\\." {
> +	    pass $gdb_test_name
> +
> +	    # Repeat the test with 'target:' sysroot.
> +	    set new_sysroot "target:"
> +	}
> +	-wrap -re "The current system root is \"target:\"\\." {
> +	    pass $gdb_test_name
> +
> +	    # Nothing else to do, we already tested with target: sysroot.
> +	}
> +	-re "$gdb_prompt $" {
> +	    pass $gdb_test_name
> +
> +	    # If already testing with any other sysroot, we probably should
> +	    # not try to adjust things, so don't do any further testing.
> +	}
> +    }
> +
> +    with_test_prefix "sysroot $new_sysroot" {
> +	if { $new_sysroot != "" } {
> +	    do_test $new_sysroot
> +	}
> +    }
> +}
>

Re: [PATCH v3 2/2] Make GDB resolve dlopen of memory mapped shared libraries

[Asaf Fisher]

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

I’ll be able to review it right after the 22nd of November.


On Fri, Nov 11, 2022 at 2:35 PM Dr Lancelot SIX <lsix@lancelotsix.com>
wrote:

> Hi Asaf and Andrew,
>
> Thanks for working on this! I have included comments below in the patch.
> On 10/11/2022 19:37, Andrew Burgess via Gdb-patches wrote:
>
> Asaf Fisher via Gdb-patches <gdb-patches@sourceware.org> <gdb-patches@sourceware.org> writes:
>
>
> Introduced `check_proc_self_file` that checks if a path used by
> inferior in dlopen is in the form of `/proc/self/...` and if so resolves
> it to `/proc/[pid]/...`
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29586
>
> Hi Asaf,
>
> Thanks for providing a fix for this, and thanks for submitting the
> copyright assignment paperwork.
>
> I took a look through your patch and ended up tweaking it a little.
> I've attached a revised version below, which I'd love to hear your
> feedback on.
>
> When making changes my goal was to extend your fix to work with
> gdbserver just like it works with native targets.  You can test
> different gdbserver setups like this:
>
>   make check-gdb TESTS="gdb.base/solib-proc-self.exp"  \
>                  RUNTESTFLAGS="--target_board=native-extended-gdbserver"
>
> This will setup gdbserver and connect GDB to it with 'target
> extended-remote', you can also test using the 'native-gdbserver' board,
> which will connect to gdbserver as just 'target remote'.
>
> Ideally any new test will pass in all three configurations (the default
> plus the two listed above), and initially, your change only fixed the
> native target case.  By moving the fix elsewhere in the shared library
> loading process I think I new have all three cases working.
>
> I also extended the test to test using a 'target:' sysroot.  By default
> when testing with the two gdbserver boards above, we set the sysroot to
> "" (the empty string), this tells GDB that remote files can be found on
> the local machine, and avoids all files accesses having to go over the
> remote protocol.  However, given the nature of this change, I figured it
> was worth testing with the 'target:' sysroot too, this means we re-run
> the tests bu sending all file accesses over the remote protocol.  That
> case is also fixed with the patch below.
>
> Where I'd most appreciate your feedback is for the algorithm by which
> the /proc/self path is spotted and converted to /proc/PID.  I've cut the
> code back a bit from what you had originally, mostly because I couldn't
> find a way to test that the extra complexity was required.  If you have
> any additional test cases that could show that the slimmed down code is
> not good enough, then that would be great.
>
> Thanks,
> Andrew
>
> ---
>
> commit 0bf4c98bc225c89a0a1ddcea727eca178ad69710
> Author: Asaf Fisher <asaffisher.dev@gmail.com> <asaffisher.dev@gmail.com>
> Date:   Fri Oct 21 17:42:05 2022 +0000
>
>     gdb: handle loading shared libraries from /proc/self/fd/
>
>     Bug PR gdb/29586 describes a situations where a shared library is
>     created in memory, then written to a memory mapped file.  The memory
>     mapped file will show up as a file descriptor within /proc/self/fd/,
>     and this path is then used with dlopen in order to call functions
>     within the in-memory shared library.
>
>     When attempting to debug this GDB hangs.  The problem is that, GDB
>     stops at the shared-library event, reads the shared library path from
>     the inferior, which is "/proc/self/fd/<NUM>", and then GDB attempts to
>     open this file.
>
>     Unfortunately, this means GDB tries to open a file within GDB's
>     /proc/self/fd/ directory, not within the inferior's directory.  In the
>     case of our hang it turns out that the file descriptor that is opened
>     is a pipe, and GDB hangs trying to read from the pipe.
>
>     However, the behaviour is really just undefined, depending on which
>     file descriptor the inferior tries to open, GDB will open, or fail to
>     open, random files within its /proc/self/fd directory.
>
>     The solution proposed in this commit is to hook into solib_find_1, and
>     spot when GDB is looking for any file in /proc/self/, if this is the
>     case, then the filename is rewritten as /proc/<PID>, where <PID> is
>     the process-id of the current inferior.
>
>     The test passes for the unix, native-gdbserver, and
>     native-extended-gdbserver targets.  When testing with either of the
>     gdbserver targets, the test is run using the default empty sysroot,
>     and also using the 'target:' sysroot.
>
>     Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29586
>
>     gdb.base/solib-proc-self.exp
>     Co-authored-by: Andrew Burgess <aburgess@redhat.com> <aburgess@redhat.com>
>
>
> diff --git a/gdb/solib.c b/gdb/solib.c
> index 7cfdd81114c..cf2d0d3bc3a 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -83,6 +83,35 @@ show_solib_search_path (struct ui_file *file, int from_tty,
>  #  define DOS_BASED_FILE_SYSTEM 0
>  #endif
>
> +/* Fix references to files in /proc/self/fd/ when opening a shared library.
> +
> +   SO_NAME is the name of the shared library being loaded.  This function
> +   returns a possibly modified name which should be used as the path to the
> +   shared library.
> +
> +   If SO_NAME starts with /proc/self, then the returned name will be
> +   modified to start with /proc/PID where 'PID' is the pid of the current
> +   inferior.  */
> +
> +static gdb::unique_xmalloc_ptr<char>
> +filter_proc_self_filenames (gdb::unique_xmalloc_ptr<char> so_name)
> +{
> +  static const char *proc_self_prefix = "/proc/self";
> +
> +  /* Is the path really a /proc/self?  */
> +  if (!startswith (so_name.get (), proc_self_prefix))
> +    return so_name;
> +
> +  /* Get the part of the path after /proc/self.  For example given
> +     '/proc/self/fd' we find the '/fd' part.  */
> +  gdb_assert (strlen (so_name.get ()) >= strlen (proc_self_prefix));
>
> I am not sure how this assert can ever fail as the test just before
> checked that so_name starts with proc_self_prefix.  How can it have a
> smaller length?
>
> +  const char *tail = so_name.get () + strlen (proc_self_prefix);
> +
> +  /* Build a replacement path.  */
> +  int inferior_pid = inferior_ptid.pid ();
> +  return xstrprintf ("/proc/%d%s", inferior_pid, tail);
> +}
> +
>  /* Return the full pathname of a binary file (the main executable or a
>     shared library file), or NULL if not found.  If FD is non-NULL, *FD
>     is set to either -1 or an open file handle for the binary file.
> @@ -172,6 +201,12 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
>  	}
>      }
>
> +  /* If the path starts /proc/self then rewrite this as /proc/PID using the
> +     current inferior's pid.  Failing to do this will cause GDB to try and
> +     open files within its proc directory, rather than the inferiors.  */
> +  temp_pathname.reset (xstrdup (in_pathname));
> +  temp_pathname = filter_proc_self_filenames (std::move (temp_pathname));
> +
>    /* Note, we're interested in IS_TARGET_ABSOLUTE_PATH, not
>       IS_ABSOLUTE_PATH.  The latter is for host paths only, while
>       IN_PATHNAME is a target path.  For example, if we're supposed to
> @@ -184,9 +219,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
>         3rd attempt, c:/foo/bar.dll ==> /sysroot/foo/bar.dll
>    */
>
> -  if (!IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) || sysroot == NULL)
> -    temp_pathname.reset (xstrdup (in_pathname));
> -  else
> +  if (IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) && sysroot != NULL)
>
> Just a nit, but you could s/NULL/nullptr while at updating this line
>
>      {
>        bool need_dir_separator;
>
> @@ -213,7 +246,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
>        /* Cat the prefixed pathname together.  */
>        temp_pathname.reset (concat (sysroot,
>  				   need_dir_separator ? SLASH_STRING : "",
> -				   in_pathname, (char *) NULL));
> +				   temp_pathname.get (), (char *) NULL));
>
> Same here. Also, could nullptr make it so the "(char *)" cast becomes
> useless?
>
>      }
>
>    /* Handle files to be accessed via the target.  */
> diff --git a/gdb/testsuite/gdb.base/solib-proc-self.cc b/gdb/testsuite/gdb.base/solib-proc-self.cc
> new file mode 100644
> index 00000000000..f2439d738a3
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/solib-proc-self.cc
> @@ -0,0 +1,73 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2022 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/> <http://www.gnu.org/licenses/>.  */
> +
> +#include <sys/mman.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <iostream>
> +#include <fstream>
> +#include <sstream>
> +#include <vector>
> +#include <unistd.h>
> +
> +#ifdef __WIN32__
> +#include <windows.h>
> +#define dlopen(name, mode) LoadLibrary (name)
> +#define dlclose(handle) FreeLibrary (handle)
> +#define dlerror() "an error occurred"
> +#else
> +#include <dlfcn.h>
> +#endif
> +
> +int
> +main ()
> +{
> +  /* Read the shared libraries contents into a buffer.  */
> +  std::ifstream read_so_file = std::ifstream (SHLIB_NAME);
> +  read_so_file.seekg (0, std::ios::end);
> +  std::streamsize size = read_so_file.tellg ();
> +  read_so_file.seekg (0, std::ios::beg);
> +  std::vector<char> buffer (size);
> +  if (!read_so_file.read (buffer.data (), size))
> +    {
> +      fprintf (stderr, "Failed to load solib\n");
> +      exit (1);
> +    }
> +
> +  /* Create a memory mapped file, then write the shared library to that
> +     new memory mapped file.  */
> +  int mem_fd = memfd_create ("test", 0);
> +  write (mem_fd, buffer.data (), buffer.size ());
> +
> +  /* Generate the /proc/self/fd/[num] path for the memory mapped file.  */
> +  std::string proc_self_fd_path;		/* break-here */
> +  std::stringstream proc_self_fd_path_stream
> +    = std::stringstream (proc_self_fd_path);
> +  proc_self_fd_path_stream << "/proc/self/fd/" << mem_fd;
> +
> +  /* Call dlopen on it.  */
> +  void *handle = dlopen (proc_self_fd_path_stream.str ().c_str (), RTLD_LAZY);
> +  if (!handle)
> +    {
> +      fprintf (stderr, "%s\n", dlerror ());
> +      exit (1);
> +    }
> +  /* It worked.  */
> +  dlclose (handle);
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/solib-proc-self.exp b/gdb/testsuite/gdb.base/solib-proc-self.exp
> new file mode 100644
> index 00000000000..1c845822490
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/solib-proc-self.exp
> @@ -0,0 +1,130 @@
> +# Copyright 2022 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/> <http://www.gnu.org/licenses/>.  */
> +
> +# Test connecting and disconnecting at shared library events.
> +
> +if {[skip_shlib_tests]} {
> +    untested "could not run to main"
> +    return 0
> +}
> +
> +standard_testfile .cc
> +
> +# Reuse an existing library, we don't care about the library contents
> +# for this test.
> +set libfile so-disc-shr
> +set libsrc "${srcdir}/${subdir}/${libfile}.c"
> +set libname "${libfile}.so"
> +set libobj [standard_output_file ${libname}]
> +
> +# Compile the shared library.
> +if { [gdb_compile_shlib $libsrc $libobj {debug}] != ""} {
> +    return -1
> +}
> +
> +# Compile the test executable.
> +if [ build_executable "failed to prepare" $testfile $srcfile \
> +	 [list shlib_load debug c++ additional_flags=-DSHLIB_NAME="${libobj}"]] {
> +    return -1
> +}
> +
> +# Start GDB and run to the point where the test program tries to dlopen a file
> +# from within /proc/self/fd/.  Catch the shared library event and check that
> +# we actually try to load a file from /proc/<INFERIOR-PID>/fd/.
> +#
> +# If SYSROOT is not the empty string, then this is set as the value of GDB's
> +# sysroot immediately after starting GDB.  The only value that is (currently)
> +# supported, other than the empty string, is 'target:'.
> +proc do_test { {sysroot ""} } {
> +    clean_restart $::binfile
> +
> +    if {$sysroot != ""} {
> +	gdb_test_no_output "set sysroot ${sysroot}"
> +    }
> +
> +    gdb_load_shlib $::libobj
> +
> +    if ![runto_main] then {
> +	return 0
> +    }
> +
> +    # Get inferior's PID for later.
> +    set inferior_pid [get_inferior_pid]
> +
> +    # Turn on the solib-events so we can see that gdb resolves everything
> +    # correctly.
> +    gdb_test_no_output "set stop-on-solib-events 1"
> +
> +    # Run to the 'break-here' marker.
> +    gdb_breakpoint [gdb_get_line_number "break-here"]
> +    gdb_continue_to_breakpoint "break-here" ".* break-here .*"
> +
> +    set memfd ""
> +    gdb_test_multiple "p mem_fd" "Get file descriptor" {
> +	-re -wrap "\\\$$::decimal = (\[^\r\n\]*)" {
> +	    set memfd $expect_out(1,string)
> +	    pass $gdb_test_name
> +	}
> +    }
> +
> +    gdb_test "continue" "Stopped due to shared library event.*" \
> +	"continue to load"
>
> I've been running this patch (as modified by Andrew) and see a failure in
> the tests.
>
> (gdb) PASS: gdb.base/solib-proc-self.exp: continue to breakpoint: break-here
> p mem_fd
> $1 = 4
> (gdb) PASS: gdb.base/solib-proc-self.exp: Get file descriptor
> continue
> Continuing.
> Stopped due to shared library event:
>   Inferior loaded /proc/7331/fd/4
> (gdb) PASS: gdb.base/solib-proc-self.exp: continue to load
> continue
> Continuing.
> Stopped due to shared library event (no libraries added or removed)
> (gdb) FAIL: gdb.base/solib-proc-self.exp: continue
>
>
> I believe this "continue" should be removed.  The test passes cleanly
> without it.
>
> Best,
> Lancelot.
>
> +
> +    # Check if inferior resolved the /proc/self/fd/[num] to /proc/[pid]/fd/[num].
> +    gdb_test "continue" \
> +	[multi_line \
> +	     "Stopped due to shared library event:" \
> +	     "  Inferior loaded (?:target:)?/proc/${inferior_pid}/fd/$memfd"]
> +}
> +
> +# First run of the test.
> +do_test
> +
> +# Possible second run of the test.  If we are using a remote target then we
> +# should consider setting the sysroot to 'target:' and re-running the test.
> +if {[target_info exists gdb_protocol]
> +    && ([target_info gdb_protocol] == "remote"
> +	|| [target_info gdb_protocol] == "extended-remote")} {
> +    # GDB will already be running after the first call to do_test, so we can
> +    # take a peek at the current sysroot setting, and decide if we should
> +    # repeat the test with a different setting.
> +
> +    set new_sysroot ""
> +    gdb_test_multiple "show sysroot" "" {
> +	-wrap -re "The current system root is \"\"\\." {
> +	    pass $gdb_test_name
> +
> +	    # Repeat the test with 'target:' sysroot.
> +	    set new_sysroot "target:"
> +	}
> +	-wrap -re "The current system root is \"target:\"\\." {
> +	    pass $gdb_test_name
> +
> +	    # Nothing else to do, we already tested with target: sysroot.
> +	}
> +	-re "$gdb_prompt $" {
> +	    pass $gdb_test_name
> +
> +	    # If already testing with any other sysroot, we probably should
> +	    # not try to adjust things, so don't do any further testing.
> +	}
> +    }
> +
> +    with_test_prefix "sysroot $new_sysroot" {
> +	if { $new_sysroot != "" } {
> +	    do_test $new_sysroot
> +	}
> +    }
> +}
>
>
>

Re: [PATCH v3 2/2] Make GDB resolve dlopen of memory mapped shared libraries

[Andrew Burgess]

Dr Lancelot SIX <lsix@lancelotsix.com> writes:

> Hi Asaf and Andrew,
>
> Thanks for working on this! I have included comments below in the patch.
>
> On 10/11/2022 19:37, Andrew Burgess via Gdb-patches wrote:
>> Asaf Fisher via Gdb-patches<gdb-patches@sourceware.org>  writes:
>>
>>> Introduced `check_proc_self_file` that checks if a path used by
>>> inferior in dlopen is in the form of `/proc/self/...` and if so resolves
>>> it to `/proc/[pid]/...`
>>>
>>> Bug:https://sourceware.org/bugzilla/show_bug.cgi?id=29586
>> Hi Asaf,
>>
>> Thanks for providing a fix for this, and thanks for submitting the
>> copyright assignment paperwork.
>>
>> I took a look through your patch and ended up tweaking it a little.
>> I've attached a revised version below, which I'd love to hear your
>> feedback on.
>>
>> When making changes my goal was to extend your fix to work with
>> gdbserver just like it works with native targets.  You can test
>> different gdbserver setups like this:
>>
>>    make check-gdb TESTS="gdb.base/solib-proc-self.exp"  \
>>                   RUNTESTFLAGS="--target_board=native-extended-gdbserver"
>>
>> This will setup gdbserver and connect GDB to it with 'target
>> extended-remote', you can also test using the 'native-gdbserver' board,
>> which will connect to gdbserver as just 'target remote'.
>>
>> Ideally any new test will pass in all three configurations (the default
>> plus the two listed above), and initially, your change only fixed the
>> native target case.  By moving the fix elsewhere in the shared library
>> loading process I think I new have all three cases working.
>>
>> I also extended the test to test using a 'target:' sysroot.  By default
>> when testing with the two gdbserver boards above, we set the sysroot to
>> "" (the empty string), this tells GDB that remote files can be found on
>> the local machine, and avoids all files accesses having to go over the
>> remote protocol.  However, given the nature of this change, I figured it
>> was worth testing with the 'target:' sysroot too, this means we re-run
>> the tests bu sending all file accesses over the remote protocol.  That
>> case is also fixed with the patch below.
>>
>> Where I'd most appreciate your feedback is for the algorithm by which
>> the /proc/self path is spotted and converted to /proc/PID.  I've cut the
>> code back a bit from what you had originally, mostly because I couldn't
>> find a way to test that the extra complexity was required.  If you have
>> any additional test cases that could show that the slimmed down code is
>> not good enough, then that would be great.
>>
>> Thanks,
>> Andrew
>>
>> ---
>>
>> commit 0bf4c98bc225c89a0a1ddcea727eca178ad69710
>> Author: Asaf Fisher<asaffisher.dev@gmail.com>
>> Date:   Fri Oct 21 17:42:05 2022 +0000
>>
>>      gdb: handle loading shared libraries from /proc/self/fd/
>>      
>>      Bug PR gdb/29586 describes a situations where a shared library is
>>      created in memory, then written to a memory mapped file.  The memory
>>      mapped file will show up as a file descriptor within /proc/self/fd/,
>>      and this path is then used with dlopen in order to call functions
>>      within the in-memory shared library.
>>      
>>      When attempting to debug this GDB hangs.  The problem is that, GDB
>>      stops at the shared-library event, reads the shared library path from
>>      the inferior, which is "/proc/self/fd/<NUM>", and then GDB attempts to
>>      open this file.
>>      
>>      Unfortunately, this means GDB tries to open a file within GDB's
>>      /proc/self/fd/ directory, not within the inferior's directory.  In the
>>      case of our hang it turns out that the file descriptor that is opened
>>      is a pipe, and GDB hangs trying to read from the pipe.
>>      
>>      However, the behaviour is really just undefined, depending on which
>>      file descriptor the inferior tries to open, GDB will open, or fail to
>>      open, random files within its /proc/self/fd directory.
>>      
>>      The solution proposed in this commit is to hook into solib_find_1, and
>>      spot when GDB is looking for any file in /proc/self/, if this is the
>>      case, then the filename is rewritten as /proc/<PID>, where <PID> is
>>      the process-id of the current inferior.
>>      
>>      The test passes for the unix, native-gdbserver, and
>>      native-extended-gdbserver targets.  When testing with either of the
>>      gdbserver targets, the test is run using the default empty sysroot,
>>      and also using the 'target:' sysroot.
>>      
>>      Bug:https://sourceware.org/bugzilla/show_bug.cgi?id=29586
>>      gdb.base/solib-proc-self.exp
>>      Co-authored-by: Andrew Burgess<aburgess@redhat.com>
>>
>> diff --git a/gdb/solib.c b/gdb/solib.c
>> index 7cfdd81114c..cf2d0d3bc3a 100644
>> --- a/gdb/solib.c
>> +++ b/gdb/solib.c
>> @@ -83,6 +83,35 @@ show_solib_search_path (struct ui_file *file, int from_tty,
>>   #  define DOS_BASED_FILE_SYSTEM 0
>>   #endif
>>   
>> +/* Fix references to files in /proc/self/fd/ when opening a shared library.
>> +
>> +   SO_NAME is the name of the shared library being loaded.  This function
>> +   returns a possibly modified name which should be used as the path to the
>> +   shared library.
>> +
>> +   If SO_NAME starts with /proc/self, then the returned name will be
>> +   modified to start with /proc/PID where 'PID' is the pid of the current
>> +   inferior.  */
>> +
>> +static gdb::unique_xmalloc_ptr<char>
>> +filter_proc_self_filenames (gdb::unique_xmalloc_ptr<char> so_name)
>> +{
>> +  static const char *proc_self_prefix = "/proc/self";
>> +
>> +  /* Is the path really a /proc/self?  */
>> +  if (!startswith (so_name.get (), proc_self_prefix))
>> +    return so_name;
>> +
>> +  /* Get the part of the path after /proc/self.  For example given
>> +     '/proc/self/fd' we find the '/fd' part.  */
>> +  gdb_assert (strlen (so_name.get ()) >= strlen (proc_self_prefix));
>
> I am not sure how this assert can ever fail as the test just before 
> checked that so_name starts with proc_self_prefix.  How can it have a 
> smaller length?
>

Removed.

>> +  const char *tail = so_name.get () + strlen (proc_self_prefix);
>> +
>> +  /* Build a replacement path.  */
>> +  int inferior_pid = inferior_ptid.pid ();
>> +  return xstrprintf ("/proc/%d%s", inferior_pid, tail);
>> +}
>> +
>>   /* Return the full pathname of a binary file (the main executable or a
>>      shared library file), or NULL if not found.  If FD is non-NULL, *FD
>>      is set to either -1 or an open file handle for the binary file.
>> @@ -172,6 +201,12 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
>>   	}
>>       }
>>   
>> +  /* If the path starts /proc/self then rewrite this as /proc/PID using the
>> +     current inferior's pid.  Failing to do this will cause GDB to try and
>> +     open files within its proc directory, rather than the inferiors.  */
>> +  temp_pathname.reset (xstrdup (in_pathname));
>> +  temp_pathname = filter_proc_self_filenames (std::move (temp_pathname));
>> +
>>     /* Note, we're interested in IS_TARGET_ABSOLUTE_PATH, not
>>        IS_ABSOLUTE_PATH.  The latter is for host paths only, while
>>        IN_PATHNAME is a target path.  For example, if we're supposed to
>> @@ -184,9 +219,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
>>          3rd attempt, c:/foo/bar.dll ==> /sysroot/foo/bar.dll
>>     */
>>   
>> -  if (!IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) || sysroot == NULL)
>> -    temp_pathname.reset (xstrdup (in_pathname));
>> -  else
>> +  if (IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) && sysroot != NULL)
>
> Just a nit, but you could s/NULL/nullptr while at updating this line
>

Fixed.

>>       {
>>         bool need_dir_separator;
>>   
>> @@ -213,7 +246,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
>>         /* Cat the prefixed pathname together.  */
>>         temp_pathname.reset (concat (sysroot,
>>   				   need_dir_separator ? SLASH_STRING : "",
>> -				   in_pathname, (char *) NULL));
>> +				   temp_pathname.get (), (char *) NULL));
> Same here. Also, could nullptr make it so the "(char *)" cast becomes 
> useless?

Fixed.

>>       }
>>   
>>     /* Handle files to be accessed via the target.  */
>> diff --git a/gdb/testsuite/gdb.base/solib-proc-self.cc b/gdb/testsuite/gdb.base/solib-proc-self.cc
>> new file mode 100644
>> index 00000000000..f2439d738a3
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/solib-proc-self.cc
>> @@ -0,0 +1,73 @@
>> +/* This testcase is part of GDB, the GNU debugger.
>> +
>> +   Copyright 2022 Free Software Foundation, Inc.
>> +
>> +   This program is free software; you can redistribute it and/or modify
>> +   it under the terms of the GNU General Public License as published by
>> +   the Free Software Foundation; either version 3 of the License, or
>> +   (at your option) any later version.
>> +
>> +   This program is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +   GNU General Public License for more details.
>> +
>> +   You should have received a copy of the GNU General Public License
>> +   along with this program.  If not, see<http://www.gnu.org/licenses/>.  */
>> +
>> +#include <sys/mman.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <iostream>
>> +#include <fstream>
>> +#include <sstream>
>> +#include <vector>
>> +#include <unistd.h>
>> +
>> +#ifdef __WIN32__
>> +#include <windows.h>
>> +#define dlopen(name, mode) LoadLibrary (name)
>> +#define dlclose(handle) FreeLibrary (handle)
>> +#define dlerror() "an error occurred"
>> +#else
>> +#include <dlfcn.h>
>> +#endif
>> +
>> +int
>> +main ()
>> +{
>> +  /* Read the shared libraries contents into a buffer.  */
>> +  std::ifstream read_so_file = std::ifstream (SHLIB_NAME);
>> +  read_so_file.seekg (0, std::ios::end);
>> +  std::streamsize size = read_so_file.tellg ();
>> +  read_so_file.seekg (0, std::ios::beg);
>> +  std::vector<char> buffer (size);
>> +  if (!read_so_file.read (buffer.data (), size))
>> +    {
>> +      fprintf (stderr, "Failed to load solib\n");
>> +      exit (1);
>> +    }
>> +
>> +  /* Create a memory mapped file, then write the shared library to that
>> +     new memory mapped file.  */
>> +  int mem_fd = memfd_create ("test", 0);
>> +  write (mem_fd, buffer.data (), buffer.size ());
>> +
>> +  /* Generate the /proc/self/fd/[num] path for the memory mapped file.  */
>> +  std::string proc_self_fd_path;		/* break-here */
>> +  std::stringstream proc_self_fd_path_stream
>> +    = std::stringstream (proc_self_fd_path);
>> +  proc_self_fd_path_stream << "/proc/self/fd/" << mem_fd;
>> +
>> +  /* Call dlopen on it.  */
>> +  void *handle = dlopen (proc_self_fd_path_stream.str ().c_str (), RTLD_LAZY);
>> +  if (!handle)
>> +    {
>> +      fprintf (stderr, "%s\n", dlerror ());
>> +      exit (1);
>> +    }
>> +  /* It worked.  */
>> +  dlclose (handle);
>> +
>> +  return 0;
>> +}
>> diff --git a/gdb/testsuite/gdb.base/solib-proc-self.exp b/gdb/testsuite/gdb.base/solib-proc-self.exp
>> new file mode 100644
>> index 00000000000..1c845822490
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/solib-proc-self.exp
>> @@ -0,0 +1,130 @@
>> +# Copyright 2022 Free Software Foundation, Inc.
>> +
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 3 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see<http://www.gnu.org/licenses/>.  */
>> +
>> +# Test connecting and disconnecting at shared library events.
>> +
>> +if {[skip_shlib_tests]} {
>> +    untested "could not run to main"
>> +    return 0
>> +}
>> +
>> +standard_testfile .cc
>> +
>> +# Reuse an existing library, we don't care about the library contents
>> +# for this test.
>> +set libfile so-disc-shr
>> +set libsrc "${srcdir}/${subdir}/${libfile}.c"
>> +set libname "${libfile}.so"
>> +set libobj [standard_output_file ${libname}]
>> +
>> +# Compile the shared library.
>> +if { [gdb_compile_shlib $libsrc $libobj {debug}] != ""} {
>> +    return -1
>> +}
>> +
>> +# Compile the test executable.
>> +if [ build_executable "failed to prepare" $testfile $srcfile \
>> +	 [list shlib_load debug c++ additional_flags=-DSHLIB_NAME="${libobj}"]] {
>> +    return -1
>> +}
>> +
>> +# Start GDB and run to the point where the test program tries to dlopen a file
>> +# from within /proc/self/fd/.  Catch the shared library event and check that
>> +# we actually try to load a file from /proc/<INFERIOR-PID>/fd/.
>> +#
>> +# If SYSROOT is not the empty string, then this is set as the value of GDB's
>> +# sysroot immediately after starting GDB.  The only value that is (currently)
>> +# supported, other than the empty string, is 'target:'.
>> +proc do_test { {sysroot ""} } {
>> +    clean_restart $::binfile
>> +
>> +    if {$sysroot != ""} {
>> +	gdb_test_no_output "set sysroot ${sysroot}"
>> +    }
>> +
>> +    gdb_load_shlib $::libobj
>> +
>> +    if ![runto_main] then {
>> +	return 0
>> +    }
>> +
>> +    # Get inferior's PID for later.
>> +    set inferior_pid [get_inferior_pid]
>> +
>> +    # Turn on the solib-events so we can see that gdb resolves everything
>> +    # correctly.
>> +    gdb_test_no_output "set stop-on-solib-events 1"
>> +
>> +    # Run to the 'break-here' marker.
>> +    gdb_breakpoint [gdb_get_line_number "break-here"]
>> +    gdb_continue_to_breakpoint "break-here" ".* break-here .*"
>> +
>> +    set memfd ""
>> +    gdb_test_multiple "p mem_fd" "Get file descriptor" {
>> +	-re -wrap "\\\$$::decimal = (\[^\r\n\]*)" {
>> +	    set memfd $expect_out(1,string)
>> +	    pass $gdb_test_name
>> +	}
>> +    }
>> +
>> +    gdb_test "continue" "Stopped due to shared library event.*" \
>> +	"continue to load"
>
> I've been running this patch (as modified by Andrew) and see a failure 
> in the tests.
>
> (gdb) PASS: gdb.base/solib-proc-self.exp: continue to breakpoint: break-here
> p mem_fd
> $1 = 4
> (gdb) PASS: gdb.base/solib-proc-self.exp: Get file descriptor
> continue
> Continuing.
> Stopped due to shared library event:
>    Inferior loaded /proc/7331/fd/4
> (gdb) PASS: gdb.base/solib-proc-self.exp: continue to load
> continue
> Continuing.
> Stopped due to shared library event (no libraries added or removed)
> (gdb) FAIL: gdb.base/solib-proc-self.exp: continue
>
> I believe this "continue" should be removed.  The test passes cleanly 
> without it.

I looked at this failure a little, turns out, for some reason, I'm
seeing two solib stop events related to the dlopen call, here's what my
gdb.log looks like:

  ...
  (gdb) PASS: gdb.base/solib-proc-self.exp: continue to breakpoint: break-here
  p mem_fd
  $1 = 4
  (gdb) PASS: gdb.base/solib-proc-self.exp: Get file descriptor
  continue
  Continuing.
  Stopped due to shared library event (no libraries added or removed)
  (gdb) PASS: gdb.base/solib-proc-self.exp: continue to load
  continue
  Continuing.
  Stopped due to shared library event:
    Inferior loaded /proc/2891392/fd/4
  (gdb) PASS: gdb.base/solib-proc-self.exp: continue

Here's the inferior backtrace at the first (no libraries added or
removed) stop:

  #0  0x00007ffff7fd85da in _dl_map_object_from_fd () from /lib64/ld-linux-x86-64.so.2
  #1  0x00007ffff7fdb366 in _dl_map_object () from /lib64/ld-linux-x86-64.so.2
  #2  0x00007ffff7fe5d55 in dl_open_worker () from /lib64/ld-linux-x86-64.so.2
  #3  0x00007ffff7ba24f8 in _dl_catch_exception () from /lib64/libc.so.6
  #4  0x00007ffff7fe58fe in _dl_open () from /lib64/ld-linux-x86-64.so.2
  #5  0x00007ffff7f8c39c in dlopen_doit () from /lib64/libdl.so.2
  #6  0x00007ffff7ba24f8 in _dl_catch_exception () from /lib64/libc.so.6
  #7  0x00007ffff7ba25c3 in _dl_catch_error () from /lib64/libc.so.6
  #8  0x00007ffff7f8cb09 in _dlerror_run () from /lib64/libdl.so.2
  #9  0x00007ffff7f8c42a in dlopen@@GLIBC_2.2.5 () from /lib64/libdl.so.2
  #10 0x0000000000402555 in main () at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/solib-proc-self.cc:63

And the backtrace at the second stop:

  #0  0x00007ffff7fe60a8 in dl_open_worker () from /lib64/ld-linux-x86-64.so.2
  #1  0x00007ffff7ba24f8 in _dl_catch_exception () from /lib64/libc.so.6
  #2  0x00007ffff7fe58fe in _dl_open () from /lib64/ld-linux-x86-64.so.2
  #3  0x00007ffff7f8c39c in dlopen_doit () from /lib64/libdl.so.2
  #4  0x00007ffff7ba24f8 in _dl_catch_exception () from /lib64/libc.so.6
  #5  0x00007ffff7ba25c3 in _dl_catch_error () from /lib64/libc.so.6
  #6  0x00007ffff7f8cb09 in _dlerror_run () from /lib64/libdl.so.2
  #7  0x00007ffff7f8c42a in dlopen@@GLIBC_2.2.5 () from /lib64/libdl.so.2
  #8  0x0000000000402555 in main () at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/solib-proc-self.cc:63

I'm going to fold the two continue calls together into a
gdb_test_multiple, but I'll also dig into why I see two stop events,
maybe there's a GDB issue here.

Thanks,
Andrew

>
> Best,
> Lancelot.
>
>> +
>> +    # Check if inferior resolved the /proc/self/fd/[num] to /proc/[pid]/fd/[num].
>> +    gdb_test "continue" \
>> +	[multi_line \
>> +	     "Stopped due to shared library event:" \
>> +	     "  Inferior loaded (?:target:)?/proc/${inferior_pid}/fd/$memfd"]
>> +}
>> +
>> +# First run of the test.
>> +do_test
>> +
>> +# Possible second run of the test.  If we are using a remote target then we
>> +# should consider setting the sysroot to 'target:' and re-running the test.
>> +if {[target_info exists gdb_protocol]
>> +    && ([target_info gdb_protocol] == "remote"
>> +	|| [target_info gdb_protocol] == "extended-remote")} {
>> +    # GDB will already be running after the first call to do_test, so we can
>> +    # take a peek at the current sysroot setting, and decide if we should
>> +    # repeat the test with a different setting.
>> +
>> +    set new_sysroot ""
>> +    gdb_test_multiple "show sysroot" "" {
>> +	-wrap -re "The current system root is \"\"\\." {
>> +	    pass $gdb_test_name
>> +
>> +	    # Repeat the test with 'target:' sysroot.
>> +	    set new_sysroot "target:"
>> +	}
>> +	-wrap -re "The current system root is \"target:\"\\." {
>> +	    pass $gdb_test_name
>> +
>> +	    # Nothing else to do, we already tested with target: sysroot.
>> +	}
>> +	-re "$gdb_prompt $" {
>> +	    pass $gdb_test_name
>> +
>> +	    # If already testing with any other sysroot, we probably should
>> +	    # not try to adjust things, so don't do any further testing.
>> +	}
>> +    }
>> +
>> +    with_test_prefix "sysroot $new_sysroot" {
>> +	if { $new_sysroot != "" } {
>> +	    do_test $new_sysroot
>> +	}
>> +    }
>> +}
>>

[PATCHv4] gdb: handle loading shared libraries from /proc/self/fd/

[Andrew Burgess]

From: Asaf Fisher via Gdb-patches <gdb-patches@sourceware.org>

Lancelot,

I dug into why the interesting shared library event is sometime the
first stop, and sometimes the second stop.  I think it depends on
whether the stap probes based mechanism is used for handling shared
library events or not.

When the probes are not used then the library is reported at the first
event (though if you continue again there is another stop, this time
with no new libraries reported).  If you are using stap probes then
the library is reported at the second stop.

I was able to reproduce the behaviour when I use a different Fedora
version to my default.  What I haven't looked into yet is why one
install uses probes, but the other does not.  I'll take a look at that
tomorrow.

However, I don't think that matters for this patch, probes or
non-probes are both valid, so I've updated the test such that it
should handle both cases fine.

Changes since v3:

  - Fixed NULL/nullptr issues Lancelot pointed out,

  - Updated the test case to take account of non-probes based shared
    library event handling.

Thanks,
Andrew

---

Bug PR gdb/29586 describes a situations where a shared library is
created in memory, then written to a memory mapped file.  The memory
mapped file will show up as a file descriptor within /proc/self/fd/,
and this path is then used with dlopen in order to call functions
within the in-memory shared library.

When attempting to debug this GDB hangs.  The problem is that, GDB
stops at the shared-library event, reads the shared library path from
the inferior, which is "/proc/self/fd/<NUM>", and then GDB attempts to
open this file.

Unfortunately, this means GDB tries to open a file within GDB's
/proc/self/fd/ directory, not within the inferior's directory.  In the
case of our hang it turns out that the file descriptor that is opened
is a pipe, and GDB hangs trying to read from the pipe.

However, the behaviour is really just undefined, depending on which
file descriptor the inferior tries to open, GDB will open, or fail to
open, random files within its /proc/self/fd directory.

The solution proposed in this commit is to hook into solib_find_1, and
spot when GDB is looking for any file in /proc/self/, if this is the
case, then the filename is rewritten as /proc/<PID>, where <PID> is
the process-id of the current inferior.

The test passes for the unix, native-gdbserver, and
native-extended-gdbserver targets.  When testing with either of the
gdbserver targets, the test is run using the default empty sysroot,
and also using the 'target:' sysroot.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29586

Co-authored-by: Andrew Burgess <aburgess@redhat.com>
---
 gdb/solib.c                                |  38 ++++-
 gdb/testsuite/gdb.base/solib-proc-self.cc  |  73 +++++++++
 gdb/testsuite/gdb.base/solib-proc-self.exp | 175 +++++++++++++++++++++
 3 files changed, 282 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/solib-proc-self.cc
 create mode 100644 gdb/testsuite/gdb.base/solib-proc-self.exp

diff --git a/gdb/solib.c b/gdb/solib.c
index 7cfdd81114c..6aabcf0d68d 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -50,6 +50,7 @@
 #include "gdb_bfd.h"
 #include "gdbsupport/filestuff.h"
 #include "gdbsupport/scoped_fd.h"
+#include "gdbsupport/pathstuff.h"
 #include "debuginfod-support.h"
 #include "source.h"
 #include "cli/cli-style.h"
@@ -83,6 +84,34 @@ show_solib_search_path (struct ui_file *file, int from_tty,
 #  define DOS_BASED_FILE_SYSTEM 0
 #endif
 
+/* Fix references to files in /proc/self/fd/ when opening a shared library.
+
+   SO_NAME is the name of the shared library being loaded.  This function
+   returns a possibly modified name which should be used as the path to the
+   shared library.
+
+   If SO_NAME starts with /proc/self, then the returned name will be
+   modified to start with /proc/PID where 'PID' is the pid of the current
+   inferior.  */
+
+static gdb::unique_xmalloc_ptr<char>
+filter_proc_self_filenames (gdb::unique_xmalloc_ptr<char> so_name)
+{
+  static const char *proc_self_prefix = "/proc/self";
+
+  /* Is the path really a /proc/self?  */
+  if (!startswith (so_name.get (), proc_self_prefix))
+    return so_name;
+
+  /* Get the part of the path after /proc/self.  For example given
+     '/proc/self/fd' we find the '/fd' part.  */
+  const char *tail = so_name.get () + strlen (proc_self_prefix);
+
+  /* Build a replacement path.  */
+  int inferior_pid = inferior_ptid.pid ();
+  return xstrprintf ("/proc/%d%s", inferior_pid, tail);
+}
+
 /* Return the full pathname of a binary file (the main executable or a
    shared library file), or NULL if not found.  If FD is non-NULL, *FD
    is set to either -1 or an open file handle for the binary file.
@@ -172,6 +201,9 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
 	}
     }
 
+  temp_pathname.reset (xstrdup (in_pathname));
+  temp_pathname = filter_proc_self_filenames (std::move (temp_pathname));
+
   /* Note, we're interested in IS_TARGET_ABSOLUTE_PATH, not
      IS_ABSOLUTE_PATH.  The latter is for host paths only, while
      IN_PATHNAME is a target path.  For example, if we're supposed to
@@ -184,9 +216,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
        3rd attempt, c:/foo/bar.dll ==> /sysroot/foo/bar.dll
   */
 
-  if (!IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) || sysroot == NULL)
-    temp_pathname.reset (xstrdup (in_pathname));
-  else
+  if (IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) && sysroot != nullptr)
     {
       bool need_dir_separator;
 
@@ -213,7 +243,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
       /* Cat the prefixed pathname together.  */
       temp_pathname.reset (concat (sysroot,
 				   need_dir_separator ? SLASH_STRING : "",
-				   in_pathname, (char *) NULL));
+				   temp_pathname.get (), nullptr));
     }
 
   /* Handle files to be accessed via the target.  */
diff --git a/gdb/testsuite/gdb.base/solib-proc-self.cc b/gdb/testsuite/gdb.base/solib-proc-self.cc
new file mode 100644
index 00000000000..f2439d738a3
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-proc-self.cc
@@ -0,0 +1,73 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2022 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <sys/mman.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <iostream>
+#include <fstream>
+#include <sstream>
+#include <vector>
+#include <unistd.h>
+
+#ifdef __WIN32__
+#include <windows.h>
+#define dlopen(name, mode) LoadLibrary (name)
+#define dlclose(handle) FreeLibrary (handle)
+#define dlerror() "an error occurred"
+#else
+#include <dlfcn.h>
+#endif
+
+int
+main ()
+{
+  /* Read the shared libraries contents into a buffer.  */
+  std::ifstream read_so_file = std::ifstream (SHLIB_NAME);
+  read_so_file.seekg (0, std::ios::end);
+  std::streamsize size = read_so_file.tellg ();
+  read_so_file.seekg (0, std::ios::beg);
+  std::vector<char> buffer (size);
+  if (!read_so_file.read (buffer.data (), size))
+    {
+      fprintf (stderr, "Failed to load solib\n");
+      exit (1);
+    }
+
+  /* Create a memory mapped file, then write the shared library to that
+     new memory mapped file.  */
+  int mem_fd = memfd_create ("test", 0);
+  write (mem_fd, buffer.data (), buffer.size ());
+
+  /* Generate the /proc/self/fd/[num] path for the memory mapped file.  */
+  std::string proc_self_fd_path;		/* break-here */
+  std::stringstream proc_self_fd_path_stream
+    = std::stringstream (proc_self_fd_path);
+  proc_self_fd_path_stream << "/proc/self/fd/" << mem_fd;
+
+  /* Call dlopen on it.  */
+  void *handle = dlopen (proc_self_fd_path_stream.str ().c_str (), RTLD_LAZY);
+  if (!handle)
+    {
+      fprintf (stderr, "%s\n", dlerror ());
+      exit (1);
+    }
+  /* It worked.  */
+  dlclose (handle);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/solib-proc-self.exp b/gdb/testsuite/gdb.base/solib-proc-self.exp
new file mode 100644
index 00000000000..794a03556ff
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-proc-self.exp
@@ -0,0 +1,175 @@
+# Copyright 2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# Test connecting and disconnecting at shared library events.
+
+if {[skip_shlib_tests]} {
+    untested "could not run to main"
+    return 0
+}
+
+standard_testfile .cc
+
+# Reuse an existing library, we don't care about the library contents
+# for this test.
+set libfile so-disc-shr
+set libsrc "${srcdir}/${subdir}/${libfile}.c"
+set libname "${libfile}.so"
+set libobj [standard_output_file ${libname}]
+
+# Compile the shared library.
+if { [gdb_compile_shlib $libsrc $libobj {debug}] != ""} {
+    return -1
+}
+
+# Compile the test executable.
+if [ build_executable "failed to prepare" $testfile $srcfile \
+	 [list shlib_load debug c++ additional_flags=-DSHLIB_NAME="${libobj}"]] {
+    return -1
+}
+
+# Start GDB and run to the point where the test program tries to dlopen a file
+# from within /proc/self/fd/.  Catch the shared library event and check that
+# we actually try to load a file from /proc/<INFERIOR-PID>/fd/.
+#
+# If SYSROOT is not the empty string, then this is set as the value of GDB's
+# sysroot immediately after starting GDB.  The only value that is (currently)
+# supported, other than the empty string, is 'target:'.
+proc do_test { {sysroot ""} } {
+    clean_restart $::binfile
+
+    if {$sysroot != ""} {
+	gdb_test_no_output "set sysroot ${sysroot}"
+    }
+
+    gdb_load_shlib $::libobj
+
+    if ![runto_main] then {
+	return 0
+    }
+
+    # Get inferior's PID for later.
+    set inferior_pid [get_inferior_pid]
+
+    # Turn on the solib-events so we can see that gdb resolves everything
+    # correctly.
+    gdb_test_no_output "set stop-on-solib-events 1"
+
+    # Run to the 'break-here' marker.
+    gdb_breakpoint [gdb_get_line_number "break-here"]
+    gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+
+    set memfd ""
+    gdb_test_multiple "p mem_fd" "Get file descriptor" {
+	-re -wrap "\\\$$::decimal = (\[^\r\n\]*)" {
+	    set memfd $expect_out(1,string)
+	    pass $gdb_test_name
+	}
+    }
+
+    # Now continue forward until the solib event is detected, and
+    # check that the loaded library is found through the /proc/PID/fd
+    # rather than /proc/self/fd.
+    #
+    # We need to handle the possibility of the interesting event
+    # showing the first, or second time we stop, as depending on which
+    # mechanism GDB is using to handle the shared library events (the
+    # newer probes based interface, or the old non-probes interface),
+    # the library will be reported at the first or second stop.
+    set saw_expected_event false
+    set saw_no_event_stop false
+    gdb_test_multiple "continue" "continue to solib evnt" {
+	-re "^continue\r\n" {
+	    exp_continue
+	}
+
+	-re "^Continuing\\.\r\n" {
+	    exp_continue
+	}
+
+	-early -re "Stopped due to shared library event \\(no libraries added or removed\\)\r\n" {
+	    # This non-interesting event shows up first when using the
+	    # probes based mechanism for dealing with shared library
+	    # events.
+	    #
+	    # We set a flag here, and, once the prompt has appeared,
+	    # we send another continue, the next event will contain
+	    # the information we want.
+	    set saw_no_event_stop true
+	    exp_continue
+	}
+
+	-re "Stopped due to shared library event:\r\n  Inferior loaded (?:target:)?/proc/${inferior_pid}/fd/$memfd\r\n" {
+	    # This event, which includes the information we are
+	    # looking for, occurs first when using the non-probes
+	    # based mechanism for handling shared library events, and
+	    # occurs second when using the probes mechanism.
+	    #
+	    # Either way, record here that we say the output we expect.
+	    set saw_expected_event true
+	    exp_continue
+	}
+
+	-re "$::gdb_prompt $" {
+	    if {$saw_no_event_stop} {
+		set saw_no_event_stop false
+		send_gdb "continue\n"
+		exp_continue
+	    } else {
+		gdb_assert {$saw_expected_event} $gdb_test_name
+	    }
+	}
+    }
+}
+
+# First run of the test.
+do_test
+
+# Possible second run of the test.  If we are using a remote target then we
+# should consider setting the sysroot to 'target:' and re-running the test.
+if {[target_info exists gdb_protocol]
+    && ([target_info gdb_protocol] == "remote"
+	|| [target_info gdb_protocol] == "extended-remote")} {
+    # GDB will already be running after the first call to do_test, so we can
+    # take a peek at the current sysroot setting, and decide if we should
+    # repeat the test with a different setting.
+
+    set new_sysroot ""
+    gdb_test_multiple "show sysroot" "" {
+	-wrap -re "The current system root is \"\"\\." {
+	    pass $gdb_test_name
+
+	    # Repeat the test with 'target:' sysroot.
+	    set new_sysroot "target:"
+	}
+	-wrap -re "The current system root is \"target:\"\\." {
+	    pass $gdb_test_name
+
+	    # Nothing else to do, we already tested with target: sysroot.
+	}
+	-re "$gdb_prompt $" {
+	    pass $gdb_test_name
+
+	    # If already testing with any other sysroot, we probably should
+	    # not try to adjust things, so don't do any further testing.
+	}
+    }
+
+    with_test_prefix "sysroot $new_sysroot" {
+	if { $new_sysroot != "" } {
+	    do_test $new_sysroot
+	}
+    }
+}

base-commit: 84f9fbe90e5429adb9dee68f04f44c92fa9e2345
-- 
2.25.4

Re: [PATCHv4] gdb: handle loading shared libraries from /proc/self/fd/

[Andrew Burgess]

Ping!

Anyone else have any thoughts on this change?

Thanks,
Andrew

Andrew Burgess <aburgess@redhat.com> writes:

> From: Asaf Fisher via Gdb-patches <gdb-patches@sourceware.org>
>
> Lancelot,
>
> I dug into why the interesting shared library event is sometime the
> first stop, and sometimes the second stop.  I think it depends on
> whether the stap probes based mechanism is used for handling shared
> library events or not.
>
> When the probes are not used then the library is reported at the first
> event (though if you continue again there is another stop, this time
> with no new libraries reported).  If you are using stap probes then
> the library is reported at the second stop.
>
> I was able to reproduce the behaviour when I use a different Fedora
> version to my default.  What I haven't looked into yet is why one
> install uses probes, but the other does not.  I'll take a look at that
> tomorrow.
>
> However, I don't think that matters for this patch, probes or
> non-probes are both valid, so I've updated the test such that it
> should handle both cases fine.
>
> Changes since v3:
>
>   - Fixed NULL/nullptr issues Lancelot pointed out,
>
>   - Updated the test case to take account of non-probes based shared
>     library event handling.
>
> Thanks,
> Andrew
>
> ---
>
> Bug PR gdb/29586 describes a situations where a shared library is
> created in memory, then written to a memory mapped file.  The memory
> mapped file will show up as a file descriptor within /proc/self/fd/,
> and this path is then used with dlopen in order to call functions
> within the in-memory shared library.
>
> When attempting to debug this GDB hangs.  The problem is that, GDB
> stops at the shared-library event, reads the shared library path from
> the inferior, which is "/proc/self/fd/<NUM>", and then GDB attempts to
> open this file.
>
> Unfortunately, this means GDB tries to open a file within GDB's
> /proc/self/fd/ directory, not within the inferior's directory.  In the
> case of our hang it turns out that the file descriptor that is opened
> is a pipe, and GDB hangs trying to read from the pipe.
>
> However, the behaviour is really just undefined, depending on which
> file descriptor the inferior tries to open, GDB will open, or fail to
> open, random files within its /proc/self/fd directory.
>
> The solution proposed in this commit is to hook into solib_find_1, and
> spot when GDB is looking for any file in /proc/self/, if this is the
> case, then the filename is rewritten as /proc/<PID>, where <PID> is
> the process-id of the current inferior.
>
> The test passes for the unix, native-gdbserver, and
> native-extended-gdbserver targets.  When testing with either of the
> gdbserver targets, the test is run using the default empty sysroot,
> and also using the 'target:' sysroot.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29586
>
> Co-authored-by: Andrew Burgess <aburgess@redhat.com>
> ---
>  gdb/solib.c                                |  38 ++++-
>  gdb/testsuite/gdb.base/solib-proc-self.cc  |  73 +++++++++
>  gdb/testsuite/gdb.base/solib-proc-self.exp | 175 +++++++++++++++++++++
>  3 files changed, 282 insertions(+), 4 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/solib-proc-self.cc
>  create mode 100644 gdb/testsuite/gdb.base/solib-proc-self.exp
>
> diff --git a/gdb/solib.c b/gdb/solib.c
> index 7cfdd81114c..6aabcf0d68d 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -50,6 +50,7 @@
>  #include "gdb_bfd.h"
>  #include "gdbsupport/filestuff.h"
>  #include "gdbsupport/scoped_fd.h"
> +#include "gdbsupport/pathstuff.h"
>  #include "debuginfod-support.h"
>  #include "source.h"
>  #include "cli/cli-style.h"
> @@ -83,6 +84,34 @@ show_solib_search_path (struct ui_file *file, int from_tty,
>  #  define DOS_BASED_FILE_SYSTEM 0
>  #endif
>  
> +/* Fix references to files in /proc/self/fd/ when opening a shared library.
> +
> +   SO_NAME is the name of the shared library being loaded.  This function
> +   returns a possibly modified name which should be used as the path to the
> +   shared library.
> +
> +   If SO_NAME starts with /proc/self, then the returned name will be
> +   modified to start with /proc/PID where 'PID' is the pid of the current
> +   inferior.  */
> +
> +static gdb::unique_xmalloc_ptr<char>
> +filter_proc_self_filenames (gdb::unique_xmalloc_ptr<char> so_name)
> +{
> +  static const char *proc_self_prefix = "/proc/self";
> +
> +  /* Is the path really a /proc/self?  */
> +  if (!startswith (so_name.get (), proc_self_prefix))
> +    return so_name;
> +
> +  /* Get the part of the path after /proc/self.  For example given
> +     '/proc/self/fd' we find the '/fd' part.  */
> +  const char *tail = so_name.get () + strlen (proc_self_prefix);
> +
> +  /* Build a replacement path.  */
> +  int inferior_pid = inferior_ptid.pid ();
> +  return xstrprintf ("/proc/%d%s", inferior_pid, tail);
> +}
> +
>  /* Return the full pathname of a binary file (the main executable or a
>     shared library file), or NULL if not found.  If FD is non-NULL, *FD
>     is set to either -1 or an open file handle for the binary file.
> @@ -172,6 +201,9 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
>  	}
>      }
>  
> +  temp_pathname.reset (xstrdup (in_pathname));
> +  temp_pathname = filter_proc_self_filenames (std::move (temp_pathname));
> +
>    /* Note, we're interested in IS_TARGET_ABSOLUTE_PATH, not
>       IS_ABSOLUTE_PATH.  The latter is for host paths only, while
>       IN_PATHNAME is a target path.  For example, if we're supposed to
> @@ -184,9 +216,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
>         3rd attempt, c:/foo/bar.dll ==> /sysroot/foo/bar.dll
>    */
>  
> -  if (!IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) || sysroot == NULL)
> -    temp_pathname.reset (xstrdup (in_pathname));
> -  else
> +  if (IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) && sysroot != nullptr)
>      {
>        bool need_dir_separator;
>  
> @@ -213,7 +243,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
>        /* Cat the prefixed pathname together.  */
>        temp_pathname.reset (concat (sysroot,
>  				   need_dir_separator ? SLASH_STRING : "",
> -				   in_pathname, (char *) NULL));
> +				   temp_pathname.get (), nullptr));
>      }
>  
>    /* Handle files to be accessed via the target.  */
> diff --git a/gdb/testsuite/gdb.base/solib-proc-self.cc b/gdb/testsuite/gdb.base/solib-proc-self.cc
> new file mode 100644
> index 00000000000..f2439d738a3
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/solib-proc-self.cc
> @@ -0,0 +1,73 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2022 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <sys/mman.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <iostream>
> +#include <fstream>
> +#include <sstream>
> +#include <vector>
> +#include <unistd.h>
> +
> +#ifdef __WIN32__
> +#include <windows.h>
> +#define dlopen(name, mode) LoadLibrary (name)
> +#define dlclose(handle) FreeLibrary (handle)
> +#define dlerror() "an error occurred"
> +#else
> +#include <dlfcn.h>
> +#endif
> +
> +int
> +main ()
> +{
> +  /* Read the shared libraries contents into a buffer.  */
> +  std::ifstream read_so_file = std::ifstream (SHLIB_NAME);
> +  read_so_file.seekg (0, std::ios::end);
> +  std::streamsize size = read_so_file.tellg ();
> +  read_so_file.seekg (0, std::ios::beg);
> +  std::vector<char> buffer (size);
> +  if (!read_so_file.read (buffer.data (), size))
> +    {
> +      fprintf (stderr, "Failed to load solib\n");
> +      exit (1);
> +    }
> +
> +  /* Create a memory mapped file, then write the shared library to that
> +     new memory mapped file.  */
> +  int mem_fd = memfd_create ("test", 0);
> +  write (mem_fd, buffer.data (), buffer.size ());
> +
> +  /* Generate the /proc/self/fd/[num] path for the memory mapped file.  */
> +  std::string proc_self_fd_path;		/* break-here */
> +  std::stringstream proc_self_fd_path_stream
> +    = std::stringstream (proc_self_fd_path);
> +  proc_self_fd_path_stream << "/proc/self/fd/" << mem_fd;
> +
> +  /* Call dlopen on it.  */
> +  void *handle = dlopen (proc_self_fd_path_stream.str ().c_str (), RTLD_LAZY);
> +  if (!handle)
> +    {
> +      fprintf (stderr, "%s\n", dlerror ());
> +      exit (1);
> +    }
> +  /* It worked.  */
> +  dlclose (handle);
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/solib-proc-self.exp b/gdb/testsuite/gdb.base/solib-proc-self.exp
> new file mode 100644
> index 00000000000..794a03556ff
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/solib-proc-self.exp
> @@ -0,0 +1,175 @@
> +# Copyright 2022 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +# Test connecting and disconnecting at shared library events.
> +
> +if {[skip_shlib_tests]} {
> +    untested "could not run to main"
> +    return 0
> +}
> +
> +standard_testfile .cc
> +
> +# Reuse an existing library, we don't care about the library contents
> +# for this test.
> +set libfile so-disc-shr
> +set libsrc "${srcdir}/${subdir}/${libfile}.c"
> +set libname "${libfile}.so"
> +set libobj [standard_output_file ${libname}]
> +
> +# Compile the shared library.
> +if { [gdb_compile_shlib $libsrc $libobj {debug}] != ""} {
> +    return -1
> +}
> +
> +# Compile the test executable.
> +if [ build_executable "failed to prepare" $testfile $srcfile \
> +	 [list shlib_load debug c++ additional_flags=-DSHLIB_NAME="${libobj}"]] {
> +    return -1
> +}
> +
> +# Start GDB and run to the point where the test program tries to dlopen a file
> +# from within /proc/self/fd/.  Catch the shared library event and check that
> +# we actually try to load a file from /proc/<INFERIOR-PID>/fd/.
> +#
> +# If SYSROOT is not the empty string, then this is set as the value of GDB's
> +# sysroot immediately after starting GDB.  The only value that is (currently)
> +# supported, other than the empty string, is 'target:'.
> +proc do_test { {sysroot ""} } {
> +    clean_restart $::binfile
> +
> +    if {$sysroot != ""} {
> +	gdb_test_no_output "set sysroot ${sysroot}"
> +    }
> +
> +    gdb_load_shlib $::libobj
> +
> +    if ![runto_main] then {
> +	return 0
> +    }
> +
> +    # Get inferior's PID for later.
> +    set inferior_pid [get_inferior_pid]
> +
> +    # Turn on the solib-events so we can see that gdb resolves everything
> +    # correctly.
> +    gdb_test_no_output "set stop-on-solib-events 1"
> +
> +    # Run to the 'break-here' marker.
> +    gdb_breakpoint [gdb_get_line_number "break-here"]
> +    gdb_continue_to_breakpoint "break-here" ".* break-here .*"
> +
> +    set memfd ""
> +    gdb_test_multiple "p mem_fd" "Get file descriptor" {
> +	-re -wrap "\\\$$::decimal = (\[^\r\n\]*)" {
> +	    set memfd $expect_out(1,string)
> +	    pass $gdb_test_name
> +	}
> +    }
> +
> +    # Now continue forward until the solib event is detected, and
> +    # check that the loaded library is found through the /proc/PID/fd
> +    # rather than /proc/self/fd.
> +    #
> +    # We need to handle the possibility of the interesting event
> +    # showing the first, or second time we stop, as depending on which
> +    # mechanism GDB is using to handle the shared library events (the
> +    # newer probes based interface, or the old non-probes interface),
> +    # the library will be reported at the first or second stop.
> +    set saw_expected_event false
> +    set saw_no_event_stop false
> +    gdb_test_multiple "continue" "continue to solib evnt" {
> +	-re "^continue\r\n" {
> +	    exp_continue
> +	}
> +
> +	-re "^Continuing\\.\r\n" {
> +	    exp_continue
> +	}
> +
> +	-early -re "Stopped due to shared library event \\(no libraries added or removed\\)\r\n" {
> +	    # This non-interesting event shows up first when using the
> +	    # probes based mechanism for dealing with shared library
> +	    # events.
> +	    #
> +	    # We set a flag here, and, once the prompt has appeared,
> +	    # we send another continue, the next event will contain
> +	    # the information we want.
> +	    set saw_no_event_stop true
> +	    exp_continue
> +	}
> +
> +	-re "Stopped due to shared library event:\r\n  Inferior loaded (?:target:)?/proc/${inferior_pid}/fd/$memfd\r\n" {
> +	    # This event, which includes the information we are
> +	    # looking for, occurs first when using the non-probes
> +	    # based mechanism for handling shared library events, and
> +	    # occurs second when using the probes mechanism.
> +	    #
> +	    # Either way, record here that we say the output we expect.
> +	    set saw_expected_event true
> +	    exp_continue
> +	}
> +
> +	-re "$::gdb_prompt $" {
> +	    if {$saw_no_event_stop} {
> +		set saw_no_event_stop false
> +		send_gdb "continue\n"
> +		exp_continue
> +	    } else {
> +		gdb_assert {$saw_expected_event} $gdb_test_name
> +	    }
> +	}
> +    }
> +}
> +
> +# First run of the test.
> +do_test
> +
> +# Possible second run of the test.  If we are using a remote target then we
> +# should consider setting the sysroot to 'target:' and re-running the test.
> +if {[target_info exists gdb_protocol]
> +    && ([target_info gdb_protocol] == "remote"
> +	|| [target_info gdb_protocol] == "extended-remote")} {
> +    # GDB will already be running after the first call to do_test, so we can
> +    # take a peek at the current sysroot setting, and decide if we should
> +    # repeat the test with a different setting.
> +
> +    set new_sysroot ""
> +    gdb_test_multiple "show sysroot" "" {
> +	-wrap -re "The current system root is \"\"\\." {
> +	    pass $gdb_test_name
> +
> +	    # Repeat the test with 'target:' sysroot.
> +	    set new_sysroot "target:"
> +	}
> +	-wrap -re "The current system root is \"target:\"\\." {
> +	    pass $gdb_test_name
> +
> +	    # Nothing else to do, we already tested with target: sysroot.
> +	}
> +	-re "$gdb_prompt $" {
> +	    pass $gdb_test_name
> +
> +	    # If already testing with any other sysroot, we probably should
> +	    # not try to adjust things, so don't do any further testing.
> +	}
> +    }
> +
> +    with_test_prefix "sysroot $new_sysroot" {
> +	if { $new_sysroot != "" } {
> +	    do_test $new_sysroot
> +	}
> +    }
> +}
>
> base-commit: 84f9fbe90e5429adb9dee68f04f44c92fa9e2345
> -- 
> 2.25.4

[PATCHv5] gdb: handle loading shared libraries from /proc/self/fd/

[Andrew Burgess]

Asaf pointed out to me (off list) that v4 fails for some cases that the
original patch supported, so here's a v5.  Specifically, if the dlopen
call is passed a non-canonical path then GDB will fail to correctly spot
that the file being opened is within /proc/self, and will still try to
open a file within GDB's /proc/self by mistake.

Changes since v4:

  - filter_proc_self_filenames now makes use of gdb_realpath_keepfile to
    (partially) canonicalize the path, this should fix the non-canonical
    path issues,

  - extended the test to include the non-canonical path case.

Changes since v3:

  - Fixed NULL/nullptr issues Lancelot pointed out,

  - Updated the test case to take account of non-probes based shared
    library event handling.

---

commit 65c1f020184e632a6de1775c00de7e0d7a97def3
Author: Asaf Fisher via Gdb-patches <gdb-patches@sourceware.org>
Date:   Fri Oct 21 17:42:05 2022 +0000

    gdb: handle loading shared libraries from /proc/self/fd/
    
    Bug PR gdb/29586 describes a situations where a shared library is
    created in memory, then written to a memory mapped file.  The memory
    mapped file will show up as a file descriptor within /proc/self/fd/,
    and this path is then used with dlopen in order to call functions
    within the in-memory shared library.
    
    When attempting to debug this GDB hangs.  The problem is that, GDB
    stops at the shared-library event, reads the shared library path from
    the inferior, which is "/proc/self/fd/<NUM>", and then GDB attempts to
    open this file.
    
    Unfortunately, this means GDB tries to open a file within GDB's
    /proc/self/fd/ directory, not within the inferior's directory.  In the
    case of our hang it turns out that the file descriptor that is opened
    is a pipe, and GDB hangs trying to read from the pipe.
    
    However, the behaviour is really just undefined, depending on which
    file descriptor the inferior tries to open, GDB will open, or fail to
    open, random files within its /proc/self/fd directory.
    
    The solution proposed in this commit is to hook into solib_find_1, and
    spot when GDB is looking for any file in /proc/self/, if this is the
    case, then the filename is rewritten as /proc/<PID>, where <PID> is
    the process-id of the current inferior.
    
    One complexity that we need to consider is how to handle non-canonical
    paths, for example, if the user opens /proc/../proc/self/fd/<NUM> then
    GDB also needs to be able to spot this.
    
    My solution is to call gdb_realpath_keepfile, this canonicalizes
    everything except the filename (the '/proc/../proc/self/fd' part in
    the above example).  The problem this then introduces is that the
    'self' is replaces with the pid of GDB itself.  I therefore check,
    after canonicalization, for a prefix of /proc/<PID OF GDB>, and if
    this is the case, I replace this with /proc/<PID OF INFERIOR>.
    
    There is one remaining issue, which I consider very unlikely, and I
    don't propose to fix.  What if the inferior actually did try to dlopen
    a file from /proc/<PID OF GDB>?  If this happened then GDB will not be
    able to distinguish this from the above case and would still replace
    the pid of GDB with the pid of the inferior.  However, this seems
    really unlikely - why would the inferior be trying to load parts of
    GDB?  So I propose to ignore this case until someone can come up with
    an actual scenario where that's useful.
    
    The test passes for the unix, native-gdbserver, and
    native-extended-gdbserver targets.  When testing with either of the
    gdbserver targets, the test is run using the default empty sysroot,
    and also using the 'target:' sysroot.
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29586
    
    Co-authored-by: Andrew Burgess <aburgess@redhat.com>

diff --git a/gdb/solib.c b/gdb/solib.c
index 59fd866b652..a627d217c85 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -50,6 +50,7 @@
 #include "gdb_bfd.h"
 #include "gdbsupport/filestuff.h"
 #include "gdbsupport/scoped_fd.h"
+#include "gdbsupport/pathstuff.h"
 #include "debuginfod-support.h"
 #include "source.h"
 #include "cli/cli-style.h"
@@ -79,6 +80,54 @@ show_solib_search_path (struct ui_file *file, int from_tty,
 #  define DOS_BASED_FILE_SYSTEM 0
 #endif
 
+/* Fix references to files in /proc/self/fd/ when opening a shared library.
+
+   SO_NAME is the name of the shared library being loaded.  This function
+   returns a possibly modified name which should be used as the path to the
+   shared library.
+
+   If SO_NAME starts with /proc/self, then the returned name will be
+   modified to start with /proc/PID where 'PID' is the pid of the current
+   inferior.  */
+
+static gdb::unique_xmalloc_ptr<char>
+filter_proc_self_filenames (gdb::unique_xmalloc_ptr<char> so_name)
+{
+  /* In order to figure out if SO_NAME points at a file in /proc/self we
+     need to canonicalize the path.  However, the whole point here is that
+     if SO_NAME does point at /proc/self then GDB will think this is its
+     self, while the inferior actually means its /proc/self.  A result of
+     this is that the file being referenced within the inferior's
+     /proc/self, might not exist within GDB's /proc/self, a call to
+     gdb_realpath would then fail.
+
+     To avoid this problem we call gdb_realpath_keepfile, this only tries
+     to canonicalize the basename of SO_NAME, and leaves the final filename
+     untouched.  */
+  so_name
+    = make_unique_xstrdup (gdb_realpath_keepfile (so_name.get ()).c_str ());
+
+  /* A result of the above canonicalization is that /proc/self will have
+     been replaced with /proc/<pid of gdb>, so that's what we need to check
+     for.  Of course, it could be the case that the inferior really did try
+     to dlopen a file within GDB's /proc/<pid> directory, in which case
+     we're going to do the wrong thing here, but that seems far less likely
+     than calling dlopen on a file within the inferior's own directory.  */
+  std::string prefix = string_printf ("/proc/%ld", (long) getpid ());
+
+  /* Is the canonical path inside GDB's /proc/<pid> directory?  */
+  if (!startswith (so_name.get (), prefix.c_str ()))
+    return so_name;
+
+  /* Get the part of the path after /proc/<pid>.  For example given
+     '/proc/123/fd' we find the '/fd' part.  */
+  const char *tail = so_name.get () + strlen (prefix.c_str ());
+
+  /* Build a replacement path within the inferiors directory.  */
+  int inferior_pid = inferior_ptid.pid ();
+  return xstrprintf ("/proc/%d%s", inferior_pid, tail);
+}
+
 /* Return the full pathname of a binary file (the main executable or a
    shared library file), or NULL if not found.  If FD is non-NULL, *FD
    is set to either -1 or an open file handle for the binary file.
@@ -168,6 +217,9 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
 	}
     }
 
+  temp_pathname.reset (xstrdup (in_pathname));
+  temp_pathname = filter_proc_self_filenames (std::move (temp_pathname));
+
   /* Note, we're interested in IS_TARGET_ABSOLUTE_PATH, not
      IS_ABSOLUTE_PATH.  The latter is for host paths only, while
      IN_PATHNAME is a target path.  For example, if we're supposed to
@@ -180,9 +232,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
        3rd attempt, c:/foo/bar.dll ==> /sysroot/foo/bar.dll
   */
 
-  if (!IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) || sysroot == NULL)
-    temp_pathname.reset (xstrdup (in_pathname));
-  else
+  if (IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) && sysroot != nullptr)
     {
       bool need_dir_separator;
 
@@ -209,7 +259,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
       /* Cat the prefixed pathname together.  */
       temp_pathname.reset (concat (sysroot,
 				   need_dir_separator ? SLASH_STRING : "",
-				   in_pathname, (char *) NULL));
+				   temp_pathname.get (), nullptr));
     }
 
   /* Handle files to be accessed via the target.  */
diff --git a/gdb/testsuite/gdb.base/solib-proc-self.cc b/gdb/testsuite/gdb.base/solib-proc-self.cc
new file mode 100644
index 00000000000..aea397a66f9
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-proc-self.cc
@@ -0,0 +1,89 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2022 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <sys/mman.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <iostream>
+#include <fstream>
+#include <sstream>
+#include <vector>
+#include <unistd.h>
+
+#ifdef __WIN32__
+#include <windows.h>
+#define dlopen(name, mode) LoadLibrary (name)
+#define dlclose(handle) FreeLibrary (handle)
+#define dlerror() "an error occurred"
+#else
+#include <dlfcn.h>
+#endif
+
+/* Combine PREFIX and NUM into a single string, joined with a '/', and
+   return the string.  */
+
+std::string
+make_library_path (const char *prefix, int num)
+{
+  std::stringstream stream;
+  stream << prefix << "/" << num;
+  return stream.str ();
+}
+
+/* Call dlopen on the library pointed to by FILENAME.  */
+
+void
+open_library (const std::string &filename)
+{
+  /* Call dlopen on FILENAME.  */
+  void *handle = dlopen (filename.c_str (), RTLD_LAZY);
+  if (handle == nullptr)
+    abort ();
+
+  /* It worked.  */
+  dlclose (handle);
+}
+
+int
+main ()
+{
+  /* Read the shared libraries contents into a buffer.  */
+  std::ifstream read_so_file = std::ifstream (SHLIB_NAME);
+  read_so_file.seekg (0, std::ios::end);
+  std::streamsize size = read_so_file.tellg ();
+  read_so_file.seekg (0, std::ios::beg);
+  std::vector<char> buffer (size);
+  if (!read_so_file.read (buffer.data (), size))
+    abort ();
+
+  /* Create a memory mapped file, then write the shared library to that
+     new memory mapped file.  */
+  int mem_fd = memfd_create ("test", 0);
+  write (mem_fd, buffer.data (), buffer.size ());
+
+  /* Generate a canonical /proc/self/fd/[num] path for the memory mapped
+     file, and call dlopen on it.  */
+  std::string filename
+    = make_library_path ("/proc/self/fd", mem_fd);	/* break-here */
+  open_library (filename);
+
+  /* Now generate a new, non-canonical filename, and call dlopen on it.  */
+  filename = make_library_path ("/proc/../proc/self/fd", mem_fd);
+  open_library (filename);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/solib-proc-self.exp b/gdb/testsuite/gdb.base/solib-proc-self.exp
new file mode 100644
index 00000000000..01e7eecfb16
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-proc-self.exp
@@ -0,0 +1,200 @@
+# Copyright 2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# Test connecting and disconnecting at shared library events.
+
+if {[skip_shlib_tests]} {
+    untested "could not run to main"
+    return 0
+}
+
+standard_testfile .cc
+
+# Reuse an existing library, we don't care about the library contents
+# for this test.
+set libfile so-disc-shr
+set libsrc "${srcdir}/${subdir}/${libfile}.c"
+set libname "${libfile}.so"
+set libobj [standard_output_file ${libname}]
+
+# Compile the shared library.
+if { [gdb_compile_shlib $libsrc $libobj {debug}] != ""} {
+    return -1
+}
+
+# Compile the test executable.
+if [ build_executable "failed to prepare" $testfile $srcfile \
+	 [list shlib_load debug c++ additional_flags=-DSHLIB_NAME="${libobj}"]] {
+    return -1
+}
+
+proc validate_library_load {inferior_pid memfd} {
+
+    # Turn on the solib-events so we can see that gdb resolves
+    # everything correctly.
+    gdb_test_no_output "set stop-on-solib-events 1"
+
+    # Now continue forward until the solib event is detected, and
+    # check that the loaded library is found through the /proc/PID/fd
+    # rather than /proc/self/fd.
+    #
+    # We need to handle the possibility of the interesting event
+    # showing the first, or second time we stop, as depending on which
+    # mechanism GDB is using to handle the shared library events (the
+    # newer probes based interface, or the old non-probes interface),
+    # the library will be reported at the first or second stop.
+    set saw_expected_event false
+    set saw_no_event_stop false
+    gdb_test_multiple "continue" "continue to solib evnt" {
+	-re "^continue\r\n" {
+	    exp_continue
+	}
+
+	-re "^Continuing\\.\r\n" {
+	    exp_continue
+	}
+
+	-early -re "Stopped due to shared library event \\(no libraries added or removed\\)\r\n" {
+	    # This non-interesting event shows up first when using the
+	    # probes based mechanism for dealing with shared library
+	    # events.
+	    #
+	    # We set a flag here, and, once the prompt has appeared,
+	    # we send another continue, the next event will contain
+	    # the information we want.
+	    set saw_no_event_stop true
+	    exp_continue
+	}
+
+	-re "Stopped due to shared library event:\r\n  Inferior loaded (?:target:)?/proc/${inferior_pid}/fd/$memfd\r\n" {
+	    # This event, which includes the information we are
+	    # looking for, occurs first when using the non-probes
+	    # based mechanism for handling shared library events, and
+	    # occurs second when using the probes mechanism.
+	    #
+	    # Either way, record here that we say the output we expect.
+	    set saw_expected_event true
+	    exp_continue
+	}
+
+	-re "$::gdb_prompt $" {
+	    if {$saw_no_event_stop} {
+		set saw_no_event_stop false
+		send_gdb "continue\n"
+		exp_continue
+	    } else {
+		gdb_assert {$saw_expected_event} $gdb_test_name
+	    }
+	}
+    }
+
+    # Turn off solib events.  We're only interested in validating the
+    # loads for now.
+    gdb_test_no_output "set stop-on-solib-events 0"
+}
+
+# Start GDB and run to the point where the test program tries to dlopen a file
+# from within /proc/self/fd/.  Catch the shared library event and check that
+# we actually try to load a file from /proc/<INFERIOR-PID>/fd/.
+#
+# If SYSROOT is not the empty string, then this is set as the value of GDB's
+# sysroot immediately after starting GDB.  The only value that is (currently)
+# supported, other than the empty string, is 'target:'.
+proc do_test { {sysroot ""} } {
+    clean_restart $::binfile
+
+    if {$sysroot != ""} {
+	gdb_test_no_output "set sysroot ${sysroot}"
+    }
+
+    gdb_load_shlib $::libobj
+
+    if ![runto_main] then {
+	return 0
+    }
+
+    # Get inferior's PID for later.
+    set inferior_pid [get_inferior_pid]
+
+    # Run to the 'break-here' marker.
+    gdb_breakpoint [gdb_get_line_number "break-here"]
+    gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+
+    set memfd ""
+    gdb_test_multiple "p mem_fd" "Get file descriptor" {
+	-re -wrap "\\\$$::decimal = (\[^\r\n\]*)" {
+	    set memfd $expect_out(1,string)
+	    pass $gdb_test_name
+	}
+    }
+
+    # The first call to open_library (in the test program) is done
+    # with a canonical path.
+    with_test_prefix "canonical path" {
+	gdb_breakpoint "open_library"
+	gdb_continue_to_breakpoint "open_library"
+	validate_library_load $inferior_pid $memfd
+    }
+
+    # The second call to open_library (in the test program) is done
+    # with a non-canonical path, however, GDB should resolve this to a
+    # canonical path for display to the user, so the output we see
+    # should be unchanged.
+    with_test_prefix "non-canonical path" {
+	gdb_breakpoint "open_library"
+	gdb_continue_to_breakpoint "open_library"
+	validate_library_load $inferior_pid $memfd
+    }
+}
+
+# First run of the test.
+do_test
+
+# Possible second run of the test.  If we are using a remote target then we
+# should consider setting the sysroot to 'target:' and re-running the test.
+if {[target_info exists gdb_protocol]
+    && ([target_info gdb_protocol] == "remote"
+	|| [target_info gdb_protocol] == "extended-remote")} {
+    # GDB will already be running after the first call to do_test, so we can
+    # take a peek at the current sysroot setting, and decide if we should
+    # repeat the test with a different setting.
+
+    set new_sysroot ""
+    gdb_test_multiple "show sysroot" "" {
+	-wrap -re "The current system root is \"\"\\." {
+	    pass $gdb_test_name
+
+	    # Repeat the test with 'target:' sysroot.
+	    set new_sysroot "target:"
+	}
+	-wrap -re "The current system root is \"target:\"\\." {
+	    pass $gdb_test_name
+
+	    # Nothing else to do, we already tested with target: sysroot.
+	}
+	-re "$gdb_prompt $" {
+	    pass $gdb_test_name
+
+	    # If already testing with any other sysroot, we probably should
+	    # not try to adjust things, so don't do any further testing.
+	}
+    }
+
+    with_test_prefix "sysroot $new_sysroot" {
+	if { $new_sysroot != "" } {
+	    do_test $new_sysroot
+	}
+    }
+}

Re: [PATCHv5] gdb: handle loading shared libraries from /proc/self/fd/

[Asaf Fisher]

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

Looks good to me!

On Thu, Dec 15, 2022 at 6:44 PM Andrew Burgess <aburgess@redhat.com> wrote:

>
> Asaf pointed out to me (off list) that v4 fails for some cases that the
> original patch supported, so here's a v5.  Specifically, if the dlopen
> call is passed a non-canonical path then GDB will fail to correctly spot
> that the file being opened is within /proc/self, and will still try to
> open a file within GDB's /proc/self by mistake.
>
> Changes since v4:
>
>   - filter_proc_self_filenames now makes use of gdb_realpath_keepfile to
>     (partially) canonicalize the path, this should fix the non-canonical
>     path issues,
>
>   - extended the test to include the non-canonical path case.
>
> Changes since v3:
>
>   - Fixed NULL/nullptr issues Lancelot pointed out,
>
>   - Updated the test case to take account of non-probes based shared
>     library event handling.
>
> ---
>
> commit 65c1f020184e632a6de1775c00de7e0d7a97def3
> Author: Asaf Fisher via Gdb-patches <gdb-patches@sourceware.org>
> Date:   Fri Oct 21 17:42:05 2022 +0000
>
>     gdb: handle loading shared libraries from /proc/self/fd/
>
>     Bug PR gdb/29586 describes a situations where a shared library is
>     created in memory, then written to a memory mapped file.  The memory
>     mapped file will show up as a file descriptor within /proc/self/fd/,
>     and this path is then used with dlopen in order to call functions
>     within the in-memory shared library.
>
>     When attempting to debug this GDB hangs.  The problem is that, GDB
>     stops at the shared-library event, reads the shared library path from
>     the inferior, which is "/proc/self/fd/<NUM>", and then GDB attempts to
>     open this file.
>
>     Unfortunately, this means GDB tries to open a file within GDB's
>     /proc/self/fd/ directory, not within the inferior's directory.  In the
>     case of our hang it turns out that the file descriptor that is opened
>     is a pipe, and GDB hangs trying to read from the pipe.
>
>     However, the behaviour is really just undefined, depending on which
>     file descriptor the inferior tries to open, GDB will open, or fail to
>     open, random files within its /proc/self/fd directory.
>
>     The solution proposed in this commit is to hook into solib_find_1, and
>     spot when GDB is looking for any file in /proc/self/, if this is the
>     case, then the filename is rewritten as /proc/<PID>, where <PID> is
>     the process-id of the current inferior.
>
>     One complexity that we need to consider is how to handle non-canonical
>     paths, for example, if the user opens /proc/../proc/self/fd/<NUM> then
>     GDB also needs to be able to spot this.
>
>     My solution is to call gdb_realpath_keepfile, this canonicalizes
>     everything except the filename (the '/proc/../proc/self/fd' part in
>     the above example).  The problem this then introduces is that the
>     'self' is replaces with the pid of GDB itself.  I therefore check,
>     after canonicalization, for a prefix of /proc/<PID OF GDB>, and if
>     this is the case, I replace this with /proc/<PID OF INFERIOR>.
>
>     There is one remaining issue, which I consider very unlikely, and I
>     don't propose to fix.  What if the inferior actually did try to dlopen
>     a file from /proc/<PID OF GDB>?  If this happened then GDB will not be
>     able to distinguish this from the above case and would still replace
>     the pid of GDB with the pid of the inferior.  However, this seems
>     really unlikely - why would the inferior be trying to load parts of
>     GDB?  So I propose to ignore this case until someone can come up with
>     an actual scenario where that's useful.
>
>     The test passes for the unix, native-gdbserver, and
>     native-extended-gdbserver targets.  When testing with either of the
>     gdbserver targets, the test is run using the default empty sysroot,
>     and also using the 'target:' sysroot.
>
>     Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29586
>
>     Co-authored-by: Andrew Burgess <aburgess@redhat.com>
>
> diff --git a/gdb/solib.c b/gdb/solib.c
> index 59fd866b652..a627d217c85 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -50,6 +50,7 @@
>  #include "gdb_bfd.h"
>  #include "gdbsupport/filestuff.h"
>  #include "gdbsupport/scoped_fd.h"
> +#include "gdbsupport/pathstuff.h"
>  #include "debuginfod-support.h"
>  #include "source.h"
>  #include "cli/cli-style.h"
> @@ -79,6 +80,54 @@ show_solib_search_path (struct ui_file *file, int
> from_tty,
>  #  define DOS_BASED_FILE_SYSTEM 0
>  #endif
>
> +/* Fix references to files in /proc/self/fd/ when opening a shared
> library.
> +
> +   SO_NAME is the name of the shared library being loaded.  This function
> +   returns a possibly modified name which should be used as the path to
> the
> +   shared library.
> +
> +   If SO_NAME starts with /proc/self, then the returned name will be
> +   modified to start with /proc/PID where 'PID' is the pid of the current
> +   inferior.  */
> +
> +static gdb::unique_xmalloc_ptr<char>
> +filter_proc_self_filenames (gdb::unique_xmalloc_ptr<char> so_name)
> +{
> +  /* In order to figure out if SO_NAME points at a file in /proc/self we
> +     need to canonicalize the path.  However, the whole point here is that
> +     if SO_NAME does point at /proc/self then GDB will think this is its
> +     self, while the inferior actually means its /proc/self.  A result of
> +     this is that the file being referenced within the inferior's
> +     /proc/self, might not exist within GDB's /proc/self, a call to
> +     gdb_realpath would then fail.
> +
> +     To avoid this problem we call gdb_realpath_keepfile, this only tries
> +     to canonicalize the basename of SO_NAME, and leaves the final
> filename
> +     untouched.  */
> +  so_name
> +    = make_unique_xstrdup (gdb_realpath_keepfile (so_name.get ()).c_str
> ());
> +
> +  /* A result of the above canonicalization is that /proc/self will have
> +     been replaced with /proc/<pid of gdb>, so that's what we need to
> check
> +     for.  Of course, it could be the case that the inferior really did
> try
> +     to dlopen a file within GDB's /proc/<pid> directory, in which case
> +     we're going to do the wrong thing here, but that seems far less
> likely
> +     than calling dlopen on a file within the inferior's own directory.
> */
> +  std::string prefix = string_printf ("/proc/%ld", (long) getpid ());
> +
> +  /* Is the canonical path inside GDB's /proc/<pid> directory?  */
> +  if (!startswith (so_name.get (), prefix.c_str ()))
> +    return so_name;
> +
> +  /* Get the part of the path after /proc/<pid>.  For example given
> +     '/proc/123/fd' we find the '/fd' part.  */
> +  const char *tail = so_name.get () + strlen (prefix.c_str ());
> +
> +  /* Build a replacement path within the inferiors directory.  */
> +  int inferior_pid = inferior_ptid.pid ();
> +  return xstrprintf ("/proc/%d%s", inferior_pid, tail);
> +}
> +
>  /* Return the full pathname of a binary file (the main executable or a
>     shared library file), or NULL if not found.  If FD is non-NULL, *FD
>     is set to either -1 or an open file handle for the binary file.
> @@ -168,6 +217,9 @@ solib_find_1 (const char *in_pathname, int *fd, bool
> is_solib)
>         }
>      }
>
> +  temp_pathname.reset (xstrdup (in_pathname));
> +  temp_pathname = filter_proc_self_filenames (std::move (temp_pathname));
> +
>    /* Note, we're interested in IS_TARGET_ABSOLUTE_PATH, not
>       IS_ABSOLUTE_PATH.  The latter is for host paths only, while
>       IN_PATHNAME is a target path.  For example, if we're supposed to
> @@ -180,9 +232,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool
> is_solib)
>         3rd attempt, c:/foo/bar.dll ==> /sysroot/foo/bar.dll
>    */
>
> -  if (!IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) || sysroot == NULL)
> -    temp_pathname.reset (xstrdup (in_pathname));
> -  else
> +  if (IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) && sysroot != nullptr)
>      {
>        bool need_dir_separator;
>
> @@ -209,7 +259,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool
> is_solib)
>        /* Cat the prefixed pathname together.  */
>        temp_pathname.reset (concat (sysroot,
>                                    need_dir_separator ? SLASH_STRING : "",
> -                                  in_pathname, (char *) NULL));
> +                                  temp_pathname.get (), nullptr));
>      }
>
>    /* Handle files to be accessed via the target.  */
> diff --git a/gdb/testsuite/gdb.base/solib-proc-self.cc
> b/gdb/testsuite/gdb.base/solib-proc-self.cc
> new file mode 100644
> index 00000000000..aea397a66f9
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/solib-proc-self.cc
> @@ -0,0 +1,89 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2022 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.
> */
> +
> +#include <sys/mman.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <iostream>
> +#include <fstream>
> +#include <sstream>
> +#include <vector>
> +#include <unistd.h>
> +
> +#ifdef __WIN32__
> +#include <windows.h>
> +#define dlopen(name, mode) LoadLibrary (name)
> +#define dlclose(handle) FreeLibrary (handle)
> +#define dlerror() "an error occurred"
> +#else
> +#include <dlfcn.h>
> +#endif
> +
> +/* Combine PREFIX and NUM into a single string, joined with a '/', and
> +   return the string.  */
> +
> +std::string
> +make_library_path (const char *prefix, int num)
> +{
> +  std::stringstream stream;
> +  stream << prefix << "/" << num;
> +  return stream.str ();
> +}
> +
> +/* Call dlopen on the library pointed to by FILENAME.  */
> +
> +void
> +open_library (const std::string &filename)
> +{
> +  /* Call dlopen on FILENAME.  */
> +  void *handle = dlopen (filename.c_str (), RTLD_LAZY);
> +  if (handle == nullptr)
> +    abort ();
> +
> +  /* It worked.  */
> +  dlclose (handle);
> +}
> +
> +int
> +main ()
> +{
> +  /* Read the shared libraries contents into a buffer.  */
> +  std::ifstream read_so_file = std::ifstream (SHLIB_NAME);
> +  read_so_file.seekg (0, std::ios::end);
> +  std::streamsize size = read_so_file.tellg ();
> +  read_so_file.seekg (0, std::ios::beg);
> +  std::vector<char> buffer (size);
> +  if (!read_so_file.read (buffer.data (), size))
> +    abort ();
> +
> +  /* Create a memory mapped file, then write the shared library to that
> +     new memory mapped file.  */
> +  int mem_fd = memfd_create ("test", 0);
> +  write (mem_fd, buffer.data (), buffer.size ());
> +
> +  /* Generate a canonical /proc/self/fd/[num] path for the memory mapped
> +     file, and call dlopen on it.  */
> +  std::string filename
> +    = make_library_path ("/proc/self/fd", mem_fd);     /* break-here */
> +  open_library (filename);
> +
> +  /* Now generate a new, non-canonical filename, and call dlopen on it.
> */
> +  filename = make_library_path ("/proc/../proc/self/fd", mem_fd);
> +  open_library (filename);
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/solib-proc-self.exp
> b/gdb/testsuite/gdb.base/solib-proc-self.exp
> new file mode 100644
> index 00000000000..01e7eecfb16
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/solib-proc-self.exp
> @@ -0,0 +1,200 @@
> +# Copyright 2022 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> */
> +
> +# Test connecting and disconnecting at shared library events.
> +
> +if {[skip_shlib_tests]} {
> +    untested "could not run to main"
> +    return 0
> +}
> +
> +standard_testfile .cc
> +
> +# Reuse an existing library, we don't care about the library contents
> +# for this test.
> +set libfile so-disc-shr
> +set libsrc "${srcdir}/${subdir}/${libfile}.c"
> +set libname "${libfile}.so"
> +set libobj [standard_output_file ${libname}]
> +
> +# Compile the shared library.
> +if { [gdb_compile_shlib $libsrc $libobj {debug}] != ""} {
> +    return -1
> +}
> +
> +# Compile the test executable.
> +if [ build_executable "failed to prepare" $testfile $srcfile \
> +        [list shlib_load debug c++
> additional_flags=-DSHLIB_NAME="${libobj}"]] {
> +    return -1
> +}
> +
> +proc validate_library_load {inferior_pid memfd} {
> +
> +    # Turn on the solib-events so we can see that gdb resolves
> +    # everything correctly.
> +    gdb_test_no_output "set stop-on-solib-events 1"
> +
> +    # Now continue forward until the solib event is detected, and
> +    # check that the loaded library is found through the /proc/PID/fd
> +    # rather than /proc/self/fd.
> +    #
> +    # We need to handle the possibility of the interesting event
> +    # showing the first, or second time we stop, as depending on which
> +    # mechanism GDB is using to handle the shared library events (the
> +    # newer probes based interface, or the old non-probes interface),
> +    # the library will be reported at the first or second stop.
> +    set saw_expected_event false
> +    set saw_no_event_stop false
> +    gdb_test_multiple "continue" "continue to solib evnt" {
> +       -re "^continue\r\n" {
> +           exp_continue
> +       }
> +
> +       -re "^Continuing\\.\r\n" {
> +           exp_continue
> +       }
> +
> +       -early -re "Stopped due to shared library event \\(no libraries
> added or removed\\)\r\n" {
> +           # This non-interesting event shows up first when using the
> +           # probes based mechanism for dealing with shared library
> +           # events.
> +           #
> +           # We set a flag here, and, once the prompt has appeared,
> +           # we send another continue, the next event will contain
> +           # the information we want.
> +           set saw_no_event_stop true
> +           exp_continue
> +       }
> +
> +       -re "Stopped due to shared library event:\r\n  Inferior loaded
> (?:target:)?/proc/${inferior_pid}/fd/$memfd\r\n" {
> +           # This event, which includes the information we are
> +           # looking for, occurs first when using the non-probes
> +           # based mechanism for handling shared library events, and
> +           # occurs second when using the probes mechanism.
> +           #
> +           # Either way, record here that we say the output we expect.
> +           set saw_expected_event true
> +           exp_continue
> +       }
> +
> +       -re "$::gdb_prompt $" {
> +           if {$saw_no_event_stop} {
> +               set saw_no_event_stop false
> +               send_gdb "continue\n"
> +               exp_continue
> +           } else {
> +               gdb_assert {$saw_expected_event} $gdb_test_name
> +           }
> +       }
> +    }
> +
> +    # Turn off solib events.  We're only interested in validating the
> +    # loads for now.
> +    gdb_test_no_output "set stop-on-solib-events 0"
> +}
> +
> +# Start GDB and run to the point where the test program tries to dlopen a
> file
> +# from within /proc/self/fd/.  Catch the shared library event and check
> that
> +# we actually try to load a file from /proc/<INFERIOR-PID>/fd/.
> +#
> +# If SYSROOT is not the empty string, then this is set as the value of
> GDB's
> +# sysroot immediately after starting GDB.  The only value that is
> (currently)
> +# supported, other than the empty string, is 'target:'.
> +proc do_test { {sysroot ""} } {
> +    clean_restart $::binfile
> +
> +    if {$sysroot != ""} {
> +       gdb_test_no_output "set sysroot ${sysroot}"
> +    }
> +
> +    gdb_load_shlib $::libobj
> +
> +    if ![runto_main] then {
> +       return 0
> +    }
> +
> +    # Get inferior's PID for later.
> +    set inferior_pid [get_inferior_pid]
> +
> +    # Run to the 'break-here' marker.
> +    gdb_breakpoint [gdb_get_line_number "break-here"]
> +    gdb_continue_to_breakpoint "break-here" ".* break-here .*"
> +
> +    set memfd ""
> +    gdb_test_multiple "p mem_fd" "Get file descriptor" {
> +       -re -wrap "\\\$$::decimal = (\[^\r\n\]*)" {
> +           set memfd $expect_out(1,string)
> +           pass $gdb_test_name
> +       }
> +    }
> +
> +    # The first call to open_library (in the test program) is done
> +    # with a canonical path.
> +    with_test_prefix "canonical path" {
> +       gdb_breakpoint "open_library"
> +       gdb_continue_to_breakpoint "open_library"
> +       validate_library_load $inferior_pid $memfd
> +    }
> +
> +    # The second call to open_library (in the test program) is done
> +    # with a non-canonical path, however, GDB should resolve this to a
> +    # canonical path for display to the user, so the output we see
> +    # should be unchanged.
> +    with_test_prefix "non-canonical path" {
> +       gdb_breakpoint "open_library"
> +       gdb_continue_to_breakpoint "open_library"
> +       validate_library_load $inferior_pid $memfd
> +    }
> +}
> +
> +# First run of the test.
> +do_test
> +
> +# Possible second run of the test.  If we are using a remote target then
> we
> +# should consider setting the sysroot to 'target:' and re-running the
> test.
> +if {[target_info exists gdb_protocol]
> +    && ([target_info gdb_protocol] == "remote"
> +       || [target_info gdb_protocol] == "extended-remote")} {
> +    # GDB will already be running after the first call to do_test, so we
> can
> +    # take a peek at the current sysroot setting, and decide if we should
> +    # repeat the test with a different setting.
> +
> +    set new_sysroot ""
> +    gdb_test_multiple "show sysroot" "" {
> +       -wrap -re "The current system root is \"\"\\." {
> +           pass $gdb_test_name
> +
> +           # Repeat the test with 'target:' sysroot.
> +           set new_sysroot "target:"
> +       }
> +       -wrap -re "The current system root is \"target:\"\\." {
> +           pass $gdb_test_name
> +
> +           # Nothing else to do, we already tested with target: sysroot.
> +       }
> +       -re "$gdb_prompt $" {
> +           pass $gdb_test_name
> +
> +           # If already testing with any other sysroot, we probably should
> +           # not try to adjust things, so don't do any further testing.
> +       }
> +    }
> +
> +    with_test_prefix "sysroot $new_sysroot" {
> +       if { $new_sysroot != "" } {
> +           do_test $new_sysroot
> +       }
> +    }
> +}
>
>

[PATCHv6] gdb: handle loading shared libraries from /proc/self/fd/

[Andrew Burgess]

From: Asaf Fisher via Gdb-patches <gdb-patches@sourceware.org>

Changes since v5:

  - Rebased onto HEAD of master, updated the test script to use
    'require allow_shlib_tests'.

Changes since v4:

  - filter_proc_self_filenames now makes use of gdb_realpath_keepfile to
    (partially) canonicalize the path, this should fix the non-canonical
    path issues,

  - extended the test to include the non-canonical path case.

Changes since v3:

  - Fixed NULL/nullptr issues Lancelot pointed out,

  - Updated the test case to take account of non-probes based shared
    library event handling.

---

Bug PR gdb/29586 describes a situations where a shared library is
created in memory, then written to a memory mapped file.  The memory
mapped file will show up as a file descriptor within /proc/self/fd/,
and this path is then used with dlopen in order to call functions
within the in-memory shared library.

When attempting to debug this GDB hangs.  The problem is that, GDB
stops at the shared-library event, reads the shared library path from
the inferior, which is "/proc/self/fd/<NUM>", and then GDB attempts to
open this file.

Unfortunately, this means GDB tries to open a file within GDB's
/proc/self/fd/ directory, not within the inferior's directory.  In the
case of our hang it turns out that the file descriptor that is opened
is a pipe, and GDB hangs trying to read from the pipe.

However, the behaviour is really just undefined, depending on which
file descriptor the inferior tries to open, GDB will open, or fail to
open, random files within its /proc/self/fd directory.

The solution proposed in this commit is to hook into solib_find_1, and
spot when GDB is looking for any file in /proc/self/, if this is the
case, then the filename is rewritten as /proc/<PID>, where <PID> is
the process-id of the current inferior.

One complexity that we need to consider is how to handle non-canonical
paths, for example, if the user opens /proc/../proc/self/fd/<NUM> then
GDB also needs to be able to spot this.

My solution is to call gdb_realpath_keepfile, this canonicalizes
everything except the filename (the '/proc/../proc/self/fd' part in
the above example).  The problem this then introduces is that the
'self' is replaces with the pid of GDB itself.  I therefore check,
after canonicalization, for a prefix of /proc/<PID OF GDB>, and if
this is the case, I replace this with /proc/<PID OF INFERIOR>.

There is one remaining issue, which I consider very unlikely, and I
don't propose to fix.  What if the inferior actually did try to dlopen
a file from /proc/<PID OF GDB>?  If this happened then GDB will not be
able to distinguish this from the above case and would still replace
the pid of GDB with the pid of the inferior.  However, this seems
really unlikely - why would the inferior be trying to load parts of
GDB?  So I propose to ignore this case until someone can come up with
an actual scenario where that's useful.

The test passes for the unix, native-gdbserver, and
native-extended-gdbserver targets.  When testing with either of the
gdbserver targets, the test is run using the default empty sysroot,
and also using the 'target:' sysroot.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29586

Co-authored-by: Andrew Burgess <aburgess@redhat.com>
---
 gdb/solib.c                                |  58 +++++-
 gdb/testsuite/gdb.base/solib-proc-self.cc  |  89 ++++++++++
 gdb/testsuite/gdb.base/solib-proc-self.exp | 197 +++++++++++++++++++++
 3 files changed, 340 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/solib-proc-self.cc
 create mode 100644 gdb/testsuite/gdb.base/solib-proc-self.exp

diff --git a/gdb/solib.c b/gdb/solib.c
index 60bdf46cb91..3181b4d5f1f 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -50,6 +50,7 @@
 #include "gdb_bfd.h"
 #include "gdbsupport/filestuff.h"
 #include "gdbsupport/scoped_fd.h"
+#include "gdbsupport/pathstuff.h"
 #include "debuginfod-support.h"
 #include "source.h"
 #include "cli/cli-style.h"
@@ -79,6 +80,54 @@ show_solib_search_path (struct ui_file *file, int from_tty,
 #  define DOS_BASED_FILE_SYSTEM 0
 #endif
 
+/* Fix references to files in /proc/self/fd/ when opening a shared library.
+
+   SO_NAME is the name of the shared library being loaded.  This function
+   returns a possibly modified name which should be used as the path to the
+   shared library.
+
+   If SO_NAME starts with /proc/self, then the returned name will be
+   modified to start with /proc/PID where 'PID' is the pid of the current
+   inferior.  */
+
+static gdb::unique_xmalloc_ptr<char>
+filter_proc_self_filenames (gdb::unique_xmalloc_ptr<char> so_name)
+{
+  /* In order to figure out if SO_NAME points at a file in /proc/self we
+     need to canonicalize the path.  However, the whole point here is that
+     if SO_NAME does point at /proc/self then GDB will think this is its
+     self, while the inferior actually means its /proc/self.  A result of
+     this is that the file being referenced within the inferior's
+     /proc/self, might not exist within GDB's /proc/self, a call to
+     gdb_realpath would then fail.
+
+     To avoid this problem we call gdb_realpath_keepfile, this only tries
+     to canonicalize the basename of SO_NAME, and leaves the final filename
+     untouched.  */
+  so_name
+    = make_unique_xstrdup (gdb_realpath_keepfile (so_name.get ()).c_str ());
+
+  /* A result of the above canonicalization is that /proc/self will have
+     been replaced with /proc/<pid of gdb>, so that's what we need to check
+     for.  Of course, it could be the case that the inferior really did try
+     to dlopen a file within GDB's /proc/<pid> directory, in which case
+     we're going to do the wrong thing here, but that seems far less likely
+     than calling dlopen on a file within the inferior's own directory.  */
+  std::string prefix = string_printf ("/proc/%ld", (long) getpid ());
+
+  /* Is the canonical path inside GDB's /proc/<pid> directory?  */
+  if (!startswith (so_name.get (), prefix.c_str ()))
+    return so_name;
+
+  /* Get the part of the path after /proc/<pid>.  For example given
+     '/proc/123/fd' we find the '/fd' part.  */
+  const char *tail = so_name.get () + strlen (prefix.c_str ());
+
+  /* Build a replacement path within the inferiors directory.  */
+  int inferior_pid = inferior_ptid.pid ();
+  return xstrprintf ("/proc/%d%s", inferior_pid, tail);
+}
+
 /* Return the full pathname of a binary file (the main executable or a
    shared library file), or NULL if not found.  If FD is non-NULL, *FD
    is set to either -1 or an open file handle for the binary file.
@@ -168,6 +217,9 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
 	}
     }
 
+  temp_pathname.reset (xstrdup (in_pathname));
+  temp_pathname = filter_proc_self_filenames (std::move (temp_pathname));
+
   /* Note, we're interested in IS_TARGET_ABSOLUTE_PATH, not
      IS_ABSOLUTE_PATH.  The latter is for host paths only, while
      IN_PATHNAME is a target path.  For example, if we're supposed to
@@ -180,9 +232,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
        3rd attempt, c:/foo/bar.dll ==> /sysroot/foo/bar.dll
   */
 
-  if (!IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) || sysroot == NULL)
-    temp_pathname.reset (xstrdup (in_pathname));
-  else
+  if (IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) && sysroot != nullptr)
     {
       bool need_dir_separator;
 
@@ -209,7 +259,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
       /* Cat the prefixed pathname together.  */
       temp_pathname.reset (concat (sysroot,
 				   need_dir_separator ? SLASH_STRING : "",
-				   in_pathname, (char *) NULL));
+				   temp_pathname.get (), nullptr));
     }
 
   /* Handle files to be accessed via the target.  */
diff --git a/gdb/testsuite/gdb.base/solib-proc-self.cc b/gdb/testsuite/gdb.base/solib-proc-self.cc
new file mode 100644
index 00000000000..aea397a66f9
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-proc-self.cc
@@ -0,0 +1,89 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2022 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <sys/mman.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <iostream>
+#include <fstream>
+#include <sstream>
+#include <vector>
+#include <unistd.h>
+
+#ifdef __WIN32__
+#include <windows.h>
+#define dlopen(name, mode) LoadLibrary (name)
+#define dlclose(handle) FreeLibrary (handle)
+#define dlerror() "an error occurred"
+#else
+#include <dlfcn.h>
+#endif
+
+/* Combine PREFIX and NUM into a single string, joined with a '/', and
+   return the string.  */
+
+std::string
+make_library_path (const char *prefix, int num)
+{
+  std::stringstream stream;
+  stream << prefix << "/" << num;
+  return stream.str ();
+}
+
+/* Call dlopen on the library pointed to by FILENAME.  */
+
+void
+open_library (const std::string &filename)
+{
+  /* Call dlopen on FILENAME.  */
+  void *handle = dlopen (filename.c_str (), RTLD_LAZY);
+  if (handle == nullptr)
+    abort ();
+
+  /* It worked.  */
+  dlclose (handle);
+}
+
+int
+main ()
+{
+  /* Read the shared libraries contents into a buffer.  */
+  std::ifstream read_so_file = std::ifstream (SHLIB_NAME);
+  read_so_file.seekg (0, std::ios::end);
+  std::streamsize size = read_so_file.tellg ();
+  read_so_file.seekg (0, std::ios::beg);
+  std::vector<char> buffer (size);
+  if (!read_so_file.read (buffer.data (), size))
+    abort ();
+
+  /* Create a memory mapped file, then write the shared library to that
+     new memory mapped file.  */
+  int mem_fd = memfd_create ("test", 0);
+  write (mem_fd, buffer.data (), buffer.size ());
+
+  /* Generate a canonical /proc/self/fd/[num] path for the memory mapped
+     file, and call dlopen on it.  */
+  std::string filename
+    = make_library_path ("/proc/self/fd", mem_fd);	/* break-here */
+  open_library (filename);
+
+  /* Now generate a new, non-canonical filename, and call dlopen on it.  */
+  filename = make_library_path ("/proc/../proc/self/fd", mem_fd);
+  open_library (filename);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/solib-proc-self.exp b/gdb/testsuite/gdb.base/solib-proc-self.exp
new file mode 100644
index 00000000000..f026d2544e6
--- /dev/null
+++ b/gdb/testsuite/gdb.base/solib-proc-self.exp
@@ -0,0 +1,197 @@
+# Copyright 2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# Test connecting and disconnecting at shared library events.
+
+require allow_shlib_tests
+
+standard_testfile .cc
+
+# Reuse an existing library, we don't care about the library contents
+# for this test.
+set libfile so-disc-shr
+set libsrc "${srcdir}/${subdir}/${libfile}.c"
+set libname "${libfile}.so"
+set libobj [standard_output_file ${libname}]
+
+# Compile the shared library.
+if { [gdb_compile_shlib $libsrc $libobj {debug}] != ""} {
+    return -1
+}
+
+# Compile the test executable.
+if [ build_executable "failed to prepare" $testfile $srcfile \
+	 [list shlib_load debug c++ additional_flags=-DSHLIB_NAME="${libobj}"]] {
+    return -1
+}
+
+proc validate_library_load {inferior_pid memfd} {
+
+    # Turn on the solib-events so we can see that gdb resolves
+    # everything correctly.
+    gdb_test_no_output "set stop-on-solib-events 1"
+
+    # Now continue forward until the solib event is detected, and
+    # check that the loaded library is found through the /proc/PID/fd
+    # rather than /proc/self/fd.
+    #
+    # We need to handle the possibility of the interesting event
+    # showing the first, or second time we stop, as depending on which
+    # mechanism GDB is using to handle the shared library events (the
+    # newer probes based interface, or the old non-probes interface),
+    # the library will be reported at the first or second stop.
+    set saw_expected_event false
+    set saw_no_event_stop false
+    gdb_test_multiple "continue" "continue to solib evnt" {
+	-re "^continue\r\n" {
+	    exp_continue
+	}
+
+	-re "^Continuing\\.\r\n" {
+	    exp_continue
+	}
+
+	-early -re "Stopped due to shared library event \\(no libraries added or removed\\)\r\n" {
+	    # This non-interesting event shows up first when using the
+	    # probes based mechanism for dealing with shared library
+	    # events.
+	    #
+	    # We set a flag here, and, once the prompt has appeared,
+	    # we send another continue, the next event will contain
+	    # the information we want.
+	    set saw_no_event_stop true
+	    exp_continue
+	}
+
+	-re "Stopped due to shared library event:\r\n  Inferior loaded (?:target:)?/proc/${inferior_pid}/fd/$memfd\r\n" {
+	    # This event, which includes the information we are
+	    # looking for, occurs first when using the non-probes
+	    # based mechanism for handling shared library events, and
+	    # occurs second when using the probes mechanism.
+	    #
+	    # Either way, record here that we say the output we expect.
+	    set saw_expected_event true
+	    exp_continue
+	}
+
+	-re "$::gdb_prompt $" {
+	    if {$saw_no_event_stop} {
+		set saw_no_event_stop false
+		send_gdb "continue\n"
+		exp_continue
+	    } else {
+		gdb_assert {$saw_expected_event} $gdb_test_name
+	    }
+	}
+    }
+
+    # Turn off solib events.  We're only interested in validating the
+    # loads for now.
+    gdb_test_no_output "set stop-on-solib-events 0"
+}
+
+# Start GDB and run to the point where the test program tries to dlopen a file
+# from within /proc/self/fd/.  Catch the shared library event and check that
+# we actually try to load a file from /proc/<INFERIOR-PID>/fd/.
+#
+# If SYSROOT is not the empty string, then this is set as the value of GDB's
+# sysroot immediately after starting GDB.  The only value that is (currently)
+# supported, other than the empty string, is 'target:'.
+proc do_test { {sysroot ""} } {
+    clean_restart $::binfile
+
+    if {$sysroot != ""} {
+	gdb_test_no_output "set sysroot ${sysroot}"
+    }
+
+    gdb_load_shlib $::libobj
+
+    if ![runto_main] then {
+	return 0
+    }
+
+    # Get inferior's PID for later.
+    set inferior_pid [get_inferior_pid]
+
+    # Run to the 'break-here' marker.
+    gdb_breakpoint [gdb_get_line_number "break-here"]
+    gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+
+    set memfd ""
+    gdb_test_multiple "p mem_fd" "Get file descriptor" {
+	-re -wrap "\\\$$::decimal = (\[^\r\n\]*)" {
+	    set memfd $expect_out(1,string)
+	    pass $gdb_test_name
+	}
+    }
+
+    # The first call to open_library (in the test program) is done
+    # with a canonical path.
+    with_test_prefix "canonical path" {
+	gdb_breakpoint "open_library"
+	gdb_continue_to_breakpoint "open_library"
+	validate_library_load $inferior_pid $memfd
+    }
+
+    # The second call to open_library (in the test program) is done
+    # with a non-canonical path, however, GDB should resolve this to a
+    # canonical path for display to the user, so the output we see
+    # should be unchanged.
+    with_test_prefix "non-canonical path" {
+	gdb_breakpoint "open_library"
+	gdb_continue_to_breakpoint "open_library"
+	validate_library_load $inferior_pid $memfd
+    }
+}
+
+# First run of the test.
+do_test
+
+# Possible second run of the test.  If we are using a remote target then we
+# should consider setting the sysroot to 'target:' and re-running the test.
+if {[target_info exists gdb_protocol]
+    && ([target_info gdb_protocol] == "remote"
+	|| [target_info gdb_protocol] == "extended-remote")} {
+    # GDB will already be running after the first call to do_test, so we can
+    # take a peek at the current sysroot setting, and decide if we should
+    # repeat the test with a different setting.
+
+    set new_sysroot ""
+    gdb_test_multiple "show sysroot" "" {
+	-wrap -re "The current system root is \"\"\\." {
+	    pass $gdb_test_name
+
+	    # Repeat the test with 'target:' sysroot.
+	    set new_sysroot "target:"
+	}
+	-wrap -re "The current system root is \"target:\"\\." {
+	    pass $gdb_test_name
+
+	    # Nothing else to do, we already tested with target: sysroot.
+	}
+	-re "$gdb_prompt $" {
+	    pass $gdb_test_name
+
+	    # If already testing with any other sysroot, we probably should
+	    # not try to adjust things, so don't do any further testing.
+	}
+    }
+
+    with_test_prefix "sysroot $new_sysroot" {
+	if { $new_sysroot != "" } {
+	    do_test $new_sysroot
+	}
+    }
+}

base-commit: 7d02a94c8f74613edb0cdde11982b22eaaa9affe
-- 
2.25.4

Re: [PATCHv6] gdb: handle loading shared libraries from /proc/self/fd/

[Pedro Alves]

On 2023-01-20 12:33 p.m., Andrew Burgess via Gdb-patches wrote:
> From: Asaf Fisher via Gdb-patches <gdb-patches@sourceware.org>
> 
> Changes since v5:
> 
>   - Rebased onto HEAD of master, updated the test script to use
>     'require allow_shlib_tests'.
> 
> Changes since v4:
> 
>   - filter_proc_self_filenames now makes use of gdb_realpath_keepfile to
>     (partially) canonicalize the path, this should fix the non-canonical
>     path issues,
> 
>   - extended the test to include the non-canonical path case.
> 
> Changes since v3:
> 
>   - Fixed NULL/nullptr issues Lancelot pointed out,
> 
>   - Updated the test case to take account of non-probes based shared
>     library event handling.
> 
> ---
> 
> Bug PR gdb/29586 describes a situations where a shared library is

"a situations" -> "a situation"

> created in memory, then written to a memory mapped file.  The memory
> mapped file will show up as a file descriptor within /proc/self/fd/,
> and this path is then used with dlopen in order to call functions
> within the in-memory shared library.
> 
> When attempting to debug this GDB hangs.  The problem is that, GDB
> stops at the shared-library event, reads the shared library path from
> the inferior, which is "/proc/self/fd/<NUM>", and then GDB attempts to
> open this file.
> 
> Unfortunately, this means GDB tries to open a file within GDB's
> /proc/self/fd/ directory, not within the inferior's directory.  In the
> case of our hang it turns out that the file descriptor that is opened
> is a pipe, and GDB hangs trying to read from the pipe.
> 
> However, the behaviour is really just undefined, depending on which
> file descriptor the inferior tries to open, GDB will open, or fail to
> open, random files within its /proc/self/fd directory.
> 
> The solution proposed in this commit is to hook into solib_find_1, and
> spot when GDB is looking for any file in /proc/self/, if this is the
> case, then the filename is rewritten as /proc/<PID>, where <PID> is
> the process-id of the current inferior.
> 
> One complexity that we need to consider is how to handle non-canonical
> paths, for example, if the user opens /proc/../proc/self/fd/<NUM> then
> GDB also needs to be able to spot this.
> 
> My solution is to call gdb_realpath_keepfile, this canonicalizes
> everything except the filename (the '/proc/../proc/self/fd' part in
> the above example).  The problem this then introduces is that the
> 'self' is replaces with the pid of GDB itself.  I therefore check,

"is replaces" -> "is replaced"


> after canonicalization, for a prefix of /proc/<PID OF GDB>, and if
> this is the case, I replace this with /proc/<PID OF INFERIOR>.
> 
> There is one remaining issue, which I consider very unlikely, and I
> don't propose to fix.  What if the inferior actually did try to dlopen
> a file from /proc/<PID OF GDB>?  If this happened then GDB will not be
> able to distinguish this from the above case and would still replace
> the pid of GDB with the pid of the inferior.  However, this seems
> really unlikely - why would the inferior be trying to load parts of
> GDB?  So I propose to ignore this case until someone can come up with
> an actual scenario where that's useful.
> 
> The test passes for the unix, native-gdbserver, and
> native-extended-gdbserver targets.  When testing with either of the
> gdbserver targets, the test is run using the default empty sysroot,
> and also using the 'target:' sysroot.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29586
> 
> Co-authored-by: Andrew Burgess <aburgess@redhat.com>
> ---
>  gdb/solib.c                                |  58 +++++-
>  gdb/testsuite/gdb.base/solib-proc-self.cc  |  89 ++++++++++
>  gdb/testsuite/gdb.base/solib-proc-self.exp | 197 +++++++++++++++++++++
>  3 files changed, 340 insertions(+), 4 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/solib-proc-self.cc
>  create mode 100644 gdb/testsuite/gdb.base/solib-proc-self.exp
> 
> diff --git a/gdb/solib.c b/gdb/solib.c
> index 60bdf46cb91..3181b4d5f1f 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -50,6 +50,7 @@
>  #include "gdb_bfd.h"
>  #include "gdbsupport/filestuff.h"
>  #include "gdbsupport/scoped_fd.h"
> +#include "gdbsupport/pathstuff.h"
>  #include "debuginfod-support.h"
>  #include "source.h"
>  #include "cli/cli-style.h"
> @@ -79,6 +80,54 @@ show_solib_search_path (struct ui_file *file, int from_tty,
>  #  define DOS_BASED_FILE_SYSTEM 0
>  #endif
>  
> +/* Fix references to files in /proc/self/fd/ when opening a shared library.
> +
> +   SO_NAME is the name of the shared library being loaded.  This function
> +   returns a possibly modified name which should be used as the path to the
> +   shared library.
> +
> +   If SO_NAME starts with /proc/self, then the returned name will be
> +   modified to start with /proc/PID where 'PID' is the pid of the current
> +   inferior.  */
> +
> +static gdb::unique_xmalloc_ptr<char>
> +filter_proc_self_filenames (gdb::unique_xmalloc_ptr<char> so_name)
> +{
> +  /* In order to figure out if SO_NAME points at a file in /proc/self we
> +     need to canonicalize the path.  However, the whole point here is that
> +     if SO_NAME does point at /proc/self then GDB will think this is its
> +     self, while the inferior actually means its /proc/self.  A result of
> +     this is that the file being referenced within the inferior's
> +     /proc/self, might not exist within GDB's /proc/self, a call to
> +     gdb_realpath would then fail.
> +
> +     To avoid this problem we call gdb_realpath_keepfile, this only tries
> +     to canonicalize the basename of SO_NAME, and leaves the final filename
> +     untouched.  */
> +  so_name
> +    = make_unique_xstrdup (gdb_realpath_keepfile (so_name.get ()).c_str ());

gdb_realpath_keepfile resolves symlinks on the local filesystem, so how can
we be using it on target: files?  There's a comment exactly about this in exec.c:

      /* gdb_realpath_keepfile resolves symlinks on the local
	 filesystem and so cannot be used for "target:" files.  */
      gdb_assert (current_program_space->exec_filename == nullptr);
      if (load_via_target)
	current_program_space->exec_filename
	  = (make_unique_xstrdup
	     (bfd_get_filename (current_program_space->exec_bfd ())));
      else
	current_program_space->exec_filename
	  = make_unique_xstrdup (gdb_realpath_keepfile
				   (scratch_pathname).c_str ());


Also, what if the host is not a Linux host, if it is one where /proc/ does
not exist at all, such as e.g., a Windows host debugging a remote Linux target?
Won't gdb_realpath_keepfile fail in that scenario?

Also, do we get here with a relative pathname?  In that case, this will use
the CWD of GDB for path expansion, which just looks wrong.

I wonder whether using the linux namespace support wouldn't be a better approach.

Now how linux_nat_target::fileio_readlink calls linux_mntns_readlink:

 gdb::optional<std::string>
 linux_nat_target::fileio_readlink (struct inferior *inf, const char *filename,
 				   fileio_error *target_errno)
 {
 ...
   len = linux_mntns_readlink (linux_nat_fileio_pid_of (inf),
 			      filename, buf, sizeof (buf));
 ...

That means that the link is resolved in the namespace of the inferior.

GDBserver has similar code.

Also note:

 $ readlink -f /proc/self
 /proc/1957246

That suggests that we would just need to call target_fileio_readlink somewhere
to resolve the path.

That would leave core files to address, as you can't enter the namespace of
a process that is already gone.  But for cores, your solution is also not safe,
as opening "/proc/inferior_pid/" from the live host system is incorrect, that
PID might as well have been assigned to some other process, if it even exists.
Reading from /proc/pid/ files must be handled specially for core files.
So that leaves me to think that the target_fileio_readlink approach could work.

> +
> +  /* A result of the above canonicalization is that /proc/self will have
> +     been replaced with /proc/<pid of gdb>, so that's what we need to check
> +     for.  Of course, it could be the case that the inferior really did try
> +     to dlopen a file within GDB's /proc/<pid> directory, in which case
> +     we're going to do the wrong thing here, but that seems far less likely
> +     than calling dlopen on a file within the inferior's own directory.  */
> +  std::string prefix = string_printf ("/proc/%ld", (long) getpid ());
> +
> +  /* Is the canonical path inside GDB's /proc/<pid> directory?  */
> +  if (!startswith (so_name.get (), prefix.c_str ()))
> +    return so_name;

If GDB's pid is "1234", and so_name is "/proc/12345/something", this will think
the path is GDB's /proc/<pid> directory.  Need to append a slash to
PREFIX to avoid this.  Also need to check for exact "/proc/$GDB_PID" then.

> +
> +  /* Get the part of the path after /proc/<pid>.  For example given
> +     '/proc/123/fd' we find the '/fd' part.  */
> +  const char *tail = so_name.get () + strlen (prefix.c_str ());
> +
> +  /* Build a replacement path within the inferiors directory.  */
> +  int inferior_pid = inferior_ptid.pid ();
> +  return xstrprintf ("/proc/%d%s", inferior_pid, tail);
> +}
> +
>  /* Return the full pathname of a binary file (the main executable or a
>     shared library file), or NULL if not found.  If FD is non-NULL, *FD
>     is set to either -1 or an open file handle for the binary file.
> @@ -168,6 +217,9 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
>  	}
>      }
>  
> +  temp_pathname.reset (xstrdup (in_pathname));
> +  temp_pathname = filter_proc_self_filenames (std::move (temp_pathname));

FWIW, this could be written as:

  temp_pathname = filter_proc_self_filenames (make_unique_xtrdup (in_pathname));

> +
>    /* Note, we're interested in IS_TARGET_ABSOLUTE_PATH, not
>       IS_ABSOLUTE_PATH.  The latter is for host paths only, while
>       IN_PATHNAME is a target path.  For example, if we're supposed to
> @@ -180,9 +232,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
>         3rd attempt, c:/foo/bar.dll ==> /sysroot/foo/bar.dll
>    */
>  
> -  if (!IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) || sysroot == NULL)
> -    temp_pathname.reset (xstrdup (in_pathname));
> -  else
> +  if (IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) && sysroot != nullptr)
>      {
>        bool need_dir_separator;
>  
> @@ -209,7 +259,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
>        /* Cat the prefixed pathname together.  */
>        temp_pathname.reset (concat (sysroot,
>  				   need_dir_separator ? SLASH_STRING : "",
> -				   in_pathname, (char *) NULL));
> +				   temp_pathname.get (), nullptr));
>      }
>  
>    /* Handle files to be accessed via the target.  */
> diff --git a/gdb/testsuite/gdb.base/solib-proc-self.cc b/gdb/testsuite/gdb.base/solib-proc-self.cc
> new file mode 100644
> index 00000000000..aea397a66f9
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/solib-proc-self.cc
> @@ -0,0 +1,89 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2022 Free Software Foundation, Inc.

Dates need update to 2022-2023.

> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <sys/mman.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <iostream>
> +#include <fstream>
> +#include <sstream>
> +#include <vector>
> +#include <unistd.h>
> +
> +#ifdef __WIN32__
> +#include <windows.h>
> +#define dlopen(name, mode) LoadLibrary (name)
> +#define dlclose(handle) FreeLibrary (handle)
> +#define dlerror() "an error occurred"

/proc/self/ doesn't exit on Windows, so this seems pointless.

Maybe also add some require !istarget windows to the .exp file.


> +#else
> +#include <dlfcn.h>
> +#endif
> +
> +/* Combine PREFIX and NUM into a single string, joined with a '/', and
> +   return the string.  */
> +
> +std::string
> +make_library_path (const char *prefix, int num)
> +{
> +  std::stringstream stream;
> +  stream << prefix << "/" << num;
> +  return stream.str ();
> +}
> +
> +/* Call dlopen on the library pointed to by FILENAME.  */
> +
> +void
> +open_library (const std::string &filename)
> +{
> +  /* Call dlopen on FILENAME.  */
> +  void *handle = dlopen (filename.c_str (), RTLD_LAZY);
> +  if (handle == nullptr)
> +    abort ();
> +
> +  /* It worked.  */
> +  dlclose (handle);
> +}
> +
> +int
> +main ()
> +{
> +  /* Read the shared libraries contents into a buffer.  */
> +  std::ifstream read_so_file = std::ifstream (SHLIB_NAME);
> +  read_so_file.seekg (0, std::ios::end);
> +  std::streamsize size = read_so_file.tellg ();
> +  read_so_file.seekg (0, std::ios::beg);
> +  std::vector<char> buffer (size);
> +  if (!read_so_file.read (buffer.data (), size))
> +    abort ();
> +
> +  /* Create a memory mapped file, then write the shared library to that
> +     new memory mapped file.  */
> +  int mem_fd = memfd_create ("test", 0);
> +  write (mem_fd, buffer.data (), buffer.size ());

Missing error checking here.

> +
> +  /* Generate a canonical /proc/self/fd/[num] path for the memory mapped
> +     file, and call dlopen on it.  */
> +  std::string filename
> +    = make_library_path ("/proc/self/fd", mem_fd);	/* break-here */
> +  open_library (filename);
> +
> +  /* Now generate a new, non-canonical filename, and call dlopen on it.  */
> +  filename = make_library_path ("/proc/../proc/self/fd", mem_fd);
> +  open_library (filename);
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/solib-proc-self.exp b/gdb/testsuite/gdb.base/solib-proc-self.exp
> new file mode 100644
> index 00000000000..f026d2544e6
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/solib-proc-self.exp
> @@ -0,0 +1,197 @@
> +# Copyright 2022 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +# Test connecting and disconnecting at shared library events.

This comment is stale.

> +
> +require allow_shlib_tests
> +
> +standard_testfile .cc
> +
> +# Reuse an existing library, we don't care about the library contents
> +# for this test.
> +set libfile so-disc-shr
> +set libsrc "${srcdir}/${subdir}/${libfile}.c"
> +set libname "${libfile}.so"
> +set libobj [standard_output_file ${libname}]
> +
> +# Compile the shared library.
> +if { [gdb_compile_shlib $libsrc $libobj {debug}] != ""} {
> +    return -1
> +}
> +
> +# Compile the test executable.
> +if [ build_executable "failed to prepare" $testfile $srcfile \
> +	 [list shlib_load debug c++ additional_flags=-DSHLIB_NAME="${libobj}"]] {
> +    return -1
> +}
> +
> +proc validate_library_load {inferior_pid memfd} {
> +
> +    # Turn on the solib-events so we can see that gdb resolves
> +    # everything correctly.
> +    gdb_test_no_output "set stop-on-solib-events 1"
> +
> +    # Now continue forward until the solib event is detected, and
> +    # check that the loaded library is found through the /proc/PID/fd
> +    # rather than /proc/self/fd.
> +    #
> +    # We need to handle the possibility of the interesting event
> +    # showing the first, or second time we stop, as depending on which
> +    # mechanism GDB is using to handle the shared library events (the
> +    # newer probes based interface, or the old non-probes interface),
> +    # the library will be reported at the first or second stop.
> +    set saw_expected_event false
> +    set saw_no_event_stop false
> +    gdb_test_multiple "continue" "continue to solib evnt" {

"solib evnt" -> "solib event"

> +	-re "^continue\r\n" {
> +	    exp_continue
> +	}
> +
> +	-re "^Continuing\\.\r\n" {
> +	    exp_continue
> +	}
> +
> +	-early -re "Stopped due to shared library event \\(no libraries added or removed\\)\r\n" {
> +	    # This non-interesting event shows up first when using the
> +	    # probes based mechanism for dealing with shared library
> +	    # events.
> +	    #
> +	    # We set a flag here, and, once the prompt has appeared,
> +	    # we send another continue, the next event will contain
> +	    # the information we want.
> +	    set saw_no_event_stop true
> +	    exp_continue
> +	}
> +
> +	-re "Stopped due to shared library event:\r\n  Inferior loaded (?:target:)?/proc/${inferior_pid}/fd/$memfd\r\n" {
> +	    # This event, which includes the information we are
> +	    # looking for, occurs first when using the non-probes
> +	    # based mechanism for handling shared library events, and
> +	    # occurs second when using the probes mechanism.
> +	    #
> +	    # Either way, record here that we say the output we expect.
> +	    set saw_expected_event true
> +	    exp_continue
> +	}
> +
> +	-re "$::gdb_prompt $" {
> +	    if {$saw_no_event_stop} {
> +		set saw_no_event_stop false
> +		send_gdb "continue\n"
> +		exp_continue
> +	    } else {
> +		gdb_assert {$saw_expected_event} $gdb_test_name
> +	    }
> +	}
> +    }
> +
> +    # Turn off solib events.  We're only interested in validating the
> +    # loads for now.
> +    gdb_test_no_output "set stop-on-solib-events 0"
> +}
> +
> +# Start GDB and run to the point where the test program tries to dlopen a file
> +# from within /proc/self/fd/.  Catch the shared library event and check that
> +# we actually try to load a file from /proc/<INFERIOR-PID>/fd/.
> +#
> +# If SYSROOT is not the empty string, then this is set as the value of GDB's
> +# sysroot immediately after starting GDB.  The only value that is (currently)
> +# supported, other than the empty string, is 'target:'.
> +proc do_test { {sysroot ""} } {
> +    clean_restart $::binfile
> +
> +    if {$sysroot != ""} {
> +	gdb_test_no_output "set sysroot ${sysroot}"
> +    }
> +
> +    gdb_load_shlib $::libobj
> +
> +    if ![runto_main] then {
> +	return 0
> +    }
> +
> +    # Get inferior's PID for later.
> +    set inferior_pid [get_inferior_pid]
> +
> +    # Run to the 'break-here' marker.
> +    gdb_breakpoint [gdb_get_line_number "break-here"]
> +    gdb_continue_to_breakpoint "break-here" ".* break-here .*"
> +
> +    set memfd ""
> +    gdb_test_multiple "p mem_fd" "Get file descriptor" {

Lowercase "Get", please.

Pedro Alves

> +	-re -wrap "\\\$$::decimal = (\[^\r\n\]*)" {
> +	    set memfd $expect_out(1,string)
> +	    pass $gdb_test_name
> +	}
> +    }
> +
> +    # The first call to open_library (in the test program) is done
> +    # with a canonical path.
> +    with_test_prefix "canonical path" {
> +	gdb_breakpoint "open_library"
> +	gdb_continue_to_breakpoint "open_library"
> +	validate_library_load $inferior_pid $memfd
> +    }
> +
> +    # The second call to open_library (in the test program) is done
> +    # with a non-canonical path, however, GDB should resolve this to a
> +    # canonical path for display to the user, so the output we see
> +    # should be unchanged.
> +    with_test_prefix "non-canonical path" {
> +	gdb_breakpoint "open_library"
> +	gdb_continue_to_breakpoint "open_library"
> +	validate_library_load $inferior_pid $memfd
> +    }
> +}
> +
> +# First run of the test.
> +do_test
> +
> +# Possible second run of the test.  If we are using a remote target then we
> +# should consider setting the sysroot to 'target:' and re-running the test.
> +if {[target_info exists gdb_protocol]
> +    && ([target_info gdb_protocol] == "remote"
> +	|| [target_info gdb_protocol] == "extended-remote")} {
> +    # GDB will already be running after the first call to do_test, so we can
> +    # take a peek at the current sysroot setting, and decide if we should
> +    # repeat the test with a different setting.
> +
> +    set new_sysroot ""
> +    gdb_test_multiple "show sysroot" "" {
> +	-wrap -re "The current system root is \"\"\\." {
> +	    pass $gdb_test_name
> +
> +	    # Repeat the test with 'target:' sysroot.
> +	    set new_sysroot "target:"
> +	}
> +	-wrap -re "The current system root is \"target:\"\\." {
> +	    pass $gdb_test_name
> +
> +	    # Nothing else to do, we already tested with target: sysroot.
> +	}
> +	-re "$gdb_prompt $" {
> +	    pass $gdb_test_name
> +
> +	    # If already testing with any other sysroot, we probably should
> +	    # not try to adjust things, so don't do any further testing.
> +	}
> +    }
> +
> +    with_test_prefix "sysroot $new_sysroot" {
> +	if { $new_sysroot != "" } {
> +	    do_test $new_sysroot
> +	}
> +    }
> +}
> 
> base-commit: 7d02a94c8f74613edb0cdde11982b22eaaa9affe
>