From: Simon Marchi <simon.marchi@efficios.com>
To: Simon Marchi <simon.marchi@efficios.com>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [PATCH 4/5] gdb: add program_space parameter to clear_solib
Date: Tue, 6 Feb 2024 22:37:54 -0500 [thread overview]
Message-ID: <66f89bb0-e088-40ee-8adb-a9b9b069f8b5@efficios.com> (raw)
In-Reply-To: <20240206171514.119244-5-simon.marchi@efficios.com>
On 2/6/24 12:14, Simon Marchi wrote:
> Make the current_program_space reference bubble up one level.
>
> Remove one unnecessary declaration of clear_solib.
>
> Change-Id: I234e2c8c0b71713364fc7b76cee2bee2b026bd6d
> ---
> gdb/corelow.c | 2 +-
> gdb/solib.c | 21 ++++++++++++---------
> gdb/solib.h | 6 +++---
> gdb/symtab.h | 4 ----
> 4 files changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/gdb/corelow.c b/gdb/corelow.c
> index 43c1f69b1317..f291b2aba191 100644
> --- a/gdb/corelow.c
> +++ b/gdb/corelow.c
> @@ -335,7 +335,7 @@ core_target::clear_core ()
>
> /* Clear out solib state while the bfd is still open. See
> comments in clear_solib in solib.c. */
> - clear_solib ();
> + clear_solib (current_program_space);
>
> current_program_space->cbfd.reset (nullptr);
> }
> diff --git a/gdb/solib.c b/gdb/solib.c
> index 98cda039a833..bd69c549b8e8 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -1180,23 +1180,26 @@ solib_keep_data_in_core (CORE_ADDR vaddr, unsigned long size)
> return false;
> }
>
> -/* Called by free_all_symtabs */
> +/* See solib.h. */
>
> void
> -clear_solib (void)
> +clear_solib (program_space *pspace)
> {
> - const solib_ops *ops = gdbarch_so_ops (current_inferior ()->arch ());
> + inferior *inf = find_inferior_for_program_space (pspace);
> + gdb_assert (inf != nullptr);
Thanks to the Linaro CI for pointing it out, this particular change
causes some regressions, for instance in gdb.base/catch-fork-kill.exp.
The assertion fails because inf is nullptr.
We're here:
#0 internal_error_loc (file=0x55adc73f51c0 "/home/smarchi/src/binutils-gdb/gdb/solib.c", line=1189,
fmt=0x55adc73f4b00 "%s: Assertion `%s' failed.") at /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:57
#1 0x000055adcb297e6d in clear_solib (pspace=0x613000011d80) at /home/smarchi/src/binutils-gdb/gdb/solib.c:1189
#2 0x000055adcb29822e in no_shared_libraries (ignored=0x0, from_tty=0) at /home/smarchi/src/binutils-gdb/gdb/solib.c:1250
#3 0x000055adca9f26d4 in program_space::~program_space (this=0x613000011d80, __in_chrg=<optimized out>)
at /home/smarchi/src/binutils-gdb/gdb/progspace.c:120
#4 0x000055adca0c570c in delete_inferior (inf=0x618000225480) at /home/smarchi/src/binutils-gdb/gdb/inferior.c:302
#5 0x000055adca0c801d in prune_inferiors () at /home/smarchi/src/binutils-gdb/gdb/inferior.c:492
What I observe is that the current inferior is #1, bound to pspace #1,
and we're deleting inferior #2, bound to pspace #2. My change doesn't
work, because at this point inferior #2 has been removed from the
inferior list, so find_inferior_for_program_space returns nullptr.
However, the current code seems wrong too, because we're using inferior
#1's arch to get hold of an solib_ops to call ops->clear_solib on,
despite deleting program space #2, which was bound to inferior #2. It
works here, because both inferiors have the same gdbarch with the same
solib_ops. But it would be possible for the two inferiors to have
different gdbarches and solib_ops.
My upcoming solib series should improve the situation, by having a
backlink from program_space to solib_ops. So we won't need an inferior
here in order to get the solib_ops. In the mean time, I'll try to
revert using current_inferior to get the arch and solib_ops, even though
I know it's wrong.
Simon
next prev parent reply other threads:[~2024-02-07 3:37 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-06 17:14 [PATCH 0/5] Random cleanup patches Simon Marchi
2024-02-06 17:14 ` [PATCH 1/5] gdb: add program_space parameter to mark_breakpoints_out Simon Marchi
2024-02-06 17:14 ` [PATCH 2/5] gdb: add inferior parameter to breakpoint_init_inferior Simon Marchi
2024-02-06 17:14 ` [PATCH 3/5] gdb: add program_space parameter to disable_breakpoints_in_shlibs Simon Marchi
2024-02-06 17:14 ` [PATCH 4/5] gdb: add program_space parameter to clear_solib Simon Marchi
2024-02-07 3:37 ` Simon Marchi [this message]
2024-02-06 17:14 ` [PATCH 5/5] gdb: remove unnecessary nullptr check in remove_user_added_objfile 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=66f89bb0-e088-40ee-8adb-a9b9b069f8b5@efficios.com \
--to=simon.marchi@efficios.com \
--cc=gdb-patches@sourceware.org \
/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).