Re: [PATCH v2 4/7] gdb, btrace: check inferior_ptid in record_btrace_target::xfer_partial()

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2021-12-02 02:15, Markus Metzger wrote:
> In record_btrace_target::xfer_partial(), we handle accessing read-only
> memory during replay.  This request only makes sense if inferior_ptid is
> either a single light-weight process or a full process.  Check that.
> ---
>  gdb/record-btrace.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
> index a25670e7e24..13665096f14 100644
> --- a/gdb/record-btrace.c
> +++ b/gdb/record-btrace.c
> @@ -1434,6 +1434,7 @@ record_btrace_target::xfer_partial (enum target_object object,
>    /* Filter out requests that don't make sense during replay.  */
>    if (replay_memory_access == replay_memory_access_read_only
>        && !record_btrace_generating_corefile
> +      && inferior_ptid.is_lwp_or_pid ()
>        && record_is_replaying (inferior_ptid))
>      {
>        switch (object)

Given your implementation of is_lwp_or_pid, that means checking that
inferior_ptid is not null_ptid nor minus_one_ptid.

I don't think minus_one_ptid is ever a valid value for inferior_ptid, if
inferior_ptid is minus_one_ptid it means there is clearly a bug
somewhere.  So I think it doesn't make sense to check for that.

As for inferior_ptid being null here... I doubt that there is a
legitimate reason for this to happen... but I suppose that it can
technically happen.  <S-Del>Tthat you could call
switch_to_inferior_no_thread (which sets inferior_ptid to null_ptid) and
then call target_ops::xfer_partial.  Maybe there's some target_object
that read a target-wide property and therefore don't need inferior_ptid
to point to a pid or an lwp?

I would lean towards saying the same thing as for patch 1/7, that this
change only ends up hiding potential future bugs.  But if you can give
me a legitimate scenario for this to happen, then I can be convinced
otherwise.

Simon