Re: [RFA 4/5] Darwin: fix thread ptid started by fork_inferior

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

On 2018-08-22 06:11, Xavier Roirand wrote:
> When debugging a program on Mac OS X Darwin, gdb stops with:
> 
> Temporary breakpoint 1 at 0x100000fb4: file /tmp/helloworld.c, line 1.
> Starting program: /private/tmp/helloworld
> [New Thread 0xb03 of process 65066]
> [New Thread 0xd03 of process 65066]
> During startup program terminated with signal SIGTRAP, Trace/breakpoint 
> trap.
> 
> When the inferior is started a thread with lwp=tid=0 is created
> and has to be fixed later by darwin_init_thread_list(). Because
> this is not done, GDB does not understand that the SIGTRAP is
> coming from GDB and not the program itself.

I think I have seen this error in my testing yesterday, though it seemed 
intermittent.  If I started a few times in a row, I would often get that 
SIGTRAP, but it would eventually work...  I can't really comment on the 
validity of the fix, so I will assume it's right (I am not at work right 
now so I can't test on the Mac).  But linux-nat does something similar 
(add a pid-only thread, then change it to its real ptid), so there is a 
precedent.

> diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
> index 96f70cf..9ad4a87 100644
> --- a/gdb/darwin-nat.c
> +++ b/gdb/darwin-nat.c
> @@ -344,8 +344,22 @@ darwin_check_new_threads (struct inferior *inf)
>  	  pti->gdb_port = new_id;
>  	  pti->msg_state = DARWIN_RUNNING;
> 
> -	  /* Add the new thread.  */
> -	  add_thread_with_info (ptid_t (inf->pid, 0, new_id), pti);
> +	  if (old_nbr == 0 && new_ix == 0)
> +            {
> +	      /* A ptid is created when the inferior is started (see
> +                 fork-child.c) with lwp=tid=0.  This ptid will be 
> renamed
> +                 later by darwin_init_thread_list (), so find this 
> previous
> +                 thread silently added.  */
> +
> +              struct thread_info *tp = find_thread_ptid (ptid_t
> (inf->pid, 0, 0));
> +              tp->priv.reset (pti);

Put a gdb_assert (tp != nullptr), so that if for some reason the thread 
is not found (because of a GDB bug), we fail with a failed assertion 
rather than a segfault.

> +            }
> +          else
> +            {
> +              /* Add the new thread.  */
> +              add_thread_with_info (ptid_t (inf->pid, 0, new_id), 
> pti);
> +             }
> +
>  	  new_thread_vec.push_back (pti);
>  	  new_ix++;
>  	  continue;
> @@ -1733,6 +1747,8 @@ thread_info_from_private_thread_info
> (darwin_thread_info *pti)
>  static void
>  darwin_init_thread_list (struct inferior *inf)
>  {
> +  ptid_t new_ptid;
> +
>    darwin_check_new_threads (inf);
> 
>    darwin_inferior *priv = get_darwin_inferior (inf);
> @@ -1743,7 +1759,11 @@ darwin_init_thread_list (struct inferior *inf)
>    struct thread_info *first_thread
>      = thread_info_from_private_thread_info (first_pti);
> 
> -  inferior_ptid = first_thread->ptid;
> +  /* Note: fork_inferior automatically add a thread but it uses a 
> wrong ptid.
> +     Fix up.  */
> +  new_ptid = ptid_t (inf->pid, 0, first_pti->gdb_port);
> +  thread_change_ptid (inferior_ptid, new_ptid);
> +  inferior_ptid = new_ptid;

You can declare new_ptid when initializing it.

Simon