[4/5] Add "target:" filename handling to find_separate_debug_file
Commit Message
This commit updates find_separate_debug_file to handle filenames
prefixed with "target:". The same-directory and DEBUG_SUBDIRECTORY
locations are checked with the prefix if supplied. The debugdir
location is checked both with and without the prefix if one is
supplied. This makes GDB able to fetch separate debug files from
remote targets and from inferiors in containers.
gdb/ChangeLog:
* gdb/symfile.c (build_debug_file_name): New argument "prefix".
(find_separate_debug_file): Separate TARGET_SYSROOT_PREFIX from
dir. In debugdir loop, also try location prepended with dir's
prefix if one was supplied.
---
gdb/ChangeLog | 7 +++++++
gdb/symfile.c | 47 +++++++++++++++++++++++++++++++++++++----------
2 files changed, 44 insertions(+), 10 deletions(-)
Comments
On 06/16/2015 10:42 AM, Gary Benson wrote:
> This commit updates find_separate_debug_file to handle filenames
> prefixed with "target:". The same-directory and DEBUG_SUBDIRECTORY
> locations are checked with the prefix if supplied. The debugdir
> location is checked both with and without the prefix if one is
> supplied. This makes GDB able to fetch separate debug files from
> remote targets and from inferiors in containers.
> + /* Try the same location but without TARGET_SYSROOT_PREFIX
> + (i.e. on the local filesystem). */
> + debugfile = build_debug_file_name (no_prefix, debugdir, dir,
> + debuglink, NULL);
Given that we have a CRC to match, shouldn't we try the local
filesystem first, avoiding the (potentially slow) remote fetching
in the case the files on the container/remote are the same of
the host's? (which I think happens often with containers).
And I guess we could skip the "target:" attempt if
target_filesystem_is_local() ?
Fetching (big) debug info files from slow remote targets will
I think lead to the desire for file chunk caching in gdb. But
I think slow debug info beats no debug info.
(Speaking of caching, AFAICS, gdb_bfd_crc/get_file_crc always read in
the whole file and do the crc check locally. It seems really silly
to read in the _whole_ file out of the target in get_file_crc, but
not cache the file's contents for subsequent same-file accesses...
And we could try pushing the CRC calculation to the target side as
well.
But guess with build id validation we'll just not care about this whole
CRC path anymore.)
Thanks,
Pedro Alves
Pedro Alves wrote:
> On 06/16/2015 10:42 AM, Gary Benson wrote:
> > This commit updates find_separate_debug_file to handle filenames
> > prefixed with "target:". The same-directory and
> > DEBUG_SUBDIRECTORY locations are checked with the prefix if
> > supplied. The debugdir location is checked both with and without
> > the prefix if one is supplied. This makes GDB able to fetch
> > separate debug files from remote targets and from inferiors in
> > containers.
> >
> > + /* Try the same location but without TARGET_SYSROOT_PREFIX
> > + (i.e. on the local filesystem). */
> > + debugfile = build_debug_file_name (no_prefix, debugdir, dir,
> > + debuglink, NULL);
>
> Given that we have a CRC to match, shouldn't we try the local
> filesystem first, avoiding the (potentially slow) remote fetching
> in the case the files on the container/remote are the same of
> the host's? (which I think happens often with containers).
I think for both remote and container cases the answer to "are they
the same" is no more than "they might be".
With containers, "target:" fetching shouldn't be very much slower
than local-filesystem fetching, just a couple more operations at
file open time. With remote it will be slower, but less likely
to be the same (maybe!)
That was my reasoning. I can switch it round if you want though.
Let me know what you'd like...
Cheers,
Gary
Gary Benson wrote:
> Pedro Alves wrote:
> > On 06/16/2015 10:42 AM, Gary Benson wrote:
> > > This commit updates find_separate_debug_file to handle filenames
> > > prefixed with "target:". The same-directory and
> > > DEBUG_SUBDIRECTORY locations are checked with the prefix if
> > > supplied. The debugdir location is checked both with and without
> > > the prefix if one is supplied. This makes GDB able to fetch
> > > separate debug files from remote targets and from inferiors in
> > > containers.
> > >
> > > + /* Try the same location but without TARGET_SYSROOT_PREFIX
> > > + (i.e. on the local filesystem). */
> > > + debugfile = build_debug_file_name (no_prefix, debugdir, dir,
> > > + debuglink, NULL);
> >
> > Given that we have a CRC to match, shouldn't we try the local
> > filesystem first, avoiding the (potentially slow) remote fetching
> > in the case the files on the container/remote are the same of the
> > host's? (which I think happens often with containers).
>
> I think for both remote and container cases the answer to "are they
> the same" is no more than "they might be".
>
> With containers, "target:" fetching shouldn't be very much slower
> than local-filesystem fetching, just a couple more operations at
> file open time. With remote it will be slower, but less likely
> to be the same (maybe!)
>
> That was my reasoning. I can switch it round if you want though.
> Let me know what you'd like...
Given the slow remote transfers thread on the other list [1],
I will switch it round (i.e. try locally first).
Thanks,
Gary
--
[1] https://sourceware.org/ml/gdb/2015-07/msg00038.html
Gary Benson wrote:
> Gary Benson wrote:
> > Pedro Alves wrote:
> > > On 06/16/2015 10:42 AM, Gary Benson wrote:
> > > > This commit updates find_separate_debug_file to handle
> > > > filenames prefixed with "target:". The same-directory and
> > > > DEBUG_SUBDIRECTORY locations are checked with the prefix if
> > > > supplied. The debugdir location is checked both with and
> > > > without the prefix if one is supplied. This makes GDB able
> > > > to fetch separate debug files from remote targets and from
> > > > inferiors in containers.
> > > >
> > > > + /* Try the same location but without TARGET_SYSROOT_PREFIX
> > > > + (i.e. on the local filesystem). */
> > > > + debugfile = build_debug_file_name (no_prefix, debugdir, dir,
> > > > + debuglink, NULL);
> > >
> > > Given that we have a CRC to match, shouldn't we try the local
> > > filesystem first, avoiding the (potentially slow) remote
> > > fetching in the case the files on the container/remote are
> > > the same of the host's? (which I think happens often with
> > > containers).
> >
> > I think for both remote and container cases the answer to "are
> > they the same" is no more than "they might be".
> >
> > With containers, "target:" fetching shouldn't be very much slower
> > than local-filesystem fetching, just a couple more operations at
> > file open time. With remote it will be slower, but less likely
> > to be the same (maybe!)
> >
> > That was my reasoning. I can switch it round if you want though.
> > Let me know what you'd like...
>
> Given the slow remote transfers thread on the other list [1],
> I will switch it round (i.e. try locally first).
> --
> [1] https://sourceware.org/ml/gdb/2015-07/msg00038.html
Actually, I now discover that the reason I did it "target:" first
was that doing it the other way around clutters up the display
with loads of warnings if the two systems are different:
Reading symbols from target:/usr/lib64/libpython2.6.so.1.0...
warning: the debug information found in "/usr/lib/debug/usr/lib64/libpython2.6.so.1.0.debug" does not match "target:/usr/lib64/libpython2.6.so.1.0" (CRC mismatch).
Reading symbols from target:/usr/lib/debug/usr/lib64/libpython2.6.so.1.0.debug...done.
done.
Reading symbols from target:/lib64/libpthread.so.0...
warning: the debug information found in "/usr/lib/debug/lib64/libpthread-2.12.so.debug" does not match "target:/lib64/libpthread.so.0" (CRC mismatch).
Reading symbols from target:/usr/lib/debug/lib64/libpthread-2.12.so.debug...done.
done.
...etc...
If you're remote debugging over a slow link and have set gdb_sysroot
to some local copy then there will be no "target:" paths here so there
will be no slowdown with a target-first search.
If you're remote debugging with local debuginfo there will be some
slowdown due to failing vFile:open: packets. One round trip per
executable/library to be precise. Is this acceptable to you?
Cheers,
Gary
On Tue, 16 Jun 2015 11:42:47 +0200, Gary Benson wrote:
> This commit updates find_separate_debug_file to handle filenames
> prefixed with "target:". The same-directory and DEBUG_SUBDIRECTORY
> locations are checked with the prefix if supplied. The debugdir
> location is checked both with and without the prefix if one is
> supplied. This makes GDB able to fetch separate debug files from
> remote targets and from inferiors in containers.
I do not have practical experience with containers but from what I know their
target is to make the container content OS-independent. That is I can run on
CentOS host a container with Ubuntu etc. Then this shortcut will not work.
I can provide Ubuntu debug info files in some /root/os/ubuntuxyz .
Similarly for Fedora 21 running on CentOS I can provide debug info files
easily with mock in /var/lib/mock/fedora-21-x86_64/root/ .
So one should be able to specify also non-/ directory.
Besides that one should be able to specify multiple directories as for example
there exist site configurations where each machine has /mnt/nfsdir mounted
directory with debug info files for all operating systems in use etc.
Unfortunately on UNIX GDB separates multiple directories by ':' which
conflicts with the "target:" prefix. But maybe one could make an exception
that "target" component would not be parsed as separate directory (which
should not clash as nobody is going to use directory name without leading
slash).
Jan
@@ -1434,16 +1434,17 @@ separate_debug_file_exists (const char *name, unsigned long crc,
/* Build the filename of a separate debug file from an arbitrary
number of components. Returns an xmalloc'd string that must
be be freed by the caller. The final argument of this function
- must be NULL to mark the end the argument list. */
+ must be NULL to mark the end the argument list. PREFIX will
+ be prepended to the result with no directory separator. */
static char *
-build_debug_file_name (const char *first, ...)
+build_debug_file_name (const char *prefix, const char *first, ...)
{
va_list ap;
const char *arg, *last;
VEC (char_ptr) *args = NULL;
struct cleanup *back_to = make_cleanup_free_char_ptr_vec (args);
- int bufsiz = 0;
+ int bufsiz = strlen (prefix);
char *buf, *tmp;
int i;
@@ -1495,7 +1496,7 @@ build_debug_file_name (const char *first, ...)
bufsiz += 1; /* Terminator. */
buf = xmalloc (bufsiz);
- buf[0] = '\0';
+ strcpy (buf, prefix);
for (i = 0; VEC_iterate (char_ptr, args, i, tmp); i++)
strcat (buf, tmp);
gdb_assert (bufsiz == strlen (buf) + 1);
@@ -1537,15 +1538,25 @@ find_separate_debug_file (const char *dir,
struct cleanup *back_to;
int ix;
const char *altdir = NULL;
+ const char *no_prefix = "";
+ const char *dir_prefix = no_prefix;
+
+ /* Separate TARGET_SYSROOT_PREFIX from directory. */
+ if (is_target_filename (dir))
+ {
+ dir_prefix = TARGET_SYSROOT_PREFIX;
+ dir += strlen (dir_prefix);
+ }
/* First try in the same directory as the original file. */
- debugfile = build_debug_file_name (dir, debuglink, NULL);
+ debugfile = build_debug_file_name (dir_prefix, dir, debuglink, NULL);
if (separate_debug_file_exists (debugfile, crc32, objfile))
return debugfile;
xfree (debugfile);
/* Then try in the subdirectory named DEBUG_SUBDIRECTORY. */
- debugfile = build_debug_file_name (dir, DEBUG_SUBDIRECTORY,
+ debugfile = build_debug_file_name (dir_prefix, dir,
+ DEBUG_SUBDIRECTORY,
debuglink, NULL);
if (separate_debug_file_exists (debugfile, crc32, objfile))
return debugfile;
@@ -1575,8 +1586,24 @@ find_separate_debug_file (const char *dir,
for (ix = 0; VEC_iterate (char_ptr, debugdir_vec, ix, debugdir); ++ix)
{
- debugfile = build_debug_file_name (debugdir, dir, debuglink,
- NULL);
+ /* First try with TARGET_SYSROOT_PREFIX if that's how DIR was
+ supplied. */
+ if (dir_prefix != no_prefix)
+ {
+ debugfile = build_debug_file_name (dir_prefix, debugdir, dir,
+ debuglink, NULL);
+ if (separate_debug_file_exists (debugfile, crc32, objfile))
+ {
+ do_cleanups (back_to);
+ return debugfile;
+ }
+ xfree (debugfile);
+ }
+
+ /* Try the same location but without TARGET_SYSROOT_PREFIX
+ (i.e. on the local filesystem). */
+ debugfile = build_debug_file_name (no_prefix, debugdir, dir,
+ debuglink, NULL);
if (separate_debug_file_exists (debugfile, crc32, objfile))
{
do_cleanups (back_to);
@@ -1586,8 +1613,8 @@ find_separate_debug_file (const char *dir,
if (altdir != NULL)
{
- debugfile = build_debug_file_name (debugdir, altdir,
- debuglink, NULL);
+ debugfile = build_debug_file_name (no_prefix, debugdir,
+ altdir, debuglink, NULL);
if (separate_debug_file_exists (debugfile, crc32, objfile))
{
do_cleanups (back_to);