[PATCH 0/5] Separate debugfile improvements

[PATCH 0/5] Separate debugfile improvements

[Gary Benson]

Hi all,

This series makes a number of improvements to the code that locates
separated debuginfo.  This series (patch 4 specifically) goes really
nicely with the remote/container improvements I made this past few
months, and it'd be really nice to get this into 7.10.

- Patch 1 pulls filename building (including buffer allocation) out of
  find_separate_debug_file into a new function, build_debug_file_name.
  This simplifies find_separate_debug_file ready for the other
  patches.  The new code avoids generating paths with repeated
  separators, but operation is otherwise unchanged.

- Patch 2 alters find_separate_debug_file's caller to supply all paths
  without trailing separators, which patch 1 of this series made
  unnecessary.  This allows paths to be compared more simply.

- Patch 3 changes how find_separate_debug_file's CANON_DIR argument is
  handled.  The new code avoids looking in CANON_DIR if it is the same
  as DIR, but operation is otherwise unchanged.

- Patch 4 updates find_separate_debug_file to handle filenames
  prefixed with "target:".  This allows GDB to locate and access
  separated debuginfo from remote targets and from inferiors in
  containers.

- Patch 5 causes GDB to look for debug files in gdb_sysroot in
  addition to the other searched locations.  This allows for easier
  analysis of core files from foreign machines.

Built and regtested on RHEL 6.6 x86_64.

Ok to commit?

Thanks,
Gary

--
http://gbenson.net/

[PATCH 1/5] Introduce build_debug_file_name

[Gary Benson]

This commit introduces a new function build_debug_file_name which
concatenates a series of filename components into a filename.
find_separate_debug_file is updated to use build_debug_file_name.
A later commit in this series will extend build_debug_file_name
to correctly handle "target:" prefixes, so it is convenient to
have filename building pulled out into one function.  For now the
only functional change here is that the original code sometimes
generated filenames with repeated directory separators while the
new code does not.

gdb/ChangeLog:

	* gdb/symfile.c (build_debug_file_name): New function.
	(find_separate_debug_file): Use the above to build filenames.
---
 gdb/ChangeLog |    5 ++
 gdb/symfile.c |  117 +++++++++++++++++++++++++++++++++++++++++---------------
 2 files changed, 90 insertions(+), 32 deletions(-)

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 0c35ffa..799133a 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1431,6 +1431,79 @@ separate_debug_file_exists (const char *name, unsigned long crc,
   return 1;
 }
 
+/* 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.  */
+
+static char *
+build_debug_file_name (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;
+  char *buf, *tmp;
+  int i;
+
+  va_start (ap, first);
+  for (arg = first; arg; arg = va_arg (ap, const char *))
+    last = arg;
+  va_end (ap);
+
+  va_start (ap, first);
+  for (arg = first; arg; arg = va_arg (ap, const char *))
+    {
+      if (arg == last)
+	tmp = xstrdup (arg);
+      else
+	{
+	  int len;
+
+	  /* Strip leading separators from subdirectories.  */
+	  if (arg != first)
+	    {
+	      while (*arg != '\0' && IS_DIR_SEPARATOR (*arg))
+		arg++;
+	    }
+
+	  /* Strip trailing separators.  */
+	  len = strlen (arg);
+
+	  while (len > 0 && IS_DIR_SEPARATOR (arg[len - 1]))
+	    len--;
+
+	  if (len > 0)
+	    {
+	      tmp = xmalloc (len + strlen (SLASH_STRING) + 1);
+	      memcpy (tmp, arg, len);
+	      strcpy (tmp + len, SLASH_STRING);
+	    }
+	  else
+	    tmp = NULL;
+	}
+
+      if (tmp != NULL)
+	{
+	  VEC_safe_push (char_ptr, args, tmp);
+	  bufsiz += strlen (tmp);
+	}
+    }
+  va_end (ap);
+
+  bufsiz += 1;  /* Terminator.  */
+
+  buf = xmalloc (bufsiz);
+  buf[0] = '\0';
+  for (i = 0; VEC_iterate (char_ptr, args, i, tmp); i++)
+    strcat (buf, tmp);
+  gdb_assert (bufsiz == strlen (buf) + 1);
+
+  do_cleanups (back_to);
+  return buf;
+}
+
 char *debug_file_directory = NULL;
 static void
 show_debug_file_directory (struct ui_file *file, int from_tty,
@@ -1461,38 +1534,22 @@ find_separate_debug_file (const char *dir,
 {
   char *debugdir;
   char *debugfile;
-  int i;
   VEC (char_ptr) *debugdir_vec;
   struct cleanup *back_to;
   int ix;
 
-  /* Set I to max (strlen (canon_dir), strlen (dir)).  */
-  i = strlen (dir);
-  if (canon_dir != NULL && strlen (canon_dir) > i)
-    i = strlen (canon_dir);
-
-  debugfile = xmalloc (strlen (debug_file_directory) + 1
-		       + i
-		       + strlen (DEBUG_SUBDIRECTORY)
-		       + strlen ("/")
-		       + strlen (debuglink)
-		       + 1);
-
   /* First try in the same directory as the original file.  */
-  strcpy (debugfile, dir);
-  strcat (debugfile, debuglink);
-
+  debugfile = build_debug_file_name (dir, debuglink, NULL);
   if (separate_debug_file_exists (debugfile, crc32, objfile))
     return debugfile;
+  xfree (debugfile);
 
   /* Then try in the subdirectory named DEBUG_SUBDIRECTORY.  */
-  strcpy (debugfile, dir);
-  strcat (debugfile, DEBUG_SUBDIRECTORY);
-  strcat (debugfile, "/");
-  strcat (debugfile, debuglink);
-
+  debugfile = build_debug_file_name (dir, DEBUG_SUBDIRECTORY,
+				     debuglink, NULL);
   if (separate_debug_file_exists (debugfile, crc32, objfile))
     return debugfile;
+  xfree (debugfile);
 
   /* Then try in the global debugfile directories.
 
@@ -1504,16 +1561,14 @@ find_separate_debug_file (const char *dir,
 
   for (ix = 0; VEC_iterate (char_ptr, debugdir_vec, ix, debugdir); ++ix)
     {
-      strcpy (debugfile, debugdir);
-      strcat (debugfile, "/");
-      strcat (debugfile, dir);
-      strcat (debugfile, debuglink);
-
+      debugfile = build_debug_file_name (debugdir, dir, debuglink,
+					 NULL);
       if (separate_debug_file_exists (debugfile, crc32, objfile))
 	{
 	  do_cleanups (back_to);
 	  return debugfile;
 	}
+      xfree (debugfile);
 
       /* If the file is in the sysroot, try using its base path in the
 	 global debugfile directory.  */
@@ -1522,21 +1577,19 @@ find_separate_debug_file (const char *dir,
 			    strlen (gdb_sysroot)) == 0
 	  && IS_DIR_SEPARATOR (canon_dir[strlen (gdb_sysroot)]))
 	{
-	  strcpy (debugfile, debugdir);
-	  strcat (debugfile, canon_dir + strlen (gdb_sysroot));
-	  strcat (debugfile, "/");
-	  strcat (debugfile, debuglink);
-
+	  debugfile = build_debug_file_name (debugdir, canon_dir
+					     + strlen (gdb_sysroot),
+					     debuglink, NULL);
 	  if (separate_debug_file_exists (debugfile, crc32, objfile))
 	    {
 	      do_cleanups (back_to);
 	      return debugfile;
 	    }
+	  xfree (debugfile);
 	}
     }
 
   do_cleanups (back_to);
-  xfree (debugfile);
   return NULL;
 }
 
-- 
1.7.1

Re: [PATCH 1/5] Introduce build_debug_file_name

[Eli Zaretskii]

> From: Gary Benson <gbenson@redhat.com>
> Cc: Cédric Buissart <cedric.buissart@gmail.com>
> Date: Tue, 16 Jun 2015 10:42:44 +0100
> 
> +/* 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.  */
> +
> +static char *
> +build_debug_file_name (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;
> +  char *buf, *tmp;
> +  int i;
> +
> +  va_start (ap, first);
> +  for (arg = first; arg; arg = va_arg (ap, const char *))
> +    last = arg;
> +  va_end (ap);
> +
> +  va_start (ap, first);
> +  for (arg = first; arg; arg = va_arg (ap, const char *))
> +    {
> +      if (arg == last)
> +	tmp = xstrdup (arg);
> +      else
> +	{
> +	  int len;
> +
> +	  /* Strip leading separators from subdirectories.  */
> +	  if (arg != first)
> +	    {
> +	      while (*arg != '\0' && IS_DIR_SEPARATOR (*arg))
> +		arg++;
> +	    }
> +
> +	  /* Strip trailing separators.  */
> +	  len = strlen (arg);
> +
> +	  while (len > 0 && IS_DIR_SEPARATOR (arg[len - 1]))
> +	    len--;

Was this logic tested with Windows-style "d:/foo" file names?  E.g.,
what will happen if the first component is "d:/"?

Thanks.

Re: [PATCH 1/5] Introduce build_debug_file_name

[Gary Benson]

Eli Zaretskii wrote:
> > From: Gary Benson <gbenson@redhat.com>
> > Cc: Cédric Buissart <cedric.buissart@gmail.com>
> > Date: Tue, 16 Jun 2015 10:42:44 +0100
> > 
> > +/* 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.  */
> > +
> > +static char *
> > +build_debug_file_name (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;
> > +  char *buf, *tmp;
> > +  int i;
> > +
> > +  va_start (ap, first);
> > +  for (arg = first; arg; arg = va_arg (ap, const char *))
> > +    last = arg;
> > +  va_end (ap);
> > +
> > +  va_start (ap, first);
> > +  for (arg = first; arg; arg = va_arg (ap, const char *))
> > +    {
> > +      if (arg == last)
> > +	tmp = xstrdup (arg);
> > +      else
> > +	{
> > +	  int len;
> > +
> > +	  /* Strip leading separators from subdirectories.  */
> > +	  if (arg != first)
> > +	    {
> > +	      while (*arg != '\0' && IS_DIR_SEPARATOR (*arg))
> > +		arg++;
> > +	    }
> > +
> > +	  /* Strip trailing separators.  */
> > +	  len = strlen (arg);
> > +
> > +	  while (len > 0 && IS_DIR_SEPARATOR (arg[len - 1]))
> > +	    len--;
> 
> Was this logic tested with Windows-style "d:/foo" file names?  E.g.,
> what will happen if the first component is "d:/"?

I don't have access to any Windows machine so I haven't been able to
test this, but I don't think the new code would regress compared to
what it replaces (which doesn't seem to have been written with Windows
in mind at all, e.g. it uses "/" rather than SLASH_STRING and has no
particular handling for drive letters).

For the case you mention nothing would be stripped (the "d" in that
path is !IS_DIR_SEPARATOR) so the filename components would be
concatenated verbatim, just as with the original code.  The resulting
filename may not make sense, but it's not a regression.

I don't believe this series should be blocked unless it breaks
something that actually worked before.

Thanks,
Gary

-- 
http://gbenson.net/

Re: [PATCH 1/5] Introduce build_debug_file_name

[Eli Zaretskii]

> Date: Wed, 17 Jun 2015 10:47:34 +0100
> From: Gary Benson <gbenson@redhat.com>
> Cc: gdb-patches@sourceware.org, cedric.buissart@gmail.com
> 
> For the case you mention nothing would be stripped (the "d" in that
> path is !IS_DIR_SEPARATOR) so the filename components would be
> concatenated verbatim, just as with the original code.  The resulting
> filename may not make sense, but it's not a regression.

But won't we produce "d://foo/bar" as result?

> I don't believe this series should be blocked unless it breaks
> something that actually worked before.

If a fix is very simple, why not make it?

Re: [PATCH 1/5] Introduce build_debug_file_name

[Gary Benson]

Eli Zaretskii wrote:
> > Date: Wed, 17 Jun 2015 10:47:34 +0100
> > From: Gary Benson <gbenson@redhat.com>
> > Cc: gdb-patches@sourceware.org, cedric.buissart@gmail.com
> > 
> > For the case you mention nothing would be stripped (the "d" in
> > that path is !IS_DIR_SEPARATOR) so the filename components would
> > be concatenated verbatim, just as with the original code.  The
> > resulting filename may not make sense, but it's not a regression.
> 
> But won't we produce "d://foo/bar" as result?

No.  build_debug_file_name strips both leading and trailing slashes.
The only ways to get a double slash from build_debug_file_name is to
pass one in in the middle of a string.

  build_debug_file_name ("d:", "foo", "bar")          -> "d:/foo/bar"
  build_debug_file_name ("d:/", "/foo", "bar")        -> "d:/foo/bar"
  build_debug_file_name ("d:///", "///foo///", "bar") -> "d:/foo/bar"

but

  build_debug_file_name("d://foo", "bar") -> "d://foo/bar"
  
> > I don't believe this series should be blocked unless it breaks
> > something that actually worked before.
> 
> If a fix is very simple, why not make it?

I don't understand, what are you asking me to fix?

Thanks,
Gary

-- 
http://gbenson.net/

Re: [PATCH 1/5] Introduce build_debug_file_name

[Eli Zaretskii]

> Date: Thu, 18 Jun 2015 11:55:28 +0100
> From: Gary Benson <gbenson@redhat.com>
> Cc: gdb-patches@sourceware.org, cedric.buissart@gmail.com
> 
> > But won't we produce "d://foo/bar" as result?
> 
> No.  build_debug_file_name strips both leading and trailing slashes.
> The only ways to get a double slash from build_debug_file_name is to
> pass one in in the middle of a string.
> 
>   build_debug_file_name ("d:", "foo", "bar")          -> "d:/foo/bar"
>   build_debug_file_name ("d:/", "/foo", "bar")        -> "d:/foo/bar"
>   build_debug_file_name ("d:///", "///foo///", "bar") -> "d:/foo/bar"
> 
> but
> 
>   build_debug_file_name("d://foo", "bar") -> "d://foo/bar"
>   
> > > I don't believe this series should be blocked unless it breaks
> > > something that actually worked before.
> > 
> > If a fix is very simple, why not make it?
> 
> I don't understand, what are you asking me to fix?

Nothing, given the above.

Thanks.

Re: [PATCH 1/5] Introduce build_debug_file_name

[Gary Benson]

Eli Zaretskii wrote:
> > Date: Thu, 18 Jun 2015 11:55:28 +0100
> > From: Gary Benson <gbenson@redhat.com>
> > Cc: gdb-patches@sourceware.org, cedric.buissart@gmail.com
> > 
> > > But won't we produce "d://foo/bar" as result?
> > 
> > No.  build_debug_file_name strips both leading and trailing slashes.
> > The only ways to get a double slash from build_debug_file_name is to
> > pass one in in the middle of a string.
> > 
> >   build_debug_file_name ("d:", "foo", "bar")          -> "d:/foo/bar"
> >   build_debug_file_name ("d:/", "/foo", "bar")        -> "d:/foo/bar"
> >   build_debug_file_name ("d:///", "///foo///", "bar") -> "d:/foo/bar"
> > 
> > but
> > 
> >   build_debug_file_name("d://foo", "bar") -> "d://foo/bar"
> >   
> > > > I don't believe this series should be blocked unless it breaks
> > > > something that actually worked before.
> > > 
> > > If a fix is very simple, why not make it?
> > 
> > I don't understand, what are you asking me to fix?
> 
> Nothing, given the above.
> 
> Thanks.

Cool, thanks Eli.

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH 1/5] Introduce build_debug_file_name

[Pedro Alves]

On 06/16/2015 10:42 AM, Gary Benson wrote:
> This commit introduces a new function build_debug_file_name which
> concatenates a series of filename components into a filename.
> find_separate_debug_file is updated to use build_debug_file_name.
> A later commit in this series will extend build_debug_file_name
> to correctly handle "target:" prefixes, so it is convenient to
> have filename building pulled out into one function.  For now the
> only functional change here is that the original code sometimes
> generated filenames with repeated directory separators while the
> new code does not.

I'd drop the "debug" from the function's name.  Sounds like a
candidate for reuse elsewhere to me.

> 
> gdb/ChangeLog:
> 
> 	* gdb/symfile.c (build_debug_file_name): New function.
> 	(find_separate_debug_file): Use the above to build filenames.
> ---
>  gdb/ChangeLog |    5 ++
>  gdb/symfile.c |  117 +++++++++++++++++++++++++++++++++++++++++---------------
>  2 files changed, 90 insertions(+), 32 deletions(-)
> 
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index 0c35ffa..799133a 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -1431,6 +1431,79 @@ separate_debug_file_exists (const char *name, unsigned long crc,
>    return 1;
>  }
>  
> +/* 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.  */

double "be be".

> +
> +static char *
> +build_debug_file_name (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;
> +  char *buf, *tmp;
> +  int i;
> +
> +  va_start (ap, first);
> +  for (arg = first; arg; arg = va_arg (ap, const char *))

arg != NULL in predicate.

> +    last = arg;
> +  va_end (ap);
> +
> +  va_start (ap, first);
> +  for (arg = first; arg; arg = va_arg (ap, const char *))

likewise.

> +    {
> +      if (arg == last)
> +	tmp = xstrdup (arg);
> +      else
> +	{
> +	  int len;
> +
> +	  /* Strip leading separators from subdirectories.  */
> +	  if (arg != first)
> +	    {
> +	      while (*arg != '\0' && IS_DIR_SEPARATOR (*arg))
> +		arg++;
> +	    }
> +
> +	  /* Strip trailing separators.  */
> +	  len = strlen (arg);
> +
> +	  while (len > 0 && IS_DIR_SEPARATOR (arg[len - 1]))
> +	    len--;
> +
> +	  if (len > 0)
> +	    {
> +	      tmp = xmalloc (len + strlen (SLASH_STRING) + 1);
> +	      memcpy (tmp, arg, len);
> +	      strcpy (tmp + len, SLASH_STRING);
> +	    }
> +	  else
> +	    tmp = NULL;
> +	}
> +
> +      if (tmp != NULL)
> +	{
> +	  VEC_safe_push (char_ptr, args, tmp);
> +	  bufsiz += strlen (tmp);

Why build the temporary VEC instead of just incrementally
building the final buf?

I think you could simplify this much if you did that,
and plus use reconcat.

> +	}
> +    }
> +  va_end (ap);
> +
> +  bufsiz += 1;  /* Terminator.  */
> +
> +  buf = xmalloc (bufsiz);
> +  buf[0] = '\0';
> +  for (i = 0; VEC_iterate (char_ptr, args, i, tmp); i++)
> +    strcat (buf, tmp);
> +  gdb_assert (bufsiz == strlen (buf) + 1);


Thanks,
Pedro Alves

Re: [PATCH 1/5] Introduce build_debug_file_name

[Gary Benson]

Pedro Alves wrote:
> On 06/16/2015 10:42 AM, Gary Benson wrote:
> > This commit introduces a new function build_debug_file_name which
> > concatenates a series of filename components into a filename.
> > find_separate_debug_file is updated to use build_debug_file_name.
> > A later commit in this series will extend build_debug_file_name to
> > correctly handle "target:" prefixes, so it is convenient to have
> > filename building pulled out into one function.  For now the only
> > functional change here is that the original code sometimes
> > generated filenames with repeated directory separators while the
> > new code does not.
> 
> I'd drop the "debug" from the function's name.  Sounds like a
> candidate for reuse elsewhere to me.

Should I put it somewhere else, maybe common-utils.c?

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH 1/5] Introduce build_debug_file_name

[Pedro Alves]

On 07/02/2015 12:18 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 06/16/2015 10:42 AM, Gary Benson wrote:
>>> This commit introduces a new function build_debug_file_name which
>>> concatenates a series of filename components into a filename.
>>> find_separate_debug_file is updated to use build_debug_file_name.
>>> A later commit in this series will extend build_debug_file_name to
>>> correctly handle "target:" prefixes, so it is convenient to have
>>> filename building pulled out into one function.  For now the only
>>> functional change here is that the original code sometimes
>>> generated filenames with repeated directory separators while the
>>> new code does not.
>>
>> I'd drop the "debug" from the function's name.  Sounds like a
>> candidate for reuse elsewhere to me.
> 
> Should I put it somewhere else, maybe common-utils.c?

I dislike common-utils.c for "kitchen-sink" reasons, though.

There's common/filestuff.c, but that looks more for low/OS level
file things.

There's substitute_path_component (and gdb_realpath / gdb_abspath)
in utils.c.  Maybe put it next to substitute_path_component, which
seems to be in the same "family" of function, and then (at some point)
we would move all file name/path manipulation routines to its own file.

WDYT?

Thanks,
Pedro Alves

Re: [PATCH 1/5] Introduce build_debug_file_name

[Gary Benson]

Pedro Alves wrote:
> On 07/02/2015 12:18 PM, Gary Benson wrote:
> > Pedro Alves wrote:
> > > On 06/16/2015 10:42 AM, Gary Benson wrote:
> > > > This commit introduces a new function build_debug_file_name
> > > > which concatenates a series of filename components into a
> > > > filename.  find_separate_debug_file is updated to use
> > > > build_debug_file_name.  A later commit in this series will
> > > > extend build_debug_file_name to correctly handle "target:"
> > > > prefixes, so it is convenient to have filename building pulled
> > > > out into one function.  For now the only functional change
> > > > here is that the original code sometimes generated filenames
> > > > with repeated directory separators while the new code does
> > > > not.
> > >
> > > I'd drop the "debug" from the function's name.  Sounds like a
> > > candidate for reuse elsewhere to me.
> > 
> > Should I put it somewhere else, maybe common-utils.c?
> 
> I dislike common-utils.c for "kitchen-sink" reasons, though.
> 
> There's common/filestuff.c, but that looks more for low/OS level
> file things.

Yeah, that was my first thought.

> There's substitute_path_component (and gdb_realpath / gdb_abspath)
> in utils.c.  Maybe put it next to substitute_path_component, which
> seems to be in the same "family" of function, and then (at some
> point) we would move all file name/path manipulation routines to its
> own file.
> 
> WDYT?

Suits me :)

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH 1/5] Introduce build_debug_file_name

[Jan Kratochvil]

On Tue, 16 Jun 2015 11:42:44 +0200, Gary Benson wrote:
> +/* 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.  */

I would describe also:
  Initial slash is left in place.  Additionally a slash is appended at the
  end.


Jan

[PATCH 2/5] Pre-strip now-unnecessary trailing directory separators

[Gary Benson]

Prior to the previous commit find_separate_debug_file required that
its DIR argument had a trailing directory separator.  This is no
longer necessary, and makes it difficult to check whether dir and
canon_dir are the same as canon_dir usually does not have a trailing
separator.  This commit updates find_separate_debug_file's caller to
not supply DIR with a trailing separator.  The next commit in the
series relies on this to avoid trying the same location twice.

gdb/ChangeLog:

	* gdb/symfile.c (find_separate_debug_file): Update comment.
	(terminate_after_last_dir_separator): Replaced with...
	(terminate_at_last_dir_separator): New function.
	(find_separate_debug_file_by_debuglink): Use the above.
---
 gdb/ChangeLog |    7 +++++++
 gdb/symfile.c |   19 ++++++++++---------
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 799133a..77aaeed 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1523,7 +1523,6 @@ show_debug_file_directory (struct ui_file *file, int from_tty,
    where the original file resides (may not be the same as
    dirname(objfile->name) due to symlinks), and DEBUGLINK as the file we are
    looking for.  CANON_DIR is the "realpath" form of DIR.
-   DIR must contain a trailing '/'.
    Returns the path of the file with separate debug info, of NULL.  */
 
 static char *
@@ -1593,12 +1592,12 @@ find_separate_debug_file (const char *dir,
   return NULL;
 }
 
-/* Modify PATH to contain only "[/]directory/" part of PATH.
-   If there were no directory separators in PATH, PATH will be empty
+/* Terminate PATH at the final directory separator.  If PATH
+   contains no directory separators then PATH will be an empty
    string on return.  */
 
 static void
-terminate_after_last_dir_separator (char *path)
+terminate_at_last_dir_separator (char *path)
 {
   int i;
 
@@ -1606,10 +1605,12 @@ terminate_after_last_dir_separator (char *path)
      followed by a slash.  The directory can be relative or absolute.  */
   for (i = strlen(path) - 1; i >= 0; i--)
     if (IS_DIR_SEPARATOR (path[i]))
-      break;
+      {
+	path[i] = '\0';
+	return;
+      }
 
-  /* If I is -1 then no directory is present there and DIR will be "".  */
-  path[i + 1] = '\0';
+  path[0] = '\0';
 }
 
 /* Find separate debuginfo for OBJFILE (using .gnu_debuglink section).
@@ -1636,7 +1637,7 @@ find_separate_debug_file_by_debuglink (struct objfile *objfile)
   cleanups = make_cleanup (xfree, debuglink);
   dir = xstrdup (objfile_name (objfile));
   make_cleanup (xfree, dir);
-  terminate_after_last_dir_separator (dir);
+  terminate_at_last_dir_separator (dir);
   canon_dir = lrealpath (dir);
 
   debugfile = find_separate_debug_file (dir, canon_dir, debuglink,
@@ -1659,7 +1660,7 @@ find_separate_debug_file_by_debuglink (struct objfile *objfile)
 	  if (symlink_dir != NULL)
 	    {
 	      make_cleanup (xfree, symlink_dir);
-	      terminate_after_last_dir_separator (symlink_dir);
+	      terminate_at_last_dir_separator (symlink_dir);
 	      if (strcmp (dir, symlink_dir) != 0)
 		{
 		  /* Different directory, so try using it.  */
-- 
1.7.1

Re: [PATCH 2/5] Pre-strip now-unnecessary trailing directory separators

[Pedro Alves]

On 06/16/2015 10:42 AM, Gary Benson wrote:
> Prior to the previous commit find_separate_debug_file required that
> its DIR argument had a trailing directory separator.  This is no
> longer necessary, and makes it difficult to check whether dir and
> canon_dir are the same as canon_dir usually does not have a trailing
> separator.  This commit updates find_separate_debug_file's caller to
> not supply DIR with a trailing separator.  The next commit in the
> series relies on this to avoid trying the same location twice.

OK.

Thanks,
Pedro Alves

[PATCH 3/5] Update how find_separate_debug_file handles CANON_DIR argument

[Gary Benson]

find_separate_debug_file's CANON_DIR argument is used to create an
alternate location to search for debug files.  This commit moves
that logic out of the loop it's in and introduces a new variable
altdir to hold the generated alternate location.  A check is added
to inhibit the altdir search if alternate location is the same as
the regular location specified in DIR.  A check was also added for
gdb_sysroot == NULL, which the original code was missing.

gdb/ChangeLog:

	* gdb/symfile.c (find_separate_debug_file): Move canon_dir
	logic out of loop.  Add check for NULL gdb_sysroot.  Don't
	check the same location twice.
---
 gdb/ChangeLog |    6 ++++++
 gdb/symfile.c |   25 +++++++++++++++++--------
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 77aaeed..bae144e 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1536,6 +1536,7 @@ find_separate_debug_file (const char *dir,
   VEC (char_ptr) *debugdir_vec;
   struct cleanup *back_to;
   int ix;
+  const char *altdir = NULL;
 
   /* First try in the same directory as the original file.  */
   debugfile = build_debug_file_name (dir, debuglink, NULL);
@@ -1558,6 +1559,20 @@ find_separate_debug_file (const char *dir,
   debugdir_vec = dirnames_to_char_ptr_vec (debug_file_directory);
   back_to = make_cleanup_free_char_ptr_vec (debugdir_vec);
 
+  /* If the file is in the sysroot, try its base path in the
+     global debugfile directory as an alternate location.  */
+  if (canon_dir != NULL
+      && gdb_sysroot != NULL && *gdb_sysroot != '\0'
+      && filename_ncmp (canon_dir, gdb_sysroot,
+			strlen (gdb_sysroot)) == 0
+      && IS_DIR_SEPARATOR (canon_dir[strlen (gdb_sysroot)]))
+    {
+      altdir = canon_dir + strlen (gdb_sysroot);
+
+      if (strcmp (altdir, dir) == 0)
+	altdir = NULL;
+    }
+
   for (ix = 0; VEC_iterate (char_ptr, debugdir_vec, ix, debugdir); ++ix)
     {
       debugfile = build_debug_file_name (debugdir, dir, debuglink,
@@ -1569,15 +1584,9 @@ find_separate_debug_file (const char *dir,
 	}
       xfree (debugfile);
 
-      /* If the file is in the sysroot, try using its base path in the
-	 global debugfile directory.  */
-      if (canon_dir != NULL
-	  && filename_ncmp (canon_dir, gdb_sysroot,
-			    strlen (gdb_sysroot)) == 0
-	  && IS_DIR_SEPARATOR (canon_dir[strlen (gdb_sysroot)]))
+      if (altdir != NULL)
 	{
-	  debugfile = build_debug_file_name (debugdir, canon_dir
-					     + strlen (gdb_sysroot),
+	  debugfile = build_debug_file_name (debugdir, altdir,
 					     debuglink, NULL);
 	  if (separate_debug_file_exists (debugfile, crc32, objfile))
 	    {
-- 
1.7.1

Re: [PATCH 3/5] Update how find_separate_debug_file handles CANON_DIR argument

[Pedro Alves]

On 06/16/2015 10:42 AM, Gary Benson wrote:
> find_separate_debug_file's CANON_DIR argument is used to create an
> alternate location to search for debug files.  This commit moves
> that logic out of the loop it's in and introduces a new variable
> altdir to hold the generated alternate location.  A check is added
> to inhibit the altdir search if alternate location is the same as
> the regular location specified in DIR.  A check was also added for
> gdb_sysroot == NULL, which the original code was missing.

I think the "gdb_sysroot != NULL" check is no longer necessary, right?

Otherwise looks fine.

Thanks,
Pedro Alves

[PATCH 4/5] Add "target:" filename handling to find_separate_debug_file

[Gary Benson]

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

diff --git a/gdb/symfile.c b/gdb/symfile.c
index bae144e..0cc940a 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -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);
-- 
1.7.1

Re: [PATCH 4/5] Add "target:" filename handling to find_separate_debug_file

[Pedro Alves]

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

Re: [PATCH 4/5] Add "target:" filename handling to find_separate_debug_file

[Gary Benson]

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

-- 
http://gbenson.net/

Re: [PATCH 4/5] Add "target:" filename handling to find_separate_debug_file

[Gary Benson]

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

Re: [PATCH 4/5] Add "target:" filename handling to find_separate_debug_file

[Gary Benson]

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

-- 
http://gbenson.net/

Re: [PATCH 4/5] Add "target:" filename handling to find_separate_debug_file

[Jan Kratochvil]

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

[PATCH 5/5] Also look for debug files in gdb_sysroot

[Gary Benson]

This commit makes GDB look for separate debug files in gdb_sysroot if
not found in the currently searched locations.  This allows for easier
analysis of core files from foreign machines using the sysroot option.
The user creates or mounts a directory containing the necessary
binaries and debuginfo and then sets GDB's sysroot to that directory
before starting their debug session.

gdb/ChangeLog:

	* gdb/symfile.c (find_separate_debug_file): In debugdir loop,
	also try alternate directory prefixed with gdb_sysroot.
---
 gdb/ChangeLog |    6 ++++++
 gdb/symfile.c |   11 +++++++++++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 0cc940a..1cd99ea 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1613,6 +1613,7 @@ find_separate_debug_file (const char *dir,
 
       if (altdir != NULL)
 	{
+	  /* Try in the alternate directory directly.  */
 	  debugfile = build_debug_file_name (no_prefix, debugdir,
 					     altdir, debuglink, NULL);
 	  if (separate_debug_file_exists (debugfile, crc32, objfile))
@@ -1621,6 +1622,16 @@ find_separate_debug_file (const char *dir,
 	      return debugfile;
 	    }
 	  xfree (debugfile);
+
+	  /* Try in the alternate directory in gdb_sysroot.  */
+	  debugfile = build_debug_file_name (gdb_sysroot, debugdir,
+					     altdir, debuglink, NULL);
+	  if (separate_debug_file_exists (debugfile, crc32, objfile))
+	    {
+	      do_cleanups (back_to);
+	      return debugfile;
+	    }
+	  xfree (debugfile);
 	}
     }
 
-- 
1.7.1

Re: [PATCH 5/5] Also look for debug files in gdb_sysroot

[Pedro Alves]

On 06/16/2015 10:42 AM, Gary Benson wrote:
> This commit makes GDB look for separate debug files in gdb_sysroot if
> not found in the currently searched locations.  This allows for easier
> analysis of core files from foreign machines using the sysroot option.
> The user creates or mounts a directory containing the necessary
> binaries and debuginfo and then sets GDB's sysroot to that directory
> before starting their debug session.

Looks fine to me, but I think this should be documented in
the manual, and maybe in the online help somewhere as well?
And NEWS too.

Thanks,
Pedro Alves

> 
> gdb/ChangeLog:
> 
> 	* gdb/symfile.c (find_separate_debug_file): In debugdir loop,
> 	also try alternate directory prefixed with gdb_sysroot.
> ---
>  gdb/ChangeLog |    6 ++++++
>  gdb/symfile.c |   11 +++++++++++
>  2 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index 0cc940a..1cd99ea 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -1613,6 +1613,7 @@ find_separate_debug_file (const char *dir,
>  
>        if (altdir != NULL)
>  	{
> +	  /* Try in the alternate directory directly.  */
>  	  debugfile = build_debug_file_name (no_prefix, debugdir,
>  					     altdir, debuglink, NULL);
>  	  if (separate_debug_file_exists (debugfile, crc32, objfile))
> @@ -1621,6 +1622,16 @@ find_separate_debug_file (const char *dir,
>  	      return debugfile;
>  	    }
>  	  xfree (debugfile);
> +
> +	  /* Try in the alternate directory in gdb_sysroot.  */
> +	  debugfile = build_debug_file_name (gdb_sysroot, debugdir,
> +					     altdir, debuglink, NULL);
> +	  if (separate_debug_file_exists (debugfile, crc32, objfile))
> +	    {
> +	      do_cleanups (back_to);
> +	      return debugfile;
> +	    }
> +	  xfree (debugfile);

[PING][PATCH 0/5] Separate debugfile improvements

[Gary Benson]

Ping:
https://sourceware.org/ml/gdb-patches/2015-06/msg00338.html

It would be nice to get this in for 7.10.

Gary Benson wrote:
> Hi all,
> 
> This series makes a number of improvements to the code that locates
> separated debuginfo.  This series (patch 4 specifically) goes really
> nicely with the remote/container improvements I made this past few
> months, and it'd be really nice to get this into 7.10.
> 
> - Patch 1 pulls filename building (including buffer allocation) out of
>   find_separate_debug_file into a new function, build_debug_file_name.
>   This simplifies find_separate_debug_file ready for the other
>   patches.  The new code avoids generating paths with repeated
>   separators, but operation is otherwise unchanged.
> 
> - Patch 2 alters find_separate_debug_file's caller to supply all paths
>   without trailing separators, which patch 1 of this series made
>   unnecessary.  This allows paths to be compared more simply.
> 
> - Patch 3 changes how find_separate_debug_file's CANON_DIR argument is
>   handled.  The new code avoids looking in CANON_DIR if it is the same
>   as DIR, but operation is otherwise unchanged.
> 
> - Patch 4 updates find_separate_debug_file to handle filenames
>   prefixed with "target:".  This allows GDB to locate and access
>   separated debuginfo from remote targets and from inferiors in
>   containers.
> 
> - Patch 5 causes GDB to look for debug files in gdb_sysroot in
>   addition to the other searched locations.  This allows for easier
>   analysis of core files from foreign machines.
> 
> Built and regtested on RHEL 6.6 x86_64.
> 
> Ok to commit?
> 
> Thanks,
> Gary
> 
> --
> http://gbenson.net/