[PATCH v2 5/6] Add support for LWP-based threads on FreeBSD.

Pedro Alves palves@redhat.com
Thu Jan 14 15:29:00 GMT 2016


[Dropping binutils.]

Hi John,

This deserves a NEWS entry.

New commands ("set debug fbsd-lwp") need to documented in
the manual, even debug ones.

This generally looks good to me.  A few minor comments inline below.

On 01/13/2016 09:45 PM, John Baldwin wrote:

> +
> +static int
> +fbsd_thread_alive (struct target_ops *ops, ptid_t ptid)
> +{
> +  if (ptid_lwp_p (ptid))
> +    {
> +      struct ptrace_lwpinfo pl;
> +
> +      if (ptrace (PT_LWPINFO, ptid_get_lwp (ptid), (caddr_t)&pl, sizeof pl)
> +	  == -1)

Space after cast.

> +	return 0;
> +#ifdef PL_FLAG_EXITED
> +      if (pl.pl_flags & PL_FLAG_EXITED)
> +	return 0;
> +#endif
> +    }
> +
> +  return 1;
> +}
> +
> +/* Convert PTID to a string.  Returns the string in a static
> +   buffer.  */
> +



> +#ifdef HAVE_STRUCT_PTRACE_LWPINFO_PL_TDNAME
> +/* Return the name assigned to a thread by an application.  Returns
> +   the string in a static buffer.  */
> +
> +static const char *
> +fbsd_thread_name (struct target_ops *self, struct thread_info *thr)
> +{
> +  struct ptrace_lwpinfo pl;
> +  struct kinfo_proc kp;
> +  int pid = ptid_get_pid (thr->ptid);
> +  long lwp = ptid_get_lwp (thr->ptid);
> +  static char buf[64];

Is this the kernel-side size limit?  Worth it of a define/comment.
Mainly looking at the xsnprintf below and wondering whether
a we could see a longer name and thus cause gdb to internal error.

> +
> +  /* Note that ptrace_lwpinfo returns the process command in pl_tdname
> +     if a name has not been set explicitly.  Return a NULL name in
> +     that case.  */
> +  fbsd_fetch_kinfo_proc (pid, &kp);
> +  if (ptrace (PT_LWPINFO, lwp, (caddr_t)&pl, sizeof pl) == -1)
> +    perror_with_name (("ptrace"));

Space after cast.

> +  if (strcmp (kp.ki_comm, pl.pl_tdname) == 0)
> +    return NULL;
> +  xsnprintf (buf, sizeof buf, "%s", pl.pl_tdname);
> +  return buf;
> +}
> +#endif
> +
> +#ifdef PT_LWP_EVENTS
> +/* Enable LWP events for a specific process.
> +
> +   To catch LWP events, PT_LWP_EVENTS is set on every traced process.
> +   This enables stops on the birth for new LWPs (excluding the "main" LWP)
> +   and the death of LWPs (excluding the last LWP in a process).  Note
> +   that unlike fork events, the LWP that creates a new LWP does not
> +   report an event.  */
> +
> +static void
> +fbsd_enable_lwp_events (pid_t pid)
> +{
> +  if (ptrace (PT_LWP_EVENTS, pid, (PTRACE_TYPE_ARG3)0, 1) == -1)
> +    perror_with_name (("ptrace"));

Space after cast.

> +}
> +#endif
> +
> +/* Add threads for any new LWPs in a process.
> +
> +   When LWP events are used, this function is only used to detect existing
> +   threads when attaching to a process.  On older systems, this function is
> +   called to discover new threads each time the thread list is updated.  */
> +
> +static void
> +fbsd_add_threads (pid_t pid)
> +{
> +  struct cleanup *cleanup;
> +  lwpid_t *lwps;
> +  int i, nlwps;
> +
> +  gdb_assert (!in_thread_list (pid_to_ptid (pid)));
> +  nlwps = ptrace (PT_GETNUMLWPS, pid, NULL, 0);
> +  if (nlwps == -1)
> +    perror_with_name (("ptrace"));
> +
> +  lwps = XCNEWVEC (lwpid_t, nlwps);
> +  cleanup = make_cleanup (xfree, lwps);
> +
> +  nlwps = ptrace (PT_GETLWPLIST, pid, (caddr_t)lwps, nlwps);
> +  if (nlwps == -1)
> +    perror_with_name (("ptrace"));
> +
> +  for (i = 0; i < nlwps; i++)
> +    {
> +      ptid_t ptid = ptid_build (pid, lwps[i], 0);
> +
> +      if (!in_thread_list (ptid))
> +	{
> +#ifdef PT_LWP_EVENTS
> +	  struct ptrace_lwpinfo pl;
> +
> +	  /* Don't add exited threads.  Note that this is only called
> +	     when attaching to a multi-threaded process.  */
> +	  if (ptrace (PT_LWPINFO, lwps[i], (caddr_t)&pl, sizeof pl) == -1)

Space after cast.

> +	    perror_with_name (("ptrace"));
> +	  if (pl.pl_flags & PL_FLAG_EXITED)
> +	    continue;
> +#endif
> +	  if (debug_fbsd_lwp)
> +	    fprintf_unfiltered (gdb_stdlog,
> +				"FLWP: adding thread for LWP %u\n",
> +				lwps[i]);
> +	  add_thread (ptid);
> +	}
> +    }
> +  do_cleanups (cleanup);
> +}
> +



> +static void (*super_resume) (struct target_ops *,
> +			     ptid_t,
> +			     int,
> +			     enum gdb_signal);
> +
> +static int
> +resume_one_thread_cb(struct thread_info *tp, void *data)

Space after _cb.

> +{
> +  ptid_t *ptid = data;
> +  int request;
> +
> +  if (ptid_get_pid (tp->ptid) != ptid_get_pid (*ptid))
> +    return 0;
> +
> +  if (ptid_get_lwp (tp->ptid) == ptid_get_lwp (*ptid))
> +    request = PT_RESUME;
> +  else
> +    request = PT_SUSPEND;
> +
> +  if (ptrace (request, ptid_get_lwp (tp->ptid), (caddr_t)0, 0) == -1)

Space after cast.

> +    perror_with_name (("ptrace"));
> +  return 0;
> +}
> +
> +static int
> +resume_all_threads_cb(struct thread_info *tp, void *data)

Missing space.


> +{
> +  ptid_t *filter = data;
> +
> +  if (!ptid_match (tp->ptid, *filter))
> +    return 0;
> +
> +  if (ptrace (PT_RESUME, ptid_get_lwp (tp->ptid), (caddr_t)0, 0) == -1)

Missing space.


> +    perror_with_name (("ptrace"));
> +  return 0;
> +}
> +



> @@ -331,18 +581,80 @@ fbsd_wait (struct target_ops *ops,
>  	  if (ptrace (PT_LWPINFO, pid, (caddr_t)&pl, sizeof pl) == -1)
>  	    perror_with_name (("ptrace"));
>  
> +	  wptid = ptid_build (pid, pl.pl_lwpid, 0);
> +
> +#ifdef PT_LWP_EVENTS
> +	  if (pl.pl_flags & PL_FLAG_EXITED)
> +	    {
> +	      /* If GDB attaches to a multi-threaded process, exiting
> +		 threads might be skipped during fbsd_post_attach that
> +		 have not yet reported their PL_FLAG_EXITED event.
> +		 Ignore EXITED events for an unknown LWP.  */
> +	      if (in_thread_list (wptid))
> +		{
> +		  if (debug_fbsd_lwp)
> +		    fprintf_unfiltered (gdb_stdlog,
> +					"FLWP: deleting thread for LWP %u\n",
> +					pl.pl_lwpid);
> +		  if (print_thread_events)
> +		    printf_unfiltered (_("[%s exited]\n"), target_pid_to_str
> +				       (wptid));
> +		  delete_thread (wptid);
> +		}
> +	      if (ptrace (PT_CONTINUE, pid, (PTRACE_TYPE_ARG3)1, 0) == -1)

Missing space.

> +		perror_with_name (("ptrace"));
> +	      continue;
> +	    }
> +#endif
> +
> +	  /* Switch to an LWP PTID on the first stop in a new process.
> +	     This is done after handling PL_FLAG_EXITED to avoid
> +	     switching to an exited LWP.  It is done before checking
> +	     PL_FLAG_BORN in case the first stop reported after
> +	     attaching to an existing process is a PL_FLAG_BORN
> +	     event.  */
> +	  if (in_thread_list (pid_to_ptid (pid)))
> +	    {
> +	      if (debug_fbsd_lwp)
> +		fprintf_unfiltered (gdb_stdlog,
> +				    "FLWP: using LWP %u for first thread\n",
> +				    pl.pl_lwpid);
> +	      thread_change_ptid (pid_to_ptid (pid), wptid);
> +	    }
> +
> +#ifdef PT_LWP_EVENTS
> +	  if (pl.pl_flags & PL_FLAG_BORN)
> +	    {
> +	      /* If GDB attaches to a multi-threaded process, newborn
> +		 threads might be added by fbsd_add_threads that have
> +		 not yet reported their PL_FLAG_BORN event.  Ignore
> +		 BORN events for an already-known LWP.  */
> +	      if (!in_thread_list (wptid))
> +		{
> +		  if (debug_fbsd_lwp)
> +		    fprintf_unfiltered (gdb_stdlog,
> +					"FLWP: adding thread for LWP %u\n",
> +					pl.pl_lwpid);
> +		  add_thread (wptid);
> +		}
> +	      ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
> +	      return wptid;
> +	    }
> +#endif
> +
>  #ifdef TDP_RFPPWAIT
>  	  if (pl.pl_flags & PL_FLAG_FORKED)
>  	    {
>  	      struct kinfo_proc kp;
> +	      ptid_t child_ptid;
>  	      pid_t child;
>  
>  	      child = pl.pl_child_pid;
>  	      ourstatus->kind = TARGET_WAITKIND_FORKED;
> -	      ourstatus->value.related_pid = pid_to_ptid (child);
>  
>  	      /* Make sure the other end of the fork is stopped too.  */
> -	      if (!fbsd_is_child_pending (child))
> +	      child_ptid = fbsd_is_child_pending (child);
> +	      if (ptid_equal (child_ptid, null_ptid))
>  		{
>  		  pid = waitpid (child, &status, 0);
>  		  if (pid == -1)
> @@ -354,6 +666,7 @@ fbsd_wait (struct target_ops *ops,
>  		    perror_with_name (("ptrace"));
>  
>  		  gdb_assert (pl.pl_flags & PL_FLAG_CHILD);
> +		  child_ptid = ptid_build(child, pl.pl_lwpid, 0);

Missing space.

>  		}
>  
>  	      /* For vfork, the child process will have the P_PPWAIT
> @@ -361,6 +674,7 @@ fbsd_wait (struct target_ops *ops,
>  	      fbsd_fetch_kinfo_proc (child, &kp);
>  	      if (kp.ki_flag & P_PPWAIT)
>  		ourstatus->kind = TARGET_WAITKIND_VFORKED;
> +	      ourstatus->value.related_pid = child_ptid;
>  
>  	      return wptid;
>  	    }
> @@ -370,7 +684,7 @@ fbsd_wait (struct target_ops *ops,
>  	      /* Remember that this child forked, but do not report it
>  		 until the parent reports its corresponding fork
>  		 event.  */
> -	      fbsd_remember_child (ptid_get_pid (wptid));
> +	      fbsd_remember_child (wptid);
>  	      continue;
>  	    }
>  #endif
> @@ -449,13 +763,19 @@ fbsd_enable_follow_fork (pid_t pid)
>    if (ptrace (PT_FOLLOW_FORK, pid, (PTRACE_TYPE_ARG3)0, 1) == -1)
>      perror_with_name (("ptrace"));
>  }
> +#endif
>  


> +/* Provide a prototype to silence -Wmissing-prototypes.  */
> +extern initialize_file_ftype _initialize_fbsd_nat;
> +
> +void
> +_initialize_fbsd_nat (void)
> +{
> +#ifdef PT_LWPINFO
> +  add_setshow_boolean_cmd ("fbsd-lwp", class_maintenance,
> +			   &debug_fbsd_lwp, _("\
> +Set debugging of FreeBSD lwp module."), _("\
> +Show debugging of FreeBSD lwp module."), _("\
> +Enables printf debugging output."),
> +			   NULL,
> +			   NULL,

Please implement a show callback, for i18n.

Thanks,
Pedro Alves



More information about the Gdb-patches mailing list