gdb: call update_thread_list after completing an inferior call
authorAndrew Burgess <aburgess@redhat.com>
Tue, 10 Oct 2023 09:00:10 +0000 (10:00 +0100)
committerAndrew Burgess <aburgess@redhat.com>
Wed, 8 Nov 2023 13:28:09 +0000 (13:28 +0000)
I noticed that if GDB is using a remote or extended-remote target,
then, if an inferior call caused a new thread to appear, or for an
existing thread to exit, then these events are not reported to the
user.

The problem is that for these targets GDB relies on a call to
update_thread_list to learn about changes to the inferior's thread
list.

If GDB doesn't pass through the normal stop code then GDB will not
call update_thread_list, and so will not report changes in the thread
list.

This commit adds an additional update_thread_list call, after which
thread events are correctly reported.

gdb/infcall.c
gdb/testsuite/gdb.threads/infcall-thread-announce.c [new file with mode: 0644]
gdb/testsuite/gdb.threads/infcall-thread-announce.exp [new file with mode: 0644]

index 0f9ad34bbb4e8e0fc8c016004eb317d4b805a7d8..e62c2205808a729b2db1ce4415b73afe8b497bb2 100644 (file)
@@ -1388,6 +1388,13 @@ call_function_by_hand_dummy (struct value *function,
 
     gdb::observers::inferior_call_post.notify (call_thread_ptid, funaddr);
 
+
+    /* As the inferior call failed, we are about to throw an error, which
+       will be caught and printed somewhere else in GDB.  We want new threads
+       to be printed before the error message, otherwise it looks odd; the
+       threads appear after GDB has reported a stop.  */
+    update_thread_list ();
+
     if (call_thread->state != THREAD_EXITED)
       {
        /* The FSM should still be the same.  */
diff --git a/gdb/testsuite/gdb.threads/infcall-thread-announce.c b/gdb/testsuite/gdb.threads/infcall-thread-announce.c
new file mode 100644 (file)
index 0000000..2ed246c
--- /dev/null
@@ -0,0 +1,192 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <assert.h>
+#include <pthread.h>
+
+/* Test can create at most this number of extra threads.  */
+#define MAX_THREADS 3
+
+/* For convenience.  */
+#define FALSE 0
+#define TRUE (!FALSE)
+
+/* Controls a thread created by this test.  */
+struct thread_descriptor
+{
+  /* The pthread handle.  Not valid unless STARTED is true.  */
+  pthread_t thr;
+
+  /* This field is set to TRUE when a thread has been created, otherwise,
+     is false.  */
+  int started;
+
+  /* A condition variable and mutex, used for synchronising between the
+     worker thread and the main thread.  */
+  pthread_cond_t cond;
+  pthread_mutex_t mutex;
+};
+
+/* Keep track of worker threads.  */
+struct thread_descriptor threads[MAX_THREADS];
+
+/* Worker thread function.  Doesn't do much.  Synchronise with the main
+   thread, mark the thread as started, and then block waiting for the main
+   thread.  Once the main thread wakes us, this thread exits.
+
+   ARG is a thread_descriptor shared with the main thread.  */
+
+void *
+thread_function (void *arg)
+{
+  int res;
+  struct thread_descriptor *thread = (struct thread_descriptor *) arg;
+
+  /* Acquire the thread's lock.  Initially the main thread holds this lock,
+     but releases it when the main thread enters a pthread_cond_wait.  */
+  res = pthread_mutex_lock (&thread->mutex);
+  assert (res == 0);
+
+  /* Mark the thread as started.  */
+  thread->started = TRUE;
+
+  /* Signal the main thread to tell it we are started.  The main thread
+     will still be blocked though as we hold the thread's lock.  */
+  res = pthread_cond_signal (&thread->cond);
+  assert (res == 0);
+
+  /* Now wait until the main thread tells us to exit.  By entering this
+     pthread_cond_wait we release the lock, which allows the main thread to
+     resume.  */
+  res = pthread_cond_wait (&thread->cond, &thread->mutex);
+  assert (res == 0);
+
+  /* The main thread woke us up.  We reacquired the thread lock as we left
+     the pthread_cond_wait, so release the lock now.  */
+  res = pthread_mutex_unlock (&thread->mutex);
+  assert (res == 0);
+
+  return NULL;
+}
+
+/* Start a new thread within the global THREADS array.  Return true if a
+   new thread was started, otherwise return false.  */
+
+int
+start_thread ()
+{
+  int idx, res;
+
+  for (idx = 0; idx < MAX_THREADS; ++idx)
+    if (!threads[idx].started)
+      break;
+
+  if (idx == MAX_THREADS)
+    return FALSE;
+
+  /* Acquire the thread lock before starting the new thread.  */
+  res = pthread_mutex_lock (&threads[idx].mutex);
+  assert (res == 0);
+
+  /* Start the new thread.  */
+  res = pthread_create (&threads[idx].thr, NULL,
+                       thread_function, &threads[idx]);
+  assert (res == 0);
+
+  /* Unlock and wait.  The thread signals us once it is ready.  */
+  res = pthread_cond_wait (&threads[idx].cond, &threads[idx].mutex);
+  assert (res == 0);
+
+  /* The worker thread is now blocked in a pthread_cond_wait and we
+     reacquired the lock as we left our own pthread_cond_wait above.  */
+  res = pthread_mutex_unlock (&threads[idx].mutex);
+  assert (res == 0);
+
+  return TRUE;
+}
+
+/* Stop a thread from within the global THREADS array.  Return true if a
+   thread was stopped, otherwise return false.  */
+int
+stop_thread ()
+{
+  /* Look for a thread that is started.  */
+  for (int idx = 0; idx < MAX_THREADS; ++idx)
+    if (threads[idx].started)
+      {
+       int res;
+
+       /* Grab the thread lock.  */
+       res = pthread_mutex_lock (&threads[idx].mutex);
+       assert (res == 0);
+
+       /* Signal the worker thread, this wakes it up, but it can't exit
+          until it acquires the thread lock, which we currently hold.  */
+       res = pthread_cond_signal (&threads[idx].cond);
+       assert (res == 0);
+
+       /* Release the thread lock, this allows the worker thread to exit.  */
+       res = pthread_mutex_unlock (&threads[idx].mutex);
+       assert (res == 0);
+
+       /* Now wait for the thread to exit.  */
+       void *retval;
+       res = pthread_join (threads[idx].thr, &retval);
+       assert (res == 0);
+       assert (retval == NULL);
+
+       /* Now the thread has exited, mark it as no longer started.  */
+       assert (threads[idx].started);
+       threads[idx].started = FALSE;
+
+       return TRUE;
+      }
+
+  return FALSE;
+}
+
+void
+init_descriptor_array ()
+{
+  for (int i = 0; i < MAX_THREADS; ++i)
+    {
+      int res;
+
+      threads[i].started = FALSE;
+      res = pthread_cond_init (&threads[i].cond, NULL);
+      assert (res == 0);
+      res = pthread_mutex_init (&threads[i].mutex, NULL);
+      assert (res == 0);
+    }
+}
+
+void
+breakpt ()
+{
+  /* Nothing.  */
+}
+
+int
+main ()
+{
+  init_descriptor_array ();
+  breakpt ();
+  start_thread ();
+  stop_thread ();
+  breakpt ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/infcall-thread-announce.exp b/gdb/testsuite/gdb.threads/infcall-thread-announce.exp
new file mode 100644 (file)
index 0000000..f480b43
--- /dev/null
@@ -0,0 +1,71 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that thread creation and thread exit events are correctly
+# announced when a thread starts, or exits, as a result of an inferior
+# function call from GDB.
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug pthreads}] == -1} {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_breakpoint breakpt
+gdb_continue_to_breakpoint "first breakpt call"
+
+set thr_count 1
+
+proc check_thread_count { adjustment } {
+    incr ::thr_count $adjustment
+
+    gdb_test "p \$_inferior_thread_count" \
+       "^\\\$$::decimal = $::thr_count"
+}
+
+with_test_prefix "starting threads" {
+    gdb_test "call (void) start_thread()" \
+       "\\\[New Thread \[^\r\n\]+\\\]" \
+       "start a new thread, return value discarded"
+    check_thread_count +1
+
+    foreach_with_prefix call_type { print call } {
+       gdb_test "$call_type start_thread()" \
+           "\\\[New Thread \[^\r\n\]+\\\]\r\n\\\$$decimal = 1" \
+           "start another new thread"
+       check_thread_count +1
+    }
+}
+
+with_test_prefix "stopping threads" {
+    gdb_test "call (void) stop_thread()" \
+       "\\\[Thread \[^\r\n\]+ exited\\\]" \
+       "stop a thread, return value discarded"
+    check_thread_count -1
+
+    foreach_with_prefix call_type { print call } {
+       gdb_test "$call_type stop_thread()" \
+           "\\\[Thread \[^\r\n\]+ exited\\\]\r\n\\\$$decimal = 1" \
+           "stop another thread"
+       check_thread_count -1
+    }
+}
+
+gdb_continue_to_breakpoint "second breakpt call"
+gdb_continue_to_end