darwin: Do not add a dummy thread
authorSimon Marchi <simon.marchi@ericsson.com>
Tue, 27 Jun 2017 08:55:36 +0000 (10:55 +0200)
committerSimon Marchi <simon.marchi@ericsson.com>
Tue, 27 Jun 2017 08:56:53 +0000 (10:56 +0200)
Starting a process on macOS/Darwin currently leads to this error:

/Users/simark/src/binutils-gdb/gdb/darwin-nat.c:383: internal-error: void darwin_check_new_threads(struct inferior *): Assertion `tp' failed.

with the corresponding partial backtrace (sorry, taken with lldb,
because well, gdb is broken :)):

    frame #9: 0x000000010004605a gdb`darwin_check_new_threads(inf=0x0000000100edf670) at darwin-nat.c:383
    frame #10: 0x0000000100045848 gdb`darwin_init_thread_list(inf=0x0000000100edf670) at darwin-nat.c:1710
    frame #11: 0x00000001000452f8 gdb`darwin_ptrace_him(pid=8375) at darwin-nat.c:1792
    frame #12: 0x0000000100041d95 gdb`fork_inferior(...) at fork-inferior.c:440
    frame #13: 0x0000000100043f82 gdb`darwin_create_inferior(...) at darwin-nat.c:1841
    frame #14: 0x000000010034ac32 gdb`run_command_1(args=0x0000000000000000, from_tty=1, tbreak_at_main=1) at infcmd.c:611

The issue was introduced by commit

  "Share fork_inferior et al with gdbserver"

because it changed the place where the dummy thread (pid, 0, 0) is added,
relative to the call to the init_trace_fun callback.  In this callback, darwin
checks for new threads in the program (there should be exactly one) in order to
update this dummy thread with the right tid.  Previously, things happened in
this order:

 - fork_inferior calls fork()
 - fork_inferior adds dummy thread
 - fork_inferior calls init_trace_fun callback, which updates the dummy
   thread info

Following the commit mentioned above, the new thread is added in the
darwin-nat code, after having called fork_inferior (in
darwin_create_inferior).  So gdb tries to do things in this order:

 - fork_inferior calls fork()
 - fork_inferior calls init_trace_fun callback, which tries to update
   the dummy thread info
 - darwin_create_inferior adds the dummy thread

The error happens while trying to update the dummy thread that has not
been added yet.

I don't think this dummy thread is necessary for darwin.  Previously, it
was fork_inferior that was adding this thread, for all targets, so
darwin had to deal with it.  Now that it's done by targets themselves,
we can just skip that on darwin.  darwin_check_new_threads called
indirectly by init_trace_fun/darwin_ptrace_him will simply notice the
new thread and add it with the right information.

My level of testing was: try to start a process and try to attach to a
process, and it seems to work somewhat like it did before.  I tried to
run the testsuite, but it leaves a huge amount of zombie processes that
launchd doesn't seem to reap, leading to exhaustion of system resources
(number of processes).

gdb/ChangeLog:

* darwin-nat.c (darwin_check_new_threads): Don't handle dummy
thread.
(darwin_init_thread_list): Don't update dummy thread.
(darwin_create_inferior, darwin_attach): Don't add a dummy thread.

gdb/ChangeLog
gdb/darwin-nat.c

index ac5965a04f44678de9e3afd24afe74d2f177072d..065f33c0a37903d2a26483ecff9dbaa3dea1576a 100644 (file)
@@ -1,3 +1,10 @@
+2017-06-27  Simon Marchi  <simon.marchi@ericsson.com>
+
+       * darwin-nat.c (darwin_check_new_threads): Don't handle dummy
+       thread.
+       (darwin_init_thread_list): Don't update dummy thread.
+       (darwin_create_inferior, darwin_attach): Don't add a dummy thread.
+
 2017-06-26  Simon Marchi  <simon.marchi@ericsson.com>
 
        * record-full.c (netorder16): Remove.
index cd6724904d6f77e88f41e333ef1f49cfb220ad20..b721da86b818068fb4df95683f68b5655ef7fe44 100644 (file)
@@ -367,29 +367,14 @@ darwin_check_new_threads (struct inferior *inf)
       if (new_ix < new_nbr && (old_ix == old_nbr || new_id < old_id))
        {
          /* A thread was created.  */
-         struct thread_info *tp;
          struct private_thread_info *pti;
 
          pti = XCNEW (struct private_thread_info);
          pti->gdb_port = new_id;
          pti->msg_state = DARWIN_RUNNING;
 
-         if (old_nbr == 0 && new_ix == 0)
-           {
-             /* A ptid is create when the inferior is started (see
-                fork-child.c) with lwp=tid=0.  This ptid will be renamed
-                later by darwin_init_thread_list ().  */
-             tp = find_thread_ptid (ptid_build (inf->pid, 0, 0));
-             gdb_assert (tp);
-             gdb_assert (tp->priv == NULL);
-             tp->priv = pti;
-           }
-         else
-           {
-             /* Add the new thread.  */
-             tp = add_thread_with_info
-               (ptid_build (inf->pid, 0, new_id), pti);
-           }
+         /* Add the new thread.  */
+         add_thread_with_info (ptid_build (inf->pid, 0, new_id), pti);
          VEC_safe_push (darwin_thread_t, thread_vec, pti);
          new_ix++;
          continue;
@@ -1701,23 +1686,38 @@ darwin_attach_pid (struct inferior *inf)
     push_target (darwin_ops);
 }
 
+/* Get the thread_info object corresponding to this private_thread_info.  */
+
+static struct thread_info *
+thread_info_from_private_thread_info (private_thread_info *pti)
+{
+  struct thread_info *it;
+
+  ALL_THREADS (it)
+    {
+      if (it->priv->gdb_port == pti->gdb_port)
+       break;
+    }
+
+  gdb_assert (it != NULL);
+
+  return it;
+}
+
 static void
 darwin_init_thread_list (struct inferior *inf)
 {
-  darwin_thread_t *thread;
-  ptid_t new_ptid;
-
   darwin_check_new_threads (inf);
 
-  gdb_assert (inf->priv->threads
-             && VEC_length (darwin_thread_t, inf->priv->threads) > 0);
-  thread = VEC_index (darwin_thread_t, inf->priv->threads, 0);
+  gdb_assert (inf->priv->threads != NULL);
+  gdb_assert (VEC_length (darwin_thread_t, inf->priv->threads) > 0);
 
-  /* Note: fork_inferior automatically add a thead but it uses a wrong ptid.
-     Fix up.  */
-  new_ptid = ptid_build (inf->pid, 0, thread->gdb_port);
-  thread_change_ptid (inferior_ptid, new_ptid);
-  inferior_ptid = new_ptid;
+  private_thread_info *first_pti
+    = VEC_index (darwin_thread_t, inf->priv->threads, 0);
+  struct thread_info *first_thread
+    = thread_info_from_private_thread_info (first_pti);
+
+  inferior_ptid = first_thread->ptid;
 }
 
 /* The child must synchronize with gdb: gdb must set the exception port
@@ -1834,23 +1834,10 @@ darwin_create_inferior (struct target_ops *ops,
                        const std::string &allargs,
                        char **env, int from_tty)
 {
-  pid_t pid;
-  ptid_t ptid;
-
   /* Do the hard work.  */
-  pid = fork_inferior (exec_file, allargs, env, darwin_ptrace_me,
-                      darwin_ptrace_him, darwin_pre_ptrace, NULL,
-                      darwin_execvp);
-
-  ptid = pid_to_ptid (pid);
-  /* Return now in case of error.  */
-  if (ptid_equal (inferior_ptid, null_ptid))
-    return;
-
-  /* We have something that executes now.  We'll be running through
-     the shell at this point (if startup-with-shell is true), but the
-     pid shouldn't change.  */
-  add_thread_silent (ptid);
+  fork_inferior (exec_file, allargs, env, darwin_ptrace_me,
+                darwin_ptrace_him, darwin_pre_ptrace, NULL,
+                darwin_execvp);
 }
 \f
 
@@ -1920,9 +1907,6 @@ darwin_attach (struct target_ops *ops, const char *args, int from_tty)
   inferior_appeared (inf, pid);
   inf->attach_flag = 1;
 
-  /* Always add a main thread.  */
-  add_thread_silent (inferior_ptid);
-
   darwin_attach_pid (inf);
 
   darwin_suspend_inferior (inf);