Re: [PATCH v2 02/24] Don't rely on inferior_ptid in record_full_wait

Pedro Alves
2019-12-20T17:49:51Z

On 11/1/19 2:54 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> The multi-target patch sets inferior_ptid to null_ptid before handling
> Pedro> a target event, and thus before calling target_wait, in order to catch
> Pedro> places in target_ops::wait implementations that are incorrectly
> Pedro> relying on inferior_ptid (which could otherwise be a ptid of a
> Pedro> different target, for example).
>
> I think it would be good to add a comment before target_ops::wait
> explaining what is required from its implementation. If other
> target_ops methods also cannot rely on inferior_ptid, then that
> documentation should be updated as well. This would make it simpler to
> know how to update an existing target, or to write a new target.

How does this sound?

diff --git i/gdb/target.h w/gdb/target.h
index 1bb7276673..14a7d3e61f 100644
--- i/gdb/target.h
+++ w/gdb/target.h
@@ -478,6 +478,13 @@ struct target_ops
TARGET_DEFAULT_NORETURN (noprocess ());
virtual void commit_resume ()
TARGET_DEFAULT_IGNORE ();
+ /* See target_wait's description. Note that implementations of
+ this method must not assume that inferior_ptid on entry is
+ pointing at the thread or inferior that ends up reporting an
+ event. The reported event could be for some other thread in
+ the current inferior or even for a different process of the
+ current target. inferior_ptid may also be null_ptid on
+ entry. */
virtual ptid_t wait (ptid_t, struct target_waitstatus *,
int TARGET_DEBUG_PRINTER (target_debug_print_options))
TARGET_DEFAULT_FUNC (default_target_wait);

Thanks,
Pedro Alves