* [PATCH] PR17917 Lookup remote build-id in remote binaries
@ 2021-08-28 5:35 Daniel Black
2021-09-09 23:05 ` Daniel Black
2021-10-15 15:13 ` [PATCH] " Simon Marchi
0 siblings, 2 replies; 11+ messages in thread
From: Daniel Black @ 2021-08-28 5:35 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] PR17917 Lookup remote build-id in remote binaries
2021-08-28 5:35 [PATCH] PR17917 Lookup remote build-id in remote binaries Daniel Black
@ 2021-09-09 23:05 ` Daniel Black
2021-09-29 7:02 ` [PING 1 Month Reminder][PATCH] " Daniel Black
2021-10-15 15:13 ` [PATCH] " Simon Marchi
1 sibling, 1 reply; 11+ messages in thread
From: Daniel Black @ 2021-09-09 23:05 UTC (permalink / raw)
To: gdb-patches
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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PING 1 Month Reminder][PATCH] PR17917 Lookup remote build-id in remote binaries
2021-09-09 23:05 ` Daniel Black
@ 2021-09-29 7:02 ` Daniel Black
2021-10-09 7:03 ` [PING 6 Weeks][PATCH] " Daniel Black
2021-10-16 7:04 ` [PING 7 Week Reminder][PATCH] " Daniel Black
0 siblings, 2 replies; 11+ messages in thread
From: Daniel Black @ 2021-09-29 7:02 UTC (permalink / raw)
To: gdb-patches
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
> >
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PING 6 Weeks][PATCH] PR17917 Lookup remote build-id in remote binaries
2021-09-29 7:02 ` [PING 1 Month Reminder][PATCH] " Daniel Black
@ 2021-10-09 7:03 ` Daniel Black
2021-10-16 7:04 ` [PING 7 Week Reminder][PATCH] " Daniel Black
1 sibling, 0 replies; 11+ messages in thread
From: Daniel Black @ 2021-10-09 7:03 UTC (permalink / raw)
To: gdb-patches
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
> > >
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] PR17917 Lookup remote build-id in remote binaries
2021-08-28 5:35 [PATCH] PR17917 Lookup remote build-id in remote binaries Daniel Black
2021-09-09 23:05 ` Daniel Black
@ 2021-10-15 15:13 ` Simon Marchi
2021-10-18 5:50 ` Daniel Black
1 sibling, 1 reply; 11+ messages in thread
From: Simon Marchi @ 2021-10-15 15:13 UTC (permalink / raw)
To: Daniel Black, gdb-patches
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PING 7 Week Reminder][PATCH] PR17917 Lookup remote build-id in remote binaries
2021-09-29 7:02 ` [PING 1 Month Reminder][PATCH] " Daniel Black
2021-10-09 7:03 ` [PING 6 Weeks][PATCH] " Daniel Black
@ 2021-10-16 7:04 ` Daniel Black
2021-10-16 10:10 ` Simon Marchi
1 sibling, 1 reply; 11+ messages in thread
From: Daniel Black @ 2021-10-16 7:04 UTC (permalink / raw)
To: gdb-patches
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
> > >
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PING 7 Week Reminder][PATCH] PR17917 Lookup remote build-id in remote binaries
2021-10-16 7:04 ` [PING 7 Week Reminder][PATCH] " Daniel Black
@ 2021-10-16 10:10 ` Simon Marchi
0 siblings, 0 replies; 11+ messages in thread
From: Simon Marchi @ 2021-10-16 10:10 UTC (permalink / raw)
To: Daniel Black, gdb-patches
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
>>>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] PR17917 Lookup remote build-id in remote binaries
2021-10-15 15:13 ` [PATCH] " Simon Marchi
@ 2021-10-18 5:50 ` Daniel Black
2021-10-18 6:06 ` [PATCHv2] Fix PR gdb/17917 Lookup " Daniel Black
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Black @ 2021-10-18 5:50 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv2] Fix PR gdb/17917 Lookup build-id in remote binaries
2021-10-18 5:50 ` Daniel Black
@ 2021-10-18 6:06 ` Daniel Black
2021-10-18 15:21 ` Simon Marchi
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Black @ 2021-10-18 6:06 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv2] Fix PR gdb/17917 Lookup build-id in remote binaries
2021-10-18 6:06 ` [PATCHv2] Fix PR gdb/17917 Lookup " Daniel Black
@ 2021-10-18 15:21 ` Simon Marchi
2021-10-18 19:12 ` Daniel Black
0 siblings, 1 reply; 11+ messages in thread
From: Simon Marchi @ 2021-10-18 15:21 UTC (permalink / raw)
To: Daniel Black, gdb-patches
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv2] Fix PR gdb/17917 Lookup build-id in remote binaries
2021-10-18 15:21 ` Simon Marchi
@ 2021-10-18 19:12 ` Daniel Black
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Black @ 2021-10-18 19:12 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-10-18 19:12 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-28 5:35 [PATCH] PR17917 Lookup remote build-id in remote binaries Daniel Black
2021-09-09 23:05 ` Daniel Black
2021-09-29 7:02 ` [PING 1 Month Reminder][PATCH] " Daniel Black
2021-10-09 7:03 ` [PING 6 Weeks][PATCH] " Daniel Black
2021-10-16 7:04 ` [PING 7 Week Reminder][PATCH] " Daniel Black
2021-10-16 10:10 ` Simon Marchi
2021-10-15 15:13 ` [PATCH] " Simon Marchi
2021-10-18 5:50 ` Daniel Black
2021-10-18 6:06 ` [PATCHv2] Fix PR gdb/17917 Lookup " Daniel Black
2021-10-18 15:21 ` Simon Marchi
2021-10-18 19:12 ` Daniel Black
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).