]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
gdb: fix for 'set suppress-cli-notifications on' missed case
authorAndrew Burgess <aburgess@redhat.com>
Sun, 28 Sep 2025 15:16:53 +0000 (16:16 +0100)
committerAndrew Burgess <aburgess@redhat.com>
Wed, 8 Oct 2025 08:25:50 +0000 (09:25 +0100)
I noticed this behaviour:

  (gdb) info threads
    Id   Target Id                               Frame
    1    Thread 0xf7dbc700 (LWP 3161872) "thr" 0xf7eb2888 in clone () from /lib/libc.so.6
  * 2    Thread 0xf7dbbb40 (LWP 3161884) "thr" breakpt () at thr.c:19
  (gdb) set suppress-cli-notifications on
  (gdb) thread 1
  (gdb) thread 1
  [Switching to thread 1 (Thread 0xf7dbc700 (LWP 3161872))]
  #0  0xf7eb2888 in clone () from /lib/libc.so.6
  (gdb)

I think that the second 'thread 1' should not produce any output just
like the 'inferior' command, continuing in the same GDB session:

  (gdb) inferior 1
  (gdb)

Without suppress-cli-notifications we would see an inferior, thread,
and frame being printed, but with suppress-cli-notifications set to
on, we get no output.

The difference in behaviours is that in inferior_command (inferior.c),
we always call notify_user_selected_context_changed, even in the case
where the inferior doesn't actually change.

In thread_command (thread.c), we have some code that catches the
thread not changed case, and calls print_selected_thread_frame.  The
notify_user_selected_context_changed function is only called if the
thread actually changes.

I did consider simply extending thread_command to check the global
cli_suppress_notification.user_selected_context state and skipping the
call to print_selected_thread_frame if suppression is on.

However, I realised that calling print_selected_thread_frame actually
introduces a bug.

When the 'thread' command is used to select the currently selected
thread, GDB still calls 'thread_selected'.  And 'thread_select' always
selects frame #0 within that thread, consider this session:

  (gdb) info threads
    Id   Target Id                              Frame
    1    Thread 0xf7dbc700 (LWP 723986) "thr" 0xf7eb2888 in clone () from /lib/libc.so.6
  * 2    Thread 0xf7dbbb40 (LWP 723990) "thr" breakpt () at thr.c:19
  (gdb) bt
  #0  breakpt () at thr.c:19
  #1  0x080491fd in thread_worker (arg=0xffff9514) at thr.c:31
  #2  0xf7f7667e in start_thread () from /lib/libpthread.so.0
  #3  0xf7eb289a in clone () from /lib/libc.so.6
  (gdb) frame 3
  #3  0xf7eb289a in clone () from /lib/libc.so.6
  (gdb) thread 2
  [Switching to thread 2 (Thread 0xf7dbbb40 (LWP 723990))]
  #0  breakpt () at thr.c:19
  19   while (stop)
  (gdb) frame
  #0  breakpt () at thr.c:19
  19   while (stop)
  (gdb)

Notice that the frame resets back to frame #0.

By only calling print_selected_thread_frame, and not calling
notify_user_selected_context_changed, this means that GDB will fail to
emit an MI async notification.  It is this async notification which
tells MI consumers that the frame has been reset to #0.

And so, I think that the correct solution is, like with the 'inferior'
command, to always call notify_user_selected_context_changed.

This does mean that in some cases unnecessary MI notifications can be
emitted, however, an MI consumer should be able to handle these.  We
could try to avoid these, but we would need to extend thread_command
to check that neither the thread OR frame has changed after the call
to thread_select, and right now, I'm not sure it's worth adding the
extra complexity.

I've rewritten the gdb.base/cli-suppress-notification.exp test to
cover more cases, especially the reselecting the same thread case.
And I've updated the gdb.mi/user-selected-context-sync.exp test to
allow for the additional MI notifications that are emitted, and to
check the frame reset case.

While working on this change, I did wonder about calls to
notify_user_selected_context_changed for frame related commands.  In
places we do elide calls to notify_user_selected_context_changed if
the frame hasn't changed.  I wondered if there were more bugs here?

I don't think there are though.  While changing the inferior will also
change the selected thread, and the selected frame.  And changing the
thread will also change the selected frame.  Changing the frame is the
"inner most" context related thing that can be changed.  There are no
side effect changes that also need to be notified, so for these cases,
I think we are fine.

Also in infrun.c I fixed a code style issue relating to
notify_user_selected_context_changed.  It's not a functional change
required by this commit, but it's related to this patch, so I'm
including it here.

Reviewed-By: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Tested-By: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Approved-By: Tom Tromey <tom@tromey.com>
gdb/infrun.c
gdb/testsuite/gdb.base/cli-suppress-notification.c
gdb/testsuite/gdb.base/cli-suppress-notification.exp
gdb/testsuite/gdb.mi/user-selected-context-sync.exp
gdb/thread.c

index 239ede968efb7a6d54c4ad75e88710fa665970fe..b3c408feb3043bc62becc7ae5bd9a6fdbb516729 100644 (file)
@@ -6845,7 +6845,8 @@ notify_normal_stop (bpstat *bs, int print_frame)
 
 /* See infrun.h.  */
 
-void notify_user_selected_context_changed (user_selected_what selection)
+void
+notify_user_selected_context_changed (user_selected_what selection)
 {
   interps_notify_user_selected_context_changed (selection);
   gdb::observers::user_selected_context_changed.notify (selection);
index 02e012f95645f32433abf33599bd41720ca7f19f..12fe04c570842dbb584e886071ca415983be0823 100644 (file)
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-static int global = 0;
+#include <stdlib.h>
+#include <pthread.h>
+#include <unistd.h>
+#include <assert.h>
+
+/* Used for thread synchronisation.  */
+
+pthread_mutex_t g_mutex = PTHREAD_MUTEX_INITIALIZER;
+pthread_cond_t g_cond = PTHREAD_COND_INITIALIZER;
+
+/* This is set by GDB.  */
+
+volatile int wait_for_gdb = 1;
+
+/* This is used to create some work for GDB to step through.  */
+
+volatile int global_var = 0;
+
+/* A simple thread worker function.  */
+
+void*
+worker_thread_func (void *arg)
+{
+  int res;
+
+  /* Grab the mutex.  This completes once the main thread is waiting.  */
+  res = pthread_mutex_lock (&g_mutex);
+  assert (res == 0);
+
+  /* Wake the main thread, letting it know that we are here.  At this
+     point the main thread is still blocked as we hold G_MUTEX.  */
+  res = pthread_cond_signal (&g_cond);
+
+  /* Now we wait.  This releases G_MUTEX and allows the main thread to
+     continue.  */
+  res = pthread_cond_wait (&g_cond, &g_mutex);
+  assert (res == 0);
+
+  /* Unlock the mutex.  We're all done now.  */
+  res = pthread_mutex_unlock (&g_mutex);
+  assert (res == 0);
+
+  return NULL;
+}
 
 int
-main ()
+main (void)
 {
-  global++;
-  global++;
+  pthread_t thr;
+  int res;
+
+  /* Lock G_MUTEX before creating the worker thread.  */
+  pthread_mutex_lock (&g_mutex);
+
+  res = pthread_create (&thr, NULL, worker_thread_func, NULL);
+  assert (res == 0);
+
+  /* Release G_MUTEX and wait for the worker thread.  */
+  res = pthread_cond_wait (&g_cond, &g_mutex);
+  assert (res == 0);
+
+  global_var++;        /* Break here.  */
+  global_var++;        /* Second.  */
+  global_var++;        /* Third.  */
+
+  while (wait_for_gdb)
+    sleep(1);
+
+  /* Notify the worker thread, it will exit once G_MUTEX is released.  */
+  pthread_cond_signal (&g_cond);
+  pthread_mutex_unlock (&g_mutex);
+
+  /* Wait for the worker to actually exit.  */
+  res = pthread_join (thr, NULL);
+  assert (res == 0);
+
+  /* Clean up the mutex and condition variable.  */
+  pthread_mutex_destroy (&g_mutex);
+  pthread_cond_destroy (&g_cond);
+
   return 0;
 }
index 6880d983a4d9a3ae88f6f3e9e368103b5d8d32dd..003f7d247120c36464ae41a9f0a3d5a32cfa5202 100644 (file)
@@ -17,7 +17,8 @@
 
 standard_testfile
 
-if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
+         {debug pthreads}] } {
     return
 }
 
@@ -25,15 +26,53 @@ if {![runto_main]} {
     return
 }
 
-gdb_test "inferior 1" ".*Switching to inferior 1 .* to thread 1 .*" \
-    "inferior switch is not suppressed"
+delete_breakpoints
+
+gdb_breakpoint [gdb_get_line_number "Break here"]
+gdb_continue_to_breakpoint "threads created"
+
+if {![use_gdb_stub]} {
+    gdb_test "add-inferior -exec $binfile" ".*" \
+       "add a second inferior"
+
+    gdb_test "inferior 2" ".*Switching to inferior 2 .*" \
+       "inferior switch is not suppressed"
+    gdb_test "info breakpoints"
+    gdb_test "info inferiors"
+    gdb_test "info connections"
+    gdb_run_cmd
+    gdb_test_multiple "" "stop at breakpoint in inferior 2" {
+       -wrap -re "Breakpoint ${::decimal}(?:\\.${::decimal})?, main .*" {
+           pass $gdb_test_name
+       }
+    }
+}
 
 gdb_test_no_output "set suppress-cli-notifications on"
-gdb_test_no_output "inferior 1" "inferior switch is suppressed"
+if {![use_gdb_stub]} {
+    gdb_test_no_output "inferior 1" \
+       "inferior switch is suppressed when changing inferior"
+}
+gdb_test_no_output "inferior 1" \
+    "inferior switch is suppressed when same inferior selected"
 gdb_test_no_output "next" "stepping is suppressed"
+gdb_test_no_output "thread 2" "switch to a new thread"
+gdb_test_no_output "thread 2" "switch to the same thread"
 
 # Now check that suppression can be turned back off.
 gdb_test_no_output "set suppress-cli-notifications off"
-gdb_test "inferior 1" ".*Switching to inferior 1 .* to thread 1 .*" \
-    "inferior switch is not suppressed again"
-gdb_test "next" "return 0;" "stepping is not suppressed"
+gdb_test "thread 1" \
+    [multi_line \
+        "\\\[Switching to thread 1\[^\r\n\]*\\\]" \
+        "#0\\s+main\[^\r\n\]+" \
+        "\[^\r\n\]+Second\\.  \\*/"]
+gdb_test_no_output "set wait_for_gdb = 0" \
+    "set wait_for_gdb in first inferior"
+
+if {![use_gdb_stub]} {
+    gdb_test "inferior 2" ".*Switching to inferior 2 .* to thread 2\\.1 .*" \
+       "inferior switch is not suppressed again"
+    gdb_test "next" ".*Second.*" "stepping is not suppressed"
+    gdb_test_no_output "set wait_for_gdb = 0" \
+       "set wait_for_gdb in second inferior"
+}
index c954057e90c4ce22e5b0dc2e97c92981750bdcc0..8a4827fd4a80c92c94eb4f412d417e7bb73fdc6d 100644 (file)
@@ -576,10 +576,8 @@ proc_with_prefix test_cli_thread { mode } {
            match_re_or_ensure_no_output $mi_re "select thread, event on MI "
        }
 
-       # Do the 'thread' command to select the same thread.  We shouldn't receive
-       # an event on MI, since we won't actually switch thread.
-
-       set mi_re ""
+       # Do the 'thread' command to select the same thread.  We
+       # should see the same thread events.
 
        with_spawn_id $gdb_main_spawn_id {
            gdb_test "thread 1.2" $cli_re "select thread again"
@@ -589,6 +587,30 @@ proc_with_prefix test_cli_thread { mode } {
            match_re_or_ensure_no_output $mi_re "select thread, event on MI again"
        }
 
+       # Use the 'frame' command to select frame 1.  In either mode
+       # the current thread is stopped, so this should work.
+       with_spawn_id $gdb_main_spawn_id {
+           gdb_test "frame 1" [make_cli_re $mode -1 -1 1] \
+               "select frame 1"
+       }
+
+       with_spawn_id $mi_spawn_id {
+           match_re_or_ensure_no_output [make_mi_re $mode 2 1 event] \
+               "select frame 1, event on MI"
+       }
+
+       # Reselect the current thread.  GDB will switch back to
+       # frame #0, and send CLI and MI notifications.
+       with_spawn_id $gdb_main_spawn_id {
+           gdb_test "thread 1.2" $cli_re \
+               "select thread again, resetting frame"
+       }
+
+       with_spawn_id $mi_spawn_id {
+           match_re_or_ensure_no_output $mi_re \
+               "select thread again, resetting frame, event on MI"
+       }
+
        # Try the 'thread' command without arguments.
 
        set cli_re "\\\[Current thread is 1\\.2.*\\\]"
@@ -623,10 +645,9 @@ proc_with_prefix test_cli_thread { mode } {
            match_re_or_ensure_no_output $mi_re "select thread, event on MI"
        }
 
-       # Do the 'thread' command to select the third thread again.  Again, we
-       # shouldn't receive an event on MI.
-
-       set mi_re ""
+       # Do the 'thread' command to select the third thread again.
+       # We should get the same thread events again as selecting the
+       # same thread still resets the frame to #0.
 
        with_spawn_id $gdb_main_spawn_id {
            gdb_test "thread 1.3" $cli_re "select thread again"
@@ -636,6 +657,33 @@ proc_with_prefix test_cli_thread { mode } {
            match_re_or_ensure_no_output $mi_re "select thread again, event on MI"
        }
 
+       # In non-stop mode the current thread is still running, so we
+       # cannot change the selected frame.
+       if { $mode eq "all-stop" } {
+           # Use the 'frame' command to select frame 1.
+           with_spawn_id $gdb_main_spawn_id {
+               gdb_test "frame 1" [make_cli_re $mode -1 -1 1] \
+                   "select frame 1"
+           }
+
+           with_spawn_id $mi_spawn_id {
+               match_re_or_ensure_no_output [make_mi_re $mode 3 1 event] \
+                   "select frame 1, event on MI"
+           }
+
+           # Reselect the current thread.  GDB will switch back to
+           # frame #0, and send CLI and MI notifications.
+           with_spawn_id $gdb_main_spawn_id {
+               gdb_test "thread 1.3" $cli_re \
+                   "select thread again, resetting frame"
+           }
+
+           with_spawn_id $mi_spawn_id {
+               match_re_or_ensure_no_output $mi_re \
+                   "select thread again, resetting frame, event on MI"
+           }
+       }
+
        # Try the 'thread' command without arguments.
 
        set cli_re "\\\[Current thread is 1\\.3 ${any}\\\]"
@@ -1106,18 +1154,44 @@ proc_with_prefix test_cli_in_mi_thread { mode cli_in_mi_mode } {
            match_re_or_ensure_no_output "$cli_re\r\n" "select thread, event on CLI"
        }
 
-       # Do the 'thread' command to select the same thread.  We shouldn't
-       # receive an event on CLI, since we won't actually switch thread.
-
-       set mi_re [make_cli_in_mi_re $command $cli_in_mi_mode $mode 0 -1 1.2 2 0]
-       set cli_re ""
+       # Do the 'thread' command to select the same thread.  We
+       # should see the same thread events.
 
        with_spawn_id $mi_spawn_id {
            mi_gdb_test $command $mi_re "select thread again"
        }
 
        with_spawn_id $gdb_main_spawn_id {
-           match_re_or_ensure_no_output $cli_re "select thread again, event on CLI"
+           match_re_or_ensure_no_output "$cli_re\r\n" "select thread again, event on CLI"
+       }
+
+       # In non-stop mode the current thread is still running, so we
+       # cannot change the selected frame.  Use 'frame' command to
+       # select frame 1.
+       set f_command [make_cli_in_mi_command $cli_in_mi_mode "frame 1"]
+       set f_cli_re [make_cli_re $mode -1 -1 1]
+       set f_mi_re [make_cli_in_mi_re $f_command \
+                        $cli_in_mi_mode $mode 1 -1 -1 2 1]
+
+       with_spawn_id $mi_spawn_id {
+           mi_gdb_test $f_command $f_mi_re "select frame 1"
+       }
+
+       with_spawn_id $gdb_main_spawn_id {
+           match_re_or_ensure_no_output "$f_cli_re\r\n" \
+               "select frame 1, event on CLI"
+       }
+
+       # Reselect the current thread.  GDB will switch back to
+       # frame #0, and send CLI and MI notifications.
+       with_spawn_id $mi_spawn_id {
+           mi_gdb_test $command $mi_re \
+               "select thread again, resetting frame"
+       }
+
+       with_spawn_id $gdb_main_spawn_id {
+           match_re_or_ensure_no_output "$cli_re\r\n" \
+               "select thread again, resetting frame, event on CLI"
        }
 
        # Try the 'thread' command without arguments.
@@ -1157,24 +1231,49 @@ proc_with_prefix test_cli_in_mi_thread { mode cli_in_mi_mode } {
            match_re_or_ensure_no_output "$cli_re\r\n" "select thread, event on CLI"
        }
 
-       # Do the 'thread' command to select the third thread again.  Again, we
-       # shouldn't receive an event on MI.
-
-       if { $mode == "all-stop" } {
-           set mi_re [make_cli_in_mi_re $command $cli_in_mi_mode $mode 0 -1 1.3 3 0]
-       } else {
-           set mi_re [make_cli_in_mi_re $command $cli_in_mi_mode $mode 0 -1 1.3 3 -1]
-       }
-       set cli_re ""
+       # Do the 'thread' command to select the third thread again.
+       # We should see the same thread events.
 
        with_spawn_id $mi_spawn_id {
            mi_gdb_test $command $mi_re "select thread again"
        }
 
        with_spawn_id $gdb_main_spawn_id {
-           match_re_or_ensure_no_output $cli_re "select thread again, event on CLI"
+           match_re_or_ensure_no_output "$cli_re\r\n" "select thread again, event on CLI"
+       }
+
+       # In non-stop mode the current thread is now running, so we
+       # cannot switch frames.  In all-stop mode all threads are
+       # stopped, so frame switching is fine.
+       if { $mode eq "all-stop" } {
+           # Use 'frame' command to select frame 1.
+           set f_command [make_cli_in_mi_command $cli_in_mi_mode "frame 1"]
+           set f_cli_re [make_cli_re $mode -1 -1 1]
+           set f_mi_re [make_cli_in_mi_re $f_command \
+                            $cli_in_mi_mode $mode 1 -1 -1 3 1]
+           with_spawn_id $mi_spawn_id {
+               mi_gdb_test $f_command $f_mi_re "select frame 1"
+           }
+
+           with_spawn_id $gdb_main_spawn_id {
+               match_re_or_ensure_no_output "$f_cli_re\r\n" \
+                   "select frame 1, event on CLI"
+           }
+
+           # Reselect the current thread.  GDB will switch back to
+           # frame #0, and send CLI and MI notifications.
+           with_spawn_id $mi_spawn_id {
+               mi_gdb_test $command $mi_re \
+                   "select thread again, resetting frame"
+           }
+
+           with_spawn_id $gdb_main_spawn_id {
+               match_re_or_ensure_no_output "$cli_re\r\n" \
+                   "select thread again, resetting frame, event on CLI"
+           }
        }
 
+
        # Try the 'thread' command without arguments.
 
        set command [make_cli_in_mi_command $cli_in_mi_mode "thread"]
index 1b855b4e0b3b302092092e75c4a1d43f754bc67c..472f41969cfc71e685fb13e7e394d451cda0e546 100644 (file)
@@ -1977,21 +1977,10 @@ thread_command (const char *tidstr, int from_tty)
     }
   else
     {
-      ptid_t previous_ptid = inferior_ptid;
-
       thread_select (tidstr, parse_thread_id (tidstr, NULL));
 
-      /* Print if the thread has not changed, otherwise an event will
-        be sent.  */
-      if (inferior_ptid == previous_ptid)
-       {
-         print_selected_thread_frame (current_uiout,
-                                      USER_SELECTED_THREAD
-                                      | USER_SELECTED_FRAME);
-       }
-      else
-       notify_user_selected_context_changed
-         (USER_SELECTED_THREAD | USER_SELECTED_FRAME);
+      notify_user_selected_context_changed
+       (USER_SELECTED_THREAD | USER_SELECTED_FRAME);
     }
 }