Re: [PATCH 1/3] gdbserver: hide fork child threads from GDB

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

On 2021-11-26 17:51, Pedro Alves wrote:
> On 2021-11-24 20:04, Simon Marchi via Gdb-patches wrote:
> 
>> I split the field in two (fork_parent / fork_child), I think it's
>> clearer this way, easier to follow which thread is the parent and which
>> is the child, and helps ensure things are consistent.  That simplifies
>> things a bit in linux_set_resume_request.  
> 
> This had been conscientiously done that way to avoid storing redundant
> information.  You could have the same thing with a single field + the
> waitstatus, wrapped in fork_child()/fork_parent() methods.  But I'm fine with
> with that you have.  I do see a different reason for taking the two-fields approach
> though -- you don't have access to the pending waitstatus in common code.
> That would be a better rationale, IMO.  OTOH, I think we will end up needing
> access to the pending waitstatus anyhow.  See below, and the review to patch #3.
> 
>> Currently, when
>> lwp->fork_relative is set, we have to deduce whether this is a parent or
>> child using the pending event.  With separate fork_parent and fork_child
>> fields, it becomes more obvious.  If the thread has a fork parent, then
>> it means it's a fork child, and vice-versa.
> 
> With clone events, in some spots I'll need to be able to know whether
> fork_child being set means the thread is stopped for a fork/vfork or a
> clone event.  Most of the code related to fork_parent/fork_child is the
> same for forks and for clones, but not all.
> 
> Wonder whether we should instead have some:
> 
>   thread_info *target_pending_child (thread_info *parent);
>   thread_info *target_pending_child_parent (thread_info *child);
>   target_waitkind target_pending_child_kind (thread_info *parent);
> 
> target methods instead of moving the fields.  I haven't thought this
> fully through, though.

At first I wasn't sure, I was thinking that having the fields directly
in thread_info would help avoid duplication, in case another OS (e.g.
NetBSD) decided to implement the same thing.  But not really: even if
the fields are in thread_info, the bulk of the work (updating these
fields) is done by the target.  Since the state is managed by the
target, target methods make sense.

I will add a "target_thread_pending_parent" in this patch, a
"target_thread_pending_child" in patch 3, and in your series you can add
a way to get the kind.  Either a new method, or I was thinking of an
output parameter on target_thread_pending_child.  Sounds good?

Simon