public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/7] Do not require "file" commands for remote targets
@ 2015-04-01 11:22 Gary Benson
  2015-04-01 11:22 ` [PATCH 1/7] Introduce exec_file_locate_attach Gary Benson
                   ` (7 more replies)
  0 siblings, 8 replies; 52+ messages in thread
From: Gary Benson @ 2015-04-01 11:22 UTC (permalink / raw)
  To: gdb-patches

Hi all,

This series builds on my 'New default sysroot "target:"' series [1]
and makes GDB able to locate and access executable files when using
remote targets.  This removes the need for a "file" command before
"target *remote ...".

Patches 1-3 cause the main executable's pathname to be prefixed with
gdb_sysroot on attach in much the same way pathnames of shared
libraries are treated.

Patches 4-6 implement the to_pid_to_exec_file method for remote
targets.  This removes the need for the "file" command when using
the "attach" command with gdbserver with multiprocess extensions:

  bash$ gdb -q
  (gdb) target extended-remote | gdbserver --multi -
  Remote debugging using | gdbserver --multi -
  Remote debugging using stdio
  (gdb) attach 31979
  Attaching to process 31979
  Attached; pid = 31979
  Reading symbols from target:/bin/bash...

Patch 7 causes GDB to attempt to locate and open the executable
in remote-target cases without multiprocess extensions:

  bash$ gdb -q
  (gdb) target remote | gdbserver - --attach 31979
  Remote debugging using | gdbserver - --attach 31979
  Attached; pid = 31979
  Remote debugging using stdio
  Reading symbols from target:/bin/bash...

and:

  bash$ gdb -q
  (gdb) target remote | gdbserver - /bin/sh
  Remote debugging using | gdbserver - /bin/sh
  Process /bin/sh created; pid = 32166
  stdin/stdout redirected
  Remote debugging using stdio
  Reading symbols from target:/bin/bash...

There is no change to GDB's behaviour if the user has specified a
main executable, with the "file" command or on startup, so existing
use cases do not change.

Built and regtested on RHEL 6.6 x86_64.

Ok to commit?

Thanks,
Gary

--
[1] https://sourceware.org/ml/gdb-patches/2015-03/msg00638.html

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 3/7] Use gdb_sysroot for main executable on attach
  2015-04-01 11:22 [PATCH 0/7] Do not require "file" commands for remote targets Gary Benson
  2015-04-01 11:22 ` [PATCH 1/7] Introduce exec_file_locate_attach Gary Benson
@ 2015-04-01 11:22 ` Gary Benson
  2015-04-01 14:53   ` Eli Zaretskii
  2015-04-15 10:43   ` Pedro Alves
  2015-04-01 11:22 ` [PATCH 2/7] Introduce exec_file_find Gary Benson
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 52+ messages in thread
From: Gary Benson @ 2015-04-01 11:22 UTC (permalink / raw)
  To: gdb-patches

This commit updates exec_file_locate_attach to use exec_file_find
to compute the full pathname of the main executable in some cases.
The net effect of this is that the main executable's path will be
prefixed with gdb_sysroot in the same way that shared library paths
currently are.

gdb/ChangeLog:

	* exec.c (solist.h): New include.
	(exec_file_locate_attach): Prefix absolute executable
	paths with gdb_sysroot if set.
	* NEWS: Mention that executable paths may be prepended
	with sysroot on attach.

gdb/doc/ChangeLog:

	* gdb.texinfo (set sysroot): Document that "set sysroot" also
	applies to executable paths if supplied to GDB as absolute.
---
 gdb/ChangeLog       |    8 ++++++++
 gdb/NEWS            |    5 +++++
 gdb/doc/ChangeLog   |    5 +++++
 gdb/doc/gdb.texinfo |   22 ++++++++++++----------
 gdb/exec.c          |   32 ++++++++++++++++++++++++--------
 5 files changed, 54 insertions(+), 18 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index f2a51a4..dec60cb 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -16,6 +16,11 @@
   system, be it local or remote.  This replaces the prefix "remote:".
   The default sysroot has been changed from "" to "target:".
 
+* Paths specified by "set sysroot" will be prepended to the path of
+  the main executable when attaching to already-running processes
+  (local and remote) if the path of the main executable is reported
+  to GDB as absolute by the operating system.
+
 * Python Scripting
 
   ** gdb.Objfile objects have a new attribute "username",
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 93e7e87..692d70e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -17800,18 +17800,20 @@ may try to load the host's libraries.  @value{GDBN} has two variables
 to specify the search directories for target libraries.
 
 @table @code
-@cindex prefix for shared library file names
+@cindex prefix for executable and shared library file names
 @cindex system root, alternate
 @kindex set solib-absolute-prefix
 @kindex set sysroot
 @item set sysroot @var{path}
 Use @var{path} as the system root for the program being debugged.  Any
-absolute shared library paths will be prefixed with @var{path}; many
-runtime loaders store the absolute paths to the shared library in the
-target program's memory.  If you use @code{set sysroot} to find shared
-libraries, they need to be laid out in the same way that they are on
-the target, with e.g.@: a @file{/lib} and @file{/usr/lib} hierarchy
-under @var{path}.
+shared library paths will be prefixed with @var{path}; many runtime
+loaders store the absolute paths to the shared library in the target
+program's memory.  When attaching to already-running processes, their
+paths will be prefixed with @var{path} if reported to @value{GDBN} as
+absolute by the operating system.  If you use @code{set sysroot} to
+find executables and shared libraries, they need to be laid out in
+the same way that they are on the target, with e.g.@: a @file{/bin},
+@file{/lib} and @file{/usr/lib} hierarchy under @var{path}.
 
 If @var{path} starts with the sequence @file{target:} and the target
 system is remote then @value{GDBN} will retrieve the target binaries
@@ -17846,7 +17848,7 @@ system:
   c:/foo/bar.dll @result{} /path/to/sysroot/c:/foo/bar.dll
 @end smallexample
 
-If that does not find the shared library, @value{GDBN} tries removing
+If that does not find the binary, @value{GDBN} tries removing
 the @samp{:} character from the drive spec, both for convenience, and,
 for the case of the host file system not supporting file names with
 colons:
@@ -17871,7 +17873,7 @@ and point the system root at @file{/path/to/sysroot}, so that
 @value{GDBN} can find the correct copies of both
 @file{c:\sys\bin\foo.dll}, and @file{z:\sys\bin\bar.dll}.
 
-If that still does not find the shared library, @value{GDBN} tries
+If that still does not find the binary, @value{GDBN} tries
 removing the whole drive spec from the target file name:
 
 @smallexample
@@ -17895,7 +17897,7 @@ location.
 
 @kindex show sysroot
 @item show sysroot
-Display the current shared library prefix.
+Display the current executable and shared library prefix.
 
 @kindex set solib-search-path
 @item set solib-search-path @var{path}
diff --git a/gdb/exec.c b/gdb/exec.c
index ab7fcbb..99c0cd4 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -42,6 +42,7 @@
 
 #include <ctype.h>
 #include <sys/stat.h>
+#include "solist.h"
 
 void (*deprecated_file_changed_hook) (char *);
 
@@ -151,15 +152,30 @@ exec_file_locate_attach (int pid, int from_tty)
   if (exec_file == NULL)
     return;
 
-  /* 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 gdb_sysroot is not empty and the discovered filename
+     is absolute then prefix the filename with gdb_sysroot.  */
+  if (gdb_sysroot != NULL && *gdb_sysroot != '\0'
+      && IS_ABSOLUTE_PATH (exec_file))
+    {
+      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 (!source_full_path_of (exec_file, &full_exec_path))
-    full_exec_path = xstrdup (exec_file);
+  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);
+    }
 
   exec_file_attach (full_exec_path, from_tty);
   symbol_file_add_main (full_exec_path, from_tty);
-- 
1.7.1

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 2/7] Introduce exec_file_find
  2015-04-01 11:22 [PATCH 0/7] Do not require "file" commands for remote targets Gary Benson
  2015-04-01 11:22 ` [PATCH 1/7] Introduce exec_file_locate_attach Gary Benson
  2015-04-01 11:22 ` [PATCH 3/7] Use gdb_sysroot for main executable on attach Gary Benson
@ 2015-04-01 11:22 ` Gary Benson
  2015-04-01 11:27 ` [PATCH 4/7] Introduce linux_pid_to_exec_file Gary Benson
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 52+ messages in thread
From: Gary Benson @ 2015-04-01 11:22 UTC (permalink / raw)
  To: gdb-patches

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, factored out from...
	(solib_find): ...here.
	(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 359a208..eaeb772 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -112,13 +112,13 @@ 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.
    If GDB_SYSROOT starts with "target:" and target filesystem
    is the local filesystem then the "target:" prefix will be
    stripped before the search starts.  This ensures that the
@@ -128,58 +128,36 @@ show_solib_search_path (struct ui_file *file, int from_tty,
    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 either the sysroot (if set) or
-   the local filesystem.
+   the local filesystem.  SOLIB_SEARCH_PATH is not used when searching
+   for the main executable.
 
    Search algorithm:
    * If a sysroot is set and path is absolute:
    *   Search for 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 NO sysroot is set, perform the following two searches:
    *   Look in inferior's $PATH.
-   *   Look in inferior's $LD_LIBRARY_PATH.
+   *   If IS_SOLIB is non-zero:
+   *     Look in inferior's $LD_LIBRARY_PATH.
    *
    * The last check avoids doing this search when targetting remote
    * machines since a 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;
-  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 = gdb_sysroot;
 
-  /* 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;
-	}
-    }
-
   if (sysroot != NULL)
     {
       /* If the absolute prefix starts with "target:" but the
@@ -351,23 +329,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);
 
@@ -378,9 +359,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 && sysroot == NULL)
+  /* 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 && sysroot == NULL)
     found_file = openp (get_in_environ (current_inferior ()->environment,
 					"LD_LIBRARY_PATH"),
 			OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH, in_pathname,
@@ -390,6 +371,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

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 1/7] Introduce exec_file_locate_attach
  2015-04-01 11:22 [PATCH 0/7] Do not require "file" commands for remote targets Gary Benson
@ 2015-04-01 11:22 ` Gary Benson
  2015-04-01 11:22 ` [PATCH 3/7] Use gdb_sysroot for main executable on attach Gary Benson
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 52+ messages in thread
From: Gary Benson @ 2015-04-01 11:22 UTC (permalink / raw)
  To: gdb-patches

This commit adds a new function, exec_file_locate_attach, which works
like exec_file_attach except that, instead of a filename argument, it
takes an integer process ID and attempts to determine the executable
filename from that.

gdb/ChangeLog:

	* gdbcore.h (exec_file_locate_attach): New declaration.
	* exec.c (exec_file_locate_attach): New function, factored
	out from...
	* infcmd.c (attach_command_post_wait): ...here.
---
 gdb/ChangeLog |    7 +++++++
 gdb/exec.c    |   31 +++++++++++++++++++++++++++++++
 gdb/gdbcore.h |    7 +++++++
 gdb/infcmd.c  |   25 ++-----------------------
 4 files changed, 47 insertions(+), 23 deletions(-)

diff --git a/gdb/exec.c b/gdb/exec.c
index 113e869..ab7fcbb 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -134,6 +134,37 @@ exec_file_clear (int from_tty)
     printf_unfiltered (_("No executable file now.\n"));
 }
 
+/* See gdbcore.h.  */
+
+void
+exec_file_locate_attach (int pid, int from_tty)
+{
+  char *exec_file, *full_exec_path = NULL;
+
+  /* Do nothing if we already have an executable filename.  */
+  exec_file = (char *) get_exec_file (0);
+  if (exec_file != NULL)
+    return;
+
+  /* Try to determine a filename from the process itself.  */
+  exec_file = target_pid_to_exec_file (pid);
+  if (exec_file == NULL)
+    return;
+
+  /* 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);
+
+  exec_file_attach (full_exec_path, from_tty);
+  symbol_file_add_main (full_exec_path, from_tty);
+}
+
 /* Set FILENAME as the new exec file.
 
    This function is intended to be behave essentially the same
diff --git a/gdb/gdbcore.h b/gdb/gdbcore.h
index 63a75f0..a437c8a 100644
--- a/gdb/gdbcore.h
+++ b/gdb/gdbcore.h
@@ -150,6 +150,13 @@ extern void core_file_command (char *filename, int from_tty);
 
 extern void exec_file_attach (const char *filename, int from_tty);
 
+/* If the filename of the main executable is unknown, attempt to
+   determine it.  If a filename is determined, proceed as though
+   it was just specified with the "file" command.  Do nothing if
+   the filename of the main executable is already known.  */
+
+extern void exec_file_locate_attach (int pid, int from_tty);
+
 extern void exec_file_clear (int from_tty);
 
 extern void validate_files (void);
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 1c70675..f028d60 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2489,8 +2489,6 @@ proceed_after_attach (int pid)
 static void
 attach_command_post_wait (char *args, int from_tty, int async_exec)
 {
-  char *exec_file;
-  char *full_exec_path = NULL;
   struct inferior *inferior;
 
   inferior = current_inferior ();
@@ -2498,27 +2496,8 @@ attach_command_post_wait (char *args, int from_tty, int async_exec)
 
   /* If no exec file is yet known, try to determine it from the
      process itself.  */
-  exec_file = (char *) get_exec_file (0);
-  if (!exec_file)
-    {
-      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.
-
-	     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);
-
-	  exec_file_attach (full_exec_path, from_tty);
-	  symbol_file_add_main (full_exec_path, from_tty);
-	}
-    }
+  if (get_exec_file (0) == NULL)
+    exec_file_locate_attach (ptid_get_pid (inferior_ptid), from_tty);
   else
     {
       reopen_exec_file ();
-- 
1.7.1

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 4/7] Introduce linux_pid_to_exec_file
  2015-04-01 11:22 [PATCH 0/7] Do not require "file" commands for remote targets Gary Benson
                   ` (2 preceding siblings ...)
  2015-04-01 11:22 ` [PATCH 2/7] Introduce exec_file_find Gary Benson
@ 2015-04-01 11:27 ` Gary Benson
  2015-04-06 16:41   ` Doug Evans
  2015-04-15  9:37   ` Pedro Alves
  2015-04-01 11:29 ` [PATCH 5/7] Implement remote_pid_to_exec_file using qXfer:exec-file:read Gary Benson
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 52+ messages in thread
From: Gary Benson @ 2015-04-01 11:27 UTC (permalink / raw)
  To: gdb-patches

This commit introduces a new function, linux_pid_to_exec_file, that
shared Linux code can use to discover the filename of the executable
that was run to create a process on the system.

gdb/ChangeLog:

	* nat/linux-nat.h (linux_pid_to_exec_file): New declaration.
	* nat/linux-nat.c: New file.
	* Makefile.in (common-linux-nat.o): New rule.
	* config/aarch64/linux.mh (NATDEPFILES): Add common-linux-nat.o.
	* config/alpha/alpha-linux.mh (NATDEPFILES): Likewise.
	* config/arm/linux.mh (NATDEPFILES): Likewise.
	* config/i386/linux.mh (NATDEPFILES): Likewise.
	* config/i386/linux64.mh (NATDEPFILES): Likewise.
	* config/ia64/linux.mh (NATDEPFILES): Likewise.
	* config/m32r/linux.mh (NATDEPFILES): Likewise.
	* config/m68k/linux.mh (NATDEPFILES): Likewise.
	* config/mips/linux.mh (NATDEPFILES): Likewise.
	* config/pa/linux.mh (NATDEPFILES): Likewise.
	* config/powerpc/linux.mh (NATDEPFILES): Likewise.
	* config/powerpc/ppc64-linux.mh (NATDEPFILES): Likewise.
	* config/powerpc/spu-linux.mh (NATDEPFILES): Likewise.
	* config/s390/linux.mh (NATDEPFILES): Likewise.
	* config/sparc/linux.mh (NATDEPFILES): Likewise.
	* config/sparc/linux64.mh (NATDEPFILES): Likewise.
	* config/tilegx/linux.mh (NATDEPFILES): Likewise.
	* config/xtensa/linux.mh (NATDEPFILES): Likewise.
	* linux-nat.c (linux_child_pid_to_exec_file): Factored out
	to new function linux_pid_to_exec_file in nat/linux-nat.c.

gdb/gdbserver/ChangeLog:

	* Makefile.in (common-linux-nat.o): New rule.
	* configure.srv (srv_linux_obj): Add common-linux-nat.o.
---
 gdb/ChangeLog                     |   26 ++++++++++++++++++++++++++
 gdb/Makefile.in                   |    4 ++++
 gdb/config/aarch64/linux.mh       |    2 +-
 gdb/config/alpha/alpha-linux.mh   |    2 +-
 gdb/config/arm/linux.mh           |    2 +-
 gdb/config/i386/linux.mh          |    2 +-
 gdb/config/i386/linux64.mh        |    2 +-
 gdb/config/ia64/linux.mh          |    2 +-
 gdb/config/m32r/linux.mh          |    2 +-
 gdb/config/m68k/linux.mh          |    2 +-
 gdb/config/mips/linux.mh          |    2 +-
 gdb/config/pa/linux.mh            |    2 +-
 gdb/config/powerpc/linux.mh       |    2 +-
 gdb/config/powerpc/ppc64-linux.mh |    2 +-
 gdb/config/powerpc/spu-linux.mh   |    3 ++-
 gdb/config/s390/linux.mh          |    2 +-
 gdb/config/sparc/linux.mh         |    2 +-
 gdb/config/sparc/linux64.mh       |    2 +-
 gdb/config/tilegx/linux.mh        |    2 +-
 gdb/config/xtensa/linux.mh        |    2 +-
 gdb/gdbserver/ChangeLog           |    5 +++++
 gdb/gdbserver/Makefile.in         |    3 +++
 gdb/gdbserver/configure.srv       |    2 +-
 gdb/linux-nat.c                   |   10 +---------
 gdb/nat/linux-nat.c               |   37 +++++++++++++++++++++++++++++++++++++
 gdb/nat/linux-nat.h               |    9 +++++++++
 26 files changed, 105 insertions(+), 28 deletions(-)
 create mode 100644 gdb/nat/linux-nat.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 907997b..6920358 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -2310,6 +2310,10 @@ x86-linux-dregs.o: ${srcdir}/nat/x86-linux-dregs.c
 	$(COMPILE) $(srcdir)/nat/x86-linux-dregs.c
 	$(POSTCOMPILE)
 
+common-linux-nat.o: ${srcdir}/nat/linux-nat.c
+	$(COMPILE) $(srcdir)/nat/linux-nat.c
+	$(POSTCOMPILE)
+
 #
 # gdb/tui/ dependencies
 #
diff --git a/gdb/config/aarch64/linux.mh b/gdb/config/aarch64/linux.mh
index 7f96e4d..d22150a 100644
--- a/gdb/config/aarch64/linux.mh
+++ b/gdb/config/aarch64/linux.mh
@@ -22,7 +22,7 @@ NAT_FILE= config/nm-linux.h
 NATDEPFILES= inf-ptrace.o fork-child.o aarch64-linux-nat.o \
 	proc-service.o linux-thread-db.o linux-nat.o linux-fork.o \
 	linux-procfs.o linux-ptrace.o linux-osdata.o linux-waitpid.o \
-	linux-personality.o
+	linux-personality.o common-linux-nat.o
 NAT_CDEPS = $(srcdir)/proc-service.list
 
 LOADLIBES= -ldl $(RDYNAMIC)
diff --git a/gdb/config/alpha/alpha-linux.mh b/gdb/config/alpha/alpha-linux.mh
index 2ea02a1..81b1199 100644
--- a/gdb/config/alpha/alpha-linux.mh
+++ b/gdb/config/alpha/alpha-linux.mh
@@ -3,7 +3,7 @@ NAT_FILE= config/nm-linux.h
 NATDEPFILES= inf-ptrace.o alpha-linux-nat.o \
 	fork-child.o proc-service.o linux-thread-db.o \
 	linux-nat.o linux-osdata.o linux-fork.o linux-procfs.o linux-ptrace.o \
-	linux-waitpid.o linux-personality.o
+	linux-waitpid.o linux-personality.o common-linux-nat.o
 NAT_CDEPS = $(srcdir)/proc-service.list
 
 # The dynamically loaded libthread_db needs access to symbols in the
diff --git a/gdb/config/arm/linux.mh b/gdb/config/arm/linux.mh
index 549bf42..e92188a 100644
--- a/gdb/config/arm/linux.mh
+++ b/gdb/config/arm/linux.mh
@@ -4,7 +4,7 @@ NAT_FILE= config/nm-linux.h
 NATDEPFILES= inf-ptrace.o fork-child.o arm-linux-nat.o \
 	proc-service.o linux-thread-db.o \
 	linux-nat.o linux-osdata.o linux-fork.o linux-procfs.o linux-ptrace.o \
-	linux-waitpid.o linux-personality.o
+	linux-waitpid.o linux-personality.o common-linux-nat.o
 NAT_CDEPS = $(srcdir)/proc-service.list
 
 LOADLIBES= -ldl $(RDYNAMIC)
diff --git a/gdb/config/i386/linux.mh b/gdb/config/i386/linux.mh
index 9be2c5f..a32d940 100644
--- a/gdb/config/i386/linux.mh
+++ b/gdb/config/i386/linux.mh
@@ -6,7 +6,7 @@ NATDEPFILES= inf-ptrace.o fork-child.o \
 	proc-service.o linux-thread-db.o \
 	linux-nat.o linux-osdata.o linux-fork.o linux-procfs.o linux-ptrace.o \
 	linux-btrace.o linux-waitpid.o linux-personality.o x86-linux.o \
-	x86-linux-dregs.o
+	x86-linux-dregs.o common-linux-nat.o
 NAT_CDEPS = $(srcdir)/proc-service.list
 
 # The dynamically loaded libthread_db needs access to symbols in the
diff --git a/gdb/config/i386/linux64.mh b/gdb/config/i386/linux64.mh
index 224a2a9..7666814 100644
--- a/gdb/config/i386/linux64.mh
+++ b/gdb/config/i386/linux64.mh
@@ -6,7 +6,7 @@ NATDEPFILES= inf-ptrace.o fork-child.o \
 	proc-service.o linux-thread-db.o linux-fork.o \
 	linux-procfs.o linux-ptrace.o linux-btrace.o \
 	linux-waitpid.o linux-personality.o x86-linux.o \
-	x86-linux-dregs.o
+	x86-linux-dregs.o common-linux-nat.o
 NAT_FILE= config/nm-linux.h
 NAT_CDEPS = $(srcdir)/proc-service.list
 
diff --git a/gdb/config/ia64/linux.mh b/gdb/config/ia64/linux.mh
index 9dce22b..8d9dc12 100644
--- a/gdb/config/ia64/linux.mh
+++ b/gdb/config/ia64/linux.mh
@@ -6,7 +6,7 @@ NATDEPFILES= inf-ptrace.o fork-child.o \
 	proc-service.o linux-thread-db.o \
 	linux-nat.o linux-osdata.o linux-fork.o \
 	linux-personality.o \
-	linux-procfs.o linux-ptrace.o linux-waitpid.o
+	linux-procfs.o linux-ptrace.o linux-waitpid.o common-linux-nat.o
 NAT_CDEPS = $(srcdir)/proc-service.list
 
 LOADLIBES = -ldl $(RDYNAMIC)
diff --git a/gdb/config/m32r/linux.mh b/gdb/config/m32r/linux.mh
index 6b810e6..d83df66 100644
--- a/gdb/config/m32r/linux.mh
+++ b/gdb/config/m32r/linux.mh
@@ -4,7 +4,7 @@ NAT_FILE= config/nm-linux.h
 NATDEPFILES= inf-ptrace.o fork-child.o				\
 	m32r-linux-nat.o proc-service.o linux-thread-db.o	\
 	linux-nat.o linux-osdata.o linux-fork.o linux-procfs.o linux-ptrace.o \
-	linux-waitpid.o linux-personality.o
+	linux-waitpid.o linux-personality.o common-linux-nat.o
 NAT_CDEPS = $(srcdir)/proc-service.list
 
 LOADLIBES= -ldl $(RDYNAMIC)
diff --git a/gdb/config/m68k/linux.mh b/gdb/config/m68k/linux.mh
index f3b3baa..1bc1d78 100644
--- a/gdb/config/m68k/linux.mh
+++ b/gdb/config/m68k/linux.mh
@@ -6,7 +6,7 @@ NATDEPFILES= inf-ptrace.o fork-child.o \
 	proc-service.o linux-thread-db.o \
 	linux-nat.o linux-osdata.o linux-fork.o linux-procfs.o linux-ptrace.o \
 	linux-personality.o \
-	linux-waitpid.o
+	linux-waitpid.o common-linux-nat.o
 NAT_CDEPS = $(srcdir)/proc-service.list
 
 # The dynamically loaded libthread_db needs access to symbols in the
diff --git a/gdb/config/mips/linux.mh b/gdb/config/mips/linux.mh
index d6a802f..577e410 100644
--- a/gdb/config/mips/linux.mh
+++ b/gdb/config/mips/linux.mh
@@ -5,7 +5,7 @@ NATDEPFILES= inf-ptrace.o fork-child.o mips-linux-nat.o \
 	linux-nat.o linux-osdata.o linux-fork.o \
 	linux-procfs.o linux-ptrace.o linux-waitpid.o \
 	linux-personality.o \
-	mips-linux-watch.o
+	mips-linux-watch.o common-linux-nat.o
 NAT_CDEPS = $(srcdir)/proc-service.list
 
 LOADLIBES = -ldl $(RDYNAMIC)
diff --git a/gdb/config/pa/linux.mh b/gdb/config/pa/linux.mh
index 9539b64..97230b8 100644
--- a/gdb/config/pa/linux.mh
+++ b/gdb/config/pa/linux.mh
@@ -4,7 +4,7 @@ NATDEPFILES= inf-ptrace.o fork-child.o \
 	hppa-linux-nat.o proc-service.o linux-thread-db.o \
 	linux-nat.o linux-osdata.o linux-fork.o \
 	linux-procfs.o linux-ptrace.o linux-waitpid.o \
-	linux-personality.o
+	linux-personality.o common-linux-nat.o
 NAT_CDEPS = $(srcdir)/proc-service.list
 
 LOADLIBES = -ldl $(RDYNAMIC)
diff --git a/gdb/config/powerpc/linux.mh b/gdb/config/powerpc/linux.mh
index 76e62c0..f8f8a6e 100644
--- a/gdb/config/powerpc/linux.mh
+++ b/gdb/config/powerpc/linux.mh
@@ -6,7 +6,7 @@ NAT_FILE= config/nm-linux.h
 NATDEPFILES= inf-ptrace.o fork-child.o \
 	ppc-linux-nat.o proc-service.o linux-thread-db.o \
 	linux-nat.o linux-osdata.o linux-fork.o linux-procfs.o linux-ptrace.o \
-	linux-waitpid.o linux-personality.o
+	linux-waitpid.o linux-personality.o common-linux-nat.o
 NAT_CDEPS = $(srcdir)/proc-service.list
 
 LOADLIBES = -ldl $(RDYNAMIC)
diff --git a/gdb/config/powerpc/ppc64-linux.mh b/gdb/config/powerpc/ppc64-linux.mh
index 7eb6507..f9194a0 100644
--- a/gdb/config/powerpc/ppc64-linux.mh
+++ b/gdb/config/powerpc/ppc64-linux.mh
@@ -6,7 +6,7 @@ NAT_FILE= config/nm-linux.h
 NATDEPFILES= inf-ptrace.o fork-child.o \
 	ppc-linux-nat.o proc-service.o linux-thread-db.o \
 	linux-nat.o linux-osdata.o linux-fork.o linux-procfs.o linux-ptrace.o \
-	linux-waitpid.o ppc-linux.o linux-personality.o
+	linux-waitpid.o ppc-linux.o linux-personality.o common-linux-nat.o
 NAT_CDEPS = $(srcdir)/proc-service.list
 
 # The PowerPC has severe limitations on TOC size, and uses them even
diff --git a/gdb/config/powerpc/spu-linux.mh b/gdb/config/powerpc/spu-linux.mh
index d44aeeb..345ac8d 100644
--- a/gdb/config/powerpc/spu-linux.mh
+++ b/gdb/config/powerpc/spu-linux.mh
@@ -4,5 +4,6 @@
 # PPU side of the Cell BE and debugging the SPU side.
 
 NATDEPFILES = spu-linux-nat.o fork-child.o inf-ptrace.o \
-	      linux-procfs.o linux-ptrace.o linux-waitpid.o linux-personality.o
+	      linux-procfs.o linux-ptrace.o linux-waitpid.o \
+	      linux-personality.o common-linux-nat.o
 
diff --git a/gdb/config/s390/linux.mh b/gdb/config/s390/linux.mh
index e1ad899..9938e73 100644
--- a/gdb/config/s390/linux.mh
+++ b/gdb/config/s390/linux.mh
@@ -4,6 +4,6 @@ NATDEPFILES= inf-ptrace.o fork-child.o s390-linux-nat.o \
 	linux-thread-db.o proc-service.o \
 	linux-nat.o linux-osdata.o linux-fork.o linux-procfs.o linux-ptrace.o \
 	linux-personality.o \
-	linux-waitpid.o
+	linux-waitpid.o common-linux-nat.o
 NAT_CDEPS = $(srcdir)/proc-service.list
 LOADLIBES = -ldl $(RDYNAMIC)
diff --git a/gdb/config/sparc/linux.mh b/gdb/config/sparc/linux.mh
index bd7fc86..30cc477 100644
--- a/gdb/config/sparc/linux.mh
+++ b/gdb/config/sparc/linux.mh
@@ -5,7 +5,7 @@ NATDEPFILES= sparc-nat.o sparc-linux-nat.o \
 	proc-service.o linux-thread-db.o \
 	linux-nat.o linux-osdata.o linux-fork.o \
 	linux-procfs.o linux-ptrace.o linux-waitpid.o \
-	linux-personality.o
+	linux-personality.o common-linux-nat.o
 NAT_CDEPS = $(srcdir)/proc-service.list
 
 # The dynamically loaded libthread_db needs access to symbols in the
diff --git a/gdb/config/sparc/linux64.mh b/gdb/config/sparc/linux64.mh
index 86f984f..73fa282 100644
--- a/gdb/config/sparc/linux64.mh
+++ b/gdb/config/sparc/linux64.mh
@@ -5,7 +5,7 @@ NATDEPFILES= sparc-nat.o sparc64-nat.o sparc64-linux-nat.o \
 	proc-service.o linux-thread-db.o \
 	linux-nat.o linux-osdata.o linux-fork.o \
 	linux-procfs.o linux-ptrace.o linux-waitpid.o \
-	linux-personality.o
+	linux-personality.o common-linux-nat.o
 NAT_CDEPS = $(srcdir)/proc-service.list
 
 # The dynamically loaded libthread_db needs access to symbols in the
diff --git a/gdb/config/tilegx/linux.mh b/gdb/config/tilegx/linux.mh
index b5edcd4..cd5528d 100644
--- a/gdb/config/tilegx/linux.mh
+++ b/gdb/config/tilegx/linux.mh
@@ -6,7 +6,7 @@ NATDEPFILES= inf-ptrace.o fork-child.o \
 	proc-service.o linux-thread-db.o \
 	linux-nat.o linux-osdata.o linux-fork.o \
 	linux-procfs.o linux-ptrace.o linux-waitpid.o \
-	linux-personality.o
+	linux-personality.o common-linux-nat.o
 
 # The dynamically loaded libthread_db needs access to symbols in the
 # gdb executable.
diff --git a/gdb/config/xtensa/linux.mh b/gdb/config/xtensa/linux.mh
index b4e59b3..9737442 100644
--- a/gdb/config/xtensa/linux.mh
+++ b/gdb/config/xtensa/linux.mh
@@ -5,7 +5,7 @@ NAT_FILE= config/nm-linux.h
 NATDEPFILES= inf-ptrace.o fork-child.o xtensa-linux-nat.o \
 	linux-thread-db.o proc-service.o \
 	linux-nat.o linux-osdata.o linux-fork.o linux-procfs.o linux-ptrace.o \
-	linux-waitpid.o linux-personality.o
+	linux-waitpid.o linux-personality.o common-linux-nat.o
 NAT_CDEPS = $(srcdir)/proc-service.list
 
 LOADLIBES = -ldl $(RDYNAMIC)
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 9d698c6..e30636c 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -615,6 +615,9 @@ x86-linux.o: ../nat/x86-linux.c
 x86-linux-dregs.o: ../nat/x86-linux-dregs.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+common-linux-nat.o: ../nat/linux-nat.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 
 aarch64.c : $(srcdir)/../regformats/aarch64.dat $(regdat_sh)
 	$(SHELL) $(regdat_sh) $(srcdir)/../regformats/aarch64.dat aarch64.c
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index 81dd235..11c9bc0 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -42,7 +42,7 @@ srv_amd64_linux_xmlfiles="i386/amd64-linux.xml i386/amd64-avx-linux.xml i386/amd
 
 # Linux object files.  This is so we don't have to repeat
 # these files over and over again.
-srv_linux_obj="linux-low.o linux-osdata.o linux-procfs.o linux-ptrace.o linux-waitpid.o linux-personality.o"
+srv_linux_obj="linux-low.o linux-osdata.o linux-procfs.o linux-ptrace.o linux-waitpid.o linux-personality.o common-linux-nat.o"
 
 # Input is taken from the "${target}" variable.
 
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 0376299..de4dc1a 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4086,15 +4086,7 @@ linux_nat_thread_name (struct target_ops *self, struct thread_info *thr)
 static char *
 linux_child_pid_to_exec_file (struct target_ops *self, int pid)
 {
-  static char buf[PATH_MAX];
-  char name[PATH_MAX];
-
-  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
-  memset (buf, 0, PATH_MAX);
-  if (readlink (name, buf, PATH_MAX - 1) <= 0)
-    strcpy (buf, name);
-
-  return buf;
+  return linux_pid_to_exec_file (pid);
 }
 
 /* Implement the to_xfer_partial interface for memory reads using the /proc
diff --git a/gdb/nat/linux-nat.c b/gdb/nat/linux-nat.c
new file mode 100644
index 0000000..b9deae3
--- /dev/null
+++ b/gdb/nat/linux-nat.c
@@ -0,0 +1,37 @@
+/* Native-dependent code for GNU/Linux
+
+   Copyright (C) 2000-2015 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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 "common-defs.h"
+#include "nat/linux-nat.h"
+
+/* See nat/linux-nat.h.  */
+
+char *
+linux_pid_to_exec_file (int pid)
+{
+  static char buf[PATH_MAX];
+  char name[PATH_MAX];
+
+  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
+  memset (buf, 0, PATH_MAX);
+  if (readlink (name, buf, PATH_MAX - 1) <= 0)
+    strcpy (buf, name);
+
+  return buf;
+}
diff --git a/gdb/nat/linux-nat.h b/gdb/nat/linux-nat.h
index 7cdaf40..81c2edc 100644
--- a/gdb/nat/linux-nat.h
+++ b/gdb/nat/linux-nat.h
@@ -78,4 +78,13 @@ extern enum target_stop_reason lwp_stop_reason (struct lwp_info *lwp);
 
 extern void linux_stop_lwp (struct lwp_info *lwp);
 
+/* Return the pathname of the executable file that was run to create
+   the process PID.  If the executable file cannot be determined, NULL
+   is returned.  Otherwise, a pointer to a character string containing
+   the pathname is returned.  This string should be copied into a
+   buffer by the client if the string will not be immediately used, or
+   if it must persist.  */
+
+extern char *linux_pid_to_exec_file (int pid);
+
 #endif /* LINUX_NAT_H */
-- 
1.7.1

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 5/7] Implement remote_pid_to_exec_file using qXfer:exec-file:read
  2015-04-01 11:22 [PATCH 0/7] Do not require "file" commands for remote targets Gary Benson
                   ` (3 preceding siblings ...)
  2015-04-01 11:27 ` [PATCH 4/7] Introduce linux_pid_to_exec_file Gary Benson
@ 2015-04-01 11:29 ` Gary Benson
  2015-04-01 14:55   ` Eli Zaretskii
  2015-04-01 11:30 ` [PATCH 6/7] Implement qXfer:exec-file:read in gdbserver Gary Benson
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 52+ messages in thread
From: Gary Benson @ 2015-04-01 11:29 UTC (permalink / raw)
  To: gdb-patches

This commit adds a new packet "qXfer:exec-file:read" to the remote
protocol that can be used to obtain the pathname of the file that
was executed to create a process on the remote system.  Support for
this packet is added to GDB and remote_ops.to_pid_to_exec_file is
implemented using it.

gdb/ChangeLog:

	* target.h (TARGET_OBJECT_EXEC_FILE): New enum value.
	* remote.c (PACKET_qXfer_exec_file): Likewise.
	(remote_protocol_features): Register the
	"qXfer:exec-file:read" feature.
	(remote_xfer_partial): Handle TARGET_OBJECT_EXEC_FILE.
	(remote_pid_to_exec_file): New function.
	(init_remote_ops): Initialize to_pid_to_exec_file.
	(_initialize_remote): Register new "set/show remote
	pid-to-exec-file-packet" command.
	* NEWS: Announce new qXfer:exec-file:read packet.

gdb/doc/ChangeLog:

	* gdb.texinfo (Remote Configuration): Document the "set/show
	remote pid-to-exec-file-packet" command.
	(General Query Packets): Document the qXfer:exec-file:read
	qSupported features.  Document the qXfer:exec-file:read packet.
---
 gdb/ChangeLog       |   13 +++++++++++++
 gdb/NEWS            |    4 ++++
 gdb/doc/ChangeLog   |    7 +++++++
 gdb/doc/gdb.texinfo |   22 ++++++++++++++++++++++
 gdb/remote.c        |   35 +++++++++++++++++++++++++++++++++++
 gdb/target.h        |    7 ++++++-
 6 files changed, 87 insertions(+), 1 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index dec60cb..1486acb 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -90,6 +90,10 @@ hwbreak stop reason
 vFile:fstat:
   Return information about files on the remote system.
 
+qXfer:exec-file:read
+  Return the pathname of the file that was executed to create a
+  process running on the remote system.
+
 * The info record command now shows the recording format and the
   branch tracing configuration for the current thread when using
   the btrace record target.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 692d70e..3c4e47a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -19677,6 +19677,10 @@ are:
 @tab @code{Z4}
 @tab @code{awatch}
 
+@item @code{pid-to-exec-file}
+@tab @code{qXfer:exec-file:read}
+@tab @code{attach}, @code{run}
+
 @item @code{target-features}
 @tab @code{qXfer:features:read}
 @tab @code{set architecture}
@@ -35900,6 +35904,11 @@ These are the currently defined stub features and their properties:
 @tab @samp{-}
 @tab Yes
 
+@item @samp{qXfer:exec-file:read}
+@tab No
+@tab @samp{-}
+@tab Yes
+
 @item @samp{qXfer:features:read}
 @tab No
 @tab @samp{-}
@@ -36103,6 +36112,10 @@ packet (@pxref{qXfer btrace read}).
 The remote stub understands the @samp{qXfer:btrace-conf:read}
 packet (@pxref{qXfer btrace-conf read}).
 
+@item qXfer:exec-file:read
+The remote stub understands the @samp{qXfer:exec-file:read} packet
+(@pxref{qXfer executable filename read}).
+
 @item qXfer:features:read
 The remote stub understands the @samp{qXfer:features:read} packet
 (@pxref{qXfer target description read}).
@@ -36417,6 +36430,15 @@ Return a description of the current branch trace configuration.
 This packet is not probed by default; the remote stub must request it
 by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
 
+@item qXfer:exec-file:read:@var{annex}:@var{offset},@var{length}
+@anchor{qXfer executable filename read}
+Return the pathname of the file that was executed to create a process
+running on the remote system.  The annex specifies the numeric process
+ID of the process to query, encoded as a hexadecimal number.
+
+This packet is not probed by default; the remote stub must request it,
+by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
+
 @item qXfer:features:read:@var{annex}:@var{offset},@var{length}
 @anchor{qXfer target description read}
 Access the @dfn{target description}.  @xref{Target Descriptions}.  The
diff --git a/gdb/remote.c b/gdb/remote.c
index 69a67a8..a307e68 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1252,6 +1252,7 @@ enum {
   PACKET_vFile_fstat,
   PACKET_qXfer_auxv,
   PACKET_qXfer_features,
+  PACKET_qXfer_exec_file,
   PACKET_qXfer_libraries,
   PACKET_qXfer_libraries_svr4,
   PACKET_qXfer_memory_map,
@@ -3963,6 +3964,8 @@ static const struct protocol_feature remote_protocol_features[] = {
   { "PacketSize", PACKET_DISABLE, remote_packet_size, -1 },
   { "qXfer:auxv:read", PACKET_DISABLE, remote_supported_packet,
     PACKET_qXfer_auxv },
+  { "qXfer:exec-file:read", PACKET_DISABLE, remote_supported_packet,
+    PACKET_qXfer_exec_file },
   { "qXfer:features:read", PACKET_DISABLE, remote_supported_packet,
     PACKET_qXfer_features },
   { "qXfer:libraries:read", PACKET_DISABLE, remote_supported_packet,
@@ -9035,6 +9038,11 @@ remote_xfer_partial (struct target_ops *ops, enum target_object object,
 				len, xfered_len,
 	&remote_protocol_packets[PACKET_qXfer_btrace_conf]);
 
+    case TARGET_OBJECT_EXEC_FILE:
+      return remote_read_qxfer (ops, "exec-file", annex, readbuf, offset,
+				len, xfered_len,
+	&remote_protocol_packets[PACKET_qXfer_exec_file]);
+
     default:
       return TARGET_XFER_E_IO;
     }
@@ -11642,6 +11650,29 @@ remote_load (struct target_ops *self, const char *name, int from_tty)
   generic_load (name, from_tty);
 }
 
+/* Accepts an integer PID; returns a string representing a file that
+   can be opened on the remote side to get the symbols for the child
+   process.  Returns NULL if the operation is not supported.  */
+
+static char *
+remote_pid_to_exec_file (struct target_ops *self, int pid)
+{
+  static char *filename = NULL;
+  char annex[9];
+
+  if (packet_support (PACKET_qXfer_exec_file) != PACKET_ENABLE)
+    return NULL;
+
+  if (filename != NULL)
+    xfree (filename);
+
+  xsnprintf (annex, sizeof (annex), "%x", pid);
+  filename = target_read_stralloc (&current_target,
+				   TARGET_OBJECT_EXEC_FILE, annex);
+
+  return filename;
+}
+
 static void
 init_remote_ops (void)
 {
@@ -11691,6 +11722,7 @@ Specify the serial device it is connected to\n\
   remote_ops.to_stop = remote_stop;
   remote_ops.to_xfer_partial = remote_xfer_partial;
   remote_ops.to_rcmd = remote_rcmd;
+  remote_ops.to_pid_to_exec_file = remote_pid_to_exec_file;
   remote_ops.to_log_command = serial_log_command;
   remote_ops.to_get_thread_local_address = remote_get_thread_local_address;
   remote_ops.to_stratum = process_stratum;
@@ -12219,6 +12251,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_auxv],
 			 "qXfer:auxv:read", "read-aux-vector", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_exec_file],
+			 "qXfer:exec-file:read", "pid-to-exec-file", 0);
+
   add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_features],
 			 "qXfer:features:read", "target-features", 0);
 
diff --git a/gdb/target.h b/gdb/target.h
index 01ec5f5..693379a 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -204,7 +204,12 @@ enum target_object
   /* Branch trace data, in XML format.  */
   TARGET_OBJECT_BTRACE,
   /* Branch trace configuration, in XML format.  */
-  TARGET_OBJECT_BTRACE_CONF
+  TARGET_OBJECT_BTRACE_CONF,
+  /* The pathname of the executable file that was run to create
+     a specified process.  ANNEX should be a string representation
+     of the process ID of the process in question, in hexadecimal
+     format.  */
+  TARGET_OBJECT_EXEC_FILE,
   /* Possible future objects: TARGET_OBJECT_FILE, ...  */
 };
 
-- 
1.7.1

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 6/7] Implement qXfer:exec-file:read in gdbserver
  2015-04-01 11:22 [PATCH 0/7] Do not require "file" commands for remote targets Gary Benson
                   ` (4 preceding siblings ...)
  2015-04-01 11:29 ` [PATCH 5/7] Implement remote_pid_to_exec_file using qXfer:exec-file:read Gary Benson
@ 2015-04-01 11:30 ` Gary Benson
  2015-04-06 17:11   ` Doug Evans
  2015-04-17 23:43   ` Possible regression on gdb.base/attach.exp when using native-extended-gdbserver (was: Re: [PATCH 6/7] Implement qXfer:exec-file:read in gdbserver) Sergio Durigan Junior
  2015-04-01 11:39 ` [PATCH 7/7] Access executable from remote system when first inferior appears Gary Benson
  2015-04-15 10:47 ` [PATCH 0/7] Do not require "file" commands for remote targets Pedro Alves
  7 siblings, 2 replies; 52+ messages in thread
From: Gary Benson @ 2015-04-01 11:30 UTC (permalink / raw)
  To: gdb-patches

This commit implements the "qXfer:exec-file:read" packet in gdbserver.

gdb/gdbserver/ChangeLog:

	* target.h (struct target_ops) <pid_to_exec_file>: New field.
	* linux-low.c (linux_target_ops): Initialize pid_to_exec_file.
	* server.c (handle_qxfer_exec_file): New function.
	(qxfer_packets): Add exec-file entry.
	(handle_query): Report qXfer:exec-file:read as supported packet.
---
 gdb/gdbserver/ChangeLog   |    8 ++++++++
 gdb/gdbserver/linux-low.c |    1 +
 gdb/gdbserver/server.c    |   40 ++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/target.h    |    8 ++++++++
 4 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index e4c5420..5ce39fb 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -6429,6 +6429,7 @@ static struct target_ops linux_target_ops = {
   NULL,
 #endif
   linux_supports_range_stepping,
+  linux_pid_to_exec_file,
 };
 
 static void
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 96b31b8..31a2c04 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1137,6 +1137,42 @@ handle_qxfer_auxv (const char *annex,
   return (*the_target->read_auxv) (offset, readbuf, len);
 }
 
+/* Handle qXfer:exec-file:read.  */
+
+static int
+handle_qxfer_exec_file (const char *const_annex,
+			gdb_byte *readbuf, const gdb_byte *writebuf,
+			ULONGEST offset, LONGEST len)
+{
+  char *annex, *file;
+  ULONGEST pid;
+  int total_len;
+
+  if (the_target->pid_to_exec_file == NULL || writebuf != NULL)
+    return -2;
+
+  annex = alloca (strlen (const_annex) + 1);
+  strcpy (annex, const_annex);
+  annex = unpack_varlen_hex (annex, &pid);
+  if (annex[0] != '\0' || pid == 0)
+    return -1;
+
+  file = (*the_target->pid_to_exec_file) (pid);
+  if (file == NULL)
+    return -1;
+
+  total_len = strlen (file);
+
+  if (offset > total_len)
+    return -1;
+
+  if (offset + len > total_len)
+    len = total_len - offset;
+
+  memcpy (readbuf, file + offset, len);
+  return len;
+}
+
 /* Handle qXfer:features:read.  */
 
 static int
@@ -1638,6 +1674,7 @@ static const struct qxfer qxfer_packets[] =
     { "auxv", handle_qxfer_auxv },
     { "btrace", handle_qxfer_btrace },
     { "btrace-conf", handle_qxfer_btrace_conf },
+    { "exec-file", handle_qxfer_exec_file},
     { "fdpic", handle_qxfer_fdpic},
     { "features", handle_qxfer_features },
     { "libraries", handle_qxfer_libraries },
@@ -2082,6 +2119,9 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
       if (target_supports_stopped_by_hw_breakpoint ())
 	strcat (own_buf, ";hwbreak+");
 
+      if (the_target->pid_to_exec_file != NULL)
+	strcat (own_buf, ";qXfer:exec-file:read+");
+
       return;
     }
 
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index 126c861..dc7802d 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -394,6 +394,14 @@ struct target_ops
 
   /* Return true if target supports range stepping.  */
   int (*supports_range_stepping) (void);
+
+  /* Return the pathname of the executable file that was run to
+     create the process PID.  If the executable file cannot be
+     determined, NULL is returned.  Otherwise, a pointer to a
+     character string containing the pathname is returned.  This
+     string should be copied into a buffer by the client if the
+     string will not be immediately used, or if it must persist.  */
+  char *(*pid_to_exec_file) (int pid);
 };
 
 extern struct target_ops *the_target;
-- 
1.7.1

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 7/7] Access executable from remote system when first inferior appears
  2015-04-01 11:22 [PATCH 0/7] Do not require "file" commands for remote targets Gary Benson
                   ` (5 preceding siblings ...)
  2015-04-01 11:30 ` [PATCH 6/7] Implement qXfer:exec-file:read in gdbserver Gary Benson
@ 2015-04-01 11:39 ` Gary Benson
  2015-04-15 10:24   ` Pedro Alves
  2015-04-15 10:47 ` [PATCH 0/7] Do not require "file" commands for remote targets Pedro Alves
  7 siblings, 1 reply; 52+ messages in thread
From: Gary Benson @ 2015-04-01 11:39 UTC (permalink / raw)
  To: gdb-patches

This commit modifies remote_add_inferior to take an extra argument
try_open_exec.  If this is nonzero, remote_add_inferior will attempt
to open this inferior's executable as the main executable if no main
executable is open already.  Callers are updated appropriately.

One testcase required updating as a result of this commit.  The test
checked that GDB's "info files" command does not crash if no main
executable is open, and relied on GDB's inability to access the main
executable over the remote protocol.  The test was updated to inhibit
this new behavior.

gdb/ChangeLog:

	* remote.c (remote_add_inferior): New argument try_open_exec.
	If nonzero, attempt to open the inferior's executable file as
	the main executable if no main executable is open already.
	All callers updated.

gdb/testsuite/ChangeLog:

	* gdb.server/server-exec-info.exp: Inhibit GDB from accessing
	the main executable over the remote protocol.
---
 gdb/ChangeLog                                 |    7 +++++++
 gdb/remote.c                                  |   18 +++++++++++++-----
 gdb/testsuite/ChangeLog                       |    5 +++++
 gdb/testsuite/gdb.server/server-exec-info.exp |    1 +
 4 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index a307e68..642bde1 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1516,10 +1516,13 @@ remote_query_attached (int pid)
    inferior.  If ATTACHED is 1, then we had just attached to this
    inferior.  If it is 0, then we just created this inferior.  If it
    is -1, then try querying the remote stub to find out if it had
-   attached to the inferior or not.  */
+   attached to the inferior or not.  If TRY_OPEN_EXEC is true then
+   attempt to open this inferior's executable as the main executable
+   if no main executable is open already.  */
 
 static struct inferior *
-remote_add_inferior (int fake_pid_p, int pid, int attached)
+remote_add_inferior (int fake_pid_p, int pid, int attached,
+		     int try_open_exec)
 {
   struct inferior *inf;
 
@@ -1553,6 +1556,11 @@ remote_add_inferior (int fake_pid_p, int pid, int attached)
   inf->attach_flag = attached;
   inf->fake_pid_p = fake_pid_p;
 
+  /* If no main executable is currently open then attempt to
+     open the file that was executed to create this inferior.  */
+  if (try_open_exec && !fake_pid_p && get_exec_file (0) == NULL)
+    exec_file_locate_attach (pid, 1);
+
   return inf;
 }
 
@@ -1643,7 +1651,7 @@ remote_notice_new_inferior (ptid_t currthread, int running)
 	  int fake_pid_p = !remote_multi_process_p (rs);
 
 	  inf = remote_add_inferior (fake_pid_p,
-				     ptid_get_pid (currthread), -1);
+				     ptid_get_pid (currthread), -1, 1);
 	}
 
       /* This is really a new thread.  Add it.  */
@@ -3413,7 +3421,7 @@ add_current_inferior_and_thread (char *wait_status)
       fake_pid_p = 1;
     }
 
-  remote_add_inferior (fake_pid_p, ptid_get_pid (inferior_ptid), -1);
+  remote_add_inferior (fake_pid_p, ptid_get_pid (inferior_ptid), -1, 1);
 
   /* Add the main thread.  */
   add_thread_silent (inferior_ptid);
@@ -4539,7 +4547,7 @@ extended_remote_attach (struct target_ops *target, const char *args,
 	     target_pid_to_str (pid_to_ptid (pid)));
     }
 
-  set_current_inferior (remote_add_inferior (0, pid, 1));
+  set_current_inferior (remote_add_inferior (0, pid, 1, 0));
 
   inferior_ptid = pid_to_ptid (pid);
 
diff --git a/gdb/testsuite/gdb.server/server-exec-info.exp b/gdb/testsuite/gdb.server/server-exec-info.exp
index ca5f5ca..c12554a 100644
--- a/gdb/testsuite/gdb.server/server-exec-info.exp
+++ b/gdb/testsuite/gdb.server/server-exec-info.exp
@@ -27,6 +27,7 @@ if [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] {
     return -1
 }
 
+gdb_test_no_output "set remote pid-to-exec-file-packet off"
 gdb_test "file" ".*" "file" \
 	 {Discard symbol table from `.*'\? \(y or n\) } "y"
 gdbserver_run ""
-- 
1.7.1

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 3/7] Use gdb_sysroot for main executable on attach
  2015-04-01 11:22 ` [PATCH 3/7] Use gdb_sysroot for main executable on attach Gary Benson
@ 2015-04-01 14:53   ` Eli Zaretskii
  2015-04-15 12:45     ` Gary Benson
  2015-04-15 10:43   ` Pedro Alves
  1 sibling, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2015-04-01 14:53 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches

> From: Gary Benson <gbenson@redhat.com>
> Date: Wed,  1 Apr 2015 12:22:17 +0100
> 
> +* Paths specified by "set sysroot" will be prepended to the path of
> +  the main executable when attaching to already-running processes
> +  (local and remote) if the path of the main executable is reported
> +  to GDB as absolute by the operating system.

Please don't use "path" when you really mean "file name".

OK with that fixed.

> +shared library paths will be prefixed with @var{path}; many runtime
> +loaders store the absolute paths to the shared library in the target
> +program's memory.  When attaching to already-running processes, their
> +paths will be prefixed with @var{path} if reported to @value{GDBN} as
> +absolute by the operating system.  If you use @code{set sysroot} to
> +find executables and shared libraries, they need to be laid out in
> +the same way that they are on the target, with e.g.@: a @file{/bin},
> +@file{/lib} and @file{/usr/lib} hierarchy under @var{path}.

Same here.  (Yes, I know that the previous text also used "path").

Thanks.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 5/7] Implement remote_pid_to_exec_file using qXfer:exec-file:read
  2015-04-01 11:29 ` [PATCH 5/7] Implement remote_pid_to_exec_file using qXfer:exec-file:read Gary Benson
@ 2015-04-01 14:55   ` Eli Zaretskii
  2015-04-06 17:00     ` Doug Evans
  0 siblings, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2015-04-01 14:55 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches

> From: Gary Benson <gbenson@redhat.com>
> Date: Wed,  1 Apr 2015 12:22:19 +0100
> 
> +qXfer:exec-file:read
> +  Return the pathname of the file that was executed to create a
            ^^^^^^^^^^^^^^^^^^^^^^^^
"the name of the file"

OK with this change.

> +@item qXfer:exec-file:read:@var{annex}:@var{offset},@var{length}
> +@anchor{qXfer executable filename read}
> +Return the pathname of the file that was executed to create a process
          ^^^^^^^^^^^^^^^^^^^^^^^^
Likewise.

Otherwise, the documentation parts are OK.

Thanks.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 4/7] Introduce linux_pid_to_exec_file
  2015-04-01 11:27 ` [PATCH 4/7] Introduce linux_pid_to_exec_file Gary Benson
@ 2015-04-06 16:41   ` Doug Evans
  2015-04-07  9:07     ` Gary Benson
  2015-04-15  9:37   ` Pedro Alves
  1 sibling, 1 reply; 52+ messages in thread
From: Doug Evans @ 2015-04-06 16:41 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches

Gary Benson <gbenson@redhat.com> writes:
> This commit introduces a new function, linux_pid_to_exec_file, that
> shared Linux code can use to discover the filename of the executable
> that was run to create a process on the system.
>
> gdb/ChangeLog:
>
> 	* nat/linux-nat.h (linux_pid_to_exec_file): New declaration.
> 	* nat/linux-nat.c: New file.
> 	* Makefile.in (common-linux-nat.o): New rule.
> 	* config/aarch64/linux.mh (NATDEPFILES): Add common-linux-nat.o.
> 	* config/alpha/alpha-linux.mh (NATDEPFILES): Likewise.
> 	* config/arm/linux.mh (NATDEPFILES): Likewise.
> 	* config/i386/linux.mh (NATDEPFILES): Likewise.
> 	* config/i386/linux64.mh (NATDEPFILES): Likewise.
> 	* config/ia64/linux.mh (NATDEPFILES): Likewise.
> 	* config/m32r/linux.mh (NATDEPFILES): Likewise.
> 	* config/m68k/linux.mh (NATDEPFILES): Likewise.
> 	* config/mips/linux.mh (NATDEPFILES): Likewise.
> 	* config/pa/linux.mh (NATDEPFILES): Likewise.
> 	* config/powerpc/linux.mh (NATDEPFILES): Likewise.
> 	* config/powerpc/ppc64-linux.mh (NATDEPFILES): Likewise.
> 	* config/powerpc/spu-linux.mh (NATDEPFILES): Likewise.
> 	* config/s390/linux.mh (NATDEPFILES): Likewise.
> 	* config/sparc/linux.mh (NATDEPFILES): Likewise.
> 	* config/sparc/linux64.mh (NATDEPFILES): Likewise.
> 	* config/tilegx/linux.mh (NATDEPFILES): Likewise.
> 	* config/xtensa/linux.mh (NATDEPFILES): Likewise.
> 	* linux-nat.c (linux_child_pid_to_exec_file): Factored out
> 	to new function linux_pid_to_exec_file in nat/linux-nat.c.
>
> ...
>
> diff --git a/gdb/nat/linux-nat.c b/gdb/nat/linux-nat.c
> new file mode 100644
> index 0000000..b9deae3
> --- /dev/null
> +++ b/gdb/nat/linux-nat.c
> @@ -0,0 +1,37 @@
> +/* Native-dependent code for GNU/Linux
> +
> +   Copyright (C) 2000-2015 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   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 "common-defs.h"
> +#include "nat/linux-nat.h"
> +
> +/* See nat/linux-nat.h.  */
> +
> +char *
> +linux_pid_to_exec_file (int pid)
> +{
> +  static char buf[PATH_MAX];
> +  char name[PATH_MAX];
> +
> +  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
> +  memset (buf, 0, PATH_MAX);
> +  if (readlink (name, buf, PATH_MAX - 1) <= 0)
> +    strcpy (buf, name);
> +
> +  return buf;
> +}
> diff --git a/gdb/nat/linux-nat.h b/gdb/nat/linux-nat.h
> index 7cdaf40..81c2edc 100644
> --- a/gdb/nat/linux-nat.h
> +++ b/gdb/nat/linux-nat.h
> @@ -78,4 +78,13 @@ extern enum target_stop_reason lwp_stop_reason (struct lwp_info *lwp);
>  
>  extern void linux_stop_lwp (struct lwp_info *lwp);
>  
> +/* Return the pathname of the executable file that was run to create
> +   the process PID.  If the executable file cannot be determined, NULL
> +   is returned.  Otherwise, a pointer to a character string containing
> +   the pathname is returned.  This string should be copied into a
> +   buffer by the client if the string will not be immediately used, or
> +   if it must persist.  */
> +
> +extern char *linux_pid_to_exec_file (int pid);
> +
>  #endif /* LINUX_NAT_H */

Hi.

I like the idea of returning NULL if the executable file cannot be
determined, but the implementation doesn't do this.

Also, while I don't have a strong opinion, it seems preferable
to return a const char *.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 5/7] Implement remote_pid_to_exec_file using qXfer:exec-file:read
  2015-04-01 14:55   ` Eli Zaretskii
@ 2015-04-06 17:00     ` Doug Evans
  2015-04-06 17:21       ` Eli Zaretskii
  0 siblings, 1 reply; 52+ messages in thread
From: Doug Evans @ 2015-04-06 17:00 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Gary Benson, gdb-patches

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Wed,  1 Apr 2015 12:22:19 +0100
>> 
>> +qXfer:exec-file:read
>> +  Return the pathname of the file that was executed to create a
>             ^^^^^^^^^^^^^^^^^^^^^^^^
> "the name of the file"
>
> OK with this change.
>
>> +@item qXfer:exec-file:read:@var{annex}:@var{offset},@var{length}
>> +@anchor{qXfer executable filename read}
>> +Return the pathname of the file that was executed to create a process
>           ^^^^^^^^^^^^^^^^^^^^^^^^
> Likewise.
>
> Otherwise, the documentation parts are OK.

"the name of the file" is ambiguous though.
Is it the full path name of the file? Or the base name?
Can the target choose? [I hope not.]

IWBN to have more clarity here.

While I realize "path" is a term associated with directories
(paths in which to find files), "full path name" is in common
use for complete/absolute file names.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 6/7] Implement qXfer:exec-file:read in gdbserver
  2015-04-01 11:30 ` [PATCH 6/7] Implement qXfer:exec-file:read in gdbserver Gary Benson
@ 2015-04-06 17:11   ` Doug Evans
  2015-04-07  9:19     ` Gary Benson
  2015-04-17 23:43   ` Possible regression on gdb.base/attach.exp when using native-extended-gdbserver (was: Re: [PATCH 6/7] Implement qXfer:exec-file:read in gdbserver) Sergio Durigan Junior
  1 sibling, 1 reply; 52+ messages in thread
From: Doug Evans @ 2015-04-06 17:11 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches

Gary Benson <gbenson@redhat.com> writes:
> This commit implements the "qXfer:exec-file:read" packet in gdbserver.
>
> gdb/gdbserver/ChangeLog:
>
> 	* target.h (struct target_ops) <pid_to_exec_file>: New field.
> 	* linux-low.c (linux_target_ops): Initialize pid_to_exec_file.
> 	* server.c (handle_qxfer_exec_file): New function.
> 	(qxfer_packets): Add exec-file entry.
> 	(handle_query): Report qXfer:exec-file:read as supported packet.
>
> ...
>
> diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
> index 126c861..dc7802d 100644
> --- a/gdb/gdbserver/target.h
> +++ b/gdb/gdbserver/target.h
> @@ -394,6 +394,14 @@ struct target_ops
>  
>    /* Return true if target supports range stepping.  */
>    int (*supports_range_stepping) (void);
> +
> +  /* Return the pathname of the executable file that was run to
> +     create the process PID.  If the executable file cannot be
> +     determined, NULL is returned.  Otherwise, a pointer to a
> +     character string containing the pathname is returned.  This
> +     string should be copied into a buffer by the client if the
> +     string will not be immediately used, or if it must persist.  */
> +  char *(*pid_to_exec_file) (int pid);
>  };
>  
>  extern struct target_ops *the_target;

IWBN to have some clarity on what the pathname result can and cannot be.

Perhaps nitpicky, but the less ambiguity the better.
I think(!) the intent is that the path name is the full
path name, but I could be wrong.

Another issue is whether the path has been real-path'd.
[all symlinks resolved]  I don't have a strong opinion on that,
but I do think we should at least require full paths to be returned here.

const char * result?

Also, I was going to say we need to pick a type for "pid" and consistently
use it, but that's a whole 'nother discussion, and this patch set
needn't be bogged down by it.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 5/7] Implement remote_pid_to_exec_file using qXfer:exec-file:read
  2015-04-06 17:00     ` Doug Evans
@ 2015-04-06 17:21       ` Eli Zaretskii
  2015-04-06 21:57         ` Doug Evans
  2015-04-07  9:08         ` Gary Benson
  0 siblings, 2 replies; 52+ messages in thread
From: Eli Zaretskii @ 2015-04-06 17:21 UTC (permalink / raw)
  To: Doug Evans; +Cc: gbenson, gdb-patches

> From: Doug Evans <xdje42@gmail.com>
> Cc: Gary Benson <gbenson@redhat.com>,  gdb-patches@sourceware.org
> Date: Mon, 06 Apr 2015 10:00:08 -0700
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Date: Wed,  1 Apr 2015 12:22:19 +0100
> >> 
> >> +qXfer:exec-file:read
> >> +  Return the pathname of the file that was executed to create a
> >             ^^^^^^^^^^^^^^^^^^^^^^^^
> > "the name of the file"
> >
> > OK with this change.
> >
> >> +@item qXfer:exec-file:read:@var{annex}:@var{offset},@var{length}
> >> +@anchor{qXfer executable filename read}
> >> +Return the pathname of the file that was executed to create a process
> >           ^^^^^^^^^^^^^^^^^^^^^^^^
> > Likewise.
> >
> > Otherwise, the documentation parts are OK.
> 
> "the name of the file" is ambiguous though.
> Is it the full path name of the file? Or the base name?

I don't know the answers, but if this is a "full path name", then
"the full absolute name of the file" will convey that.

> While I realize "path" is a term associated with directories
> (paths in which to find files), "full path name" is in common
> use for complete/absolute file names.

Yes, we just want to use the latter rather than the former.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 5/7] Implement remote_pid_to_exec_file using qXfer:exec-file:read
  2015-04-06 17:21       ` Eli Zaretskii
@ 2015-04-06 21:57         ` Doug Evans
  2015-04-07  6:09           ` Eli Zaretskii
  2015-04-07  9:08         ` Gary Benson
  1 sibling, 1 reply; 52+ messages in thread
From: Doug Evans @ 2015-04-06 21:57 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Gary Benson, gdb-patches

On Mon, Apr 6, 2015 at 10:21 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Doug Evans <xdje42@gmail.com>
>> Cc: Gary Benson <gbenson@redhat.com>,  gdb-patches@sourceware.org
>> Date: Mon, 06 Apr 2015 10:00:08 -0700
>>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>> >> Date: Wed,  1 Apr 2015 12:22:19 +0100
>> >>
>> >> +qXfer:exec-file:read
>> >> +  Return the pathname of the file that was executed to create a
>> >             ^^^^^^^^^^^^^^^^^^^^^^^^
>> > "the name of the file"
>> >
>> > OK with this change.
>> >
>> >> +@item qXfer:exec-file:read:@var{annex}:@var{offset},@var{length}
>> >> +@anchor{qXfer executable filename read}
>> >> +Return the pathname of the file that was executed to create a process
>> >           ^^^^^^^^^^^^^^^^^^^^^^^^
>> > Likewise.
>> >
>> > Otherwise, the documentation parts are OK.
>>
>> "the name of the file" is ambiguous though.
>> Is it the full path name of the file? Or the base name?
>
> I don't know the answers, but if this is a "full path name", then
> "the full absolute name of the file" will convey that.
>
>> While I realize "path" is a term associated with directories
>> (paths in which to find files), "full path name" is in common
>> use for complete/absolute file names.
>
> Yes, we just want to use the latter rather than the former.

I'm curious why "full path name" is a problem though.
It is such a common term.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 5/7] Implement remote_pid_to_exec_file using qXfer:exec-file:read
  2015-04-06 21:57         ` Doug Evans
@ 2015-04-07  6:09           ` Eli Zaretskii
  0 siblings, 0 replies; 52+ messages in thread
From: Eli Zaretskii @ 2015-04-07  6:09 UTC (permalink / raw)
  To: Doug Evans; +Cc: gbenson, gdb-patches

> Date: Mon, 6 Apr 2015 14:57:35 -0700
> From: Doug Evans <xdje42@gmail.com>
> Cc: Gary Benson <gbenson@redhat.com>, 
> 	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
> 
> I'm curious why "full path name" is a problem though.

Because the GNU Coding Standards ask us not to use it.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 4/7] Introduce linux_pid_to_exec_file
  2015-04-06 16:41   ` Doug Evans
@ 2015-04-07  9:07     ` Gary Benson
  2015-04-08  3:15       ` Doug Evans
  0 siblings, 1 reply; 52+ messages in thread
From: Gary Benson @ 2015-04-07  9:07 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

Doug Evans wrote:
> Gary Benson <gbenson@redhat.com> writes:
> > diff --git a/gdb/nat/linux-nat.c b/gdb/nat/linux-nat.c
> > new file mode 100644
> > index 0000000..b9deae3
> > --- /dev/null
> > +++ b/gdb/nat/linux-nat.c
> > @@ -0,0 +1,37 @@
...
> > +/* See nat/linux-nat.h.  */
> > +
> > +char *
> > +linux_pid_to_exec_file (int pid)
> > +{
> > +  static char buf[PATH_MAX];
> > +  char name[PATH_MAX];
> > +
> > +  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
> > +  memset (buf, 0, PATH_MAX);
> > +  if (readlink (name, buf, PATH_MAX - 1) <= 0)
> > +    strcpy (buf, name);
> > +
> > +  return buf;
> > +}
> > diff --git a/gdb/nat/linux-nat.h b/gdb/nat/linux-nat.h
> > index 7cdaf40..81c2edc 100644
> > --- a/gdb/nat/linux-nat.h
> > +++ b/gdb/nat/linux-nat.h
> > @@ -78,4 +78,13 @@
> >  
> >  extern void linux_stop_lwp (struct lwp_info *lwp);
> >  
> > +/* Return the pathname of the executable file that was run to create
> > +   the process PID.  If the executable file cannot be determined, NULL
> > +   is returned.  Otherwise, a pointer to a character string containing
> > +   the pathname is returned.  This string should be copied into a
> > +   buffer by the client if the string will not be immediately used, or
> > +   if it must persist.  */
> > +
> > +extern char *linux_pid_to_exec_file (int pid);
> > +
> >  #endif /* LINUX_NAT_H */
> 
> I like the idea of returning NULL if the executable file cannot be
> determined, but the implementation doesn't do this.

You're right, I will remove that part of the documentation comment.

> Also, while I don't have a strong opinion, it seems preferable
> to return a const char *.

The function is the implementation of target_pid_to_exec_file, via
a wrapper, and that does not return const.  I briefly attempted to
constify this but it snowballed.

Thanks,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 5/7] Implement remote_pid_to_exec_file using qXfer:exec-file:read
  2015-04-06 17:21       ` Eli Zaretskii
  2015-04-06 21:57         ` Doug Evans
@ 2015-04-07  9:08         ` Gary Benson
  2015-04-08  1:57           ` Doug Evans
  1 sibling, 1 reply; 52+ messages in thread
From: Gary Benson @ 2015-04-07  9:08 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Doug Evans, gdb-patches

Eli Zaretskii wrote:
> > From: Doug Evans <xdje42@gmail.com>
> > Cc: Gary Benson <gbenson@redhat.com>,  gdb-patches@sourceware.org
> > Date: Mon, 06 Apr 2015 10:00:08 -0700
> > 
> > Eli Zaretskii <eliz@gnu.org> writes:
> > 
> > > > Date: Wed,  1 Apr 2015 12:22:19 +0100
> > > > 
> > > > +qXfer:exec-file:read
> > > > +  Return the pathname of the file that was executed to create a
> > >             ^^^^^^^^^^^^^^^^^^^^^^^^
> > > "the name of the file"
> > >
> > > OK with this change.
> > >
> > > > +@item qXfer:exec-file:read:@var{annex}:@var{offset},@var{length}
> > > > +@anchor{qXfer executable filename read}
> > > > +Return the pathname of the file that was executed to create a process
> > >           ^^^^^^^^^^^^^^^^^^^^^^^^
> > > Likewise.
> > >
> > > Otherwise, the documentation parts are OK.
> > 
> > "the name of the file" is ambiguous though.
> > Is it the full path name of the file? Or the base name?
> 
> I don't know the answers, but if this is a "full path name", then
> "the full absolute name of the file" will convey that.

It should be absolute.  I will use "the full absolute name of the
file" as Eli suggests.

Thanks,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 6/7] Implement qXfer:exec-file:read in gdbserver
  2015-04-06 17:11   ` Doug Evans
@ 2015-04-07  9:19     ` Gary Benson
  0 siblings, 0 replies; 52+ messages in thread
From: Gary Benson @ 2015-04-07  9:19 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

Doug Evans wrote:
> Gary Benson <gbenson@redhat.com> writes:
> > diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
> > index 126c861..dc7802d 100644
> > --- a/gdb/gdbserver/target.h
> > +++ b/gdb/gdbserver/target.h
> > @@ -394,6 +394,14 @@ struct target_ops
> >  
> >    /* Return true if target supports range stepping.  */
> >    int (*supports_range_stepping) (void);
> > +
> > +  /* Return the pathname of the executable file that was run to
> > +     create the process PID.  If the executable file cannot be
> > +     determined, NULL is returned.  Otherwise, a pointer to a
> > +     character string containing the pathname is returned.  This
> > +     string should be copied into a buffer by the client if the
> > +     string will not be immediately used, or if it must persist.  */
> > +  char *(*pid_to_exec_file) (int pid);
> >  };
> >  
> >  extern struct target_ops *the_target;
> 
> IWBN to have some clarity on what the pathname result can and cannot be.
> 
> Perhaps nitpicky, but the less ambiguity the better.  I think(!) the
> intent is that the path name is the full path name, but I could be
> wrong.
> 
> Another issue is whether the path has been real-path'd.  [all
> symlinks resolved] I don't have a strong opinion on that, but I do
> think we should at least require full paths to be returned here.

I will use the "the full absolute name of the file" as Eli suggested
for the documentation.  I will also change the gdb/target.h version
to match.

It's not necessary for all symlinks to be resolved.  I think the
intention is for the function to return what the operating system
says the executable's filename is, but of course non of these old
parts of GDB are documented to that extent so I'm guessing.

> const char * result?

Not const for the same reason as the GDB side.

> Also, I was going to say we need to pick a type for "pid" and
> consistently use it, but that's a whole 'nother discussion, and
> this patch set needn't be bogged down by it.

Quite :)

Thanks,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 5/7] Implement remote_pid_to_exec_file using qXfer:exec-file:read
  2015-04-07  9:08         ` Gary Benson
@ 2015-04-08  1:57           ` Doug Evans
  2015-04-08  6:00             ` Eli Zaretskii
  0 siblings, 1 reply; 52+ messages in thread
From: Doug Evans @ 2015-04-08  1:57 UTC (permalink / raw)
  To: Gary Benson; +Cc: Eli Zaretskii, gdb-patches

On Tue, Apr 7, 2015 at 2:08 AM, Gary Benson <gbenson@redhat.com> wrote:
> Eli Zaretskii wrote:
>> > From: Doug Evans <xdje42@gmail.com>
>> > Cc: Gary Benson <gbenson@redhat.com>,  gdb-patches@sourceware.org
>> > Date: Mon, 06 Apr 2015 10:00:08 -0700
>> >
>> > Eli Zaretskii <eliz@gnu.org> writes:
>> >
>> > > > Date: Wed,  1 Apr 2015 12:22:19 +0100
>> > > >
>> > > > +qXfer:exec-file:read
>> > > > +  Return the pathname of the file that was executed to create a
>> > >             ^^^^^^^^^^^^^^^^^^^^^^^^
>> > > "the name of the file"
>> > >
>> > > OK with this change.
>> > >
>> > > > +@item qXfer:exec-file:read:@var{annex}:@var{offset},@var{length}
>> > > > +@anchor{qXfer executable filename read}
>> > > > +Return the pathname of the file that was executed to create a process
>> > >           ^^^^^^^^^^^^^^^^^^^^^^^^
>> > > Likewise.
>> > >
>> > > Otherwise, the documentation parts are OK.
>> >
>> > "the name of the file" is ambiguous though.
>> > Is it the full path name of the file? Or the base name?
>>
>> I don't know the answers, but if this is a "full path name", then
>> "the full absolute name of the file" will convey that.
>
> It should be absolute.  I will use "the full absolute name of the
> file" as Eli suggests.

Is the "full" in "full absolute name" redundant?

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 4/7] Introduce linux_pid_to_exec_file
  2015-04-07  9:07     ` Gary Benson
@ 2015-04-08  3:15       ` Doug Evans
  2015-04-08  8:06         ` Gary Benson
  0 siblings, 1 reply; 52+ messages in thread
From: Doug Evans @ 2015-04-08  3:15 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches

On Tue, Apr 7, 2015 at 2:07 AM, Gary Benson <gbenson@redhat.com> wrote:
> Doug Evans wrote:
>> Gary Benson <gbenson@redhat.com> writes:
>> > diff --git a/gdb/nat/linux-nat.c b/gdb/nat/linux-nat.c
>> > new file mode 100644
>> > index 0000000..b9deae3
>> > --- /dev/null
>> > +++ b/gdb/nat/linux-nat.c
>> > @@ -0,0 +1,37 @@
> ...
>> > +/* See nat/linux-nat.h.  */
>> > +
>> > +char *
>> > +linux_pid_to_exec_file (int pid)
>> > +{
>> > +  static char buf[PATH_MAX];
>> > +  char name[PATH_MAX];
>> > +
>> > +  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
>> > +  memset (buf, 0, PATH_MAX);
>> > +  if (readlink (name, buf, PATH_MAX - 1) <= 0)
>> > +    strcpy (buf, name);
>> > +
>> > +  return buf;
>> > +}

Hi. Another nit.

Since readlink does not nul-terminate the string, I think it would be
clearer to explicitly nul-terminate the result instead of doing
the initial memset. I realize the current linux-nat.c version
doesn't do this, but we can still improve things here.

[One might also want the code to protect itself in the case
where the buf is not large enough, but one can leave that
for now since we assume PATH_MAX is big enough
throughout.]

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 5/7] Implement remote_pid_to_exec_file using qXfer:exec-file:read
  2015-04-08  1:57           ` Doug Evans
@ 2015-04-08  6:00             ` Eli Zaretskii
  0 siblings, 0 replies; 52+ messages in thread
From: Eli Zaretskii @ 2015-04-08  6:00 UTC (permalink / raw)
  To: Doug Evans; +Cc: gbenson, gdb-patches

> Date: Tue, 7 Apr 2015 18:57:39 -0700
> From: Doug Evans <xdje42@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
> 
> Is the "full" in "full absolute name" redundant?

To some extent, yes.  But not enough for me to protest.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 4/7] Introduce linux_pid_to_exec_file
  2015-04-08  3:15       ` Doug Evans
@ 2015-04-08  8:06         ` Gary Benson
  0 siblings, 0 replies; 52+ messages in thread
From: Gary Benson @ 2015-04-08  8:06 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

Doug Evans wrote:
> On Tue, Apr 7, 2015 at 2:07 AM, Gary Benson <gbenson@redhat.com> wrote:
> > Doug Evans wrote:
> > > Gary Benson <gbenson@redhat.com> writes:
> > > > +/* See nat/linux-nat.h.  */
> > > > +
> > > > +char *
> > > > +linux_pid_to_exec_file (int pid)
> > > > +{
> > > > +  static char buf[PATH_MAX];
> > > > +  char name[PATH_MAX];
> > > > +
> > > > +  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
> > > > +  memset (buf, 0, PATH_MAX);
> > > > +  if (readlink (name, buf, PATH_MAX - 1) <= 0)
> > > > +    strcpy (buf, name);
> > > > +
> > > > +  return buf;
> > > > +}
> 
> Hi. Another nit.
> 
> Since readlink does not nul-terminate the string, I think it would
> be clearer to explicitly nul-terminate the result instead of doing
> the initial memset. I realize the current linux-nat.c version
> doesn't do this, but we can still improve things here.

Ok, I'll do that.

> [One might also want the code to protect itself in the case where
> the buf is not large enough, but one can leave that for now since
> we assume PATH_MAX is big enough throughout.]

I don't know how you'd tell, readlink doesn't seem to indicate whether
it truncated or not.

Cheers,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 4/7] Introduce linux_pid_to_exec_file
  2015-04-01 11:27 ` [PATCH 4/7] Introduce linux_pid_to_exec_file Gary Benson
  2015-04-06 16:41   ` Doug Evans
@ 2015-04-15  9:37   ` Pedro Alves
  2015-04-15 13:14     ` [PATCH 4/7 v2] Introduce linux_proc_pid_to_exec_file Gary Benson
  1 sibling, 1 reply; 52+ messages in thread
From: Pedro Alves @ 2015-04-15  9:37 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 04/01/2015 12:22 PM, Gary Benson wrote:
> This commit introduces a new function, linux_pid_to_exec_file, that
> shared Linux code can use to discover the filename of the executable
> that was run to create a process on the system.
> 
> gdb/ChangeLog:
> 
> 	* nat/linux-nat.h (linux_pid_to_exec_file): New declaration.
> 	* nat/linux-nat.c: New file.
> 	* Makefile.in (common-linux-nat.o): New rule.
> 	* config/aarch64/linux.mh (NATDEPFILES): Add common-linux-nat.o.
> 	* config/alpha/alpha-linux.mh (NATDEPFILES): Likewise.
> 	* config/arm/linux.mh (NATDEPFILES): Likewise.
> 	* config/i386/linux.mh (NATDEPFILES): Likewise.
> 	* config/i386/linux64.mh (NATDEPFILES): Likewise.
> 	* config/ia64/linux.mh (NATDEPFILES): Likewise.
> 	* config/m32r/linux.mh (NATDEPFILES): Likewise.
> 	* config/m68k/linux.mh (NATDEPFILES): Likewise.
> 	* config/mips/linux.mh (NATDEPFILES): Likewise.
> 	* config/pa/linux.mh (NATDEPFILES): Likewise.
> 	* config/powerpc/linux.mh (NATDEPFILES): Likewise.
> 	* config/powerpc/ppc64-linux.mh (NATDEPFILES): Likewise.
> 	* config/powerpc/spu-linux.mh (NATDEPFILES): Likewise.
> 	* config/s390/linux.mh (NATDEPFILES): Likewise.
> 	* config/sparc/linux.mh (NATDEPFILES): Likewise.
> 	* config/sparc/linux64.mh (NATDEPFILES): Likewise.
> 	* config/tilegx/linux.mh (NATDEPFILES): Likewise.
> 	* config/xtensa/linux.mh (NATDEPFILES): Likewise.
> 	* linux-nat.c (linux_child_pid_to_exec_file): Factored out
> 	to new function linux_pid_to_exec_file in nat/linux-nat.c.

Sorry for pushing back a after you touched all these files, but,
there's already a natural place for this shared function.

> --- /dev/null
> +++ b/gdb/nat/linux-nat.c
> @@ -0,0 +1,37 @@
> +/* Native-dependent code for GNU/Linux
...
> +char *
> +linux_pid_to_exec_file (int pid)
> +{
> +  static char buf[PATH_MAX];
> +  char name[PATH_MAX];
> +
> +  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
> +  memset (buf, 0, PATH_MAX);
> +  if (readlink (name, buf, PATH_MAX - 1) <= 0)
> +    strcpy (buf, name);
> +
> +  return buf;
> +}

Instead please move this function to nat/linux-procfs.c,
(and call it linux_proc_pid_to_exec_file).

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 7/7] Access executable from remote system when first inferior appears
  2015-04-01 11:39 ` [PATCH 7/7] Access executable from remote system when first inferior appears Gary Benson
@ 2015-04-15 10:24   ` Pedro Alves
  2015-04-15 13:56     ` Gary Benson
  0 siblings, 1 reply; 52+ messages in thread
From: Pedro Alves @ 2015-04-15 10:24 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 04/01/2015 12:22 PM, Gary Benson wrote:
> This commit modifies remote_add_inferior to take an extra argument
> try_open_exec.  If this is nonzero, remote_add_inferior will attempt
> to open this inferior's executable as the main executable if no main
> executable is open already.  Callers are updated appropriately.
> 
> One testcase required updating as a result of this commit.  The test
> checked that GDB's "info files" command does not crash if no main
> executable is open, and relied on GDB's inability to access the main
> executable over the remote protocol.  The test was updated to inhibit
> this new behavior.

So this is significant user-visible change too.  I think it deserves
an example in the commit log, and a NEWS entry.  The manual should
probably be updated to explain/mention this too.  We already mention
something like this in the "attach" docs:

  "When you use @code{attach}, the debugger finds the program running in
  the process first (...)"

Otherwise looks good.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 3/7] Use gdb_sysroot for main executable on attach
  2015-04-01 11:22 ` [PATCH 3/7] Use gdb_sysroot for main executable on attach Gary Benson
  2015-04-01 14:53   ` Eli Zaretskii
@ 2015-04-15 10:43   ` Pedro Alves
  1 sibling, 0 replies; 52+ messages in thread
From: Pedro Alves @ 2015-04-15 10:43 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 04/01/2015 12:22 PM, Gary Benson wrote:

> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -17800,18 +17800,20 @@ may try to load the host's libraries.  @value{GDBN} has two variables
>  to specify the search directories for target libraries.
>  
>  @table @code
> -@cindex prefix for shared library file names
> +@cindex prefix for executable and shared library file names
>  @cindex system root, alternate
>  @kindex set solib-absolute-prefix
>  @kindex set sysroot
>  @item set sysroot @var{path}
>  Use @var{path} as the system root for the program being debugged.  Any
> -absolute shared library paths will be prefixed with @var{path}; many
> -runtime loaders store the absolute paths to the shared library in the
> -target program's memory.  If you use @code{set sysroot} to find shared
> -libraries, they need to be laid out in the same way that they are on
> -the target, with e.g.@: a @file{/lib} and @file{/usr/lib} hierarchy
> -under @var{path}.
> +shared library paths will be prefixed with @var{path}; many runtime
> +loaders store the absolute paths to the shared library in the target
> +program's memory.  When attaching to already-running processes, their

This "When attaching to already-running processes" part confuses me,
as the sysroot is also prepended to paths in the "run" case.

Otherwise looks good to me.

> +paths will be prefixed with @var{path} if reported to @value{GDBN} as
> +absolute by the operating system.  If you use @code{set sysroot} to
> +find executables and shared libraries, they need to be laid out in
> +the same way that they are on the target, with e.g.@: a @file{/bin},
> +@file{/lib} and @file{/usr/lib} hierarchy under @var{path}.
>  
>  If @var{path} starts with the sequence @file{target:} and the target
>  system is remote then @value{GDBN} will retrieve the target binaries
> @@ -17846,7 +17848,7 @@ system:
>    c:/foo/bar.dll @result{} /path/to/sysroot/c:/foo/bar.dll
>  @end smallexample

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 0/7] Do not require "file" commands for remote targets
  2015-04-01 11:22 [PATCH 0/7] Do not require "file" commands for remote targets Gary Benson
                   ` (6 preceding siblings ...)
  2015-04-01 11:39 ` [PATCH 7/7] Access executable from remote system when first inferior appears Gary Benson
@ 2015-04-15 10:47 ` Pedro Alves
  2015-04-15 12:02   ` Gary Benson
  2015-04-15 14:16   ` Gary Benson
  7 siblings, 2 replies; 52+ messages in thread
From: Pedro Alves @ 2015-04-15 10:47 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 04/01/2015 12:22 PM, Gary Benson wrote:
> Hi all,
> 
> This series builds on my 'New default sysroot "target:"' series [1]
> and makes GDB able to locate and access executable files when using
> remote targets.  This removes the need for a "file" command before
> "target *remote ...".
> 
> Patches 1-3 cause the main executable's pathname to be prefixed with
> gdb_sysroot on attach in much the same way pathnames of shared
> libraries are treated.
> 
> Patches 4-6 implement the to_pid_to_exec_file method for remote
> targets.  This removes the need for the "file" command when using
> the "attach" command with gdbserver with multiprocess extensions:
> 
>   bash$ gdb -q
>   (gdb) target extended-remote | gdbserver --multi -
>   Remote debugging using | gdbserver --multi -
>   Remote debugging using stdio
>   (gdb) attach 31979
>   Attaching to process 31979
>   Attached; pid = 31979
>   Reading symbols from target:/bin/bash...
> 
> Patch 7 causes GDB to attempt to locate and open the executable
> in remote-target cases without multiprocess extensions:

Should be "with multiprocess extensions".  Without those,
we don't know the target's pid, and thus can't use
pid-to-exec-file.

> 
>   bash$ gdb -q
>   (gdb) target remote | gdbserver - --attach 31979
>   Remote debugging using | gdbserver - --attach 31979
>   Attached; pid = 31979
>   Remote debugging using stdio
>   Reading symbols from target:/bin/bash...
> 
> and:
> 
>   bash$ gdb -q
>   (gdb) target remote | gdbserver - /bin/sh
>   Remote debugging using | gdbserver - /bin/sh
>   Process /bin/sh created; pid = 32166
>   stdin/stdout redirected
>   Remote debugging using stdio
>   Reading symbols from target:/bin/bash...
> 
> There is no change to GDB's behaviour if the user has specified a
> main executable, with the "file" command or on startup, so existing
> use cases do not change.
> 
> Built and regtested on RHEL 6.6 x86_64.
> 
> Ok to commit?

I commented on a couple things, but overall this looks good to me.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 0/7] Do not require "file" commands for remote targets
  2015-04-15 10:47 ` [PATCH 0/7] Do not require "file" commands for remote targets Pedro Alves
@ 2015-04-15 12:02   ` Gary Benson
  2015-04-15 12:16     ` Pedro Alves
  2015-04-15 14:16   ` Gary Benson
  1 sibling, 1 reply; 52+ messages in thread
From: Gary Benson @ 2015-04-15 12:02 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves wrote:
> On 04/01/2015 12:22 PM, Gary Benson wrote:
> > Patch 7 causes GDB to attempt to locate and open the executable
> > in remote-target cases without multiprocess extensions:
> 
> Should be "with multiprocess extensions".  Without those, we don't
> know the target's pid, and thus can't use pid-to-exec-file.

How about:

  Patch 7 causes GDB to attempt to locate and open the executable
  for remote targets not using extended-remote:

> > 
> >   bash$ gdb -q
> >   (gdb) target remote | gdbserver - --attach 31979
> >   Remote debugging using | gdbserver - --attach 31979
> >   Attached; pid = 31979
> >   Remote debugging using stdio
> >   Reading symbols from target:/bin/bash...
> > 
> > and:
> > 
> >   bash$ gdb -q
> >   (gdb) target remote | gdbserver - /bin/sh
> >   Remote debugging using | gdbserver - /bin/sh
> >   Process /bin/sh created; pid = 32166
> >   stdin/stdout redirected
> >   Remote debugging using stdio
> >   Reading symbols from target:/bin/bash...

Cheers,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 0/7] Do not require "file" commands for remote targets
  2015-04-15 12:02   ` Gary Benson
@ 2015-04-15 12:16     ` Pedro Alves
  0 siblings, 0 replies; 52+ messages in thread
From: Pedro Alves @ 2015-04-15 12:16 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches

On 04/15/2015 01:02 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 04/01/2015 12:22 PM, Gary Benson wrote:
>>> Patch 7 causes GDB to attempt to locate and open the executable
>>> in remote-target cases without multiprocess extensions:
>>
>> Should be "with multiprocess extensions".  Without those, we don't
>> know the target's pid, and thus can't use pid-to-exec-file.
> 
> How about:
> 
>   Patch 7 causes GDB to attempt to locate and open the executable
>   for remote targets not using extended-remote:

That's better.

Though it affects extended-remote just the same.  E.g.,:

  $ gdbserver --multi :9999 /bin/sh
  ...

  (gdb) target extended-remote :9999
  ...
  Reading symbols from target:/bin/bash...


Or without passing a binary to gdbserver:

  $ gdbserver --multi :9999
  ...

  (gdb) target extended-remote :9999
  ...
  (gdb) set remote exec-file /bin/sh
  ...
  (gdb) run
  ...
  (gdb) disconnect
  (gdb) quit

now reconnect in extended-remote with another gdb instance:

  $ gdb
  ...
  (gdb) target extended-remote :9999
   ...
  Reading symbols from target:/bin/bash...


So even more correct even would be to say that patch 7 causes GDB
to attempt to locate and open the executable of processes already
being debugged by the remote target, on initial connection,
with the simplest example being:

  bash$ gdb -q
  (gdb) target remote | gdbserver - /bin/sh
  Remote debugging using | gdbserver - /bin/sh
  Process /bin/sh created; pid = 32166
  stdin/stdout redirected
  Remote debugging using stdio
  Reading symbols from target:/bin/bash...

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 3/7] Use gdb_sysroot for main executable on attach
  2015-04-01 14:53   ` Eli Zaretskii
@ 2015-04-15 12:45     ` Gary Benson
  0 siblings, 0 replies; 52+ messages in thread
From: Gary Benson @ 2015-04-15 12:45 UTC (permalink / raw)
  To: Eli Zaretskii, Pedro Alves; +Cc: gdb-patches

Eli Zaretskii wrote:
> > From: Gary Benson <gbenson@redhat.com>
> > Date: Wed,  1 Apr 2015 12:22:17 +0100
> > 
> > +* Paths specified by "set sysroot" will be prepended to the path of
> > +  the main executable when attaching to already-running processes
> > +  (local and remote) if the path of the main executable is reported
> > +  to GDB as absolute by the operating system.
> 
> Please don't use "path" when you really mean "file name".
> 
> > +shared library paths will be prefixed with @var{path}; many runtime
> > +loaders store the absolute paths to the shared library in the target
> > +program's memory.  When attaching to already-running processes, their
> > +paths will be prefixed with @var{path} if reported to @value{GDBN} as
> > +absolute by the operating system.  If you use @code{set sysroot} to
> > +find executables and shared libraries, they need to be laid out in
> > +the same way that they are on the target, with e.g.@: a @file{/bin},
> > +@file{/lib} and @file{/usr/lib} hierarchy under @var{path}.
> 
> Same here.  (Yes, I know that the previous text also used "path").

Pedro Alves wrote:
> This "When attaching to already-running processes" part confuses me,
> as the sysroot is also prepended to paths in the "run" case.

How about these replacements for those two chunks:

NEWS:
+* The system root specified by "set sysroot" will be prepended to the
+  filename of the main executable (if reported to GDB as absolute by
+  the operating system) when starting processes remotely, and when
+  attaching to already-running local or remote processes.

gdb.texinfo:
 Use @var{path} as the system root for the program being debugged. Any
 absolute shared library paths will be prefixed with @var{path}; many
 runtime loaders store the absolute paths to the shared library in the
-target program's memory.  If you use @code{set sysroot} to find shared
-libraries, they need to be laid out in the same way that they are on
-the target, with e.g.@: a @file{/lib} and @file{/usr/lib} hierarchy
-under @var{path}.
+target program's memory.  When starting processes remotely, and when
+attaching to already-running processes (local or remote), their
+filenames will be prefixed with @var{path} if reported to @value{GDBN}
+as absolute by the operating system.  If you use @code{set sysroot} to
+find executables and shared libraries, they need to be laid out in the
+same way that they are on the target, with e.g.@: a @file{/bin},
+@file{/lib} and @file{/usr/lib} hierarchy under @var{path}.

At this point in the series the prefixing only happens when attaching
to local processes--attaching to some remote processes appears in
patch 5, and attaching to all remote processes and to remote processes
we start is patch 7--but I didn't want to jiggle the doc at every
step.  The above chunks are for the whole thing, which I can leave
here in patch 3 or move to patch 7, whatever people prefer.

Cheers,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 4/7 v2] Introduce linux_proc_pid_to_exec_file
  2015-04-15  9:37   ` Pedro Alves
@ 2015-04-15 13:14     ` Gary Benson
  2015-04-15 16:01       ` Pedro Alves
  0 siblings, 1 reply; 52+ messages in thread
From: Gary Benson @ 2015-04-15 13:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Pedro Alves, Doug Evans

This commit introduces a new function linux_proc_pid_to_exec_file
that shared Linux code can use to discover the filename of the
executable that was run to create a process on the system.

gdb/ChangeLog:

	* nat/linux-procfs.h (linux_proc_pid_to_exec_file):
	New declaration.
	* nat/linux-procfs.c (linux_proc_pid_to_exec_file):
	New function, factored out from...
	* linux-nat.c (linux_child_pid_to_exec_file): ...here.
---
 gdb/ChangeLog          |    8 ++++++++
 gdb/linux-nat.c        |   10 +---------
 gdb/nat/linux-procfs.c |   19 +++++++++++++++++++
 gdb/nat/linux-procfs.h |    6 ++++++
 4 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 6c198cf..b04aa68 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4106,15 +4106,7 @@ linux_nat_thread_name (struct target_ops *self, struct thread_info *thr)
 static char *
 linux_child_pid_to_exec_file (struct target_ops *self, int pid)
 {
-  static char buf[PATH_MAX];
-  char name[PATH_MAX];
-
-  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
-  memset (buf, 0, PATH_MAX);
-  if (readlink (name, buf, PATH_MAX - 1) <= 0)
-    strcpy (buf, name);
-
-  return buf;
+  return linux_proc_pid_to_exec_file (pid);
 }
 
 /* Implement the to_xfer_partial interface for memory reads using the /proc
diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c
index 7599b32..44364c5 100644
--- a/gdb/nat/linux-procfs.c
+++ b/gdb/nat/linux-procfs.c
@@ -273,3 +273,22 @@ linux_proc_task_list_dir_exists (pid_t pid)
   xsnprintf (pathname, sizeof (pathname), "/proc/%ld/task", (long) pid);
   return (stat (pathname, &buf) == 0);
 }
+
+/* See linux-procfs.h.  */
+
+char *
+linux_proc_pid_to_exec_file (int pid)
+{
+  static char buf[PATH_MAX];
+  char name[PATH_MAX];
+  ssize_t len;
+
+  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
+  len = readlink (name, buf, PATH_MAX - 1);
+  if (len <= 0)
+    strcpy (buf, name);
+  else
+    buf[len] = '\0';
+
+  return buf;
+}
diff --git a/gdb/nat/linux-procfs.h b/gdb/nat/linux-procfs.h
index c4f5788..fdbf383 100644
--- a/gdb/nat/linux-procfs.h
+++ b/gdb/nat/linux-procfs.h
@@ -73,4 +73,10 @@ extern void linux_proc_attach_tgid_threads (pid_t pid,
 /* Return true if the /proc/PID/task/ directory exists.  */
 extern int linux_proc_task_list_dir_exists (pid_t pid);
 
+/* Return the full absolute name of the executable file that was run
+   to create the process PID.  The returned value persists until this
+   function is next called.  */
+
+extern char *linux_proc_pid_to_exec_file (int pid);
+
 #endif /* COMMON_LINUX_PROCFS_H */
-- 
1.7.1

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 7/7] Access executable from remote system when first inferior appears
  2015-04-15 10:24   ` Pedro Alves
@ 2015-04-15 13:56     ` Gary Benson
  2015-04-15 14:06       ` Gary Benson
                         ` (3 more replies)
  0 siblings, 4 replies; 52+ messages in thread
From: Gary Benson @ 2015-04-15 13:56 UTC (permalink / raw)
  To: Pedro Alves, Eli Zaretskii; +Cc: gdb-patches

Pedro Alves wrote:
> On 04/01/2015 12:22 PM, Gary Benson wrote:
> > This commit modifies remote_add_inferior to take an extra argument
> > try_open_exec.  If this is nonzero, remote_add_inferior will
> > attempt to open this inferior's executable as the main executable
> > if no main executable is open already.  Callers are updated
> > appropriately.
> > 
> > One testcase required updating as a result of this commit.  The
> > test checked that GDB's "info files" command does not crash if no
> > main executable is open, and relied on GDB's inability to access
> > the main executable over the remote protocol.  The test was
> > updated to inhibit this new behavior.
> 
> So this is significant user-visible change too.  I think it deserves
> an example in the commit log, and a NEWS entry.  The manual should
> probably be updated to explain/mention this too.  We already mention
> something like this in the "attach" docs:
> 
>   "When you use @code{attach}, the debugger finds the program running in
>   the process first (...)"

How about these?

diff --git a/gdb/NEWS b/gdb/NEWS
index b11a6fc..e88210f 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -36,6 +36,11 @@
   the operating system) when starting processes remotely, and when
   attaching to already-running local or remote processes.
 
+* GDB now supports automatic location and retrieval of executable
+  files from remote targets.  Remote debugging can now be initiated
+  using only a "target remote" or "target extended-remote" command
+  (no "set sysroot" or "file" commands are required).
+
 * Python Scripting
 
   ** gdb.Objfile objects have a new attribute "username",
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 964f9c4..4f9c21b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2531,7 +2531,8 @@ programs on bare-board targets that lack an operating system.  You must
 also have permission to send the process a signal.
 
 When you use @code{attach}, the debugger finds the program running in
-the process first by looking in the current working directory, then (if
+the process first by querying the operating system.  If this fails,
+@value{GDBN} looks first in the current working directory, then (if
 the program is not found) by using the source file search path
 (@pxref{Source Path, ,Specifying Source Directories}).  You can also use
 the @code{file} command to load the program.  @xref{Files, ,Commands to

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 7/7] Access executable from remote system when first inferior appears
  2015-04-15 13:56     ` Gary Benson
@ 2015-04-15 14:06       ` Gary Benson
  2015-04-15 16:15         ` Pedro Alves
  2015-04-15 16:09       ` Pedro Alves
                         ` (2 subsequent siblings)
  3 siblings, 1 reply; 52+ messages in thread
From: Gary Benson @ 2015-04-15 14:06 UTC (permalink / raw)
  To: Pedro Alves, Eli Zaretskii; +Cc: gdb-patches

Gary Benson wrote:
> Pedro Alves wrote:
> > On 04/01/2015 12:22 PM, Gary Benson wrote:
> > > This commit modifies remote_add_inferior to take an extra argument
> > > try_open_exec.  If this is nonzero, remote_add_inferior will
> > > attempt to open this inferior's executable as the main executable
> > > if no main executable is open already.  Callers are updated
> > > appropriately.
> > > 
> > > One testcase required updating as a result of this commit.  The
> > > test checked that GDB's "info files" command does not crash if no
> > > main executable is open, and relied on GDB's inability to access
> > > the main executable over the remote protocol.  The test was
> > > updated to inhibit this new behavior.
> > 
> > So this is significant user-visible change too.  I think it deserves
> > an example in the commit log, and a NEWS entry.  The manual should
> > probably be updated to explain/mention this too.  We already mention
> > something like this in the "attach" docs:
> > 
> >   "When you use @code{attach}, the debugger finds the program running in
> >   the process first (...)"
> 
> How about these?
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index b11a6fc..e88210f 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -36,6 +36,11 @@
>    the operating system) when starting processes remotely, and when
>    attaching to already-running local or remote processes.
>  
> +* GDB now supports automatic location and retrieval of executable
> +  files from remote targets.  Remote debugging can now be initiated
> +  using only a "target remote" or "target extended-remote" command
> +  (no "set sysroot" or "file" commands are required).
> +
>  * Python Scripting
>  
>    ** gdb.Objfile objects have a new attribute "username",
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 964f9c4..4f9c21b 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -2531,7 +2531,8 @@ programs on bare-board targets that lack an operating system.  You must
>  also have permission to send the process a signal.
>  
>  When you use @code{attach}, the debugger finds the program running in
> -the process first by looking in the current working directory, then (if
> +the process first by querying the operating system.  If this fails,
> +@value{GDBN} looks first in the current working directory, then (if
>  the program is not found) by using the source file search path
>  (@pxref{Source Path, ,Specifying Source Directories}).  You can also use
>  the @code{file} command to load the program.  @xref{Files, ,Commands to

Also, I added this to the commit message:

  With this commit, remote debugging can now be initiated using only a
  "target remote" or "target extended-remote" command; no "set sysroot"
  or "file" commands are required, e.g.

    bash$ gdb -q
    (gdb) target remote | gdbserver - /bin/sh
    Remote debugging using | gdbserver - /bin/sh
    Process /bin/sh created; pid = 32166
    stdin/stdout redirected
    Remote debugging using stdio
    Reading symbols from target:/bin/bash...

Is that ok?

Cheers,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 0/7] Do not require "file" commands for remote targets
  2015-04-15 10:47 ` [PATCH 0/7] Do not require "file" commands for remote targets Pedro Alves
  2015-04-15 12:02   ` Gary Benson
@ 2015-04-15 14:16   ` Gary Benson
  2015-04-15 16:20     ` Pedro Alves
  1 sibling, 1 reply; 52+ messages in thread
From: Gary Benson @ 2015-04-15 14:16 UTC (permalink / raw)
  To: Pedro Alves, Doug Evans, Eli Zaretskii; +Cc: gdb-patches

Pedro Alves wrote:
> I commented on a couple things, but overall this looks good to me.

Pedro, I made all the changes you, Doug and Eli suggested.
Significant updates are:

 Updated NEWS and doc for patch 3:
 https://sourceware.org/ml/gdb-patches/2015-04/msg00560.html

 Updated patch 4:
 https://sourceware.org/ml/gdb-patches/2015-04/msg00562.html

 New NEWS and doc for patch 7:
 https://sourceware.org/ml/gdb-patches/2015-04/msg00564.html

 New commit message for patch 7:
 https://sourceware.org/ml/gdb-patches/2015-04/msg00565.html

Are you ok for me to commit with these changes?

Cheers,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 4/7 v2] Introduce linux_proc_pid_to_exec_file
  2015-04-15 13:14     ` [PATCH 4/7 v2] Introduce linux_proc_pid_to_exec_file Gary Benson
@ 2015-04-15 16:01       ` Pedro Alves
  0 siblings, 0 replies; 52+ messages in thread
From: Pedro Alves @ 2015-04-15 16:01 UTC (permalink / raw)
  To: Gary Benson, gdb-patches; +Cc: Doug Evans

On 04/15/2015 02:14 PM, Gary Benson wrote:
> This commit introduces a new function linux_proc_pid_to_exec_file
> that shared Linux code can use to discover the filename of the
> executable that was run to create a process on the system.

I'd have the preferred the readlink change to be a separate change,
but, it's not worth it to split now.

> 
> gdb/ChangeLog:
> 
> 	* nat/linux-procfs.h (linux_proc_pid_to_exec_file):
> 	New declaration.
> 	* nat/linux-procfs.c (linux_proc_pid_to_exec_file):
> 	New function, factored out from...
> 	* linux-nat.c (linux_child_pid_to_exec_file): ...here.


OK.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 7/7] Access executable from remote system when first inferior appears
  2015-04-15 13:56     ` Gary Benson
  2015-04-15 14:06       ` Gary Benson
@ 2015-04-15 16:09       ` Pedro Alves
  2015-04-16  8:23         ` Gary Benson
  2015-04-15 16:13       ` Pedro Alves
  2015-04-15 16:21       ` Eli Zaretskii
  3 siblings, 1 reply; 52+ messages in thread
From: Pedro Alves @ 2015-04-15 16:09 UTC (permalink / raw)
  To: Gary Benson, Eli Zaretskii; +Cc: gdb-patches

On 04/15/2015 02:55 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 04/01/2015 12:22 PM, Gary Benson wrote:
>>> This commit modifies remote_add_inferior to take an extra argument
>>> try_open_exec.  If this is nonzero, remote_add_inferior will
>>> attempt to open this inferior's executable as the main executable
>>> if no main executable is open already.  Callers are updated
>>> appropriately.
>>>
>>> One testcase required updating as a result of this commit.  The
>>> test checked that GDB's "info files" command does not crash if no
>>> main executable is open, and relied on GDB's inability to access
>>> the main executable over the remote protocol.  The test was
>>> updated to inhibit this new behavior.
>>
>> So this is significant user-visible change too.  I think it deserves
>> an example in the commit log, and a NEWS entry.  The manual should
>> probably be updated to explain/mention this too.  We already mention
>> something like this in the "attach" docs:
>>
>>   "When you use @code{attach}, the debugger finds the program running in
>>   the process first (...)"
> 
> How about these?
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index b11a6fc..e88210f 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -36,6 +36,11 @@
>    the operating system) when starting processes remotely, and when
>    attaching to already-running local or remote processes.
>  
> +* GDB now supports automatic location and retrieval of executable
> +  files from remote targets.  Remote debugging can now be initiated
> +  using only a "target remote" or "target extended-remote" command
> +  (no "set sysroot" or "file" commands are required).

LGTM, though it should probably say

  See "New remote packets" below/above.

too, to hint that this needs remote target support.  It's what we
did in the range-stepping NEWS entry, for example.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 7/7] Access executable from remote system when first inferior appears
  2015-04-15 13:56     ` Gary Benson
  2015-04-15 14:06       ` Gary Benson
  2015-04-15 16:09       ` Pedro Alves
@ 2015-04-15 16:13       ` Pedro Alves
  2015-04-16  9:30         ` Gary Benson
  2015-04-15 16:21       ` Eli Zaretskii
  3 siblings, 1 reply; 52+ messages in thread
From: Pedro Alves @ 2015-04-15 16:13 UTC (permalink / raw)
  To: Gary Benson, Eli Zaretskii; +Cc: gdb-patches

On 04/15/2015 02:55 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> > On 04/01/2015 12:22 PM, Gary Benson wrote:
>>> > > This commit modifies remote_add_inferior to take an extra argument
>>> > > try_open_exec.  If this is nonzero, remote_add_inferior will
>>> > > attempt to open this inferior's executable as the main executable
>>> > > if no main executable is open already.  Callers are updated
>>> > > appropriately.
>>> > > 
>>> > > One testcase required updating as a result of this commit.  The
>>> > > test checked that GDB's "info files" command does not crash if no
>>> > > main executable is open, and relied on GDB's inability to access
>>> > > the main executable over the remote protocol.  The test was
>>> > > updated to inhibit this new behavior.
>> > 
>> > So this is significant user-visible change too.  I think it deserves
>> > an example in the commit log, and a NEWS entry.  The manual should
>> > probably be updated to explain/mention this too.  We already mention
>> > something like this in the "attach" docs:
>> > 
>> >   "When you use @code{attach}, the debugger finds the program running in
>> >   the process first (...)"
> How about these?
> 

Missed the manual bit:

>  
>    ** gdb.Objfile objects have a new attribute "username",
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 964f9c4..4f9c21b 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -2531,7 +2531,8 @@ programs on bare-board targets that lack an operating system.  You must
>  also have permission to send the process a signal.
>  
>  When you use @code{attach}, the debugger finds the program running in
> -the process first by looking in the current working directory, then (if
> +the process first by querying the operating system.  If this fails,
> +@value{GDBN} looks first in the current working directory, then (if
>  the program is not found) by using the source file search path
>  (@pxref{Source Path, ,Specifying Source Directories}).  You can also use
>  the @code{file} command to load the program.  @xref{Files, ,Commands to

This looks right, but it's not what I was thinking.  The "attach" reference
was just to give an example of where we document that gdb find the
program for the user.  I think we should say something like that in
the "target remote" documentation too.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 7/7] Access executable from remote system when first inferior appears
  2015-04-15 14:06       ` Gary Benson
@ 2015-04-15 16:15         ` Pedro Alves
  0 siblings, 0 replies; 52+ messages in thread
From: Pedro Alves @ 2015-04-15 16:15 UTC (permalink / raw)
  To: Gary Benson, Eli Zaretskii; +Cc: gdb-patches

On 04/15/2015 03:05 PM, Gary Benson wrote:
> Also, I added this to the commit message:
> 
>   With this commit, remote debugging can now be initiated using only a
>   "target remote" or "target extended-remote" command; no "set sysroot"
>   or "file" commands are required, e.g.
> 
>     bash$ gdb -q
>     (gdb) target remote | gdbserver - /bin/sh
>     Remote debugging using | gdbserver - /bin/sh
>     Process /bin/sh created; pid = 32166
>     stdin/stdout redirected
>     Remote debugging using stdio
>     Reading symbols from target:/bin/bash...
> 
> Is that ok?
> 

OK.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 0/7] Do not require "file" commands for remote targets
  2015-04-15 14:16   ` Gary Benson
@ 2015-04-15 16:20     ` Pedro Alves
  2015-04-17  9:01       ` Gary Benson
  0 siblings, 1 reply; 52+ messages in thread
From: Pedro Alves @ 2015-04-15 16:20 UTC (permalink / raw)
  To: Gary Benson, Doug Evans, Eli Zaretskii; +Cc: gdb-patches

On 04/15/2015 03:16 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> I commented on a couple things, but overall this looks good to me.
> 
> Pedro, I made all the changes you, Doug and Eli suggested.
> Significant updates are:
> 
>  Updated NEWS and doc for patch 3:
>  https://sourceware.org/ml/gdb-patches/2015-04/msg00560.html
> 
>  Updated patch 4:
>  https://sourceware.org/ml/gdb-patches/2015-04/msg00562.html
> 
>  New NEWS and doc for patch 7:
>  https://sourceware.org/ml/gdb-patches/2015-04/msg00564.html
> 
>  New commit message for patch 7:
>  https://sourceware.org/ml/gdb-patches/2015-04/msg00565.html
> 
> Are you ok for me to commit with these changes?

Once all the above are resolved, yes.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 7/7] Access executable from remote system when first inferior appears
  2015-04-15 13:56     ` Gary Benson
                         ` (2 preceding siblings ...)
  2015-04-15 16:13       ` Pedro Alves
@ 2015-04-15 16:21       ` Eli Zaretskii
  3 siblings, 0 replies; 52+ messages in thread
From: Eli Zaretskii @ 2015-04-15 16:21 UTC (permalink / raw)
  To: Gary Benson; +Cc: palves, gdb-patches

> Date: Wed, 15 Apr 2015 14:55:55 +0100
> From: Gary Benson <gbenson@redhat.com>
> Cc: gdb-patches@sourceware.org
> 
> How about these?

Fine with me, thanks.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 7/7] Access executable from remote system when first inferior appears
  2015-04-15 16:09       ` Pedro Alves
@ 2015-04-16  8:23         ` Gary Benson
  0 siblings, 0 replies; 52+ messages in thread
From: Gary Benson @ 2015-04-16  8:23 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Eli Zaretskii, gdb-patches

Pedro Alves wrote:
> On 04/15/2015 02:55 PM, Gary Benson wrote:
> > diff --git a/gdb/NEWS b/gdb/NEWS
> > index b11a6fc..e88210f 100644
> > --- a/gdb/NEWS
> > +++ b/gdb/NEWS
> > @@ -36,6 +36,11 @@
> >  
> > +* GDB now supports automatic location and retrieval of executable
> > +  files from remote targets.  Remote debugging can now be initiated
> > +  using only a "target remote" or "target extended-remote" command
> > +  (no "set sysroot" or "file" commands are required).
> 
> LGTM, though it should probably say
> 
>   See "New remote packets" below/above.
> 
> too, to hint that this needs remote target support.  It's what we
> did in the range-stepping NEWS entry, for example.

Ok, I added it.

Cheers,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 7/7] Access executable from remote system when first inferior appears
  2015-04-15 16:13       ` Pedro Alves
@ 2015-04-16  9:30         ` Gary Benson
  2015-04-16  9:53           ` Pedro Alves
  2015-04-16 15:05           ` Eli Zaretskii
  0 siblings, 2 replies; 52+ messages in thread
From: Gary Benson @ 2015-04-16  9:30 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Eli Zaretskii, gdb-patches

Pedro Alves wrote:
> Missed the manual bit:
> On 04/15/2015 02:55 PM, Gary Benson wrote:
> > [snip]
> 
> This looks right, but it's not what I was thinking.  The "attach"
> reference was just to give an example of where we document that gdb
> find the program for the user.  I think we should say something like
> that in the "target remote" documentation too.

Ah, I see what you mean.  How about the following?  If it's ok I'll
merge that piece with this final patch.

Cheers,
Gary

---
2015-04-16  Gary Benson <gbenson@redhat.com>

	* gdb.texinfo (Connecting to a Remote Target): Mention that
	GDB can access program files from remote targets that support
	qXfer:exec-file:read and Host I/O packets.

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 964f9c4..e7872dd 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18952,10 +18952,16 @@
 @node Connecting
 @section Connecting to a Remote Target
 
-On the @value{GDBN} host machine, you will need an unstripped copy of
-your program, since @value{GDBN} needs symbol and debugging information.
-Start up @value{GDBN} as usual, using the name of the local copy of your
-program as the first argument.
+@value{GDBN} needs an unstripped copy of your program to access symbol
+and debugging information.  Some remote targets@footnote{@xref{qXfer
+executable filename read}, and @ref{Host I/O Packets}.} allow
+@value{GDBN} to access program files over the same connection used to
+communicate with @value{GDBN}.  With such a target, the only command
+you need is @code{target remote}.  If the target you are using does
+not have this support then you will need an unstripped copy of your
+program on the @value{GDBN} host machine.  Start up @value{GDBN} as
+usual, using the name of the local copy of your program as the first
+argument.
 
 @cindex @code{target remote}
 @value{GDBN} can communicate with the target over a serial line, or

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 7/7] Access executable from remote system when first inferior appears
  2015-04-16  9:30         ` Gary Benson
@ 2015-04-16  9:53           ` Pedro Alves
  2015-04-16 11:47             ` Gary Benson
  2015-04-16 15:06             ` Eli Zaretskii
  2015-04-16 15:05           ` Eli Zaretskii
  1 sibling, 2 replies; 52+ messages in thread
From: Pedro Alves @ 2015-04-16  9:53 UTC (permalink / raw)
  To: Gary Benson; +Cc: Eli Zaretskii, gdb-patches

On 04/16/2015 10:30 AM, Gary Benson wrote:
> -On the @value{GDBN} host machine, you will need an unstripped copy of
> -your program, since @value{GDBN} needs symbol and debugging information.
> -Start up @value{GDBN} as usual, using the name of the local copy of your
> -program as the first argument.
> +@value{GDBN} needs an unstripped copy of your program to access symbol
> +and debugging information.  Some remote targets@footnote{@xref{qXfer
> +executable filename read}, and @ref{Host I/O Packets}.} allow
> +@value{GDBN} to access program files over the same connection used to
> +communicate with @value{GDBN}.  With such a target, the only command
> +you need is @code{target remote}.  If the target you are using does
> +not have this support then you will need an unstripped copy of your
> +program on the @value{GDBN} host machine.  Start up @value{GDBN} as
> +usual, using the name of the local copy of your program as the first
> +argument.

Even if the target supports this, if the target's file copy is
stripped, which it usually is, you'll still need to manually pass the
unstripped program to gdb.  I think we should rephrase it a bit making
sure that is clear.

Maybe something like this? :

-On the @value{GDBN} host machine, you will need an unstripped copy of
-your program, since @value{GDBN} needs symbol and debugging information.
-Start up @value{GDBN} as usual, using the name of the local copy of your
-program as the first argument.
+@value{GDBN} needs an unstripped copy of your program to access symbol
+and debugging information.  Some remote targets@footnote{@xref{qXfer
+executable filename read}, and @ref{Host I/O Packets}.} allow
+@value{GDBN} to access program files over the same connection used to
+communicate with @value{GDBN}.  With such a target, unless the remote
+program file is stripped, the only command you need is
+@code{target remote}.  Otherwise, start up @value{GDBN} using the
+name of the local unstripped copy of your program as the first
+argument, or use the @code{file} command.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 7/7] Access executable from remote system when first inferior appears
  2015-04-16  9:53           ` Pedro Alves
@ 2015-04-16 11:47             ` Gary Benson
  2015-04-16 15:06             ` Eli Zaretskii
  1 sibling, 0 replies; 52+ messages in thread
From: Gary Benson @ 2015-04-16 11:47 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Eli Zaretskii, gdb-patches

Pedro Alves wrote:
> Maybe something like this? :
> 
> -On the @value{GDBN} host machine, you will need an unstripped copy of
> -your program, since @value{GDBN} needs symbol and debugging information.
> -Start up @value{GDBN} as usual, using the name of the local copy of your
> -program as the first argument.
> +@value{GDBN} needs an unstripped copy of your program to access symbol
> +and debugging information.  Some remote targets@footnote{@xref{qXfer
> +executable filename read}, and @ref{Host I/O Packets}.} allow
> +@value{GDBN} to access program files over the same connection used to
> +communicate with @value{GDBN}.  With such a target, unless the remote
> +program file is stripped, the only command you need is
> +@code{target remote}.  Otherwise, start up @value{GDBN} using the
> +name of the local unstripped copy of your program as the first
> +argument, or use the @code{file} command.

Looks good to me.  Eli, is this ok?

Cheers,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 7/7] Access executable from remote system when first inferior appears
  2015-04-16  9:30         ` Gary Benson
  2015-04-16  9:53           ` Pedro Alves
@ 2015-04-16 15:05           ` Eli Zaretskii
  2015-04-16 19:34             ` Gary Benson
  1 sibling, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2015-04-16 15:05 UTC (permalink / raw)
  To: Gary Benson; +Cc: palves, gdb-patches

> Date: Thu, 16 Apr 2015 10:30:13 +0100
> From: Gary Benson <gbenson@redhat.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sourceware.org
> 
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -18952,10 +18952,16 @@
>  @node Connecting
>  @section Connecting to a Remote Target
>  
> -On the @value{GDBN} host machine, you will need an unstripped copy of
> -your program, since @value{GDBN} needs symbol and debugging information.
> -Start up @value{GDBN} as usual, using the name of the local copy of your
> -program as the first argument.
> +@value{GDBN} needs an unstripped copy of your program to access symbol
> +and debugging information.  Some remote targets@footnote{@xref{qXfer
> +executable filename read}, and @ref{Host I/O Packets}.} allow
> +@value{GDBN} to access program files over the same connection used to
> +communicate with @value{GDBN}.  With such a target, the only command
> +you need is @code{target remote}.  If the target you are using does
> +not have this support then you will need an unstripped copy of your
> +program on the @value{GDBN} host machine.  Start up @value{GDBN} as
> +usual, using the name of the local copy of your program as the first
> +argument.

Thanks, but please don't put cross-references in a footnote.  Put it
in parentheses instead, and use @pxref.

Otherwise, this is okay with me.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 7/7] Access executable from remote system when first inferior appears
  2015-04-16  9:53           ` Pedro Alves
  2015-04-16 11:47             ` Gary Benson
@ 2015-04-16 15:06             ` Eli Zaretskii
  2015-04-16 15:23               ` Pedro Alves
  1 sibling, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2015-04-16 15:06 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gbenson, gdb-patches

> Date: Thu, 16 Apr 2015 10:53:39 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sourceware.org
> 
> +communicate with @value{GDBN}.  With such a target, unless the remote
> +program file is stripped, the only command you need is
> +@code{target remote}.

The use of "unless" here makes this sentence harder to understand.  I
suggest a simpler "if the remote program is unstripped" instead.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 7/7] Access executable from remote system when first inferior appears
  2015-04-16 15:06             ` Eli Zaretskii
@ 2015-04-16 15:23               ` Pedro Alves
  0 siblings, 0 replies; 52+ messages in thread
From: Pedro Alves @ 2015-04-16 15:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gbenson, gdb-patches

On 04/16/2015 04:06 PM, Eli Zaretskii wrote:
>> Date: Thu, 16 Apr 2015 10:53:39 +0100
>> From: Pedro Alves <palves@redhat.com>
>> CC: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sourceware.org
>>
>> +communicate with @value{GDBN}.  With such a target, unless the remote
>> +program file is stripped, the only command you need is
>> +@code{target remote}.
> 
> The use of "unless" here makes this sentence harder to understand.  I
> suggest a simpler "if the remote program is unstripped" instead.

Nice, I like it.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 7/7] Access executable from remote system when first inferior appears
  2015-04-16 15:05           ` Eli Zaretskii
@ 2015-04-16 19:34             ` Gary Benson
  0 siblings, 0 replies; 52+ messages in thread
From: Gary Benson @ 2015-04-16 19:34 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: palves, gdb-patches

Eli Zaretskii wrote:
> > Date: Thu, 16 Apr 2015 10:30:13 +0100
> > From: Gary Benson <gbenson@redhat.com>
> > Cc: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sourceware.org
> > 
> > --- a/gdb/doc/gdb.texinfo
> > +++ b/gdb/doc/gdb.texinfo
> > @@ -18952,10 +18952,16 @@
> >  @node Connecting
> >  @section Connecting to a Remote Target
> >  
> > -On the @value{GDBN} host machine, you will need an unstripped copy of
> > -your program, since @value{GDBN} needs symbol and debugging information.
> > -Start up @value{GDBN} as usual, using the name of the local copy of your
> > -program as the first argument.
> > +@value{GDBN} needs an unstripped copy of your program to access symbol
> > +and debugging information.  Some remote targets@footnote{@xref{qXfer
> > +executable filename read}, and @ref{Host I/O Packets}.} allow
> > +@value{GDBN} to access program files over the same connection used to
> > +communicate with @value{GDBN}.  With such a target, the only command
> > +you need is @code{target remote}.  If the target you are using does
> > +not have this support then you will need an unstripped copy of your
> > +program on the @value{GDBN} host machine.  Start up @value{GDBN} as
> > +usual, using the name of the local copy of your program as the first
> > +argument.
> 
> Thanks, but please don't put cross-references in a footnote.  Put it
> in parentheses instead, and use @pxref.
> 
> Otherwise, this is okay with me.

Thanks Eli.

Cheers,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 0/7] Do not require "file" commands for remote targets
  2015-04-15 16:20     ` Pedro Alves
@ 2015-04-17  9:01       ` Gary Benson
  0 siblings, 0 replies; 52+ messages in thread
From: Gary Benson @ 2015-04-17  9:01 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Doug Evans, Eli Zaretskii, gdb-patches

Pedro Alves wrote:
> On 04/15/2015 03:16 PM, Gary Benson wrote:
> > Pedro Alves wrote:
> > > I commented on a couple things, but overall this looks good to me.
> > 
> > Pedro, I made all the changes you, Doug and Eli suggested.
> > Significant updates are:
> > 
> >  Updated NEWS and doc for patch 3:
> >  https://sourceware.org/ml/gdb-patches/2015-04/msg00560.html
> > 
> >  Updated patch 4:
> >  https://sourceware.org/ml/gdb-patches/2015-04/msg00562.html
> > 
> >  New NEWS and doc for patch 7:
> >  https://sourceware.org/ml/gdb-patches/2015-04/msg00564.html
> > 
> >  New commit message for patch 7:
> >  https://sourceware.org/ml/gdb-patches/2015-04/msg00565.html
> > 
> > Are you ok for me to commit with these changes?
> 
> Once all the above are resolved, yes.

Thank you all for the reviews and contributions, this is now pushed.

Cheers,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Possible regression on gdb.base/attach.exp when using native-extended-gdbserver (was: Re: [PATCH 6/7] Implement qXfer:exec-file:read in gdbserver)
  2015-04-01 11:30 ` [PATCH 6/7] Implement qXfer:exec-file:read in gdbserver Gary Benson
  2015-04-06 17:11   ` Doug Evans
@ 2015-04-17 23:43   ` Sergio Durigan Junior
  2015-04-20  9:13     ` Gary Benson
  2015-04-20 10:41     ` [OB PATCH] Fix three test failures with extended remote targets Gary Benson
  1 sibling, 2 replies; 52+ messages in thread
From: Sergio Durigan Junior @ 2015-04-17 23:43 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches

On Wednesday, April 01 2015, Gary Benson wrote:

> This commit implements the "qXfer:exec-file:read" packet in gdbserver.

Hi Gary,

I haven't really investigated further, but I saw that this commit
introduced a regression on the gdb.base/attach.exp test.  This happened
on some builders; for example:

  <http://gdb-build.sergiodj.net/builders/Fedora-x86_64-native-extended-gdbserver-m32/builds/887/steps/regressions/logs/regressions>

As far as I have checked, this failure manifests only when testing the
native-extended-gdbserver variant.

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: Possible regression on gdb.base/attach.exp when using native-extended-gdbserver (was: Re: [PATCH 6/7] Implement qXfer:exec-file:read in gdbserver)
  2015-04-17 23:43   ` Possible regression on gdb.base/attach.exp when using native-extended-gdbserver (was: Re: [PATCH 6/7] Implement qXfer:exec-file:read in gdbserver) Sergio Durigan Junior
@ 2015-04-20  9:13     ` Gary Benson
  2015-04-20 10:41     ` [OB PATCH] Fix three test failures with extended remote targets Gary Benson
  1 sibling, 0 replies; 52+ messages in thread
From: Gary Benson @ 2015-04-20  9:13 UTC (permalink / raw)
  To: Sergio Durigan Junior; +Cc: gdb-patches

Sergio Durigan Junior wrote:
> On Wednesday, April 01 2015, Gary Benson wrote:
> > This commit implements the "qXfer:exec-file:read" packet in gdbserver.
> 
> I haven't really investigated further, but I saw that this commit
> introduced a regression on the gdb.base/attach.exp test.  This happened
> on some builders; for example:
> 
>   <http://gdb-build.sergiodj.net/builders/Fedora-x86_64-native-extended-gdbserver-m32/builds/887/steps/regressions/logs/regressions>
> 
> As far as I have checked, this failure manifests only when testing the
> native-extended-gdbserver variant.

Ok, looking into it.

Thanks,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [OB PATCH] Fix three test failures with extended remote targets
  2015-04-17 23:43   ` Possible regression on gdb.base/attach.exp when using native-extended-gdbserver (was: Re: [PATCH 6/7] Implement qXfer:exec-file:read in gdbserver) Sergio Durigan Junior
  2015-04-20  9:13     ` Gary Benson
@ 2015-04-20 10:41     ` Gary Benson
  1 sibling, 0 replies; 52+ messages in thread
From: Gary Benson @ 2015-04-20 10:41 UTC (permalink / raw)
  To: gdb-patches; +Cc: Sergio Durigan Junior

Hi all,

This commit fixes three gdb.base/attach.exp failures when using
extended remote targets.  The failures occurred because GDB now
locates and loads files when attaching on remote targets if the
remote target supports qXfer:exec-file:read; the filenames were
shown but with "target:" prefixes which the test has been updated
to handle.

Thanks Sergio for spotting the failure and narrowing it down to
a specific commit (and for doing all the buildbot stuff you do
generally!)

Cheers,
Gary


gdb/testsuite/ChangeLog:

	* gdb.base/attach.exp: Fix three extended remote failures.
---
 gdb/testsuite/ChangeLog           |    4 ++++
 gdb/testsuite/gdb.base/attach.exp |   21 +++++++++++++++++----
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp
index 38b19b5..f2ebe3a 100644
--- a/gdb/testsuite/gdb.base/attach.exp
+++ b/gdb/testsuite/gdb.base/attach.exp
@@ -66,6 +66,19 @@ proc do_attach_tests {} {
     global subdir
     global timeout
     
+    # Figure out a regular expression that will match the sysroot,
+    # noting that the default sysroot is "target:", and also noting
+    # that GDB will strip "target:" from the start of filenames when
+    # operating on the local filesystem
+    set sysroot ""
+    set test "show sysroot"
+    gdb_test_multiple $test $test {
+	-re "The current system root is \"(.*)\"\..*${gdb_prompt} $" {
+	    set sysroot $expect_out(1,string)
+	}
+    }
+    regsub "^target:" "$sysroot" "(target:)?" sysroot
+
     # Start the program running and then wait for a bit, to be sure
     # that it can be attached to.
 
@@ -219,17 +232,17 @@ proc do_attach_tests {} {
     set test "attach2, with no file"
     set found_exec_file 0
     gdb_test_multiple "attach $testpid" "$test" {
-	-re "Attaching to process $testpid.*Load new symbol table from \"$escapedbinfile\.exe\".*y or n. $" {
+	-re "Attaching to process $testpid.*Load new symbol table from \"$sysroot$escapedbinfile\.exe\".*y or n. $" {
 	    # On Cygwin, the DLL's symbol tables are loaded prior to the
 	    # executable's symbol table.  This in turn always results in
 	    # asking the user for actually loading the symbol table of the
 	    # executable.
-	    gdb_test "y" "Reading symbols from $escapedbinfile\.\.\.*done." \
+	    gdb_test "y" "Reading symbols from $sysroot$escapedbinfile\.\.\.*done." \
 		"$test (reset file)"
 
 	    set found_exec_file 1
 	}
-	-re "Attaching to process $testpid.*Reading symbols from $escapedbinfile.*main.*at .*$gdb_prompt $" {
+	-re "Attaching to process $testpid.*Reading symbols from $sysroot$escapedbinfile.*main.*at .*$gdb_prompt $" {
 	    pass "$test"
 	    set found_exec_file 1
 	}
@@ -298,7 +311,7 @@ proc do_attach_tests {} {
 	"before attach3, flush exec"
 
     gdb_test "attach $testpid" \
-	"Attaching to process $testpid.*Reading symbols from $escapedbinfile.*main.*at .*" \
+	"Attaching to process $testpid.*Reading symbols from $sysroot$escapedbinfile.*main.*at .*" \
 	"attach when process' a.out not in cwd"
 
     set test "after attach3, exit"
-- 
1.7.1

^ permalink raw reply	[flat|nested] 52+ messages in thread

end of thread, other threads:[~2015-04-20 10:41 UTC | newest]

Thread overview: 52+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-01 11:22 [PATCH 0/7] Do not require "file" commands for remote targets Gary Benson
2015-04-01 11:22 ` [PATCH 1/7] Introduce exec_file_locate_attach Gary Benson
2015-04-01 11:22 ` [PATCH 3/7] Use gdb_sysroot for main executable on attach Gary Benson
2015-04-01 14:53   ` Eli Zaretskii
2015-04-15 12:45     ` Gary Benson
2015-04-15 10:43   ` Pedro Alves
2015-04-01 11:22 ` [PATCH 2/7] Introduce exec_file_find Gary Benson
2015-04-01 11:27 ` [PATCH 4/7] Introduce linux_pid_to_exec_file Gary Benson
2015-04-06 16:41   ` Doug Evans
2015-04-07  9:07     ` Gary Benson
2015-04-08  3:15       ` Doug Evans
2015-04-08  8:06         ` Gary Benson
2015-04-15  9:37   ` Pedro Alves
2015-04-15 13:14     ` [PATCH 4/7 v2] Introduce linux_proc_pid_to_exec_file Gary Benson
2015-04-15 16:01       ` Pedro Alves
2015-04-01 11:29 ` [PATCH 5/7] Implement remote_pid_to_exec_file using qXfer:exec-file:read Gary Benson
2015-04-01 14:55   ` Eli Zaretskii
2015-04-06 17:00     ` Doug Evans
2015-04-06 17:21       ` Eli Zaretskii
2015-04-06 21:57         ` Doug Evans
2015-04-07  6:09           ` Eli Zaretskii
2015-04-07  9:08         ` Gary Benson
2015-04-08  1:57           ` Doug Evans
2015-04-08  6:00             ` Eli Zaretskii
2015-04-01 11:30 ` [PATCH 6/7] Implement qXfer:exec-file:read in gdbserver Gary Benson
2015-04-06 17:11   ` Doug Evans
2015-04-07  9:19     ` Gary Benson
2015-04-17 23:43   ` Possible regression on gdb.base/attach.exp when using native-extended-gdbserver (was: Re: [PATCH 6/7] Implement qXfer:exec-file:read in gdbserver) Sergio Durigan Junior
2015-04-20  9:13     ` Gary Benson
2015-04-20 10:41     ` [OB PATCH] Fix three test failures with extended remote targets Gary Benson
2015-04-01 11:39 ` [PATCH 7/7] Access executable from remote system when first inferior appears Gary Benson
2015-04-15 10:24   ` Pedro Alves
2015-04-15 13:56     ` Gary Benson
2015-04-15 14:06       ` Gary Benson
2015-04-15 16:15         ` Pedro Alves
2015-04-15 16:09       ` Pedro Alves
2015-04-16  8:23         ` Gary Benson
2015-04-15 16:13       ` Pedro Alves
2015-04-16  9:30         ` Gary Benson
2015-04-16  9:53           ` Pedro Alves
2015-04-16 11:47             ` Gary Benson
2015-04-16 15:06             ` Eli Zaretskii
2015-04-16 15:23               ` Pedro Alves
2015-04-16 15:05           ` Eli Zaretskii
2015-04-16 19:34             ` Gary Benson
2015-04-15 16:21       ` Eli Zaretskii
2015-04-15 10:47 ` [PATCH 0/7] Do not require "file" commands for remote targets Pedro Alves
2015-04-15 12:02   ` Gary Benson
2015-04-15 12:16     ` Pedro Alves
2015-04-15 14:16   ` Gary Benson
2015-04-15 16:20     ` Pedro Alves
2015-04-17  9:01       ` Gary Benson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).