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.