Age | Commit message (Collapse) | Author | Files | Lines |
|
Teach the Windows target to temporarily pause all threads when we
change the debug registers for a watchpoint. Implements the same
logic as Linux uses:
~~~
/* (...) if threads are running when the
mirror changes, a temporary and transparent stop on all threads
is forced so they can get their copy of the debug registers
updated on re-resume. (...) */
~~~
On Linux, we send each thread a SIGSTOP to step them. On Windows,
SuspendThread itself doesn't cause any asynchronous debug event to be
reported. However, we've implemented windows_nat_target::stop such
that it uses SuspendThread, and then queues a pending GDB_SIGNAL_0
stop on the thread. That results in a user-visible stop, while here
we want a non-user-visible stop. So what we do is re-use that
windows_nat_target::stop stopping mechanism, but add an external vs
internal stopping kind distinction. An internal stop results in
windows_nat_target::wait immediately re-resuming the thread.
Note we don't make the debug registers poking code SuspendThread ->
write debug registers -> ContinueThread itself, because SuspendThread
is actually asynchronous and may take a bit to stop the thread (a
following GetThreadContext blocks until the thread is actually
suspended), and, there will be several debug register writes when a
watchpoint is set, because we have to set all of DR0, DR1, DR2, DR3,
and DR7. Defering the actualy writes to ::wait avoids a bunch of
SuspendThread/ResumeThread sequences, so in principle should be
faster.
Change-Id: I39c2492c7aac06d23ef8f287f4afe3747b7bc53f
|
|
This patch adds non-stop support to the native Windows target.
This is made possible by the ContinueDebugEvent DBG_REPLY_LATER flag:
https://learn.microsoft.com/en-us/windows/win32/api/debugapi/nf-debugapi-continuedebugevent
Supported in Windows 10, version 1507 or above, this flag causes
dwThreadId to replay the existing breaking event after the target
continues. By calling the SuspendThread API against dwThreadId, a
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
debugger can resume other threads in the process and later return to
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
the breaking.
^^^^^^^^^^^^
The patch adds a new comment section in gdb/windows-nat.c providing an
overall picture of how all-stop / non-stop work.
Without DBG_REPLY_LATER, if we SuspendThread the thread, and then
immediately ContinueDebugThread(DBG_CONTINUE) before getting back to
the prompt, we could still have non-stop mode working, however, then
users wouldn't have a chance to decide whether to pass the signal to
the inferior the next time they resume the program, as that is done by
passing DBG_EXCEPTION_NOT_HANDLED to ContinueDebugEvent, and that has
already been called.
The patch adds that DBG_REPLY_LATER handling, and also adds support
for target_stop, so the core can pause threads at its discretion.
This pausing does not use the same mechanisms used in
windows_nat_target::interrupt, as those inject a new thread in the
inferior. Instead, for each thread the core wants paused, it uses
SuspendThread, and enqueues a pending GDB_SIGNAL_0 stop on the thread.
Since DBG_REPLY_LATER only exists on Windows 10 and later, we only
enable non-stop mode on Windows 10 and later.
Since having the target backend work in non-stop mode adds features
compared to old all-stop mode (signal/exception passing/suppression is
truly per-thread), this patch also switches the backend to do
all-stop-on-top-of-non-stop, by having
windows_nat_target::always_non_stop_p return true if non-stop mode is
possible. To be clear, this just changes how the backend works in
coordination with infrun. The user-visible mode default mode is still
all-stop. The difference is that infrun is responsible for stopping
all threads when needed, instead of the backend (actually the kernel)
always doing that before reporting an event to infrun.
There is no displaced stepping support, but that's "just" a missed
optimization to be done later.
Cygwin signals handling was a major headache, but I managed to get it
working. See the "Cygwin signals" description section I added at the
top of windows-nat.c.
Change-Id: Id71aef461c43c244120635b5bedc638fe77c31fb
|
|
Per
<https://learn.microsoft.com/en-us/windows/win32/api/debugapi/nf-debugapi-continuedebugevent>,
DBG_REPLY_LATER is "Supported in Windows 10, version 1507 or above, ..."
Since we support versions of Windows older than 10, we need to know
whether DBG_REPLY_LATER is available. And we need to know this before
starting any inferior.
This adds a function that probes for support (and caches the result),
by trying to call ContinueDebugEvent on pid=0,tid=0 with
DBG_REPLY_LATER, and inspecting the resulting error.
Suggested-by: Hannes Domani <ssbssa@yahoo.de>
Suggested-by: Eli Zaretskii <eliz@gnu.org>
Change-Id: Ia27b981aeecaeef430ec90cebc5b3abdce00449d
|
|
Several watchpoint-related testcases, such as
gdb.threads/watchthreads.exp for example, when tested with the backend
in non-stop mode, exposed an interesting detail of the Windows debug
API that wasn't considered before. The symptom observed is spurious
SIGTRAPs, like:
Thread 1 "watchthreads" received signal SIGTRAP, Trace/breakpoint trap.
0x00000001004010b1 in main () at .../src/gdb/testsuite/gdb.threads/watchthreads.c:48
48 args[i] = 1; usleep (1); /* Init value. */
After a good amount of staring at logs and headscratching, I realized
the problem:
#0 - It all starts in the fact that multiple threads can hit an event
at the same time. Say, a watchpoint for thread A, and a
breakpoint for thread B.
#1 - Say, WaitForDebugEvent reports the breakpoint hit for thread B
first, then GDB for some reason decides to update debug
registers, and continue. Updating debug registers means writing
the debug registers to _all_ threads, with SetThreadContext.
#2 - WaitForDebugEvent reports the watchpoint hit for thread A.
Watchpoint hits are reported as EXCEPTION_SINGLE_STEP.
#3 - windows-nat checks the Dr6 debug register to check if the step
was a watchpoint or hardware breakpoint stop, and finds that Dr6
is completely cleared. So windows-nat reports a plain SIGTRAP
(given EXCEPTION_SINGLE_STEP) to the core.
#4 - Thread A was not supposed to be stepping, so infrun reports the
SIGTRAP to the user as a random signal.
The strange part is #3 above. Why was Dr6 cleared?
Turns out what (at least in Windows 10 & 11), writing to _any_ debug
register has the side effect of clearing Dr6, even if you write the
same values the registers already had, back to the registers.
I confirmed it clearly by adding this hack to GDB:
if (th->context.ContextFlags == 0)
{
th->context.ContextFlags = CONTEXT_DEBUGGER_DR;
/* Get current values of debug registers. */
CHECK (GetThreadContext (th->h, &th->context));
DEBUG_EVENTS ("For 0x%x (once), Dr6=0x%llx", th->tid, th->context.Dr6);
/* Write debug registers back to thread, same values,
and re-read them. */
CHECK (SetThreadContext (th->h, &th->context));
CHECK (GetThreadContext (th->h, &th->context));
DEBUG_EVENTS ("For 0x%x (twice), Dr6=0x%llx", th->tid, th->context.Dr6);
}
Which showed Dr6=0 after the write + re-read:
[windows events] fill_thread_context: For 0x6a0 (once), Dr6=0xffff0ff1
[windows events] fill_thread_context: For 0x6a0 (twice), Dr6=0x0
This commit fixes the issue by detecting that a thread has a pending
watchpoint hit to report (Dr6 has interesting bits set), and if so,
avoid mofiying any debug register. Instead, let the pending
watchpoint hit be reported by WaitForDebugEvent. If infrun did want
to modify watchpoints, it will still be done when the thread is
eventually re-resumed after the pending watchpoint hit is reported.
(infrun knows how to gracefully handle the case of a watchpoint hit
for a watchpoint that has since been deleted.)
Change-Id: I21a3daa9e34eecfa054f0fea706e5ab40aabe70a
|
|
Consolidate the WoW64 & non-WoW64 paths in
windows_per_inferior::continue_one_thread to avoid code duplication.
The next patch will add more code to this function, and this
unification avoids writing that new code twice.
Change-Id: I794aadb412a3b8081212e4acf2af80d3edba7392
|
|
The Windows backend functions that manipulate the x86 debug registers
are called "cygwin_foo", which is outdated, because native MinGW gdb
also uses those functions, they are not Cygwin-specific. Rename them
to "windows_foo" to avoid confusion.
Change-Id: I46df3b44f5272adadf960da398342a3cbdb98533
|
|
windows_nat_target::windows_continue, when it finds a resumed thread
that has a pending event, does:
/* There's no need to really continue, because there's already
another event pending. However, we do need to inform the
event loop of this. */
serial_event_set (m_wait_event);
return TRUE;
If we have more than one pending event ready to be consumed, and,
windows_nat_target::wait returns without calling
windows_nat_target::windows_continue, which it will with the non-stop
support in the following patch, then we will miss waking up the event
loop.
This patch makes windows-nat.c manage the serial_event similarly to
how linux-nat.c does it. Clear it on entry to
windows_nat_target::wait, and set it if there may be more events to
process. With this, there's no need to set it from
windows_nat_target::wait_for_debug_event_main_thread, so the patch
also makes us not do it.
Change-Id: I44e1682721aa4866f1dbb052b3cfb4870fb13579
|
|
After the previous patches, struct pending_stop only contains one
field. So move that field into the windows_thread_info structure
directly, and eliminate struct pending_stop.
Change-Id: I7955884b3f378d8b39b908f6252d215f6568b367
|
|
Both GDB and GDBserver have similar code to read the $_siginfo data.
This patch moves the bulk of it to gdb/nat/windows-nat.c so it can be
shared.
Change-Id: I47fc0d3323be5b6f6fcfe912b768051a41910666
|
|
The next patch will move some duplicated code in gdb and gdbserver to
gdb/nat/windows-nat.c, where it would be convenient to get at the
Windows process info of a given Windows thread info, from within a
windows_thread_info method.
I first thought of passing down the windows_process_info pointer as
argument to the windows_thread_info method, but that looked a bit odd.
I think it looks better to just add a back pointer, so that's what
this patch does. The following patch will then add a use of it.
I suspect this will help moving more duplicated code to
gdb/nat/windows-nat.c in the future, too.
Change-Id: I47fc0d3323be5b6f6fcfe912b768051a41910666
|
|
With non-stop mode support, each thread has its own "last event", and
so printing $_siginfo should print the siginfo of the selected thread.
Likewise, with all-stop and scheduler-locking.
This patch reworks the siginfo functions in gdb/windows-nat.c and
gdbserver/win32-low.cc to reuse the exception record already saved
within each thread's 'last_event' field.
Here's an example of what you'll see after the whole non-stop series:
(gdb) thread apply all p -pretty -- $_siginfo
Thread 3 (Thread 2612.0x1470):
$1 = {
ExceptionCode = DBG_CONTROL_C,
ExceptionFlags = 0,
ExceptionRecord = 0x0,
ExceptionAddress = 0x7ffd0583e929 <KERNELBASE!EncodeRemotePointer+8249>,
NumberParameters = 0,
{
ExceptionInformation = {0 <repeats 15 times>},
AccessViolationInformation = {
Type = READ_ACCESS_VIOLATION,
Address = 0x0
}
}
}
Thread 2 (Thread 2612.0x1704):
$2 = {
ExceptionCode = SINGLE_STEP,
ExceptionFlags = 0,
ExceptionRecord = 0x0,
ExceptionAddress = 0x7ffd080ad6e4 <ntdll!ZwDelayExecution+20>,
NumberParameters = 0,
{
ExceptionInformation = {0 <repeats 15 times>},
AccessViolationInformation = {
Type = READ_ACCESS_VIOLATION,
Address = 0x0
}
}
}
Thread 1 (Thread 2612.0x434):
$3 = {
ExceptionCode = BREAKPOINT,
ExceptionFlags = 0,
ExceptionRecord = 0x0,
ExceptionAddress = 0x7ff6f691174c <main+185>,
NumberParameters = 1,
{
ExceptionInformation = {0 <repeats 15 times>},
AccessViolationInformation = {
Type = READ_ACCESS_VIOLATION,
Address = 0x0
}
}
}
(gdb)
This was in non-stop mode, and the program originally had two threads.
Thread 1 stopped for a breakpoint, then thread 2 was manually
interrupted/paused and then single-stepped. And then I typed Ctrl-C
in the inferior's terminal, which made Windows inject thread 3 in the
inferior, and report a DBG_CONTROL_C exception for it.
Change-Id: I5d4f1b62f59e8aef3606642c6524df2362b0fb7d
|
|
With non-stop mode, each thread is controlled independently of the
others, and each thread has its own independent reason for its last
stop.
Thus, any thread-specific state that is currently per-process must be
converted to per-thread state.
This patch converts windows_process_info::last_sig to per-thread
state, moving it to windows_thread_info instead.
This adjusts both native gdb and gdbserver.
Change-Id: Ice09a5d932c912210608d5af25e1898f823e3c99
|
|
With non-stop mode, each thread is controlled independently of the
others, and each thread has its own independent reason for its last
stop.
Thus, any thread-specific state that is currently per-process must be
converted to per-thread state.
This patch converts windows_process_info::current_event, moving it to
windows_thread_info instead, renamed to last_event.
Since each thread will have its own copy of its last Windows debug
event, we no longer need the same information stored in struct
pending_stop.
Since windows_process.current_event no longer exists, we need to pass
the current event as parameter to a number of methods.
This adjusts both native gdb and gdbserver.
Change-Id: Ice09a5d932c912210608d5af25e1898f823e3c99
|
|
Surprisingly (to me), enabling scheduler locking on Windows currently
fails:
(gdb)
set scheduler-locking on
Target 'native' cannot support this command.
The backend itself does support scheduler-locking. This patch
implements windows_nat_target::get_thread_control_capabilities so that
the core knows schedlocking works for this target.
Change-Id: Ie762d3768fd70e4ac398c8bcc03c3213bfa26a6a
|
|
Passing a signal to a thread other than the one that last reported an
event would be later possible with DBG_REPLY_LATER and a backend
working in non-stop mode.
With an all-stop backend that isn't possible, so at least don't
incorrectly consider passing DBG_EXCEPTION_NOT_HANDLED if the thread
that we're going to call ContinueDebugEvent for is not the one that
the user issued "signal SIG" on.
Change-Id: I32915623b5036fb902f9830ce2d6f0b1ccf1f5cf
|
|
This will be used in subsequent patches to avoid using
DBG_EXCEPTION_NOT_HANDLED on the wrong thread.
Change-Id: I32915623b5036fb902f9830ce2d6f0b1ccf1f5cf
|
|
windows_process.desired_stop_thread_id doesn't work for non-stop, as
in that case every thread will have its own independent
WaitForDebugEvent event.
Instead, detect whether we have been reported a stop that was not
supposed to be reported by simply checking whether the thread that is
reporting the event is suspended. This is now easilly possible since
each thread's suspend state is kept in sync with whether infrun wants
the thread executing or not.
windows_process.desired_stop_thread_id was also used as thread to pass
to windows_continue. However, we don't really need that either.
windows_continue is used to update the thread's registers, unsuspend
them, and then finally call ContinueDebugEvent. In most cases, we
only need the ContinueDebugEvent step, so we can convert the
windows_continue calls to continue_last_debug_event_main_thread calls.
The exception is when we see a thread creation event -- in that case,
we need to update the debug registers of the new thread. We can use
continue_one_thread for that.
Since the pending stop is now stored in windows_thread_info,
get_windows_debug_event needs to avoid reaching the bottom code if
there's no thread associated with the event anymore (i.e.,
EXIT_THREAD_DEBUG_EVENT / EXIT_PROCESS_DEBUG_EVENT).
I considered whether it would be possible to keep the pending_stop
handling code shared in gdb/nat/windows-nat.c, in this patch and
throughout the series, but I conclused that it isn't worth it, until
gdbserver is taught about async and non-stop as well.
The pending_stop struct will eventually be eliminated later down the
series.
Change-Id: Ib7c8e8d16edc0900b7c411976c5d70cf93931c1c
|
|
I noticed that windows_nat_target::get_windows_debug_event does not
copy the event recorded in pending stop to
windows_process.current_event. This seems like an oversight. The
equivalent code in gdbserver/win32-low.cc does copy it.
This change will become moot later in the series, but I figure its
still clearer to correct the buglet as preparatory patch.
Change-Id: Ic8935d854cf67a3a3c4edcbc1a1e8957b800d907
|
|
This factors some code out of windows_nat_target::windows_continue
into a new windows_continue_one function. This will make the
following patch easier to read (as well as the resulting code itself).
Change-Id: I14a0386b1b8b03015e86273060af173b5130e375
|
|
windows_continue already has two boolean parameters:
(..., int killed, bool last_call = false)
A patch later in the series would need a third. Instead, convert
windows_continue to use an optional enum-flags parameter instead of
multiple booleans.
Change-Id: I17c4d8a12b662190f972c380f838cb3317bd2e1e
|
|
We have code using do_synchronously to call continue_last_debug_event,
and later patches in the series would need to add the same code in few
more places. Factor it out to a continue_last_debug_event_main_thread
function so these other places in future patches can just call it.
Change-Id: I945e668d2b3daeb9de968219925a7b3c7c7ce9ed
|
|
The current code suspends a thread just before calling
GetThreadContext. You can only call GetThreadContext if the thread is
suspended. But, after WaitForDebugEvent, all threads are implicitly
suspended. So I don't think we even needed to call SuspendThread
explictly at all before our GetThreadContext calls.
However, suspending threads when we're about to present a stop to gdb
simplifies adding non-stop support later. This way, the windows
SuspendThread state corresponds to whether a thread is suspended or
resumed from the core's perspective. Curiously, I noticed that Wine's
winedbg does something similar:
https://github.com/wine-mirror/wine/blob/234943344f7495d1e072338f0e06fa2d5cbf0aa1/programs/winedbg/gdbproxy.c#L651
This makes it much easier to reason about a thread's suspend state,
and simplifies adding non-stop mode later on.
Change-Id: Ifd6889a8afc041fad33cd1c4500e38941da6781b
|
|
The logic in windows_nat_target::wait, where we decide what to do
depending on the result from get_windows_debug_event is harder to
grasp than it looks.
It is not easy to tell what should happen when in async mode
get_windows_debug_event returns that there's no event to process.
And then, if get_windows_debug_event returns null_ptid /
TARGET_WAITKIND_SPURIOUS, then we need to issue a ContinueDebugEvent.
There's also this comment in windows_nat_target::wait, which we're not
really implementing today:
~~~~
/* We loop when we get a non-standard exception rather than return
with a SPURIOUS because resume can try and step or modify things,
which needs a current_thread->h. But some of these exceptions mark
the birth or death of threads, which mean that the current thread
isn't necessarily what you think it is. */
~~~~
This patch changes things a bit so that the code is more obvious:
- look at the status kind, instead of ptid_t.
- add an explicit early return case for no-event.
- add an explicit case for TARGET_WAITKIND_SPURIOUS.
- with those, we no longer need to handle the case of find_thread not
finding a thread, so we can drop one indentation level.
Change-Id: I76c41762e1f893a7ff23465856ccf6a44af1f0e7
|
|
After the previous patches, thread_rec is no longer called anywhere.
Delete it.
Change-Id: Ib14e5807fc427e1c3c4a393a9ea7b36b6047a2d7
|
|
There's a single call to thread_rec(DONT_SUSPEND), in
windows_process_info::handle_exception.
In GDB, the windows-nat.c thread_rec implementation avoids actually
calling SuspendThread on the event thread by doing:
th->suspended = -1;
I am not exactly sure why, but it kind of looks like it is done as an
optimization, avoiding a SuspendThread call? It is probably done for
the same reason as the code touched in the previous patch avoided
suspending the event thread.
This however gets in the way of non-stop mode, which will really want
to SuspendThread the event thread for DBG_REPLY_LATER.
In gdbserver's thread_rec implementation DONT_SUSPEND is ignored, and
thread_rec actually always suspends, which really suggests that
SuspendThread on the event thread is really not a problem. I really
can't imagine why it would be.
DONT_SUSPEND invalidates the thread's context, but there is no need to
invalidate the context when we get an event for a thread, because we
invalidate it when we previously resumed the thread.
So, we can just remove the thread_rec call from
windows_process_info::handle_exception. That's what this patch does.
Change-Id: I0f328542bda6d8268814ca1ee4ae7a478098ecf2
|
|
Replace thread_rec(INVALIDATE_CONTEXT) calls with find_thread, and
invalidate_context / suspend calls in the spots that might need those.
I don't know why does the INVALIDATE_CONTEXT implementation in GDB
avoid suspending the event thread:
case INVALIDATE_CONTEXT:
if (ptid.lwp () != current_event.dwThreadId)
th->suspend ();
Checks for a global "current_event" get in the way of non-stop support
later in the series, as each thread will have its own "last debug
event". Regardless, it should be fine to suspend the event thread.
As a data point, the GDBserver implementation always suspends. So
this patch does not try to avoid suspending the event thread on the
native side either.
Change-Id: I8d2f0a749d23329956e62362a7007189902dddb5
|
|
We don't need reload_context, because we can get the same information
out of th->context.ContextFlags. If ContextFlags is zero, then we
need to fetch the context out of the inferior thread. This is what
gdbserver does too.
Change-Id: Ied566037c81383414c46c77713bdd1aec6377b23
|
|
handle_output_debug_string returns a Windows thread id, so it should
return a DWORD instead of an int.
Change-Id: Icbd071a1a37de8a0fc8918bd13254a8d40311e32
|
|
thread_rec(DONT_INVALIDATE_CONTEXT)
The goal of the next few patches is to eliminate thread_rec
completely. This is the first patch in that effort.
thread_rec(DONT_INVALIDATE_CONTEXT) is really just a thread lookup
with no side effects, so this adds a find_thread function that lets
you do that.
Change-Id: Ie486badce00e234b10caa478b066c34537103e3f
|
|
current_process.dr needs to be per-thread for non-stop. Actually, it
doesn't even need to exist at all. We have the x86_debug_reg_state
recording intent, and then the
cygwin_get_dr/cygwin_get_dr6/cygwin_get_dr7 functions are registered
as x86_dr_low_type vector functions, so they should return the current
value in the inferior's registers. See this comment in x86-dregs.c:
~~~
/* In non-stop/async, threads can be running while we change the
global dr_mirror (and friends). Say, we set a watchpoint, and
let threads resume. Now, say you delete the watchpoint, or
add/remove watchpoints such that dr_mirror changes while threads
are running. On targets that support non-stop,
inserting/deleting watchpoints updates the global dr_mirror only.
It does not update the real thread's debug registers; that's only
done prior to resume. Instead, if threads are running when the
mirror changes, a temporary and transparent stop on all threads
is forced so they can get their copy of the debug registers
updated on re-resume. Now, say, a thread hit a watchpoint before
having been updated with the new dr_mirror contents, and we
haven't yet handled the corresponding SIGTRAP. If we trusted
dr_mirror below, we'd mistake the real trapped address (from the
last time we had updated debug registers in the thread) with
whatever was currently in dr_mirror. So to fix this, dr_mirror
always represents intention, what we _want_ threads to have in
debug registers. To get at the address and cause of the trap, we
need to read the state the thread still has in its debug
registers.
In sum, always get the current debug register values the current
thread has, instead of trusting the global mirror. If the thread
was running when we last changed watchpoints, the mirror no
longer represents what was set in this thread's debug
registers. */
~~~
This patch makes the Windows native target follow that model as well.
I don't understand why would windows_nat_target::resume want to call
SetThreadContext itself. That duplicates things as it is currently
worrying about setting the debug registers as well. windows_continue
also does that, and windows_nat_target::resume always calls it. So
this patch simplifies windows_nat_target::resume too.
Change-Id: I2fe460341b598ad293ea60d5f702b10cefc30711
|
|
In windows_nat_target::do_initial_windows_stuff, there's no point in
setting windows_process.current_event.dwProcessId. It's a nop, given
the following memset.
Change-Id: I2fe460341b598ad293ea60d5f702b10cefc30711
|
|
Since gdb-10 there is a heap-use-after free happening if starting the
target in TUI triggers a re-reading of symbols.
It can be reproduced with:
$ gdb -q -batch a.out -ex "tui enable" -ex "shell touch a.out" -ex start
==28392== Invalid read of size 1
==28392== at 0x79E97E: lookup_global_or_static_symbol(char const*, block_enum, objfile*, domain_enum) (symtab.h:503)
==28392== by 0x79F859: lookup_global_symbol(char const*, block const*, domain_enum) (symtab.c:2641)
==28392== by 0x79F8E9: language_defn::lookup_symbol_nonlocal(char const*, block const*, domain_enum) const (symtab.c:2473)
==28392== by 0x7A66EE: lookup_symbol_aux(char const*, symbol_name_match_type, block const*, domain_enum, language, field_of_this_result*) (symtab.c:2150)
==28392== by 0x7A68C9: lookup_symbol_in_language(char const*, block const*, domain_enum, language, field_of_this_result*) (symtab.c:1958)
==28392== by 0x7A6A25: lookup_symbol(char const*, block const*, domain_enum, field_of_this_result*) (symtab.c:1970)
==28392== by 0x77120F: select_source_symtab() (source.c:319)
==28392== by 0x7EE2D5: tui_get_begin_asm_address(gdbarch**, unsigned long*) (tui-disasm.c:401)
==28392== by 0x807558: tui_display_main() (tui-winsource.c:55)
==28392== by 0x7937B5: clear_symtab_users(enum_flags<symfile_add_flag>) (functional:2464)
==28392== by 0x794F40: reread_symbols(int) (symfile.c:2690)
==28392== by 0x6497D1: run_command_1(char const*, int, run_how) (infcmd.c:398)
==28392== Address 0x4e67848 is 3,864 bytes inside a block of size 4,064 free'd
==28392== at 0x4A0A430: free (vg_replace_malloc.c:446)
==28392== by 0x936B63: _obstack_free (obstack.c:280)
==28392== by 0x79541E: reread_symbols(int) (symfile.c:2579)
==28392== by 0x6497D1: run_command_1(char const*, int, run_how) (infcmd.c:398)
==28392== by 0x4FFC45: cmd_func(cmd_list_element*, char const*, int) (cli-decode.c:2735)
==28392== by 0x7DAB50: execute_command(char const*, int) (top.c:575)
==28392== by 0x5D2B43: command_handler(char const*) (event-top.c:552)
==28392== by 0x5D3A50: command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) (event-top.c:788)
==28392== by 0x5D1F4B: gdb_rl_callback_handler(char*) (event-top.c:259)
==28392== by 0x857B3F: rl_callback_read_char (callback.c:290)
==28392== by 0x5D215D: gdb_rl_callback_read_char_wrapper_noexcept() (event-top.c:195)
==28392== by 0x5D232F: gdb_rl_callback_read_char_wrapper(void*) (event-top.c:234)
The problem is that tui_display_main is called by the all_objfiles_removed
hook, which tries to access the symbol cache.
This symbol cache is actually stale at this point, and would have been
flushed immediately afterwards by that same all_objfiles_removed hook.
It's not possible to tell the hook to call the observers in a specific
order, but in this case the tui_all_objfiles_removed observer is actually
not needed, since it only calls tui_display_main, and a 'main' can only
be found if objfiles are added, not removed.
So the fix is to simply remove the tui_all_objfiles_removed observer.
The clearing of the source window (if symbols were removed by e.g. 'file'
without arguments) still works, since this is done by the
tui_before_prompt observer.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31697
Approved-By: Tom Tromey <tom@tromey.com>
|
|
While rebasing this series[1] past this commit:
commit 4bb20a6244b7091a9a7a2ae35dfbd7e8db27550a
Date: Wed Mar 20 04:13:18 2024 -0700
gdbserver: Clear X86_XSTATE_MPX bits in xcr0 on x32
I worried that there could be other paths that might result in an xcr0
value which has X86_XSTATE_MPX set in x32 mode. As everyone
eventually calls amd64_create_target_description to build their target
description, I figured we could assert in here that if X86_XSTATE_MPX
is set then we should not be an x32 target, this will uncover any
other bugs in this area.
I'm not currently able to build/run any x32 binaries, so I have no way
to test this, but the author of commit 4bb20a6244b7091 did test this
series with that assert in place and didn't see any problems.
[1] https://inbox.sourceware.org/gdb-patches/cover.1714143669.git.aburgess@redhat.com
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31511
Approved-By: Felix Willgerodt <felix.willgerodt@intel.com>
|
|
The code in gdb.base/watchpoint-running.exp that is trying to skip
testing with hardware watchpoints also skips testing with software
watchpoints if hardware watchpoints aren't supported by the target.
This fixes it.
Change-Id: Iaed62ac827b32b4fd73b732ad81fa4a5aa5784ba
|
|
Remove accidentally leftover commented-out line from
gdb.base/watchpoint-running.exp.
Change-Id: Ie1c3b85997d2ca92a2159a539d24b02fd3c9e697
|
|
A recent commit refactored with_rocm_gpu_lock:
commit fbb0edfe60edf4ca01884151e6d9b1353aaa0a7e
Date: Sat May 4 10:41:09 2024 +0200
[gdb/testsuite] Factor out proc with_lock
Factor out proc with_lock from with_rocm_gpu_lock, and move required procs
lock_file_acquire and lock_file_release to lib/gdb-utils.exp.
This causes regressions in gdb.rocm/*.exp (as well as in downstream
rocgdb). The issue can be reproduced in the following minimal test:
load_lib rocm.exp
set foo hello
with_rocm_gpu_lock {
verbose -logs $foo
}
The issue is that the body to execute under the lock is executed in the
context of with_rocm_gpu_lock (uplevel 1 used in with_lock) instead of
in the context of the "original" caller.
This patch adjusted with_rocm_gpu_lock to account for the new extra
frame in the call stack between the caller of with_rocm_gpu_lock and
where the code execution is triggered.
Approved-By: Tom de Vries <tdevries@suse.de>
Change-Id: I79ce2c9615012215867ed5bb60144abe7dce28fe
|
|
With a x86_64-pc-mingw32 toolchain there is a build issue
whether or not the --disable-threading option is used.
The problem happens because _WIN32_WINNT is defined to 0x501
before #include <mutex> which makes the compilation abort
due to missing support for __gthread_cond_t in std_mutex.h,
which is conditional on _WIN32_WINNT >= 0x600.
Fix the case when --disable-threading is used, by only
including <mutex> in gdb/complaints.c when STD_CXX_THREAD
is defined.
Additionally make the configure script try to #include <mutex>
to automatically select --disable-threading when the header file
is not able to compile.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
When running the testsuite on a system with kernel.yama.ptrace_scope set to 1,
we run into attach failures.
Fix this by recognizing "ptrace: Operation not permitted" in
can_spawn_for_attach.
Tested on aarch64-linux and x86_64-linux.
Approved-By: Pedro Alves <pedro@palves.net>
|
|
In commit ed8fd0a342f ("[gdb/exp] Fix cast handling for indirection"), I
introduced the behaviour that even though we have:
...
(gdb) p *a_loc ()
'a_loc' has unknown return type; cast the call to its declared return type
...
we get:
...
(gdb) p (char)*a_loc ()
$1 = 97 'a'
...
In other words, the unknown return type of a_loc is inferred from the cast,
effectually evaluating:
...
(gdb) p (char)*(char *)a_loc ()
...
This is convient for the case that errno is defined as:
...
#define errno (*__errno_location ())
...
and the return type of __errno_location is unknown but the macro definition is
known, such that we can use:
...
(gdb) p (int)errno
...
instead of
...
(gdb) p *(int *)__errno_location ()
...
However, as Pedro has pointed out in post-commit review [1], this makes it
harder to reason about the semantics of an expression.
For instance, this:
...
(gdb) p (long long)*a_loc ()"
...
would be evaluated without debug info as:
...
(gdb) p (long long)*(long long *)a_loc ()"
...
but with debug info as:
...
(gdb) p (long long)*(char *)a_loc ()"
...
Fix this by instead simply erroring out for this case:
...
(gdb) p (char)*a_loc ()
'a_loc' has unknown return type; cast the call to its declared return type
...
Tested on x86_64-linux.
Approved-By: Pedro Alves <pedro@palves.net>
[1] https://sourceware.org/pipermail/gdb-patches/2024-May/208821.html
|
|
If threads are disabled, either by --disable-threading explicitely, or by
missing std::thread support, you get the following ASAN error when
loading symbols:
==7310==ERROR: AddressSanitizer: heap-use-after-free on address 0x614000002128 at pc 0x00000098794a bp 0x7ffe37e6af70 sp 0x7ffe37e6af68
READ of size 1 at 0x614000002128 thread T0
#0 0x987949 in index_cache_store_context::store() const ../../gdb/dwarf2/index-cache.c:163
#1 0x943467 in cooked_index_worker::write_to_cache(cooked_index const*, deferred_warnings*) const ../../gdb/dwarf2/cooked-index.c:601
#2 0x1705e39 in std::function<void ()>::operator()() const /gcc/9/include/c++/9.2.0/bits/std_function.h:690
#3 0x1705e39 in gdb::task_group::impl::~impl() ../../gdbsupport/task-group.cc:38
0x614000002128 is located 232 bytes inside of 408-byte region [0x614000002040,0x6140000021d8)
freed by thread T0 here:
#0 0x7fd75ccf8ea5 in operator delete(void*, unsigned long) ../../.././libsanitizer/asan/asan_new_delete.cc:177
#1 0x9462e5 in cooked_index::index_for_writing() ../../gdb/dwarf2/cooked-index.h:689
#2 0x9462e5 in operator() ../../gdb/dwarf2/cooked-index.c:657
#3 0x9462e5 in _M_invoke /gcc/9/include/c++/9.2.0/bits/std_function.h:300
It's happening because cooked_index_worker::wait always returns true in
this case, which tells cooked_index::wait it can delete the m_state
cooked_index_worker member, but cooked_index_worker::write_to_cache tries
to access it immediately afterwards.
Fixed by making cooked_index_worker::wait only return true if desired_state
is CACHE_DONE, same as if threading was enabled, so m_state will not be
prematurely deleted.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31694
Approved-By: Tom Tromey <tom@tromey.com>
|
|
All the calls to dwarf2_per_objfile::adjust have been removed, so we
can remove this function entirely.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31261
|
|
Currently, read_attribute_value calls dwarf2_per_objfile::adjust on
any address. This seems wrong, because the address may not even be in
the text section.
Luckily, this call is also not needed, because read_func_scope calls
'relocate', which does the same work.
|
|
read_call_site_scope does not need to call 'adjust', because in
general the call site is not a symbol address, but rather just the
address of some particular call.
|
|
As with the previous patch, this patch removes some calls to
dwarf2_per_objfile::adjust. These calls are not needed by the cooked
indexer, as it does not create symbols or look up symbols by address.
The call in dwarf2_ranges_read is similarly not needed, as it is only
used to update an addrmap; and in any case I believe this particular
call is only reached by the indexer.
|
|
dwarf2_per_objfile::adjust applies gdbarch_adjust_dwarf2_addr to an
address, leaving the result unrelocated. However, this adjustment is
only needed for text-section symbols -- it isn't needed for any sort
of address mapping. Therefore, these calls can be removed from
read_addrmap_from_aranges and create_addrmap_from_gdb_index.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Make target check//% is the gdb variant of a similar gcc make target [1].
When running tests using check//%:
...
$ cd build/gdb
$ make check//unix/{-fPIE/-pie,-fno-PIE/-no-pie} -j2 TESTS=gdb.server/*.exp
...
we get:
...
$ cat build/gdb/testsuite.unix.-fPIE.-pie/cache/portnum
2427
$ cat build/gdb/testsuite.unix.-fno-PIE.-no-pie/cache/portnum
2423
...
The problem is that there are two portnum files used in parallel.
Fix this by:
- creating a common lockdir build/gdb/testsuite.lockdir for make target
check//%,
- passing this down to the runtests invocations using variable GDB_LOCK_DIR,
and
- using GDB_LOCK_DIR in lock_dir.
Tested on aarch64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
PR testsuite/31632
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31632
[1] https://gcc.gnu.org/install/test.html
|
|
When instrumenting get_portnum using:
...
puts "PORTNUM: $res"
...
and running:
...
$ cd build/gdb
$ make check-parallel -j2 TESTS=gdb.server/*.exp
...
we run into:
...
Running gdb.server/abspath.exp ...
PORTNUM: 2345
...
and:
...
Running gdb.server/bkpt-other-inferior.exp ...
PORTNUM: 2345
...
This is because the test-cases are run in independent runtest invocations.
Fix this by handling the parallel case in get_portnum using:
- a file $objdir/cache/portnum to keep the portnum variable, and
- a file $objdir/cache/portnum.lock to serialize access to it.
Tested on aarch64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The lock directory returned by lock_dir is currently $objdir.
It seems possible to leave a stale lock file that blocks progress in a
following run.
Fix this by using a directory that is guaranteed to be initially empty when
using GDB_PARALLEL, like temp or cache.
In gdb/testsuite/README I found:
...
cache in particular is used to share data across invocations of runtest
...
which seems appropriate, so let's use cache for this.
Tested on aarch64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
In lib/rocm.exp we have:
...
set gpu_lock_filename $objdir/gpu-parallel.lock
...
This decides both the lock file name and directory.
Factor out a new proc lock_dir that decides on the directory, leaving just:
...
set gpu_lock_filename gpu-parallel.lock
...
Tested on aarch64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Factor out proc with_lock from with_rocm_gpu_lock, and move required procs
lock_file_acquire and lock_file_release to lib/gdb-utils.exp.
Tested on aarch64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
|