[PATCH 0/2] Use gdb_sysroot for main executable on attach

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/2] Use gdb_sysroot for main executable on attach

Gary Benson
Hi all,

This series modifies GDB to prefix the main executable's path with
gdb_sysroot under certain circumstances, namely:

 * The path supplied by target_pid_to_exec_file is absolute, and
 * gdb_sysroot is set and not remote.

This logic is skipped for remote gdb_sysroots because the subsequent
code does not support opening the main executable over the remote
protocol.  This is something I intend to rectify with future patches
but the ability to use gdb_sysroot like this is useful for attaching
to processes running in chroot jails and containerized environments
so I am submitting this series independently.

Built and regtested on RHEL 6.6 x86_64.

Ok to commit?

Thanks,
Gary

--
http://gbenson.net/
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] Use exec_file_find to locate executable on attach (sometimes)

Gary Benson
This commit updates attach_command_post_wait to use exec_file_find
to compute the full pathname of the main executable if the pathname
returned by target_pid_to_exec_file is absolute and if gdb_sysroot
is non-empty and not remote.  The net effect of this is to prefix
the main executable's path with gdb_sysroot if gdb_sysroot is set;
this is useful for attaching to processes running in chroot jails
or containerized environments.  This logic is skipped for remote
gdb_sysroots because the subsequent code does not support opening
the main executable remotely.

gdb/ChangeLog:

        * infcmd.c (filenames.h): New include.
        (remote.h): Likewise.
        (solist.h): Likewise.
        (attach_command_post_wait): Prefix absolute executable
        paths with gdb_sysroot if set and not remote.
---
 gdb/ChangeLog |    8 ++++++++
 gdb/infcmd.c  |   37 +++++++++++++++++++++++++++++--------
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 3737b8f..23c7a6c 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -54,6 +54,9 @@
 #include "continuations.h"
 #include "linespec.h"
 #include "cli/cli-utils.h"
+#include "filenames.h"
+#include "remote.h"
+#include "solist.h"
 
 /* Local functions: */
 
@@ -2493,16 +2496,34 @@ attach_command_post_wait (char *args, int from_tty, int async_exec)
       exec_file = target_pid_to_exec_file (ptid_get_pid (inferior_ptid));
       if (exec_file)
  {
-  /* It's possible we don't have a full path, but rather just a
-     filename.  Some targets, such as HP-UX, don't provide the
-     full path, sigh.
+  /* If there is a gdb_sysroot and exec_file is absolute we
+     prepend gdb_sysroot to exec_file.  This is currently
+     disabled for remote sysroot as the subsequent code cannot
+     yet cope with these.  */
+  if (IS_ABSOLUTE_PATH (exec_file)
+      && gdb_sysroot != NULL && *gdb_sysroot != '\0'
+      && !remote_filename_p (gdb_sysroot))
+    {
+      int fd = -1;
+
+      full_exec_path = exec_file_find (exec_file, &fd);
+      if (fd >= 0)
+ close (fd);
+    }
 
-     Attempt to qualify the filename against the source path.
-     (If that fails, we'll just fall back on the original
-     filename.  Not much more we can do...)  */
+  if (full_exec_path == NULL)
+    {
+      /* It's possible we don't have a full path, but rather
+ just a filename.  Some targets, such as HP-UX, don't
+ provide the full path, sigh.
+
+ Attempt to qualify the filename against the source
+ path.  (If that fails, we'll just fall back on the
+ original filename.  Not much more we can do...)  */
 
-  if (!source_full_path_of (exec_file, &full_exec_path))
-    full_exec_path = xstrdup (exec_file);
+      if (!source_full_path_of (exec_file, &full_exec_path))
+ full_exec_path = xstrdup (exec_file);
+    }
 
   exec_file_attach (full_exec_path, from_tty);
   symbol_file_add_main (full_exec_path, from_tty);
--
1.7.1

Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] Introduce new function exec_file_find

Gary Benson
In reply to this post by Gary Benson
This commit adds a new function, exec_file_find, which computes the
full pathname of the main executable in much the same way solib_find
does for pathnames of shared libraries.  The bulk of the existing
solib_find was moved into a new static function solib_find_1, with
exec_file_find and solib_find being small wrappers for solib_find_1.

gdb/ChangeLog:

        * solist.h (exec_file_find): New declaration.
        * solib.c (solib_find_1): New function.
        (solib_find): Moved most logic into the above.
        (exec_file_find): New function.
---
 gdb/ChangeLog |    7 +++
 gdb/solib.c   |  147 ++++++++++++++++++++++++++++++++++++++------------------
 gdb/solist.h  |    3 +
 3 files changed, 110 insertions(+), 47 deletions(-)

diff --git a/gdb/solib.c b/gdb/solib.c
index 98d5cfd..fc8be01 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -112,69 +112,47 @@ show_solib_search_path (struct ui_file *file, int from_tty,
 #  define DOS_BASED_FILE_SYSTEM 0
 #endif
 
-/* Returns the full pathname of the shared library file, or NULL if
-   not found.  (The pathname is malloc'ed; it needs to be freed by the
-   caller.)  *FD is set to either -1 or an open file handle for the
-   library.
+/* Return the full pathname of a binary file (the main executable
+   or a shared library file), or NULL if not found.  The returned
+   pathname is malloc'ed and must be freed by the caller.  *FD is
+   set to either -1 or an open file handle for the binary file.
 
    Global variable GDB_SYSROOT is used as a prefix directory
-   to search for shared libraries if they have an absolute path.
+   to search for binary files if they have an absolute path.
 
    Global variable SOLIB_SEARCH_PATH is used as a prefix directory
    (or set of directories, as in LD_LIBRARY_PATH) to search for all
-   shared libraries if not found in GDB_SYSROOT.
+   shared libraries if not found in GDB_SYSROOT.  SOLIB_SEARCH_PATH
+   is not used when searching for the main executable.
 
    Search algorithm:
    * If there is a gdb_sysroot and path is absolute:
    *   Search for gdb_sysroot/path.
    * else
    *   Look for it literally (unmodified).
-   * Look in SOLIB_SEARCH_PATH.
-   * If available, use target defined search function.
+   * If IS_SOLIB is non-zero:
+   *   Look in SOLIB_SEARCH_PATH.
+   *   If available, use target defined search function.
    * If gdb_sysroot is NOT set, perform the following two searches:
-   *   Look in inferior's $PATH.
+   *   If IS_SOLIB is non-zero:
+   *     Look in inferior's $PATH.
    *   Look in inferior's $LD_LIBRARY_PATH.
    *
    * The last check avoids doing this search when targetting remote
    * machines since gdb_sysroot will almost always be set.
 */
 
-char *
-solib_find (char *in_pathname, int *fd)
+static char *
+solib_find_1 (char *in_pathname, int *fd, int is_solib)
 {
   const struct target_so_ops *ops = solib_ops (target_gdbarch ());
   int found_file = -1;
   char *temp_pathname = NULL;
   int gdb_sysroot_is_empty;
-  const char *solib_symbols_extension
-    = gdbarch_solib_symbols_extension (target_gdbarch ());
   const char *fskind = effective_target_file_system_kind ();
   struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
   char *sysroot = NULL;
 
-  /* If solib_symbols_extension is set, replace the file's
-     extension.  */
-  if (solib_symbols_extension)
-    {
-      char *p = in_pathname + strlen (in_pathname);
-
-      while (p > in_pathname && *p != '.')
- p--;
-
-      if (*p == '.')
- {
-  char *new_pathname;
-
-  new_pathname = alloca (p - in_pathname + 1
- + strlen (solib_symbols_extension) + 1);
-  memcpy (new_pathname, in_pathname, p - in_pathname + 1);
-  strcpy (new_pathname + (p - in_pathname) + 1,
-  solib_symbols_extension);
-
-  in_pathname = new_pathname;
- }
-    }
-
   gdb_sysroot_is_empty = (gdb_sysroot == NULL || *gdb_sysroot == 0);
 
   if (!gdb_sysroot_is_empty)
@@ -332,23 +310,26 @@ solib_find (char *in_pathname, int *fd)
  in_pathname++;
     }
 
-  /* If not found, search the solib_search_path (if any).  */
-  if (found_file < 0 && solib_search_path != NULL)
+  /* If not found, and we're looking for a solib, search the
+     solib_search_path (if any).  */
+  if (is_solib && found_file < 0 && solib_search_path != NULL)
     found_file = openp (solib_search_path,
  OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH,
  in_pathname, O_RDONLY | O_BINARY, &temp_pathname);
 
-  /* If not found, next search the solib_search_path (if any) for the basename
-     only (ignoring the path).  This is to allow reading solibs from a path
-     that differs from the opened path.  */
-  if (found_file < 0 && solib_search_path != NULL)
+  /* If not found, and we're looking for a solib, next search the
+     solib_search_path (if any) for the basename only (ignoring the
+     path).  This is to allow reading solibs from a path that differs
+     from the opened path.  */
+  if (is_solib && found_file < 0 && solib_search_path != NULL)
     found_file = openp (solib_search_path,
  OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH,
  target_lbasename (fskind, in_pathname),
  O_RDONLY | O_BINARY, &temp_pathname);
 
-  /* If not found, try to use target supplied solib search method.  */
-  if (found_file < 0 && ops->find_and_open_solib)
+  /* If not found, and we're looking for a solib, try to use target
+     supplied solib search method.  */
+  if (is_solib && found_file < 0 && ops->find_and_open_solib)
     found_file = ops->find_and_open_solib (in_pathname, O_RDONLY | O_BINARY,
    &temp_pathname);
 
@@ -359,9 +340,9 @@ solib_find (char *in_pathname, int *fd)
  OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH, in_pathname,
  O_RDONLY | O_BINARY, &temp_pathname);
 
-  /* If not found, next search the inferior's $LD_LIBRARY_PATH
-     environment variable.  */
-  if (found_file < 0 && gdb_sysroot_is_empty)
+  /* If not found, and we're looking for a solib, next search the
+     inferior's $LD_LIBRARY_PATH environment variable.  */
+  if (is_solib && found_file < 0 && gdb_sysroot_is_empty)
     found_file = openp (get_in_environ (current_inferior ()->environment,
  "LD_LIBRARY_PATH"),
  OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH, in_pathname,
@@ -371,6 +352,78 @@ solib_find (char *in_pathname, int *fd)
   return temp_pathname;
 }
 
+/* Return the full pathname of the main executable, or NULL if not
+   found.  The returned pathname is malloc'ed and must be freed by
+   the caller.  *FD is set to either -1 or an open file handle for
+   the main executable.
+
+   The search algorithm used is described in solib_find_1's comment
+   above.  */
+
+char *
+exec_file_find (char *in_pathname, int *fd)
+{
+  char *result = solib_find_1 (in_pathname, fd, 0);
+
+  if (result == NULL)
+    {
+      const char *fskind = effective_target_file_system_kind ();
+
+      if (fskind == file_system_kind_dos_based)
+ {
+  char *new_pathname;
+
+  new_pathname = alloca (strlen (in_pathname) + 5);
+  strcpy (new_pathname, in_pathname);
+  strcat (new_pathname, ".exe");
+
+  result = solib_find_1 (new_pathname, fd, 0);
+ }
+    }
+
+  return result;
+}
+
+/* Return the full pathname of a shared library file, or NULL if not
+   found.  The returned pathname is malloc'ed and must be freed by
+   the caller.  *FD is set to either -1 or an open file handle for
+   the shared library.
+
+   The search algorithm used is described in solib_find_1's comment
+   above.  */
+
+char *
+solib_find (char *in_pathname, int *fd)
+{
+  const char *solib_symbols_extension
+    = gdbarch_solib_symbols_extension (target_gdbarch ());
+
+  /* If solib_symbols_extension is set, replace the file's
+     extension.  */
+  if (solib_symbols_extension != NULL)
+    {
+      char *p = in_pathname + strlen (in_pathname);
+
+      while (p > in_pathname && *p != '.')
+ p--;
+
+      if (*p == '.')
+ {
+  char *new_pathname;
+
+  new_pathname = alloca (p - in_pathname + 1
+ + strlen (solib_symbols_extension) + 1);
+  memcpy (new_pathname, in_pathname, p - in_pathname + 1);
+  strcpy (new_pathname + (p - in_pathname) + 1,
+  solib_symbols_extension);
+
+  in_pathname = new_pathname;
+ }
+    }
+
+  return solib_find_1 (in_pathname, fd, 1);
+}
+
 /* Open and return a BFD for the shared library PATHNAME.  If FD is not -1,
    it is used as file handle to open the file.  Throws an error if the file
    could not be opened.  Handles both local and remote file access.
diff --git a/gdb/solist.h b/gdb/solist.h
index 148bec1..7021f5c 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -176,6 +176,9 @@ void free_so (struct so_list *so);
 /* Return address of first so_list entry in master shared object list.  */
 struct so_list *master_so_list (void);
 
+/* Find main executable binary file.  */
+extern char *exec_file_find (char *in_pathname, int *fd);
+
 /* Find shared library binary file.  */
 extern char *solib_find (char *in_pathname, int *fd);
 
--
1.7.1

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Use gdb_sysroot for main executable on attach

Mark Kettenis
In reply to this post by Gary Benson
> From: Gary Benson <[hidden email]>
> Date: Thu,  5 Mar 2015 11:37:39 +0000
>
> Hi all,
>
> This series modifies GDB to prefix the main executable's path with
> gdb_sysroot under certain circumstances, namely:
>
>  * The path supplied by target_pid_to_exec_file is absolute, and
>  * gdb_sysroot is set and not remote.
>
> This logic is skipped for remote gdb_sysroots because the subsequent
> code does not support opening the main executable over the remote
> protocol.  This is something I intend to rectify with future patches
> but the ability to use gdb_sysroot like this is useful for attaching
> to processes running in chroot jails and containerized environments
> so I am submitting this series independently.
>
> Built and regtested on RHEL 6.6 x86_64.
>
> Ok to commit?

What problem are you trying to fix?
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Use gdb_sysroot for main executable on attach

Gary Benson
Mark Kettenis wrote:

> > From: Gary Benson <[hidden email]>
> >
> > This series modifies GDB to prefix the main executable's path with
> > gdb_sysroot under certain circumstances, namely:
> >
> >  * The path supplied by target_pid_to_exec_file is absolute, and
> >  * gdb_sysroot is set and not remote.
> >
> > This logic is skipped for remote gdb_sysroots because the subsequent
> > code does not support opening the main executable over the remote
> > protocol.  This is something I intend to rectify with future patches
> > but the ability to use gdb_sysroot like this is useful for attaching
> > to processes running in chroot jails and containerized environments
> > so I am submitting this series independently.
>
> What problem are you trying to fix?

I'm mostly trying to eliminate the extra work users have to do in
order to attach to remote processes or to attach to local processes
running in containers.

For remote processes you have to do something like:

  set sysroot remote:
  file /path/to/local/copy/of/binary
  target remote WHEREVER

or:
 
  set sysroot /path/to/local/copy
  file /path/to/local/copy/of/binary
  target remote WHEREVER

I would ideally like to get to the situation where the only command
you need is "target remote ...", but a step in that direction is
removing the required "file" command.

For processes running in containers you have to do something like:

  set sysroot /proc/PID/root
  file /proc/PID/exe
  attach PID

Again, I'd like to get to the situation where the only command you
need is "attach PID".  This series removes the need for the "file"
command in this sequence, but you still need the "set sysroot".

For the ultimate solution (removing the need for "set sysroot")
Pedro suggested a new option, "set sysroot target:" that would be
the default and would mean "if the target is remote, pull binaries
over the remote protocol; if the target is local, grab them from
the filesystem."  There's details here:

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

Aside from removing the need for the user to set a sysroot for
these two cases (where GDB has enough information to imply what
the user is asking for) it unblocks multi-inferior debugging
when different inferiors require different sysroots, something
that can't be done right now.  I don't think this is relevant
for remote debugging right now as we only support one gdbserver
connection at a time, but it is relevant for containers where
you might need to debug, eg, a webserver in one container
talking to a database server in another.

I would also like to make it possible to fetch stripped debug
info over the remote protocol if the user desires it, so that's
something else I'm thinking about with this work.

Cheers,
Gary

--
http://gbenson.net/
Reply | Threaded
Open this post in threaded view
|

[PING][PATCH 0/2] Use gdb_sysroot for main executable on attach

Gary Benson
In reply to this post by Gary Benson
Ping:
https://sourceware.org/ml/gdb-patches/2015-03/msg00149.html

So far Mark asked a question to which I responded:
https://sourceware.org/ml/gdb-patches/2015-03/msg00152.html

Gary Benson wrote:

> Hi all,
>
> This series modifies GDB to prefix the main executable's path with
> gdb_sysroot under certain circumstances, namely:
>
>  * The path supplied by target_pid_to_exec_file is absolute, and
>  * gdb_sysroot is set and not remote.
>
> This logic is skipped for remote gdb_sysroots because the subsequent
> code does not support opening the main executable over the remote
> protocol.  This is something I intend to rectify with future patches
> but the ability to use gdb_sysroot like this is useful for attaching
> to processes running in chroot jails and containerized environments
> so I am submitting this series independently.
>
> Built and regtested on RHEL 6.6 x86_64.
>
> Ok to commit?
>
> Thanks,
> Gary
>
> --
> http://gbenson.net/
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Use gdb_sysroot for main executable on attach

Gary Benson
In reply to this post by Gary Benson
Hi all,

Please don't review this series, I'm going to rework and resubmit
them as part of a larger series I'm working on.

Cheers,
Gary

Gary Benson wrote:

> Hi all,
>
> This series modifies GDB to prefix the main executable's path with
> gdb_sysroot under certain circumstances, namely:
>
>  * The path supplied by target_pid_to_exec_file is absolute, and
>  * gdb_sysroot is set and not remote.
>
> This logic is skipped for remote gdb_sysroots because the subsequent
> code does not support opening the main executable over the remote
> protocol.  This is something I intend to rectify with future patches
> but the ability to use gdb_sysroot like this is useful for attaching
> to processes running in chroot jails and containerized environments
> so I am submitting this series independently.
>
> Built and regtested on RHEL 6.6 x86_64.
>
> Ok to commit?
>
> Thanks,
> Gary
>
> --
> http://gbenson.net/