* [PATCH] gdb: ensure has dwarf info before reading DWZ file
@ 2024-03-30 11:01 Lancelot SIX
2024-04-01 16:09 ` Tom Tromey
0 siblings, 1 reply; 5+ messages in thread
From: Lancelot SIX @ 2024-03-30 11:01 UTC (permalink / raw)
To: gdb-patches; +Cc: Lancelot SIX
I recent change (e9b738dfbdc "Avoid race when reading dwz file") moved
the call to dwarf2_read_dwz_file from dwarf2_initialize_objfile to
dwarf2_has_info.
Before that patch, dwarf2_initialize_objfile was only called when
dwarf2_has_info returned true, and since that patch it is always called.
When reading a file that has no debug info (.debug_info/.debug_abbrev
sections), but has a .gnu_debugaltlink section, GDB’s behavior is
different. I can observe this when loading
/lib/x86_64-linux-gnu/libtinfo.so on Ubuntu 22.04 (or while debugging
any program dynamically loading this library).
Before e9b738dfbdc, we had:
$ ./gdb/gdb -data-directory ./gdb/data-directory -q /lib/x86_64-linux-gnu/libtinfo.so
Reading symbols from /lib/x86_64-linux-gnu/libtinfo.so...
(No debugging symbols found in /lib/x86_64-linux-gnu/libtinfo.so)
(gdb)
while after we have:
$ ./gdb/gdb -data-directory ./gdb/data-directory -q /lib/x86_64-linux-gnu/libtinfo.so
Reading symbols from /lib/x86_64-linux-gnu/libtinfo.so...
warning: could not find '.gnu_debugaltlink' file for /usr/lib/x86_64-linux-gnu/libtinfo.so.6.3
(No debugging symbols found in /lib/x86_64-linux-gnu/libtinfo.so)
(gdb)
This patch restores the previous behavior of only trying to load the
DWZ file for objfiles when the main part of the debuginfo is present
(i.e. when dwarf2_has_info returns true). We still make sure that
dwarf2_read_dwz_file is called at most once per objfile.
A consequence of this change is that the per_bfd->dwz_file optional
object can now remain empty (instead of containing a nullptr), so also
this patch also adjusts dwarf2_get_dwz_file to account for this
possibility. This effectively reverts the changes to
dwarf2_get_dwz_file done by e9b738dfbdc.
Regression tested on x86_64-linux-gnu Ubuntu 22.04.
---
gdb/dwarf2/dwz.c | 13 +++++++++----
gdb/dwarf2/read.c | 36 +++++++++++++++++++-----------------
2 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/gdb/dwarf2/dwz.c b/gdb/dwarf2/dwz.c
index 1eb4816fb47..c7cdba24076 100644
--- a/gdb/dwarf2/dwz.c
+++ b/gdb/dwarf2/dwz.c
@@ -282,9 +282,14 @@ dwarf2_read_dwz_file (dwarf2_per_objfile *per_objfile)
struct dwz_file *
dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd, bool require)
{
- gdb_assert (per_bfd->dwz_file.has_value ());
- dwz_file *result = per_bfd->dwz_file->get ();
- if (require && result == nullptr)
- error (_("could not read '.gnu_debugaltlink' section"));
+ gdb_assert (!require || per_bfd->dwz_file.has_value ());
+
+ dwz_file *result = nullptr;
+ if (per_bfd->dwz_file.has_value ())
+ {
+ result = per_bfd->dwz_file->get ();
+ if (require && result == nullptr)
+ error (_("could not read '.gnu_debugaltlink' section"));
+ }
return result;
}
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 31313bc88b3..9e37011ddf0 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1362,11 +1362,11 @@ dwarf2_has_info (struct objfile *objfile,
return false;
dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
+ bool just_created = false;
if (per_objfile == NULL)
{
dwarf2_per_bfd *per_bfd;
- bool just_created = false;
/* We can share a "dwarf2_per_bfd" with other objfiles if the
BFD doesn't require relocations.
@@ -1398,27 +1398,29 @@ dwarf2_has_info (struct objfile *objfile,
}
per_objfile = dwarf2_objfile_data_key.emplace (objfile, objfile, per_bfd);
+ }
+
+ const bool has_info = (!per_objfile->per_bfd->info.is_virtual
+ && per_objfile->per_bfd->info.s.section != nullptr
+ && !per_objfile->per_bfd->abbrev.is_virtual
+ && per_objfile->per_bfd->abbrev.s.section != nullptr);
- if (just_created)
+ if (just_created && has_info)
+ {
+ /* Try to fetch any potential dwz file early, while still on
+ the main thread. Also, be sure to do it just once per
+ BFD, to avoid races. */
+ try
{
- /* Try to fetch any potential dwz file early, while still on
- the main thread. Also, be sure to do it just once per
- BFD, to avoid races. */
- try
- {
- dwarf2_read_dwz_file (per_objfile);
- }
- catch (const gdb_exception_error &err)
- {
- warning (_("%s"), err.what ());
- }
+ dwarf2_read_dwz_file (per_objfile);
+ }
+ catch (const gdb_exception_error &err)
+ {
+ warning (_("%s"), err.what ());
}
}
- return (!per_objfile->per_bfd->info.is_virtual
- && per_objfile->per_bfd->info.s.section != NULL
- && !per_objfile->per_bfd->abbrev.is_virtual
- && per_objfile->per_bfd->abbrev.s.section != NULL);
+ return has_info;
}
/* See declaration. */
base-commit: 13ed3225004896142dcb0fbe24411b66f17dfc8e
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gdb: ensure has dwarf info before reading DWZ file
2024-03-30 11:01 [PATCH] gdb: ensure has dwarf info before reading DWZ file Lancelot SIX
@ 2024-04-01 16:09 ` Tom Tromey
2024-04-02 8:40 ` Six, Lancelot
0 siblings, 1 reply; 5+ messages in thread
From: Tom Tromey @ 2024-04-01 16:09 UTC (permalink / raw)
To: Lancelot SIX; +Cc: gdb-patches
>>>>> "Lancelot" == Lancelot SIX <lancelot.six@amd.com> writes:
Lancelot> A consequence of this change is that the per_bfd->dwz_file optional
Lancelot> object can now remain empty (instead of containing a nullptr), so also
Lancelot> this patch also adjusts dwarf2_get_dwz_file to account for this
Lancelot> possibility. This effectively reverts the changes to
Lancelot> dwarf2_get_dwz_file done by e9b738dfbdc.
I'm curious about the motivation for this change.
Can dwarf2_get_dwz_file be called when dwarf2_has_info returns false?
Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] gdb: ensure has dwarf info before reading DWZ file
2024-04-01 16:09 ` Tom Tromey
@ 2024-04-02 8:40 ` Six, Lancelot
2024-04-02 13:16 ` Tom Tromey
0 siblings, 1 reply; 5+ messages in thread
From: Six, Lancelot @ 2024-04-02 8:40 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
[AMD Official Use Only - General]
> Lancelot> A consequence of this change is that the per_bfd->dwz_file
> optional
> Lancelot> object can now remain empty (instead of containing a nullptr), so
> also
> Lancelot> this patch also adjusts dwarf2_get_dwz_file to account for this
> Lancelot> possibility. This effectively reverts the changes to
> Lancelot> dwarf2_get_dwz_file done by e9b738dfbdc.
>
> I'm curious about the motivation for this change.
> Can dwarf2_get_dwz_file be called when dwarf2_has_info returns false?
>
> Tom
It can be called when doing "save gdb-index" (dwarf2/index-write.c: save_gdb_index_command). This case is exercised by "gdb.dwarf2/gdb-index-nodebug.exp".
Maybe a better approach could be to modify save_gdb_index_command to check if there is any debug info before calling write_dwarf_index, but the current try/catch approach is the construct currently taking core of that. Having dwarf2_get_dwz_file return nullptr in that case does made sense to me, but I can also go another direction.
Best,
Lancelot.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gdb: ensure has dwarf info before reading DWZ file
2024-04-02 8:40 ` Six, Lancelot
@ 2024-04-02 13:16 ` Tom Tromey
2024-04-03 12:55 ` Lancelot SIX
0 siblings, 1 reply; 5+ messages in thread
From: Tom Tromey @ 2024-04-02 13:16 UTC (permalink / raw)
To: Six, Lancelot; +Cc: Tom Tromey, gdb-patches
> It can be called when doing "save gdb-index" (dwarf2/index-write.c:
> save_gdb_index_command). This case is exercised by
> "gdb.dwarf2/gdb-index-nodebug.exp".
> Maybe a better approach could be to modify save_gdb_index_command to
> check if there is any debug info before calling write_dwarf_index, but
> the current try/catch approach is the construct currently taking core
> of that. Having dwarf2_get_dwz_file return nullptr in that case does
> made sense to me, but I can also go another direction.
This seems fine, I don't think you need to change anything.
Approved-By: Tom Tromey <tom@tromey.com>
Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gdb: ensure has dwarf info before reading DWZ file
2024-04-02 13:16 ` Tom Tromey
@ 2024-04-03 12:55 ` Lancelot SIX
0 siblings, 0 replies; 5+ messages in thread
From: Lancelot SIX @ 2024-04-03 12:55 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
> Approved-By: Tom Tromey <tom@tromey.com>
>
> Tom
Thanks,
I have just pushed this patch.
Best,
Lancelot.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-04-03 12:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-30 11:01 [PATCH] gdb: ensure has dwarf info before reading DWZ file Lancelot SIX
2024-04-01 16:09 ` Tom Tromey
2024-04-02 8:40 ` Six, Lancelot
2024-04-02 13:16 ` Tom Tromey
2024-04-03 12:55 ` Lancelot SIX
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox