[PATCH] PR17917 Lookup remote build-id in remote binaries

[PATCH] PR17917 Lookup remote build-id in remote binaries

[Daniel Black]

When a file contains TARGET_SYSROOT_PREFIX, skip the lreadpath
lookup and just pass the resolution though to gdb_bfd_open.

(gdb)  set debug separate-debug-file 1
(gdb)  target extended-remote  :44663
Remote debugging using :44663
warning: Can not parse XML target description; XML support was disabled at compile time
Reading /usr/sbin/mariadbd from remote target...
warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
Reading /usr/sbin/mariadbd from remote target...
Reading symbols from target:/usr/sbin/mariadbd...

Looking for separate debug info (build-id) for target:/usr/sbin/mariadbd
  Trying /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug... no, unable to compute real path
  Trying target:/usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug...Reading /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug from remote target...
 yes!
Reading /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug from remote target...
Reading symbols from target:/usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug...
---
 gdb/build-id.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/gdb/build-id.c b/gdb/build-id.c
index d2f2576d96d..f2dfe697513 100644
--- a/gdb/build-id.c
+++ b/gdb/build-id.c
@@ -82,7 +82,9 @@ build_id_to_debug_bfd_1 (const std::string &link, size_t build_id_len,
 
   /* lrealpath() is expensive even for the usually non-existent files.  */
   gdb::unique_xmalloc_ptr<char> filename;
-  if (access (link.c_str (), F_OK) == 0)
+  if (startswith (link, TARGET_SYSROOT_PREFIX))
+    filename = make_unique_xstrdup( link.c_str ());
+  else if (access (link.c_str (), F_OK) == 0)
     filename.reset (lrealpath (link.c_str ()));
 
   if (filename == NULL)
@@ -162,18 +164,12 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,
 
       /* Try to look under the sysroot as well.  If the sysroot is
 	 "/the/sysroot", it will give
-	 "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug".
+	 "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug". */
 
-	 Don't do it if the sysroot is the target system ("target:").  It
-	 could work in theory, but the lrealpath in build_id_to_debug_bfd_1
-	 only works with local paths.  */
-      if (strcmp (gdb_sysroot, TARGET_SYSROOT_PREFIX) != 0)
-	{
-	  link = gdb_sysroot + link;
-	  debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
-	  if (debug_bfd != NULL)
-	    return debug_bfd;
-	}
+      link = gdb_sysroot + link;
+      debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
+      if (debug_bfd != NULL)
+	return debug_bfd;
     }
 
   return {};
-- 
2.31.1

Re: [PATCH] PR17917 Lookup remote build-id in remote binaries

[Daniel Black]

Hi,

Just a reminder.

Per the removed comment in this patch, lreadpath cannot be run on a
remote server, not without some gdbserver implementation anyway.

This patch pushes the logic around a remote server detection down into
build_id_to_debug_bfd_1.

When it is a remote server, we skip the lrealpath and pass the path
through the link directly.

Is there a better way to resolve this 6+ year old bug?

On Sat, Aug 28, 2021 at 3:35 PM Daniel Black <daniel@mariadb.org> wrote:
>
> When a file contains TARGET_SYSROOT_PREFIX, skip the lreadpath
> lookup and just pass the resolution though to gdb_bfd_open.
>
> (gdb)  set debug separate-debug-file 1
> (gdb)  target extended-remote  :44663
> Remote debugging using :44663
> warning: Can not parse XML target description; XML support was disabled at compile time
> Reading /usr/sbin/mariadbd from remote target...
> warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
> Reading /usr/sbin/mariadbd from remote target...
> Reading symbols from target:/usr/sbin/mariadbd...
>
> Looking for separate debug info (build-id) for target:/usr/sbin/mariadbd
>   Trying /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug... no, unable to compute real path
>   Trying target:/usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug...Reading /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug from remote target...
>  yes!
> Reading /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug from remote target...
> Reading symbols from target:/usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug...
> ---
>  gdb/build-id.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/gdb/build-id.c b/gdb/build-id.c
> index d2f2576d96d..f2dfe697513 100644
> --- a/gdb/build-id.c
> +++ b/gdb/build-id.c
> @@ -82,7 +82,9 @@ build_id_to_debug_bfd_1 (const std::string &link, size_t build_id_len,
>
>    /* lrealpath() is expensive even for the usually non-existent files.  */
>    gdb::unique_xmalloc_ptr<char> filename;
> -  if (access (link.c_str (), F_OK) == 0)
> +  if (startswith (link, TARGET_SYSROOT_PREFIX))
> +    filename = make_unique_xstrdup( link.c_str ());
> +  else if (access (link.c_str (), F_OK) == 0)
>      filename.reset (lrealpath (link.c_str ()));
>
>    if (filename == NULL)
> @@ -162,18 +164,12 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,
>
>        /* Try to look under the sysroot as well.  If the sysroot is
>          "/the/sysroot", it will give
> -        "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug".
> +        "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug". */
>
> -        Don't do it if the sysroot is the target system ("target:").  It
> -        could work in theory, but the lrealpath in build_id_to_debug_bfd_1
> -        only works with local paths.  */
> -      if (strcmp (gdb_sysroot, TARGET_SYSROOT_PREFIX) != 0)
> -       {
> -         link = gdb_sysroot + link;
> -         debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
> -         if (debug_bfd != NULL)
> -           return debug_bfd;
> -       }
> +      link = gdb_sysroot + link;
> +      debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
> +      if (debug_bfd != NULL)
> +       return debug_bfd;
>      }
>
>    return {};
> --
> 2.31.1
>

Re: [PING 1 Month Reminder][PATCH] PR17917 Lookup remote build-id in remote binaries

[Daniel Black]

On Fri, Sep 10, 2021 at 9:05 AM Daniel Black <daniel@mariadb.org> wrote:
>
> Hi,
>
> Just a reminder.
>
> Per the removed comment in this patch, lreadpath cannot be run on a
> remote server, not without some gdbserver implementation anyway.
>
> This patch pushes the logic around a remote server detection down into
> build_id_to_debug_bfd_1.
>
> When it is a remote server, we skip the lrealpath and pass the path
> through the link directly.
>
> Is there a better way to resolve this 6+ year old bug?
>
> On Sat, Aug 28, 2021 at 3:35 PM Daniel Black <daniel@mariadb.org> wrote:
> >
> > When a file contains TARGET_SYSROOT_PREFIX, skip the lreadpath
> > lookup and just pass the resolution though to gdb_bfd_open.
> >
> > (gdb)  set debug separate-debug-file 1
> > (gdb)  target extended-remote  :44663
> > Remote debugging using :44663
> > warning: Can not parse XML target description; XML support was disabled at compile time
> > Reading /usr/sbin/mariadbd from remote target...
> > warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
> > Reading /usr/sbin/mariadbd from remote target...
> > Reading symbols from target:/usr/sbin/mariadbd...
> >
> > Looking for separate debug info (build-id) for target:/usr/sbin/mariadbd
> >   Trying /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug... no, unable to compute real path
> >   Trying target:/usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug...Reading /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug from remote target...
> >  yes!
> > Reading /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug from remote target...
> > Reading symbols from target:/usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug...
> > ---
> >  gdb/build-id.c | 20 ++++++++------------
> >  1 file changed, 8 insertions(+), 12 deletions(-)
> >
> > diff --git a/gdb/build-id.c b/gdb/build-id.c
> > index d2f2576d96d..f2dfe697513 100644
> > --- a/gdb/build-id.c
> > +++ b/gdb/build-id.c
> > @@ -82,7 +82,9 @@ build_id_to_debug_bfd_1 (const std::string &link, size_t build_id_len,
> >
> >    /* lrealpath() is expensive even for the usually non-existent files.  */
> >    gdb::unique_xmalloc_ptr<char> filename;
> > -  if (access (link.c_str (), F_OK) == 0)
> > +  if (startswith (link, TARGET_SYSROOT_PREFIX))
> > +    filename = make_unique_xstrdup( link.c_str ());
> > +  else if (access (link.c_str (), F_OK) == 0)
> >      filename.reset (lrealpath (link.c_str ()));
> >
> >    if (filename == NULL)
> > @@ -162,18 +164,12 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,
> >
> >        /* Try to look under the sysroot as well.  If the sysroot is
> >          "/the/sysroot", it will give
> > -        "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug".
> > +        "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug". */
> >
> > -        Don't do it if the sysroot is the target system ("target:").  It
> > -        could work in theory, but the lrealpath in build_id_to_debug_bfd_1
> > -        only works with local paths.  */
> > -      if (strcmp (gdb_sysroot, TARGET_SYSROOT_PREFIX) != 0)
> > -       {
> > -         link = gdb_sysroot + link;
> > -         debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
> > -         if (debug_bfd != NULL)
> > -           return debug_bfd;
> > -       }
> > +      link = gdb_sysroot + link;
> > +      debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
> > +      if (debug_bfd != NULL)
> > +       return debug_bfd;
> >      }
> >
> >    return {};
> > --
> > 2.31.1
> >

Re: [PING 6 Weeks][PATCH] PR17917 Lookup remote build-id in remote binaries

[Daniel Black]

On Wed, Sep 29, 2021 at 5:02 PM Daniel Black <daniel@mariadb.org> wrote:
>
> On Fri, Sep 10, 2021 at 9:05 AM Daniel Black <daniel@mariadb.org> wrote:
> >
> > Hi,
> >
> > Just a reminder.
> >
> > Per the removed comment in this patch, lreadpath cannot be run on a
> > remote server, not without some gdbserver implementation anyway.
> >
> > This patch pushes the logic around a remote server detection down into
> > build_id_to_debug_bfd_1.
> >
> > When it is a remote server, we skip the lrealpath and pass the path
> > through the link directly.
> >
> > Is there a better way to resolve this 6+ year old bug?
> >
> > On Sat, Aug 28, 2021 at 3:35 PM Daniel Black <daniel@mariadb.org> wrote:
> > >
> > > When a file contains TARGET_SYSROOT_PREFIX, skip the lreadpath
> > > lookup and just pass the resolution though to gdb_bfd_open.
> > >
> > > (gdb)  set debug separate-debug-file 1
> > > (gdb)  target extended-remote  :44663
> > > Remote debugging using :44663
> > > warning: Can not parse XML target description; XML support was disabled at compile time
> > > Reading /usr/sbin/mariadbd from remote target...
> > > warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
> > > Reading /usr/sbin/mariadbd from remote target...
> > > Reading symbols from target:/usr/sbin/mariadbd...
> > >
> > > Looking for separate debug info (build-id) for target:/usr/sbin/mariadbd
> > >   Trying /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug... no, unable to compute real path
> > >   Trying target:/usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug...Reading /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug from remote target...
> > >  yes!
> > > Reading /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug from remote target...
> > > Reading symbols from target:/usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug...
> > > ---
> > >  gdb/build-id.c | 20 ++++++++------------
> > >  1 file changed, 8 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/gdb/build-id.c b/gdb/build-id.c
> > > index d2f2576d96d..f2dfe697513 100644
> > > --- a/gdb/build-id.c
> > > +++ b/gdb/build-id.c
> > > @@ -82,7 +82,9 @@ build_id_to_debug_bfd_1 (const std::string &link, size_t build_id_len,
> > >
> > >    /* lrealpath() is expensive even for the usually non-existent files.  */
> > >    gdb::unique_xmalloc_ptr<char> filename;
> > > -  if (access (link.c_str (), F_OK) == 0)
> > > +  if (startswith (link, TARGET_SYSROOT_PREFIX))
> > > +    filename = make_unique_xstrdup( link.c_str ());
> > > +  else if (access (link.c_str (), F_OK) == 0)
> > >      filename.reset (lrealpath (link.c_str ()));
> > >
> > >    if (filename == NULL)
> > > @@ -162,18 +164,12 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,
> > >
> > >        /* Try to look under the sysroot as well.  If the sysroot is
> > >          "/the/sysroot", it will give
> > > -        "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug".
> > > +        "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug". */
> > >
> > > -        Don't do it if the sysroot is the target system ("target:").  It
> > > -        could work in theory, but the lrealpath in build_id_to_debug_bfd_1
> > > -        only works with local paths.  */
> > > -      if (strcmp (gdb_sysroot, TARGET_SYSROOT_PREFIX) != 0)
> > > -       {
> > > -         link = gdb_sysroot + link;
> > > -         debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
> > > -         if (debug_bfd != NULL)
> > > -           return debug_bfd;
> > > -       }
> > > +      link = gdb_sysroot + link;
> > > +      debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
> > > +      if (debug_bfd != NULL)
> > > +       return debug_bfd;
> > >      }
> > >
> > >    return {};
> > > --
> > > 2.31.1
> > >

Re: [PATCH] PR17917 Lookup remote build-id in remote binaries

[Simon Marchi]

On 2021-08-28 1:35 a.m., Daniel Black via Gdb-patches wrote:
> When a file contains TARGET_SYSROOT_PREFIX, skip the lreadpath

I don't really understand "a file contains TARGET_SYSROOT_PREFIX"...
which file?  Do you mean when the sysroot is TARGET_SYSROOT_PREFIX
("target:")?

> lookup and just pass the resolution though to gdb_bfd_open.
>
> (gdb)  set debug separate-debug-file 1
> (gdb)  target extended-remote  :44663

Here, if can you add a "show sysroot" to be clear (if I understand the
problem correctly)?

> Remote debugging using :44663
> warning: Can not parse XML target description; XML support was disabled at compile time
> Reading /usr/sbin/mariadbd from remote target...
> warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
> Reading /usr/sbin/mariadbd from remote target...
> Reading symbols from target:/usr/sbin/mariadbd...
>
> Looking for separate debug info (build-id) for target:/usr/sbin/mariadbd
>   Trying /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug... no, unable to compute real path
>   Trying target:/usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug...Reading /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug from remote target...
>  yes!
> Reading /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug from remote target...
> Reading symbols from target:/usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug...

The commit message is just missing a bit of information to be
understable by someone not familiar with the problem.  Can you please
make explicit what doesn't work and what the patch enables?  I suppose
it's something like "GDB doesn't support loading separate debug files
using build-id through the target filesystem, make this possible".

The code change looks pretty much good to me, I just have minor questions
and comments below.

> ---
>  gdb/build-id.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/gdb/build-id.c b/gdb/build-id.c
> index d2f2576d96d..f2dfe697513 100644
> --- a/gdb/build-id.c
> +++ b/gdb/build-id.c
> @@ -82,7 +82,9 @@ build_id_to_debug_bfd_1 (const std::string &link, size_t build_id_len,
>
>    /* lrealpath() is expensive even for the usually non-existent files.  */
>    gdb::unique_xmalloc_ptr<char> filename;
> -  if (access (link.c_str (), F_OK) == 0)
> +  if (startswith (link, TARGET_SYSROOT_PREFIX))
> +    filename = make_unique_xstrdup( link.c_str ());

Swap parenthesis and space above.

> +  else if (access (link.c_str (), F_OK) == 0)
>      filename.reset (lrealpath (link.c_str ()));

Do you (or anybody) happen to know why we do an lrealpath here?  If
"link" points to a symlink, can't we just pass that to gdb_bfd_open,
which will end up opening the right file anyway?

>
>    if (filename == NULL)
> @@ -162,18 +164,12 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,
>
>        /* Try to look under the sysroot as well.  If the sysroot is
>  	 "/the/sysroot", it will give
> -	 "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug".
> +	 "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug". */
>
> -	 Don't do it if the sysroot is the target system ("target:").  It
> -	 could work in theory, but the lrealpath in build_id_to_debug_bfd_1
> -	 only works with local paths.  */
> -      if (strcmp (gdb_sysroot, TARGET_SYSROOT_PREFIX) != 0)
> -	{
> -	  link = gdb_sysroot + link;
> -	  debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
> -	  if (debug_bfd != NULL)
> -	    return debug_bfd;
> -	}
> +      link = gdb_sysroot + link;
> +      debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
> +      if (debug_bfd != NULL)
> +	return debug_bfd;
>      }

Should we skip the above if gdb_sysroot is empty?  Otherwise we will end
up looking twice at the same place, which is just a bit wasteful.  That
already happens today, but it would be nice to avoid it:


    $ ./gdb -nx -q --data-directory=data-directory -iex "set debug separate-debug-file" -iex "set sysroot" a.out
    Reading symbols from a.out...

    Looking for separate debug info (build-id) for /home/smarchi/build/binutils-gdb/gdb/a.out
      Trying /usr/lib/debug/.build-id/3c/c1c9c57fc70125b2ff804253a4174c9cd19b53.debug... no, unable to compute real path
      Trying /usr/lib/debug/.build-id/3c/c1c9c57fc70125b2ff804253a4174c9cd19b53.debug... no, unable to compute real path
    (No debugging symbols found in a.out)

Simon

Re: [PING 7 Week Reminder][PATCH] PR17917 Lookup remote build-id in remote binaries

[Daniel Black]

On Wed, Sep 29, 2021 at 5:02 PM Daniel Black <daniel@mariadb.org> wrote:
>
> On Fri, Sep 10, 2021 at 9:05 AM Daniel Black <daniel@mariadb.org> wrote:
> >
> > Hi,
> >
> > Just a reminder.
> >
> > Per the removed comment in this patch, lreadpath cannot be run on a
> > remote server, not without some gdbserver implementation anyway.
> >
> > This patch pushes the logic around a remote server detection down into
> > build_id_to_debug_bfd_1.
> >
> > When it is a remote server, we skip the lrealpath and pass the path
> > through the link directly.
> >
> > Is there a better way to resolve this 6+ year old bug?
> >
> > On Sat, Aug 28, 2021 at 3:35 PM Daniel Black <daniel@mariadb.org> wrote:
> > >
> > > When a file contains TARGET_SYSROOT_PREFIX, skip the lreadpath
> > > lookup and just pass the resolution though to gdb_bfd_open.
> > >
> > > (gdb)  set debug separate-debug-file 1
> > > (gdb)  target extended-remote  :44663
> > > Remote debugging using :44663
> > > warning: Can not parse XML target description; XML support was disabled at compile time
> > > Reading /usr/sbin/mariadbd from remote target...
> > > warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
> > > Reading /usr/sbin/mariadbd from remote target...
> > > Reading symbols from target:/usr/sbin/mariadbd...
> > >
> > > Looking for separate debug info (build-id) for target:/usr/sbin/mariadbd
> > >   Trying /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug... no, unable to compute real path
> > >   Trying target:/usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug...Reading /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug from remote target...
> > >  yes!
> > > Reading /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug from remote target...
> > > Reading symbols from target:/usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug...
> > > ---
> > >  gdb/build-id.c | 20 ++++++++------------
> > >  1 file changed, 8 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/gdb/build-id.c b/gdb/build-id.c
> > > index d2f2576d96d..f2dfe697513 100644
> > > --- a/gdb/build-id.c
> > > +++ b/gdb/build-id.c
> > > @@ -82,7 +82,9 @@ build_id_to_debug_bfd_1 (const std::string &link, size_t build_id_len,
> > >
> > >    /* lrealpath() is expensive even for the usually non-existent files.  */
> > >    gdb::unique_xmalloc_ptr<char> filename;
> > > -  if (access (link.c_str (), F_OK) == 0)
> > > +  if (startswith (link, TARGET_SYSROOT_PREFIX))
> > > +    filename = make_unique_xstrdup( link.c_str ());
> > > +  else if (access (link.c_str (), F_OK) == 0)
> > >      filename.reset (lrealpath (link.c_str ()));
> > >
> > >    if (filename == NULL)
> > > @@ -162,18 +164,12 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,
> > >
> > >        /* Try to look under the sysroot as well.  If the sysroot is
> > >          "/the/sysroot", it will give
> > > -        "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug".
> > > +        "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug". */
> > >
> > > -        Don't do it if the sysroot is the target system ("target:").  It
> > > -        could work in theory, but the lrealpath in build_id_to_debug_bfd_1
> > > -        only works with local paths.  */
> > > -      if (strcmp (gdb_sysroot, TARGET_SYSROOT_PREFIX) != 0)
> > > -       {
> > > -         link = gdb_sysroot + link;
> > > -         debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
> > > -         if (debug_bfd != NULL)
> > > -           return debug_bfd;
> > > -       }
> > > +      link = gdb_sysroot + link;
> > > +      debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
> > > +      if (debug_bfd != NULL)
> > > +       return debug_bfd;
> > >      }
> > >
> > >    return {};
> > > --
> > > 2.31.1
> > >

Re: [PING 7 Week Reminder][PATCH] PR17917 Lookup remote build-id in remote binaries

[Simon Marchi]

I replied to this yesterday:

https://sourceware.org/pipermail/gdb-patches/2021-October/182579.html

Simon

On 2021-10-16 03:04, Daniel Black via Gdb-patches wrote:
> On Wed, Sep 29, 2021 at 5:02 PM Daniel Black <daniel@mariadb.org> wrote:
>>
>> On Fri, Sep 10, 2021 at 9:05 AM Daniel Black <daniel@mariadb.org> wrote:
>>>
>>> Hi,
>>>
>>> Just a reminder.
>>>
>>> Per the removed comment in this patch, lreadpath cannot be run on a
>>> remote server, not without some gdbserver implementation anyway.
>>>
>>> This patch pushes the logic around a remote server detection down into
>>> build_id_to_debug_bfd_1.
>>>
>>> When it is a remote server, we skip the lrealpath and pass the path
>>> through the link directly.
>>>
>>> Is there a better way to resolve this 6+ year old bug?
>>>
>>> On Sat, Aug 28, 2021 at 3:35 PM Daniel Black <daniel@mariadb.org> wrote:
>>>>
>>>> When a file contains TARGET_SYSROOT_PREFIX, skip the lreadpath
>>>> lookup and just pass the resolution though to gdb_bfd_open.
>>>>
>>>> (gdb)  set debug separate-debug-file 1
>>>> (gdb)  target extended-remote  :44663
>>>> Remote debugging using :44663
>>>> warning: Can not parse XML target description; XML support was disabled at compile time
>>>> Reading /usr/sbin/mariadbd from remote target...
>>>> warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
>>>> Reading /usr/sbin/mariadbd from remote target...
>>>> Reading symbols from target:/usr/sbin/mariadbd...
>>>>
>>>> Looking for separate debug info (build-id) for target:/usr/sbin/mariadbd
>>>>   Trying /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug... no, unable to compute real path
>>>>   Trying target:/usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug...Reading /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug from remote target...
>>>>  yes!
>>>> Reading /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug from remote target...
>>>> Reading symbols from target:/usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug...
>>>> ---
>>>>  gdb/build-id.c | 20 ++++++++------------
>>>>  1 file changed, 8 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/gdb/build-id.c b/gdb/build-id.c
>>>> index d2f2576d96d..f2dfe697513 100644
>>>> --- a/gdb/build-id.c
>>>> +++ b/gdb/build-id.c
>>>> @@ -82,7 +82,9 @@ build_id_to_debug_bfd_1 (const std::string &link, size_t build_id_len,
>>>>
>>>>    /* lrealpath() is expensive even for the usually non-existent files.  */
>>>>    gdb::unique_xmalloc_ptr<char> filename;
>>>> -  if (access (link.c_str (), F_OK) == 0)
>>>> +  if (startswith (link, TARGET_SYSROOT_PREFIX))
>>>> +    filename = make_unique_xstrdup( link.c_str ());
>>>> +  else if (access (link.c_str (), F_OK) == 0)
>>>>      filename.reset (lrealpath (link.c_str ()));
>>>>
>>>>    if (filename == NULL)
>>>> @@ -162,18 +164,12 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,
>>>>
>>>>        /* Try to look under the sysroot as well.  If the sysroot is
>>>>          "/the/sysroot", it will give
>>>> -        "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug".
>>>> +        "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug". */
>>>>
>>>> -        Don't do it if the sysroot is the target system ("target:").  It
>>>> -        could work in theory, but the lrealpath in build_id_to_debug_bfd_1
>>>> -        only works with local paths.  */
>>>> -      if (strcmp (gdb_sysroot, TARGET_SYSROOT_PREFIX) != 0)
>>>> -       {
>>>> -         link = gdb_sysroot + link;
>>>> -         debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
>>>> -         if (debug_bfd != NULL)
>>>> -           return debug_bfd;
>>>> -       }
>>>> +      link = gdb_sysroot + link;
>>>> +      debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
>>>> +      if (debug_bfd != NULL)
>>>> +       return debug_bfd;
>>>>      }
>>>>
>>>>    return {};
>>>> --
>>>> 2.31.1
>>>>

Re: [PATCH] PR17917 Lookup remote build-id in remote binaries

[Daniel Black]

Thanks for the review.

On Sat, Oct 16, 2021 at 2:13 AM Simon Marchi <simark@simark.ca> wrote:
>
> On 2021-08-28 1:35 a.m., Daniel Black via Gdb-patches wrote:
> > When a file contains TARGET_SYSROOT_PREFIX, skip the lreadpath
>
> I don't really understand "a file contains TARGET_SYSROOT_PREFIX"...
> which file?  Do you mean when the sysroot is TARGET_SYSROOT_PREFIX
> ("target:")?

Attempted load of a build-id beginning with target:

I'll get something that's hopefully right.

> > lookup and just pass the resolution though to gdb_bfd_open.
> >
> > (gdb)  set debug separate-debug-file 1
> > (gdb)  target extended-remote  :44663
>
> Here, if can you add a "show sysroot" to be clear (if I understand the
> problem correctly)?
>
> > Remote debugging using :44663
> > warning: Can not parse XML target description; XML support was disabled at compile time
> > Reading /usr/sbin/mariadbd from remote target...
> > warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
> > Reading /usr/sbin/mariadbd from remote target...
> > Reading symbols from target:/usr/sbin/mariadbd...
> >
> > Looking for separate debug info (build-id) for target:/usr/sbin/mariadbd
> >   Trying /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug... no, unable to compute real path
> >   Trying target:/usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug...Reading /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug from remote target...
> >  yes!
> > Reading /usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug from remote target...
> > Reading symbols from target:/usr/lib/debug/.build-id/57/c35022dc84dd28479db41cc278f625776c4adc.debug...
>
> The commit message is just missing a bit of information to be
> understable by someone not familiar with the problem.  Can you please
> make explicit what doesn't work and what the patch enables?  I suppose
> it's something like "GDB doesn't support loading separate debug files
> using build-id through the target filesystem, make this possible".
>
> The code change looks pretty much good to me, I just have minor questions
> and comments below.
>
> > ---
> >  gdb/build-id.c | 20 ++++++++------------
> >  1 file changed, 8 insertions(+), 12 deletions(-)
> >
> > diff --git a/gdb/build-id.c b/gdb/build-id.c
> > index d2f2576d96d..f2dfe697513 100644
> > --- a/gdb/build-id.c
> > +++ b/gdb/build-id.c
> > @@ -82,7 +82,9 @@ build_id_to_debug_bfd_1 (const std::string &link, size_t build_id_len,
> >
> >    /* lrealpath() is expensive even for the usually non-existent files.  */
> >    gdb::unique_xmalloc_ptr<char> filename;
> > -  if (access (link.c_str (), F_OK) == 0)
> > +  if (startswith (link, TARGET_SYSROOT_PREFIX))
> > +    filename = make_unique_xstrdup( link.c_str ());
>
> Swap parenthesis and space above.

ack.

> > +  else if (access (link.c_str (), F_OK) == 0)
> >      filename.reset (lrealpath (link.c_str ()));
>
> Do you (or anybody) happen to know why we do an lrealpath here?  If
> "link" points to a symlink, can't we just pass that to gdb_bfd_open,
> which will end up opening the right file anyway?

Seems it originated in aa28a74efb27

I can't see an obvious reason to do so.

It just looks like other paths got resolved this way at the time and
this pattern was copied.

> >    if (filename == NULL)
> > @@ -162,18 +164,12 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,
> >
> >        /* Try to look under the sysroot as well.  If the sysroot is
> >        "/the/sysroot", it will give
> > -      "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug".
> > +      "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug". */
> >
> > -      Don't do it if the sysroot is the target system ("target:").  It
> > -      could work in theory, but the lrealpath in build_id_to_debug_bfd_1
> > -      only works with local paths.  */
> > -      if (strcmp (gdb_sysroot, TARGET_SYSROOT_PREFIX) != 0)
> > -     {
> > -       link = gdb_sysroot + link;
> > -       debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
> > -       if (debug_bfd != NULL)
> > -         return debug_bfd;
> > -     }
> > +      link = gdb_sysroot + link;
> > +      debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
> > +      if (debug_bfd != NULL)
> > +     return debug_bfd;
> >      }
>
> Should we skip the above if gdb_sysroot is empty?  Otherwise we will end
> up looking twice at the same place, which is just a bit wasteful.  That
> already happens today, but it would be nice to avoid it:


sure. since your change e0700ba44c56 this can just be:

--- a/gdb/build-id.c
+++ b/gdb/build-id.c
@@ -166,10 +166,13 @@ build_id_to_bfd_suffix (size_t build_id_len,
const bfd_byte *build_id,
         "/the/sysroot", it will give
         "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug". */

-      link = gdb_sysroot + link;
-      debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
-      if (debug_bfd != NULL)
-       return debug_bfd;
+      if (!gdb_sysroot.empty ())
+       {
+          link = gdb_sysroot + link;
+          debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
+          if (debug_bfd != NULL)
+            return debug_bfd;
+       }
     }


I'll append a V2 patch to this thread.

[PATCHv2] Fix PR gdb/17917 Lookup build-id in remote binaries

[Daniel Black]

GDB didn't support loading build-id debug files
using on the remote target filesystem.

This is the case when gdbserver attached to a process
and a gdb target remote occurs over tcp.

With this change we make build-id lookups possible:

(gdb) show debug-file-directory
The directory where separate debug symbols are searched for is "/usr/local/lib/debug".
(gdb) set debug-file-directory /usr/lib/debug
(gdb) show sysroot
The current system root is "target:".
(gdb) target extended-remote :46615
Remote debugging using :46615
warning: Can not parse XML target description; XML support was disabled at compile time
Reading /usr/sbin/mariadbd from remote target...
warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
Reading /usr/sbin/mariadbd from remote target...
Reading symbols from target:/usr/sbin/mariadbd...
Reading /usr/lib/debug/.build-id/6e/0a874dca5a7ff831396ddc0785d939a192efe3.debug from remote target...
Reading /usr/lib/debug/.build-id/6e/0a874dca5a7ff831396ddc0785d939a192efe3.debug from remote target...
Reading symbols from target:/usr/lib/debug/.build-id/6e/0a874dca5a7ff831396ddc0785d939a192efe3.debug...
Reading /lib/x86_64-linux-gnu/libpcre2-8.so.0 from remote target...
...

Before this change, the lookups would have been (GNU gdb (GDB) Fedora 10.2-3.fc34):

(gdb) target extended-remote :46615
Remote debugging using :46615
Reading /usr/sbin/mariadbd from remote target...
warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
Reading /usr/sbin/mariadbd from remote target...
Reading symbols from target:/usr/sbin/mariadbd...
Reading /usr/sbin/0a874dca5a7ff831396ddc0785d939a192efe3.debug from remote target...
Reading /usr/sbin/.debug/0a874dca5a7ff831396ddc0785d939a192efe3.debug from remote target...
Reading /usr/lib/debug//usr/sbin/0a874dca5a7ff831396ddc0785d939a192efe3.debug from remote target...
Reading /usr/lib/debug/usr/sbin//0a874dca5a7ff831396ddc0785d939a192efe3.debug from remote target...
Reading target:/usr/lib/debug/usr/sbin//0a874dca5a7ff831396ddc0785d939a192efe3.debug from remote target...
Missing separate debuginfo for target:/usr/sbin/mariadbd
Try: dnf --enablerepo='*debug*' install /usr/lib/debug/.build-id/6e/0a874dca5a7ff831396ddc0785d939a192efe3.debug
(No debugging symbols found in target:/usr/sbin/mariadbd)

Observe it didn't look for /usr/lib/debug/.build-id/6e/0a874dca5a7ff831396ddc0785d939a192efe3.debug
on the remote target (where it is) and expected them to be installed locally.

As a minor optimization, this also changes the build-id lookup such that if
sysroot is empty, no second lookup of the same location is performed.
---
 gdb/build-id.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/gdb/build-id.c b/gdb/build-id.c
index 553d6cec3d2..093e5a14294 100644
--- a/gdb/build-id.c
+++ b/gdb/build-id.c
@@ -82,7 +82,9 @@ build_id_to_debug_bfd_1 (const std::string &link, size_t build_id_len,
 
   /* lrealpath() is expensive even for the usually non-existent files.  */
   gdb::unique_xmalloc_ptr<char> filename;
-  if (access (link.c_str (), F_OK) == 0)
+  if (startswith (link, TARGET_SYSROOT_PREFIX))
+    filename = make_unique_xstrdup (link.c_str ());
+  else if (access (link.c_str (), F_OK) == 0)
     filename.reset (lrealpath (link.c_str ()));
 
   if (filename == NULL)
@@ -162,12 +164,9 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,
 
       /* Try to look under the sysroot as well.  If the sysroot is
 	 "/the/sysroot", it will give
-	 "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug".
+	 "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug". */
 
-	 Don't do it if the sysroot is the target system ("target:").  It
-	 could work in theory, but the lrealpath in build_id_to_debug_bfd_1
-	 only works with local paths.  */
-      if (gdb_sysroot != TARGET_SYSROOT_PREFIX)
+      if (!gdb_sysroot.empty ())
 	{
 	  link = gdb_sysroot + link;
 	  debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
-- 
2.31.1

Re: [PATCHv2] Fix PR gdb/17917 Lookup build-id in remote binaries

[Simon Marchi]

On 2021-10-18 2:06 a.m., Daniel Black via Gdb-patches wrote:
> GDB didn't support loading build-id debug files
> using on the remote target filesystem.
> 
> This is the case when gdbserver attached to a process
> and a gdb target remote occurs over tcp.
> 
> With this change we make build-id lookups possible:
> 
> (gdb) show debug-file-directory
> The directory where separate debug symbols are searched for is "/usr/local/lib/debug".
> (gdb) set debug-file-directory /usr/lib/debug
> (gdb) show sysroot
> The current system root is "target:".
> (gdb) target extended-remote :46615
> Remote debugging using :46615
> warning: Can not parse XML target description; XML support was disabled at compile time
> Reading /usr/sbin/mariadbd from remote target...
> warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
> Reading /usr/sbin/mariadbd from remote target...
> Reading symbols from target:/usr/sbin/mariadbd...
> Reading /usr/lib/debug/.build-id/6e/0a874dca5a7ff831396ddc0785d939a192efe3.debug from remote target...
> Reading /usr/lib/debug/.build-id/6e/0a874dca5a7ff831396ddc0785d939a192efe3.debug from remote target...
> Reading symbols from target:/usr/lib/debug/.build-id/6e/0a874dca5a7ff831396ddc0785d939a192efe3.debug...
> Reading /lib/x86_64-linux-gnu/libpcre2-8.so.0 from remote target...
> ...
> 
> Before this change, the lookups would have been (GNU gdb (GDB) Fedora 10.2-3.fc34):
> 
> (gdb) target extended-remote :46615
> Remote debugging using :46615
> Reading /usr/sbin/mariadbd from remote target...
> warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead.
> Reading /usr/sbin/mariadbd from remote target...
> Reading symbols from target:/usr/sbin/mariadbd...
> Reading /usr/sbin/0a874dca5a7ff831396ddc0785d939a192efe3.debug from remote target...
> Reading /usr/sbin/.debug/0a874dca5a7ff831396ddc0785d939a192efe3.debug from remote target...
> Reading /usr/lib/debug//usr/sbin/0a874dca5a7ff831396ddc0785d939a192efe3.debug from remote target...
> Reading /usr/lib/debug/usr/sbin//0a874dca5a7ff831396ddc0785d939a192efe3.debug from remote target...
> Reading target:/usr/lib/debug/usr/sbin//0a874dca5a7ff831396ddc0785d939a192efe3.debug from remote target...
> Missing separate debuginfo for target:/usr/sbin/mariadbd
> Try: dnf --enablerepo='*debug*' install /usr/lib/debug/.build-id/6e/0a874dca5a7ff831396ddc0785d939a192efe3.debug
> (No debugging symbols found in target:/usr/sbin/mariadbd)
> 
> Observe it didn't look for /usr/lib/debug/.build-id/6e/0a874dca5a7ff831396ddc0785d939a192efe3.debug
> on the remote target (where it is) and expected them to be installed locally.
> 
> As a minor optimization, this also changes the build-id lookup such that if
> sysroot is empty, no second lookup of the same location is performed.
> ---
>  gdb/build-id.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/build-id.c b/gdb/build-id.c
> index 553d6cec3d2..093e5a14294 100644
> --- a/gdb/build-id.c
> +++ b/gdb/build-id.c
> @@ -82,7 +82,9 @@ build_id_to_debug_bfd_1 (const std::string &link, size_t build_id_len,
>  
>    /* lrealpath() is expensive even for the usually non-existent files.  */
>    gdb::unique_xmalloc_ptr<char> filename;
> -  if (access (link.c_str (), F_OK) == 0)
> +  if (startswith (link, TARGET_SYSROOT_PREFIX))
> +    filename = make_unique_xstrdup (link.c_str ());
> +  else if (access (link.c_str (), F_OK) == 0)
>      filename.reset (lrealpath (link.c_str ()));
>  
>    if (filename == NULL)
> @@ -162,12 +164,9 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,
>  
>        /* Try to look under the sysroot as well.  If the sysroot is
>  	 "/the/sysroot", it will give
> -	 "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug".
> +	 "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug". */
>  
> -	 Don't do it if the sysroot is the target system ("target:").  It
> -	 could work in theory, but the lrealpath in build_id_to_debug_bfd_1
> -	 only works with local paths.  */
> -      if (gdb_sysroot != TARGET_SYSROOT_PREFIX)
> +      if (!gdb_sysroot.empty ())
>  	{
>  	  link = gdb_sysroot + link;
>  	  debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
> -- 
> 2.31.1
> 

This is fine with me.  I would just make the following change on top to avoid duplicating the
string if not necessary.  Is it ok if I merge your patch with that change?


From aab1c3b36a10daa4d765a3dfac6553f9a1a04d64 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>
Date: Mon, 18 Oct 2021 11:19:59 -0400
Subject: [PATCH] fixup

Change-Id: I8a0a8569e346540117a75387197471b805269733
---
 gdb/build-id.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/gdb/build-id.c b/gdb/build-id.c
index 093e5a142945..d68b7a7af577 100644
--- a/gdb/build-id.c
+++ b/gdb/build-id.c
@@ -81,11 +81,15 @@ build_id_to_debug_bfd_1 (const std::string &link, size_t build_id_len,
     }

   /* lrealpath() is expensive even for the usually non-existent files.  */
-  gdb::unique_xmalloc_ptr<char> filename;
+  gdb::unique_xmalloc_ptr<char> filename_holder;
+  const char *filename = nullptr;
   if (startswith (link, TARGET_SYSROOT_PREFIX))
-    filename = make_unique_xstrdup (link.c_str ());
+    filename = link.c_str ();
   else if (access (link.c_str (), F_OK) == 0)
-    filename.reset (lrealpath (link.c_str ()));
+    {
+      filename_holder.reset (lrealpath (link.c_str ()));
+      filename = filename_holder.get ();
+    }

   if (filename == NULL)
     {
@@ -96,7 +100,7 @@ build_id_to_debug_bfd_1 (const std::string &link, size_t build_id_len,
     }

   /* We expect to be silent on the non-existing files.  */
-  gdb_bfd_ref_ptr debug_bfd = gdb_bfd_open (filename.get (), gnutarget);
+  gdb_bfd_ref_ptr debug_bfd = gdb_bfd_open (filename, gnutarget);

   if (debug_bfd == NULL)
     {
@@ -164,7 +168,7 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,

       /* Try to look under the sysroot as well.  If the sysroot is
 	 "/the/sysroot", it will give
-	 "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug". */
+	 "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug".  */

       if (!gdb_sysroot.empty ())
 	{
-- 
2.26.2

Re: [PATCHv2] Fix PR gdb/17917 Lookup build-id in remote binaries

[Daniel Black]

On Tue, Oct 19, 2021 at 2:21 AM Simon Marchi <simark@simark.ca> wrote:

> This is fine with me.  I would just make the following change on top to avoid duplicating the
> string if not necessary.  Is it ok if I merge your patch with that change?
>

sure. Thanks.