[v2,02/24] Don't rely on inferior_ptid in record_full_wait

Message ID 683ef49e-3485-aa20-62dd-e43e7019ab41@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves Dec. 20, 2019, 5:49 p.m. UTC
  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?



Thanks,
Pedro Alves
  

Comments

Tom Tromey Dec. 20, 2019, 6:57 p.m. UTC | #1
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> How does this sound?

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

Looks good, thanks.

Tom
  

Patch

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);