Re: [PATCH] Linux/ptrace: don't convert ptids when asking inf-ptrace layer to resume LWP

[Pedro Alves <palves at redhat dot com>]

On 03/03/2015 04:04 PM, Mark Kettenis wrote:
>> > Date: Tue, 03 Mar 2015 15:12:48 +0000
>> > From: Pedro Alves <palves@redhat.com>
>> > 


>> > Or is it that if there are multiple kernel threads in the
>> > process, ptrace(PTRACE_CONT, PID) on bsd actually resumes
>> > them all?  Then other things must be broken anyway.
> I can only speak for OpenBSD here, but yes, on OpenBSD, if you pass
> "PID" here, all threads within a process are resumed.  If you want to
> resume an individual thread, you need to pass its thread ID.  Thread
> IDs are also integers, but start at THREAD_PID_OFFSET, which is
> currently defined as 1000000.
> 
> Are things broken?  Perhaps.  GDB used to properly support an
> all-stop/all-go model, and things still seem to work mostly ok.
> Perhaps this diff will actually make things better if there are places
> where GDB wants to resume or step a single thread that isn't the
> thread that stopped the process in the first place.

Yes, I assume so.  Things like schedlock and stepping over a
breakpoint must have been subtly broken thus far, if they have
been letting all threads run while core gdb wants only one
thread to run.

> 
> I've always considered it a serious flaw that Linux doesn't have a way
> to resume the entire process and that we need almost 5000 lines of
> code to deal with the consequences.

:-)

> 
>> > I was assuming that on BSD targets that use this method,
>> > there would only be one thread in the core thread list, and
>> > it would either have LWPID==0, or have PID==LWPID, thus it didn't
>> > matter if get_ptrace_pid returned the PID or the LWPID.
> That assumption is incorrect.  

OK.

> I see that this assumption has made its way into infrun.c:
> 
>           inferior_ptid = ptid_build (child_pid, child_pid, 0);
> 
> That's wrong.  The OS-specific code should fill in the LWPID part with
> the appropriate value by using thread_change_ptid().  AFAICT, the
> linux-nat.c code does that properly.

I think you're pointing at the follow fork code.  That was born out
of a generalization of code that used to live in linux-nat.c, inf-ptrace.c,
etc. (d83ad864).  The old inf-ptrace.c code used to do:

      /* Switch inferior_ptid out of the parent's way.  */
      inferior_ptid = pid_to_ptid (fpid);

      /* Delete the parent.  */
      detach_inferior (pid);

      add_thread_silent (inferior_ptid);


'child_pid' is set at the top of follow_fork_inferior, like:

  child_pid
    = ptid_get_pid (inferior_thread ()->pending_follow.value.related_pid);

I think I even remember that long ago "related_pid" used to be
a single int, not a ptid.

This is again another case of losing information.  I think we could
easily make that:

  child_pid = inferior_thread ()->pending_follow.value.related_pid;

and drop that ptid_build, and things should work everywhere.

I have no idea if "follow fork" actually works on BSD targets correctly.
A buildbot slave would catch breakages like these.  ( hint :-) )

> 
>> > If there anything that actually creates other threads with
>> > a different LWPID on these targets?
> The initial thread ID of an OpenBSD process will be PID + THREAD_PID_OFFSET.
> 

Thanks, didn't know that.

Pedro Alves