public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 3/5] Rename TARGET_SYSROOT_PREFIX as TARGET_FILENAME_PREFIX
  2015-07-28 15:36 [PATCH 0/5] Change how "target:" gets into filenames Gary Benson
@ 2015-07-28 15:36 ` Gary Benson
  2015-07-28 15:36 ` [PATCH 2/5] Update Valgrind GDB special case Gary Benson
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Gary Benson @ 2015-07-28 15:36 UTC (permalink / raw)
  To: gdb-patches
  Cc: Pedro Alves, Sandra Loosemore, Paul_Koning, Jan Kratochvil,
	Joel Brobecker

TARGET_SYSROOT_PREFIX is misnamed, as its use is not limited to
the system root. This commit renames it as TARGET_FILENAME_PREFIX.

gdb/ChangeLog:

	* gdb_bfd.h (TARGET_SYSROOT_PREFIX): Rename from this...
	(TARGET_FILENAME_PREFIX): ...to this.  Update all uses.
---
 gdb/ChangeLog    |    5 +++++
 gdb/exec.c       |    6 +++---
 gdb/gdb_bfd.c    |    6 +++---
 gdb/gdb_bfd.h    |   10 +++++-----
 gdb/remote-sim.c |    2 +-
 gdb/solib.c      |    6 +++---
 6 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/gdb/exec.c b/gdb/exec.c
index f1b1049..bbb7798 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -225,7 +225,7 @@ exec_file_attach (const char *filename, int from_tty)
       if (is_target_filename (filename))
 	{
 	  if (target_filesystem_is_local ())
-	    filename += strlen (TARGET_SYSROOT_PREFIX);
+	    filename += strlen (TARGET_FILENAME_PREFIX);
 	  else
 	    load_via_target = 1;
 	}
@@ -235,8 +235,8 @@ exec_file_attach (const char *filename, int from_tty)
 	  /* gdb_bfd_fopen does not support "target:" filenames.  */
 	  if (write_files)
 	    warning (_("writing into executable files is "
-		       "not supported for %s sysroots"),
-		     TARGET_SYSROOT_PREFIX);
+		       "not supported for %s filenames"),
+		     TARGET_FILENAME_PREFIX);
 
 	  scratch_pathname = xstrdup (filename);
 	  make_cleanup (xfree, scratch_pathname);
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 1781d80..3afcf5f 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -144,7 +144,7 @@ eq_bfd (const void *a, const void *b)
 int
 is_target_filename (const char *name)
 {
-  return startswith (name, TARGET_SYSROOT_PREFIX);
+  return startswith (name, TARGET_FILENAME_PREFIX);
 }
 
 /* See gdb_bfd.h.  */
@@ -223,7 +223,7 @@ gdb_bfd_iovec_fileio_open (struct bfd *abfd, void *inferior)
   gdb_assert (is_target_filename (filename));
 
   fd = target_fileio_open ((struct inferior *) inferior,
-			   filename + strlen (TARGET_SYSROOT_PREFIX),
+			   filename + strlen (TARGET_FILENAME_PREFIX),
 			   FILEIO_O_RDONLY, 0,
 			   &target_errno);
   if (fd == -1)
@@ -336,7 +336,7 @@ gdb_bfd_open (const char *name, const char *target, int fd)
 				      gdb_bfd_iovec_fileio_fstat);
 	}
 
-      name += strlen (TARGET_SYSROOT_PREFIX);
+      name += strlen (TARGET_FILENAME_PREFIX);
     }
 
   if (gdb_bfd_cache == NULL)
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index 8fbdf36..2e1e901 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -27,20 +27,20 @@ DECLARE_REGISTRY (bfd);
 /* If supplied a path starting with this sequence, gdb_bfd_open will
    open BFDs using target fileio operations.  */
 
-#define TARGET_SYSROOT_PREFIX "target:"
+#define TARGET_FILENAME_PREFIX "target:"
 
-/* Returns nonzero if NAME starts with TARGET_SYSROOT_PREFIX, zero
+/* Returns nonzero if NAME starts with TARGET_FILENAME_PREFIX, zero
    otherwise.  */
 
 int is_target_filename (const char *name);
 
 /* Returns nonzero if the filename associated with ABFD starts with
-   TARGET_SYSROOT_PREFIX, zero otherwise.  */
+   TARGET_FILENAME_PREFIX, zero otherwise.  */
 
 int gdb_bfd_has_target_filename (struct bfd *abfd);
 
 /* Open a read-only (FOPEN_RB) BFD given arguments like bfd_fopen.
-   If NAME starts with TARGET_SYSROOT_PREFIX then the BFD will be
+   If NAME starts with TARGET_FILENAME_PREFIX then the BFD will be
    opened using target fileio operations if necessary.  Returns NULL
    on error.  On success, returns a new reference to the BFD, which
    must be freed with gdb_bfd_unref.  BFDs returned by this call are
@@ -48,7 +48,7 @@ int gdb_bfd_has_target_filename (struct bfd *abfd);
    then after this call it is owned by BFD.  If the BFD was not
    accessed using target fileio operations then the filename
    associated with the BFD and accessible with bfd_get_filename will
-   not be exactly NAME but rather NAME with TARGET_SYSROOT_PREFIX
+   not be exactly NAME but rather NAME with TARGET_FILENAME_PREFIX
    stripped.  */
 
 struct bfd *gdb_bfd_open (const char *name, const char *target, int fd);
diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index 82c129d..58e1ce3 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -675,7 +675,7 @@ gdbsim_open (const char *args, int from_tty)
 
   sysroot = gdb_sysroot;
   if (is_target_filename (sysroot))
-    sysroot += strlen (TARGET_SYSROOT_PREFIX);
+    sysroot += strlen (TARGET_FILENAME_PREFIX);
 
   if (remote_debug)
     fprintf_unfiltered (gdb_stdlog,
diff --git a/gdb/solib.c b/gdb/solib.c
index eb933c0..32931da 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -167,7 +167,7 @@ solib_find_1 (char *in_pathname, int *fd, int is_solib)
      for all local files regardless of whether a "target:" prefix was
      used.  */
   if (is_target_filename (sysroot) && target_filesystem_is_local ())
-    sysroot += strlen (TARGET_SYSROOT_PREFIX);
+    sysroot += strlen (TARGET_FILENAME_PREFIX);
 
   /* Strip any trailing slashes from the absolute prefix.  */
   prefix_len = orig_prefix_len = strlen (sysroot);
@@ -239,7 +239,7 @@ solib_find_1 (char *in_pathname, int *fd, int is_solib)
 	There's no need to check for drive spec explicitly, as we only
 	get here if IN_PATHNAME is considered an absolute path.  */
       need_dir_separator = !(IS_DIR_SEPARATOR (in_pathname[0])
-			     || strcmp (TARGET_SYSROOT_PREFIX, sysroot) == 0);
+			     || strcmp (TARGET_FILENAME_PREFIX, sysroot) == 0);
 
       /* Cat the prefixed pathname together.  */
       temp_pathname = concat (sysroot,
@@ -1477,7 +1477,7 @@ gdb_sysroot_changed (char *ignored, int from_tty,
 		     struct cmd_list_element *e)
 {
   const char *old_prefix = "remote:";
-  const char *new_prefix = TARGET_SYSROOT_PREFIX;
+  const char *new_prefix = TARGET_FILENAME_PREFIX;
 
   if (startswith (gdb_sysroot, old_prefix))
     {
-- 
1.7.1

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

* [PATCH 4/5] Use TARGET_FILENAME_PREFIX as the system root in some cases
  2015-07-28 15:36 [PATCH 0/5] Change how "target:" gets into filenames Gary Benson
  2015-07-28 15:36 ` [PATCH 3/5] Rename TARGET_SYSROOT_PREFIX as TARGET_FILENAME_PREFIX Gary Benson
  2015-07-28 15:36 ` [PATCH 2/5] Update Valgrind GDB special case Gary Benson
@ 2015-07-28 15:36 ` Gary Benson
  2015-07-28 15:40   ` Eli Zaretskii
  2015-07-28 15:45 ` [PATCH 1/5] Revert default system root back to "" Gary Benson
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Gary Benson @ 2015-07-28 15:36 UTC (permalink / raw)
  To: gdb-patches
  Cc: Pedro Alves, Sandra Loosemore, Paul_Koning, Jan Kratochvil,
	Joel Brobecker

This commit updates solib_find_1 to use TARGET_FILENAME_PREFIX as the
system root if auto-target-prefix is enabled and gdb_sysroot is empty
and the target filesystem is not the local filesystem.

gdb/ChangeLog:

	* solib.c (auto_target_prefix): New static variable.
	(solib_find_1): Use TARGET_FILENAME_PREFIX as sysroot
	in some cases.
	(show_auto_target_prefix): New function.
	(_initialize_solib): New "set/show auto-target-prefix"
	commands.
	* NEWS: Mention that GDB will use "target:" as the system
	root in some cases.  Mention new "set/show auto-target-prefix"
	commands.

gdb/doc/ChangeLog:

	* gdb.texinfo (Commands to Specify Files): Document the
	"set/show auto-target-prefix" commands.
---
 gdb/ChangeLog       |   12 ++++++++++
 gdb/NEWS            |    8 +++++-
 gdb/doc/ChangeLog   |    5 ++++
 gdb/doc/gdb.texinfo |   11 +++++++++
 gdb/solib.c         |   62 +++++++++++++++++++++++++++++++++++++++++++-------
 5 files changed, 88 insertions(+), 10 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 7ecdf93..3e22b82 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -40,7 +40,9 @@
   prefixed with "target:" to tell GDB to access shared libraries from
   the target system, be it local or remote.  This replaces the prefix
   "remote:", which is automatically converted to "target:" for
-  backward compatibility.
+  backward compatibility.  If the system root is unset, GDB will use
+  "target:" as the system root where necessary if auto-target-prefix
+  is enabled.  See "New commands" below.
 
 * The system root specified by "set sysroot" will be prepended to the
   filename of the main executable (if reported to GDB as absolute by
@@ -193,6 +195,10 @@ maint set|show btrace pt skip-pad
   Set and show whether PAD packets are skipped when computing the
   packet history.
 
+set auto-target-prefix
+show auto-target-prefix
+  Control automatic prefixing of binary filenames with "target:".
+
 * The command 'thread apply all' can now support new option '-ascending'
   to call its specified command for all threads in ascending order.
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9e2ecd1..9ac6e2f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18091,6 +18091,17 @@ location.
 @item show sysroot
 Display the current executable and shared library prefix.
 
+@kindex set auto-target-prefix
+@item set solib-search-path @var{mode}
+If @var{mode} is @code{on}, and the system root is empty, @value{GDBN}
+will use @file{target:} as the system root if that is necessary for
+@value{GDBN} to be able to access the target binaries.
+
+@kindex show auto-target-prefix
+@item show auto-solib-add
+Display the current target-prefixing mode.
+@end table
+
 @kindex set solib-search-path
 @item set solib-search-path @var{path}
 If this variable is set, @var{path} is a colon-separated list of
diff --git a/gdb/solib.c b/gdb/solib.c
index 32931da..4fdfba0 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -105,6 +105,11 @@ show_solib_search_path (struct ui_file *file, int from_tty,
 		    value);
 }
 
+/* If nonzero, prefix executable and shared library filenames with
+   TARGET_FILENAME_PREFIX when attaching to processes whose filesystem
+   is not the local filesystem.  */
+static int auto_target_prefix = 1;
+
 /* Same as HAVE_DOS_BASED_FILE_SYSTEM, but useable as an rvalue.  */
 #if (HAVE_DOS_BASED_FILE_SYSTEM)
 #  define DOS_BASED_FILE_SYSTEM 1
@@ -124,7 +129,10 @@ show_solib_search_path (struct ui_file *file, int from_tty,
    is the local filesystem then the "target:" prefix will be
    stripped before the search starts.  This ensures that the
    same search algorithm is used for local files regardless of
-   whether a "target:" prefix was used.
+   whether a "target:" prefix was used.  If the target filesystem
+   is not the local filesystem then "target:" will be used as the
+   prefix directory for binary files if GDB_SYSROOT is empty and
+   AUTO_TARGET_PREFIX is nonzero.
 
    Global variable SOLIB_SEARCH_PATH is used as a prefix directory
    (or set of directories, as in LD_LIBRARY_PATH) to search for all
@@ -160,14 +168,30 @@ solib_find_1 (char *in_pathname, int *fd, int is_solib)
   char *sysroot = gdb_sysroot;
   int prefix_len, orig_prefix_len;
 
-  /* If the absolute prefix starts with "target:" but the filesystem
-     accessed by the target_fileio_* methods is the local filesystem
-     then we strip the "target:" prefix now and work with the local
-     filesystem.  This ensures that the same search algorithm is used
-     for all local files regardless of whether a "target:" prefix was
-     used.  */
-  if (is_target_filename (sysroot) && target_filesystem_is_local ())
-    sysroot += strlen (TARGET_FILENAME_PREFIX);
+  if (target_filesystem_is_local ())
+    {
+      /* If the absolute prefix starts with "target:" but the
+	 filesystem accessed by the target_fileio_* methods is the
+	 local filesystem then we strip the "target:" prefix now and
+	 work with the local filesystem.  This ensures that the same
+	 search algorithm is used for all local files regardless of
+	 whether a "target:" prefix was used.  */
+      if (is_target_filename (sysroot))
+	sysroot += strlen (TARGET_FILENAME_PREFIX);
+    }
+  else if (auto_target_prefix && *gdb_sysroot == '\0')
+    {
+      /* Set the absolute prefix to "target:" for executable files
+	 and for shared libraries whose executable filename has a
+	 "target:"-prefix.  */
+      if (!is_solib
+	  || (exec_filename != NULL
+	      && is_target_filename (exec_filename)))
+	{
+	  sysroot = xstrdup (TARGET_FILENAME_PREFIX);
+	  make_cleanup (xfree, sysroot);
+	}
+    }
 
   /* Strip any trailing slashes from the absolute prefix.  */
   prefix_len = orig_prefix_len = strlen (sysroot);
@@ -1507,6 +1531,15 @@ show_auto_solib_add (struct ui_file *file, int from_tty,
 		    value);
 }
 
+static void
+show_auto_target_prefix (struct ui_file *file, int from_tty,
+			 struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file,
+		    _("Automatic prefixing of binary filenames is %s.\n"),
+		    value);
+}
+
 
 /* Handler for library-specific lookup of global symbol NAME in OBJFILE.  Call
    the library-specific handler if it is installed for the current target.  */
@@ -1714,4 +1747,15 @@ PATH and LD_LIBRARY_PATH."),
 				     reload_shared_libraries,
 				     show_solib_search_path,
 				     &setlist, &showlist);
+
+  add_setshow_boolean_cmd ("auto-target-prefix", class_support,
+			   &auto_target_prefix, _("\
+Set automatic prefixing of binary filenames."), _("\
+Show automatic prefixing of binary filenames."), _("\
+If \"on\", filenames of binaries will be prefixed with \"target:\"\n\
+when attaching to processes whose filesystems GDB cannot access\n\
+directly."),
+			   NULL,
+			   show_auto_target_prefix,
+			   &setlist, &showlist);
 }
-- 
1.7.1

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

* [PATCH 0/5] Change how "target:" gets into filenames
@ 2015-07-28 15:36 Gary Benson
  2015-07-28 15:36 ` [PATCH 3/5] Rename TARGET_SYSROOT_PREFIX as TARGET_FILENAME_PREFIX Gary Benson
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Gary Benson @ 2015-07-28 15:36 UTC (permalink / raw)
  To: gdb-patches
  Cc: Pedro Alves, Sandra Loosemore, Paul_Koning, Jan Kratochvil,
	Joel Brobecker

Hi all,

This is a continuation of this thread:
https://sourceware.org/ml/gdb/2015-07/msg00038.html

In summary, GDB since April automatically locates and fetches
binary files from remote systems.  A number of people would
like this not to happen.

This automation hinged on changing the default sysroot from ""
to "target:".  This series reverts that change, and instead
updates solib_find_1 to use "target:" as the sysroot where it
would be necessary to access the files.

Most use cases proceed as before, but with this series the
files are *not* transferred if you supply GDB an executable
either on the command line or with a "file" command.

Remote examples:

  $ gdb ./a.out
  (gdb) target remote :9999
  # no "target:" prefix (=no files transferred)

  $ gdb
  (gdb) file a.out
  (gdb) target remote :9999
  # no "target:" prefix (=no files transferred)

  $ gdb
  (gdb) target remote :9999
  # "target:" prefix, files transferred

Aside from fixing this issue, GDB with this change has the advantage
that users don't see the "target:" prefix unless either they are
using GDB in a way that didn't work in 7.9 (remote target without
"file" and "set sysroot" commands, containerized target) or they
explitictly set it themselves (e.g. "set sysroot target:/foo/bar").

I've Cc'd Joel because, if this is the way we want to go, it would
be nice to have this in 7.10 as it minimises the difference from
7.9 and would avoid having 7.10 introduce the "target:" sysroot
only for 7.11 to remove it.

I've Cc'd Jan because he's working on making sysroot be a search
path. (defaulting to something like "/", "target:/").  I think you
could do that from this series by treating the auto-target-prefix
boolean as a stepping-stone that could be removed.  If nothing else
this series is a map of the places you'll need to update :)

Built and regtested on RHEL 6.6 x86_64.

Opinions?  (Should I commit this, and if so where?)

Cheers,
Gary

--
http://gbenson.net/

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

* [PATCH 2/5] Update Valgrind GDB special case
  2015-07-28 15:36 [PATCH 0/5] Change how "target:" gets into filenames Gary Benson
  2015-07-28 15:36 ` [PATCH 3/5] Rename TARGET_SYSROOT_PREFIX as TARGET_FILENAME_PREFIX Gary Benson
@ 2015-07-28 15:36 ` Gary Benson
  2015-07-28 15:36 ` [PATCH 4/5] Use TARGET_FILENAME_PREFIX as the system root in some cases Gary Benson
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Gary Benson @ 2015-07-28 15:36 UTC (permalink / raw)
  To: gdb-patches
  Cc: Pedro Alves, Sandra Loosemore, Paul_Koning, Jan Kratochvil,
	Joel Brobecker

This commit updates the vgdb handling in remote_filesystem_is_local
to reflect the new default system root of "".

gdb/ChangeLog:

	* remote.c (remote_filesystem_is_local): Update vgdb handling.
---
 gdb/ChangeLog |    4 ++++
 gdb/remote.c  |    5 ++---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 69da508..8b7e9d1 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -10567,9 +10567,8 @@ remote_filesystem_is_local (struct target_ops *self)
      on the local filesystem: it does not implement remote get
      and users are not expected to set a sysroot.  To handle
      this case we treat the remote filesystem as local if the
-     sysroot is exactly TARGET_SYSROOT_PREFIX and if the stub
-     does not support vFile:open.  */
-  if (strcmp (gdb_sysroot, TARGET_SYSROOT_PREFIX) == 0)
+     sysroot is empty and the stub does not support vFile:open.  */
+  if (*gdb_sysroot == '\0')
     {
       enum packet_support ps = packet_support (PACKET_vFile_open);
 
-- 
1.7.1

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

* Re: [PATCH 4/5] Use TARGET_FILENAME_PREFIX as the system root in some cases
  2015-07-28 15:36 ` [PATCH 4/5] Use TARGET_FILENAME_PREFIX as the system root in some cases Gary Benson
@ 2015-07-28 15:40   ` Eli Zaretskii
  0 siblings, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2015-07-28 15:40 UTC (permalink / raw)
  To: Gary Benson
  Cc: gdb-patches, palves, sandra, Paul_Koning, jan.kratochvil, brobecker

> From: Gary Benson <gbenson@redhat.com>
> Cc: Pedro Alves <palves@redhat.com>,        Sandra Loosemore <sandra@codesourcery.com>, Paul_Koning@Dell.com,        Jan Kratochvil <jan.kratochvil@redhat.com>,        Joel Brobecker <brobecker@adacore.com>
> Date: Tue, 28 Jul 2015 16:36:11 +0100
> 
> This commit updates solib_find_1 to use TARGET_FILENAME_PREFIX as the
> system root if auto-target-prefix is enabled and gdb_sysroot is empty
> and the target filesystem is not the local filesystem.
> 
> gdb/ChangeLog:
> 
> 	* solib.c (auto_target_prefix): New static variable.
> 	(solib_find_1): Use TARGET_FILENAME_PREFIX as sysroot
> 	in some cases.
> 	(show_auto_target_prefix): New function.
> 	(_initialize_solib): New "set/show auto-target-prefix"
> 	commands.
> 	* NEWS: Mention that GDB will use "target:" as the system
> 	root in some cases.  Mention new "set/show auto-target-prefix"
> 	commands.
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Commands to Specify Files): Document the
> 	"set/show auto-target-prefix" commands.

OK for the documentation parts.

Thanks.

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

* [PATCH 5/5] Update exec_file_find callers
  2015-07-28 15:36 [PATCH 0/5] Change how "target:" gets into filenames Gary Benson
                   ` (3 preceding siblings ...)
  2015-07-28 15:45 ` [PATCH 1/5] Revert default system root back to "" Gary Benson
@ 2015-07-28 15:45 ` Gary Benson
  2015-07-29 17:04 ` [PATCH 0/5] Change how "target:" gets into filenames Jan Kratochvil
  5 siblings, 0 replies; 13+ messages in thread
From: Gary Benson @ 2015-07-28 15:45 UTC (permalink / raw)
  To: gdb-patches
  Cc: Pedro Alves, Sandra Loosemore, Paul_Koning, Jan Kratochvil,
	Joel Brobecker

This commit updates the two callers of exec_file_find to make the call
regardless of what gdb_sysroot is set to.

Also, one of the callers used exec_file_find for absolute filenames
only, but the other used it in all cases.  The latter is probably
wrong, and has been updated to only use exec_file_find for absolute
filenames.

gdb/ChangeLog:

	* exec.c (exec_file_locate_attach): Use exec_file_find
	regardless of what gdb_sysroot is set to.
	* infrun.c (follow_exec): Likewise, but only if the new
	filename is absolute.
---
 gdb/ChangeLog |    7 +++++++
 gdb/exec.c    |    6 +++---
 gdb/infrun.c  |    5 ++++-
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/gdb/exec.c b/gdb/exec.c
index bbb7798..dcfa971 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -152,9 +152,9 @@ exec_file_locate_attach (int pid, int from_tty)
   if (exec_file == NULL)
     return;
 
-  /* If gdb_sysroot is not empty and the discovered filename
-     is absolute then prefix the filename with gdb_sysroot.  */
-  if (*gdb_sysroot != '\0' && IS_ABSOLUTE_PATH (exec_file))
+  /* If the discovered filename is absolute then prefix the filename
+     with the target prefix (if necessary) and gdb_sysroot.  */
+  if (IS_ABSOLUTE_PATH (exec_file))
     full_exec_path = exec_file_find (exec_file, NULL);
 
   if (full_exec_path == NULL)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 445a612..0b4f867 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -61,6 +61,7 @@
 #include "target-dcache.h"
 #include "terminal.h"
 #include "solist.h"
+#include "filenames.h"
 
 /* Prototypes for local functions */
 
@@ -1133,7 +1134,9 @@ follow_exec (ptid_t ptid, char *execd_pathname)
 
   breakpoint_init_inferior (inf_execd);
 
-  if (*gdb_sysroot != '\0')
+  /* If the discovered filename is absolute then prefix the filename
+     with the target prefix (if necessary) and gdb_sysroot.  */
+  if (IS_ABSOLUTE_PATH (execd_pathname))
     {
       char *name = exec_file_find (execd_pathname, NULL);
 
-- 
1.7.1

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

* [PATCH 1/5] Revert default system root back to ""
  2015-07-28 15:36 [PATCH 0/5] Change how "target:" gets into filenames Gary Benson
                   ` (2 preceding siblings ...)
  2015-07-28 15:36 ` [PATCH 4/5] Use TARGET_FILENAME_PREFIX as the system root in some cases Gary Benson
@ 2015-07-28 15:45 ` Gary Benson
  2015-07-28 15:58   ` Eli Zaretskii
  2015-07-28 15:45 ` [PATCH 5/5] Update exec_file_find callers Gary Benson
  2015-07-29 17:04 ` [PATCH 0/5] Change how "target:" gets into filenames Jan Kratochvil
  5 siblings, 1 reply; 13+ messages in thread
From: Gary Benson @ 2015-07-28 15:45 UTC (permalink / raw)
  To: gdb-patches
  Cc: Pedro Alves, Sandra Loosemore, Paul_Koning, Jan Kratochvil,
	Joel Brobecker

This commit reverts the default system root from "target:" to "".

gdb/ChangeLog:

	* main.c (captured_main): Revert default sysroot back to "".
	* NEWS: Remove comment about default sysroot being "target:".
---
 gdb/ChangeLog |    5 +++++
 gdb/NEWS      |    3 +--
 gdb/main.c    |    7 ++-----
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 7ce9758..7ecdf93 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -39,8 +39,7 @@
 * Directory names supplied to the "set sysroot" commands may be
   prefixed with "target:" to tell GDB to access shared libraries from
   the target system, be it local or remote.  This replaces the prefix
-  "remote:".  The default sysroot has been changed from "" to
-  "target:".  "remote:" is automatically converted to "target:" for
+  "remote:", which is automatically converted to "target:" for
   backward compatibility.
 
 * The system root specified by "set sysroot" will be prepended to the
diff --git a/gdb/main.c b/gdb/main.c
index aecd60a..2d642f9 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -545,11 +545,8 @@ captured_main (void *data)
   gdb_sysroot = relocate_gdb_directory (TARGET_SYSTEM_ROOT,
 					TARGET_SYSTEM_ROOT_RELOCATABLE);
 
-  if (gdb_sysroot == NULL || *gdb_sysroot == '\0')
-    {
-      xfree (gdb_sysroot);
-      gdb_sysroot = xstrdup (TARGET_SYSROOT_PREFIX);
-    }
+  if (gdb_sysroot == NULL)
+    gdb_sysroot = xstrdup ("");
 
   debug_file_directory = relocate_gdb_directory (DEBUGDIR,
 						 DEBUGDIR_RELOCATABLE);
-- 
1.7.1

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

* Re: [PATCH 1/5] Revert default system root back to ""
  2015-07-28 15:45 ` [PATCH 1/5] Revert default system root back to "" Gary Benson
@ 2015-07-28 15:58   ` Eli Zaretskii
  0 siblings, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2015-07-28 15:58 UTC (permalink / raw)
  To: Gary Benson
  Cc: gdb-patches, palves, sandra, Paul_Koning, jan.kratochvil, brobecker

> From: Gary Benson <gbenson@redhat.com>
> Cc: Pedro Alves <palves@redhat.com>,        Sandra Loosemore <sandra@codesourcery.com>, Paul_Koning@Dell.com,        Jan Kratochvil <jan.kratochvil@redhat.com>,        Joel Brobecker <brobecker@adacore.com>
> Date: Tue, 28 Jul 2015 16:36:08 +0100
> 
> This commit reverts the default system root from "target:" to "".
> 
> gdb/ChangeLog:
> 
> 	* main.c (captured_main): Revert default sysroot back to "".
> 	* NEWS: Remove comment about default sysroot being "target:".

OK for NEWS.

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

* Re: [PATCH 0/5] Change how "target:" gets into filenames
  2015-07-28 15:36 [PATCH 0/5] Change how "target:" gets into filenames Gary Benson
                   ` (4 preceding siblings ...)
  2015-07-28 15:45 ` [PATCH 5/5] Update exec_file_find callers Gary Benson
@ 2015-07-29 17:04 ` Jan Kratochvil
  5 siblings, 0 replies; 13+ messages in thread
From: Jan Kratochvil @ 2015-07-29 17:04 UTC (permalink / raw)
  To: Gary Benson
  Cc: gdb-patches, Pedro Alves, Sandra Loosemore, Paul_Koning, Joel Brobecker

On Tue, 28 Jul 2015 17:36:07 +0200, Gary Benson wrote:
> I've Cc'd Jan because he's working on making sysroot be a search
> path. (defaulting to something like "/", "target:/").  I think you
> could do that from this series by treating the auto-target-prefix
> boolean as a stepping-stone that could be removed.

I have looked at this series that it probably does what it describes.
Still I think the patchset of mine is going to replace some part of mostly all
of it when build-ids are in use.


Jan

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

* Re: [PATCH 4/5] Use TARGET_FILENAME_PREFIX as the system root in some cases
  2015-07-28 17:35 [PATCH 4/5] Use TARGET_FILENAME_PREFIX as the system root in some cases Doug Evans
  2015-07-28 20:02 ` Sandra Loosemore
@ 2015-08-04 16:09 ` Joel Brobecker
  1 sibling, 0 replies; 13+ messages in thread
From: Joel Brobecker @ 2015-08-04 16:09 UTC (permalink / raw)
  To: Doug Evans
  Cc: Gary Benson, gdb-patches, Pedro Alves, Sandra Loosemore,
	Paul_Koning, Jan Kratochvil

> I agree that we should get this resolved for 7.10 though.

Agreed as well.

-- 
Joel

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

* Re: [PATCH 4/5] Use TARGET_FILENAME_PREFIX as the system root in some cases
  2015-07-28 20:02 ` Sandra Loosemore
@ 2015-07-29 10:30   ` Gary Benson
  0 siblings, 0 replies; 13+ messages in thread
From: Gary Benson @ 2015-07-29 10:30 UTC (permalink / raw)
  To: Sandra Loosemore
  Cc: Doug Evans, gdb-patches, Pedro Alves, Paul_Koning,
	Jan Kratochvil, Joel Brobecker

Sandra Loosemore wrote:
> On 07/28/2015 11:35 AM, Doug Evans wrote:
> > If, after doing:
> >
> > (gdb) target remote :9999
> >
> > the user was first prompted with something like:
> > 
> > "Warning: I have no way to find files with debug info locally,
> > and auto-target-prefix is set to "on",
> > so I will try to fetch these files from the target.
> > This may take time.  If you want to avoid having me try to transfer
> > files from the target, you can do the following:
> > blah blah blah
> > Are you sure you want to continue?"
> > [suitably cleaned up, I didn't want to spend any time wordsmithing that]
> > 
> > then that may be sufficient.  What do others think?
> 
> The problem with forcing the user to answer question here is that
> it'll screw up a lot of GDB startup scripts.  Maybe stuff like
> launching GDB from Eclipse, too.

Yeah, I don't like having questions.  A warning followed by something
the user can interrupt is something the user only has to deal with the
once (by interrupting, adding something to their .gdbinit, and
restarting).  A question that the user has to answer *every time* is
something the user has to deal with every time.

> My preference would be to make GDB give a message when transferring
> files from the target, like
> 
> Reading /full/path/to/libc-2.21.so (12236020 bytes) from target.
> Use "set sysroot" to set a local pathname for this file instead.
> 
> and make the read operation interruptible so that if it's taking too
> long, the user can stop the transfer and do the "set sysroot" thing
> as suggested.

Ok, I will look into this.

Cheers,
Gary

-- 
http://gbenson.net/

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

* Re: [PATCH 4/5] Use TARGET_FILENAME_PREFIX as the system root in some cases
  2015-07-28 17:35 [PATCH 4/5] Use TARGET_FILENAME_PREFIX as the system root in some cases Doug Evans
@ 2015-07-28 20:02 ` Sandra Loosemore
  2015-07-29 10:30   ` Gary Benson
  2015-08-04 16:09 ` Joel Brobecker
  1 sibling, 1 reply; 13+ messages in thread
From: Sandra Loosemore @ 2015-07-28 20:02 UTC (permalink / raw)
  To: Doug Evans
  Cc: Gary Benson, gdb-patches, Pedro Alves, Paul_Koning,
	Jan Kratochvil, Joel Brobecker

On 07/28/2015 11:35 AM, Doug Evans wrote:
>
> One thing that comes to mind is that there's no indication/warning
> here of the potential massive responsiveness hit people may take
> if they turn this feature on, plus an explanation of what's going
> on, or how they can do things differently to avoid it.

Yes.  The delay would be more tolerable if users were given some clue 
what GDB is doing.

> If, after doing:
>
> (gdb) target remote :9999
>
> the user was first prompted with something like:
>
> "Warning: I have no way to find files with debug info locally,
> and auto-target-prefix is set to "on",
> so I will try to fetch these files from the target.
> This may take time.  If you want to avoid having me try to transfer
> files from the target, you can do the following:
> blah blah blah
> Are you sure you want to continue?"
> [suitably cleaned up, I didn't want to spend any time wordsmithing that]
>
> then that may be sufficient.  What do others think?

The problem with forcing the user to answer question here is that it'll 
screw up a lot of GDB startup scripts.  Maybe stuff like launching GDB 
from Eclipse, too.

My preference would be to make GDB give a message when transferring 
files from the target, like

Reading /full/path/to/libc-2.21.so (12236020 bytes) from target.
Use "set sysroot" to set a local pathname for this file instead.

and make the read operation interruptible so that if it's taking too 
long, the user can stop the transfer and do the "set sysroot" thing as 
suggested.

-Sandra

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

* Re: [PATCH 4/5] Use TARGET_FILENAME_PREFIX as the system root in some cases
@ 2015-07-28 17:35 Doug Evans
  2015-07-28 20:02 ` Sandra Loosemore
  2015-08-04 16:09 ` Joel Brobecker
  0 siblings, 2 replies; 13+ messages in thread
From: Doug Evans @ 2015-07-28 17:35 UTC (permalink / raw)
  To: Gary Benson
  Cc: gdb-patches, Pedro Alves, Sandra Loosemore, Paul_Koning,
	Jan Kratochvil, Joel Brobecker

Gary Benson writes:
  > This commit updates solib_find_1 to use TARGET_FILENAME_PREFIX as the
  > system root if auto-target-prefix is enabled and gdb_sysroot is empty
  > and the target filesystem is not the local filesystem.
  >
  > gdb/ChangeLog:
  >
  > 	* solib.c (auto_target_prefix): New static variable.
  > 	(solib_find_1): Use TARGET_FILENAME_PREFIX as sysroot
  > 	in some cases.
  > 	(show_auto_target_prefix): New function.
  > 	(_initialize_solib): New "set/show auto-target-prefix"
  > 	commands.
  > 	* NEWS: Mention that GDB will use "target:" as the system
  > 	root in some cases.  Mention new "set/show auto-target-prefix"
  > 	commands.
  >
  > gdb/doc/ChangeLog:
  >
  > 	* gdb.texinfo (Commands to Specify Files): Document the
  > 	"set/show auto-target-prefix" commands.

Hi.

Still not sure whether the subtlety between these two will trip people up.

$ gdb
(gdb) file a.out
(gdb) target remote :9999
# no "target:" prefix (=no files transferred)

$ gdb
(gdb) target remote :9999
# "target:" prefix, files transferred

One thing that comes to mind is that there's no indication/warning
here of the potential massive responsiveness hit people may take
if they turn this feature on, plus an explanation of what's going
on, or how they can do things differently to avoid it.

If, after doing:

(gdb) target remote :9999

the user was first prompted with something like:

"Warning: I have no way to find files with debug info locally,
and auto-target-prefix is set to "on",
so I will try to fetch these files from the target.
This may take time.  If you want to avoid having me try to transfer
files from the target, you can do the following:
blah blah blah
Are you sure you want to continue?"
[suitably cleaned up, I didn't want to spend any time wordsmithing that]

then that may be sufficient.  What do others think?

I agree that we should get this resolved for 7.10 though.

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

end of thread, other threads:[~2015-08-04 16:09 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-28 15:36 [PATCH 0/5] Change how "target:" gets into filenames Gary Benson
2015-07-28 15:36 ` [PATCH 3/5] Rename TARGET_SYSROOT_PREFIX as TARGET_FILENAME_PREFIX Gary Benson
2015-07-28 15:36 ` [PATCH 2/5] Update Valgrind GDB special case Gary Benson
2015-07-28 15:36 ` [PATCH 4/5] Use TARGET_FILENAME_PREFIX as the system root in some cases Gary Benson
2015-07-28 15:40   ` Eli Zaretskii
2015-07-28 15:45 ` [PATCH 1/5] Revert default system root back to "" Gary Benson
2015-07-28 15:58   ` Eli Zaretskii
2015-07-28 15:45 ` [PATCH 5/5] Update exec_file_find callers Gary Benson
2015-07-29 17:04 ` [PATCH 0/5] Change how "target:" gets into filenames Jan Kratochvil
2015-07-28 17:35 [PATCH 4/5] Use TARGET_FILENAME_PREFIX as the system root in some cases Doug Evans
2015-07-28 20:02 ` Sandra Loosemore
2015-07-29 10:30   ` Gary Benson
2015-08-04 16:09 ` Joel Brobecker

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).