mirror of
https://github.com/bminor/binutils-gdb.git
synced 2025-12-05 23:23:09 +00:00
bfea7d3059
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>
100 lines
2.6 KiB
C
100 lines
2.6 KiB
C
/* This testcase is part of GDB, the GNU debugger.
|
|
|
|
Copyright 2020-2025 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 <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 (void)
|
|
{
|
|
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;
|
|
}
|