From: "Tomar, Sourabh Singh" <SourabhSingh.Tomar@amd.com>
To: Keith Seitz <keiths@redhat.com>,
"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: RE: [PATCH] Fix macro info lookup for binaries containing DWARFv5 line table
Date: Mon, 24 May 2021 11:36:46 +0000 [thread overview]
Message-ID: <DM4PR12MB5295EC527524B0D3719ABEA79D269@DM4PR12MB5295.namprd12.prod.outlook.com> (raw)
In-Reply-To: <22ab603a-35e1-4048-3ccc-6738a13889df@redhat.com>
[AMD Public Use]
Hello Keith,
Could you please share your plan WRT to this patch.
Do you want to take it forward ? or you want me to take this forward.
Thanks,
Sourabh
-----Original Message-----
From: Keith Seitz <keiths@redhat.com>
Sent: Friday, May 14, 2021 8:26 PM
To: Tomar, Sourabh Singh <SourabhSingh.Tomar@amd.com>; gdb-patches@sourceware.org
Subject: Re: [PATCH] Fix macro info lookup for binaries containing DWARFv5 line table
[CAUTION: External Email]
Coincidentally, I happened to be looking at gdb/27306 about a similar problem, and I was curious to see how our patches interacted.
On 5/12/21 10:16 AM, Sourabh Singh Tomar via Gdb-patches wrote:
> As one may notice that dir index `0` will always contain absoulte
> path(Not including the file name as), this causes GDB to append the
> path + filename to construt macro info `main_file->filename`. This
> leads to overriding of the macro filename which will cause macro info
> lookup failure due to mismatch in `main_file->filename` and
> `sal.symtab->filename`.
gdb/27306 highlights a similar problem with gcc using -gdwarf-5:
cat -n undef-macro.c
1 #define FOO 1
2
3 int main(int argc, char **argv)
4 {
5 return FOO;
6 }
$ gcc -gdwarf-5 undef-macro.c -o undef-macro $ ./gdb -nx -q undef-macro -ex start -ex "p FOO"
(gdb) start
Temporary breakpoint 1 at 0x401111: file foo.c, line 5.
Starting program: undef-macro
Temporary breakpoint 1, main (argc=1, argv=0x7fffffffdd48) at undef-macro.c:5
5 return FOO;
No symbol "FOO" in current context.
The problem is that file name returned by file_file_name is /not/ relative to the compilation directory. Ever.
From file_file_name:
const file_entry *fe = file_name_at (file);
if (!IS_ABSOLUTE_PATH (fe->name))
{
const char *dir = fe->include_dir (this);
/* The file at index 0 (valid starting with DWARF 5) is already
relative to the compilation directory. */
if (file != 0)
return gdb::unique_xmalloc_ptr<char> (concat (dir, SLASH_STRING,
fe->name,
(char *) NULL));
Even with your patch, gdb will still return an absolute path for the above test.
As you've already mentioned, macro_start_file and macro_lookup_inclusion require that macro_source_file.filename be relative to the compliation directory. Furthermore, the documentation for macro_source_file.filename is stricter:
/* A source file --- possibly a header file. This filename is relative to
the compilation directory (table->comp_dir), it exactly matches the
symtab->filename content. */
const char *filename;
So if symtab->filename and macro_source_filename are required to be the same, why are we bothering with prepending directories? We already have access to the symtab in `fe'.
This is the version I was playing with...
diff --git a/gdb/dwarf2/line-header.c b/gdb/dwarf2/line-header.c index 7575297f966..117c5a42cc5 100644
--- a/gdb/dwarf2/line-header.c
+++ b/gdb/dwarf2/line-header.c
@@ -69,15 +69,10 @@ line_header::file_file_name (int file) const
{
const file_entry *fe = file_name_at (file);
- if (!IS_ABSOLUTE_PATH (fe->name))
- {
- const char *dir = fe->include_dir (this);
- if (dir != NULL)
- return gdb::unique_xmalloc_ptr<char> (concat (dir, SLASH_STRING,
- fe->name,
- (char *) NULL));
- }
- return make_unique_xstrdup (fe->name);
+ /* macro_source_file requires: "This filename is relative to the
+ compilation directory, it exactly matches the symtab->filename
+ content." */
+ return make_unique_xstrdup (fe->symtab->filename);
}
else
{
By happy coincidence, this has the same effect of your patch, but it also fixes gdb/27306.
I checked the origins of file_file_name and it appears that this function was split out from file_full_name, but this directory prepending bit was never removed to fulfill the "relative" requirement of the stated documentation.
[This is from commit 233d95b548ec948c4a6d01cd05c307385dd615fb.]
I hate to post counter-patches, but in this case, I think we are really both attempting to solve the same design problem with line_header::file_file_name.
> This problem does not pops up with GCC compiled binaries, since GCC
> does not emits DWARFv5 compliant line tables.
This test (as written) does not pass using gcc:
$ make check TESTS=gdb.base/dwarf5-macro.exp [snip] Running /home/keiths/work/gdb/branches/virgin/linux/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/dwarf5-macro.exp ...
FAIL: gdb.base/dwarf5-macro.exp: info macro HELLO_GDB
=== gdb Summary ===
# of unexpected failures 1
However, if I force -gdwarf-5 for gcc, too:
$ make check TESTS=gdb.base/dwarf5-macro.exp RUNTESTFLAGS="unix/-g3/-gdwarf-5"
[snip]
Running /home/keiths/work/gdb/branches/virgin/linux/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/dwarf5-macro.exp ...
=== gdb Summary ===
# of expected passes 1
I suggest that be corrected -- just force gcc to emit dwarf-5 debuginfo as well. IMO, the correct, compiler-agnostic way of dealing with this is via the DWARF assembler, but I am not going to suggest you do that.
I'd be happy if the test simply didn't FAIL on non-clang targets.
Keith
next prev parent reply other threads:[~2021-05-24 11:36 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-12 17:16 Sourabh Singh Tomar
2021-05-12 17:31 ` Tomar, Sourabh Singh
2021-05-13 15:47 ` Tom Tromey
2021-05-13 18:20 ` Simon Marchi
2021-05-14 14:56 ` Keith Seitz
2021-05-14 18:21 ` Tomar, Sourabh Singh
2021-05-14 18:50 ` Simon Marchi
2021-05-14 19:20 ` Tomar, Sourabh Singh
2021-05-14 20:56 ` Simon Marchi
2021-05-24 11:36 ` Tomar, Sourabh Singh [this message]
2021-05-24 18:47 ` Keith Seitz
2021-06-08 18:48 ` Keith Seitz
2021-06-22 17:01 ` [RFC] PING [Re: [PATCH] Fix macro info lookup for binaries containing DWARFv5 line table] Keith Seitz
2021-06-22 20:52 ` Simon Marchi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM4PR12MB5295EC527524B0D3719ABEA79D269@DM4PR12MB5295.namprd12.prod.outlook.com \
--to=sourabhsingh.tomar@amd.com \
--cc=gdb-patches@sourceware.org \
--cc=keiths@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).