public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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 ` Gary Benson
  2015-07-28 15:40   ` Eli Zaretskii
  0 siblings, 1 reply; 6+ 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] 6+ messages in thread

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

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
  -- strict thread matches above, loose matches on Subject: below --
2015-07-28 15:36 [PATCH 0/5] Change how "target:" gets into filenames 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

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