Imagelibrary/binutils-gdb
1
0
Fork 1
mirror of https://github.com/bminor/binutils-gdb.git synced 2025-11-16 04:24:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

gdb: fix for 'set suppress-cli-notifications on' missed case

Browse Source 
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>
This commit is contained in:
Andrew Burgess
2025-09-28 16:16:53 +01:00
parent c1a7d03958
commit bfea7d3059
5 changed files with 250 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
gdb/infrun.c
View 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);

81
gdb/testsuite/gdb.base/cli-suppress-notification.c
View File

@@ -15,12 +15,85 @@
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;
}

53
gdb/testsuite/gdb.base/cli-suppress-notification.exp
View 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"
}

147
gdb/testsuite/gdb.mi/user-selected-context-sync.exp
View 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"]

15
gdb/thread.c
View 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);
}
}