This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/3] Pass inferior down to target_detach and to_detach
- From: Pedro Alves <palves at redhat dot com>
- To: Simon Marchi <simon dot marchi at ericsson dot com>, gdb-patches at sourceware dot org
- Date: Thu, 18 Jan 2018 16:41:32 +0000
- Subject: Re: [PATCH 2/3] Pass inferior down to target_detach and to_detach
- Authentication-results: sourceware.org; auth=none
- References: <1514699454-18587-1-git-send-email-simon.marchi@ericsson.com> <1514699454-18587-2-git-send-email-simon.marchi@ericsson.com>
On 12/31/2017 05:50 AM, Simon Marchi wrote:
> The to_detach target_ops method implementations are currently expected
> to work on current_inferior/inferior_ptid. In order to make things more
> explicit, and remove some "shadow" parameter passing through globals,
> this patch adds an "inferior" parameter to to_detach. Implementations
> will be expected to use this instead of relying on the global. However,
> to keep things simple, this patch only does the minimum that is
> necessary to add the parameter. The following patch gives an example of
> how one such implementation would be adapted. If the approach is deemed
> good, we can then look into adapting more implementations. Until then,
> they'll continue to work as they do currently.
On the multi-target work/branch, I ended up hanging the current
target stack to the current inferior. Which means that in practice
we have to switch the current inferior/thread before calling any target
method. I can't see any other practical way. I've pondered making
every target method take an inferior or some kind of "execution
context" object as parameter, but that's a massive undertaking.
The result is still better for relying more on thread pointers
instead of inferior_ptid / ptid_t, though, IMO.
Note that in practice, even with your patch (in master) we still have
to switch the current inferior before calling target_detach anyway,
for making sure things like the current program space is set correctly,
target_gdbarch(), target description, removing breakpoints, accessing memory,
registers, etc., like here:
> /* Note that the detach above makes PARENT_INF dangling. */
> @@ -976,7 +976,7 @@ handle_vfork_child_exec_or_exit (int exec)
> }
> }
>
> - target_detach (0);
> + target_detach (inf->vfork_parent, 0);
... this still relies on the switch_to_thread call a bit above.
Or look at the prepare_to_detach call in target_detach.
All that said, I'm totally fine with incremental progress. Actually,
it's probably the only way to go!
So I'm fine with the patch, though, I think we should add a
temporary assertion in target_detach, like:
gdb_assert (inf == current_inferior ());
until everything beneath either uses an explicit inferior,
or switches the current inferior temporarily.
Thanks,
Pedro Alves