Pedro Alves [Tue, 7 May 2024 19:41:37 +0000 (20:41 +0100)]
Windows gdb: Avoid writing debug registers if watchpoint hit pending
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 with 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 that (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));
[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 modifying 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.)
Move the fill_thread_context method from windows_nat_target to
windows_per_inferior so it can be used by gdbserver too.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I21a3daa9e34eecfa054f0fea706e5ab40aabe70a
commit-id:a28f8d4e
Pedro Alves [Wed, 17 May 2023 16:05:43 +0000 (17:05 +0100)]
Windows gdb: cygwin_set_dr => windows_set_dr, etc.
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.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I46df3b44f5272adadf960da398342a3cbdb98533
commit-id:896523e0
Pedro Alves [Mon, 22 May 2023 10:29:44 +0000 (11:29 +0100)]
Windows gdb: Change serial_event management
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 a later 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.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I44e1682721aa4866f1dbb052b3cfb4870fb13579
commit-id:669a42f6
Pedro Alves [Wed, 17 May 2023 13:34:53 +0000 (14:34 +0100)]
Windows gdb+gdbserver: Eliminate struct pending_stop
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.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I7955884b3f378d8b39b908f6252d215f6568b367
commit-id:fb68c808
Pedro Alves [Fri, 30 Aug 2024 15:02:21 +0000 (16:02 +0100)]
Add backpointer from windows_thread_info to windows_process_info
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.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I47fc0d3323be5b6f6fcfe912b768051a41910666
Pedro Alves [Mon, 22 May 2023 16:33:16 +0000 (17:33 +0100)]
Windows gdb+gdbserver: Make siginfo_er per-thread state
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:
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.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I5d4f1b62f59e8aef3606642c6524df2362b0fb7d
commit-id:e0f75dea
Pedro Alves [Thu, 11 May 2023 17:41:27 +0000 (18:41 +0100)]
Windows gdbserver: Eliminate soft-interrupt mechanism
I noticed that faked_breakpoint is write only. And then I hacked
win32_process_target::request_interrupt to force it to stop threads
using the soft_interrupt_requested mechanism (which suspends threads,
and then fakes a breakpoint event in the main thread), and saw that it
no longer works -- gdbserver crashes accessing a NULL current_thread,
because fake_breakpoint_event does not switch to a thread.
This code was originally added for Windows CE, as neither
GenerateConsoleCtrlEvent nor DebugBreakProcess worked there. Windows
CE support has since been removed.
We nowadays require Windows XP or later, and XP has DebugBreakProcess.
The soft_interrupt_requested mechanism has other problems, like for
example faking the event in the main thread, even if that thread was
previously stopped, due to scheduler-locking.
A following patch will add a similar mechanism stopping all threads
with SuspendThread to native GDB, for non-stop mode, which doesn't
have these problems. It's different enough from this old code that I
think we should just rip the old code out, and reimplement it from
scratch (based on gdb's version) when we need it.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I89e98233a9c40c6dcba7c8e1dacee08603843fb1
Pedro Alves [Thu, 11 May 2023 22:07:33 +0000 (23:07 +0100)]
Windows gdb: Enable "set scheduler-locking on"
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.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Ie762d3768fd70e4ac398c8bcc03c3213bfa26a6a
Pedro Alves [Thu, 11 May 2023 11:15:36 +0000 (12:15 +0100)]
Windows gdbserver: Fix scheduler-locking
This rewrites win32_process_target::resume such that scheduler-locking
is implemented properly.
It also uses the new get_last_debug_event_ptid function to avoid
considering passing a signal to the wrong thread, like done for the
native side in a previous patch.
Note this code/comment being removed:
- /* Yes, we're ignoring resume_info[0].thread. It'd be tricky to make
- the Windows resume code do the right thing for thread switching. */
- tid = windows_process.current_event.dwThreadId;
This meant that scheduler-locking currently is broken badly unless you
stay in the thread that last reported an event. If you switch to a
different thread from the one that last reported an event and step,
you get a spurious SIGTRAP in the thread that last reported a stop,
not the one that you tried to step:
(gdb) t 1
[Switching to thread 1 (Thread 3908)]
#0 0x00007fffc768d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
(gdb) set scheduler-locking on
(gdb) set disassemble-next-line on
(gdb) frame
#0 0x00007fffc768d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
=> 0x00007fffc768d6e4 <ntdll!ZwDelayExecution+20>: c3 ret
(gdb) si
Thread 3 received signal SIGTRAP, Trace/breakpoint trap.
[Switching to Thread 2660]
0x00007fffc4e4e92e in KERNELBASE!EncodeRemotePointer () from target:C:/Windows/System32/KernelBase.dll
=> 0x00007fffc4e4e92e <KERNELBASE!EncodeRemotePointer+8254>: eb 78 jmp 0x7fffc4e4e9a8 <KERNELBASE!EncodeRemotePointer+8376>
(gdb)
Note how we switched to thread 1, stepped, and GDBserver still stepped
thread 3... This is fixed by this patch. We now get:
(gdb) info threads
Id Target Id Frame
1 Thread 920 0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
2 Thread 8528 0x00007ffe03730ad4 in ntdll!ZwWaitForWorkViaWorkerFactory () from target:C:/Windows/SYSTEM32/ntdll.dll
3 Thread 3128 0x00007ffe03730ad4 in ntdll!ZwWaitForWorkViaWorkerFactory () from target:C:/Windows/SYSTEM32/ntdll.dll
* 4 Thread 7164 0x00007ffe0102e929 in KERNELBASE!EncodeRemotePointer () from target:C:/Windows/System32/KernelBase.dll
5 Thread 8348 0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
6 Thread 2064 0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
(gdb) t 1
[Switching to thread 1 (Thread 920)]
#0 0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
(gdb) set scheduler-locking on
(gdb) si
0x00007ffe0372d6e4 in ntdll!ZwDelayExecution () from target:C:/Windows/SYSTEM32/ntdll.dll
(gdb) si
0x00007ffe00f9b44e in SleepEx () from target:C:/Windows/System32/KernelBase.dll
(gdb) si
0x00007ffe00f9b453 in SleepEx () from target:C:/Windows/System32/KernelBase.dll
I.e., we kept stepping the right thread, thread 1.
Note we stopped again at 0x00007ffe0372d6e4 the first time (same PC
the thread already was at before the first stepi) because the thread
had been stopped at a syscall, so that's normal:
(gdb) disassemble
Dump of assembler code for function ntdll!ZwDelayExecution:
0x00007ffe0372d6d0 <+0>: mov %rcx,%r10
0x00007ffe0372d6d3 <+3>: mov $0x34,%eax
0x00007ffe0372d6d8 <+8>: testb $0x1,0x7ffe0308
0x00007ffe0372d6e0 <+16>: jne 0x7ffe0372d6e5 <ntdll!ZwDelayExecution+21>
0x00007ffe0372d6e2 <+18>: syscall
=> 0x00007ffe0372d6e4 <+20>: ret
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I44f4fe4cb98592517569c6716b9d189f42db25a0
commit-id:2a7b7d8e
Pedro Alves [Thu, 11 May 2023 11:27:27 +0000 (12:27 +0100)]
Windows gdb: Can't pass signal to thread other than last stopped thread
Passing a signal to a thread other than the one that last reported an
event will be later possible with DBG_REPLY_LATER and the Windows
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.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I27092ecfbf0904ebce02dff07d9104d22f3d8f0e
commit-id:30c8d0ce
Pedro Alves [Tue, 7 May 2024 15:04:50 +0000 (16:04 +0100)]
Windows gdb+gdbserver: Elim desired_stop_thread_id / rework pending_stops
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.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Ib7c8e8d16edc0900b7c411976c5d70cf93931c1c
commit-id:0b93b3f0
Pedro Alves [Thu, 18 May 2023 18:13:45 +0000 (19:13 +0100)]
Windows gdb: Pending stop and current_event
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.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Ic8935d854cf67a3a3c4edcbc1a1e8957b800d907
Pedro Alves [Tue, 9 May 2023 19:34:50 +0000 (20:34 +0100)]
Windows gdb: Factor code out of windows_nat_target::windows_continue
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).
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I14a0386b1b8b03015e86273060af173b5130e375
Pedro Alves [Thu, 21 Oct 2021 17:16:58 +0000 (18:16 +0100)]
Windows gdb: Introduce windows_continue_flags
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.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I17c4d8a12b662190f972c380f838cb3317bd2e1e
commit-id:e669e7de
Pedro Alves [Thu, 21 Oct 2021 17:16:58 +0000 (18:16 +0100)]
Windows gdb: Introduce continue_last_debug_event_main_thread
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.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I945e668d2b3daeb9de968219925a7b3c7c7ce9ed
commit-id:ee04461a
Pedro Alves [Tue, 9 May 2023 09:27:04 +0000 (10:27 +0100)]
Windows gdb+gdbserver: Move suspending thread to when returning event
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.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Ifd6889a8afc041fad33cd1c4500e38941da6781b
commit-id:c4d2c92e
Pedro Alves [Thu, 11 May 2023 12:16:09 +0000 (13:16 +0100)]
Windows gdb: Simplify windows_nat_target::wait
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.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I76c41762e1f893a7ff23465856ccf6a44af1f0e7
commit-id:aff7fc4a
Pedro Alves [Tue, 9 May 2023 09:13:08 +0000 (10:13 +0100)]
Windows gdb+gdbserver: Eliminate DONT_SUSPEND
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.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I0f328542bda6d8268814ca1ee4ae7a478098ecf2
Pedro Alves [Mon, 8 May 2023 20:36:28 +0000 (21:36 +0100)]
Windows gdb+gdbserver: Eliminate thread_rec(INVALIDATE_CONTEXT) calls
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.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I8d2f0a749d23329956e62362a7007189902dddb5
Pedro Alves [Tue, 30 Apr 2024 14:33:58 +0000 (15:33 +0100)]
Windows gdb: Eliminate reload_context
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.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: Ied566037c81383414c46c77713bdd1aec6377b23
Tom de Vries [Fri, 24 Apr 2026 17:30:41 +0000 (19:30 +0200)]
[gdb/tui] Expand WDB reference
In gdb/tui/tui.c I came across:
...
/* General functions for the WDB TUI.
...
Initially I thought this was a WDB->GDB typo, but after searching a bit I
found out that this is a reference to HP Wildebeest, a port of GDB to HP-UX.
Change the comment line to use "General functions for the GDB TUI", and
instead mention it here in a more descriptive form:
...
- Contributed by Hewlett-Packard Company.
+ Contributed by Hewlett-Packard Company. Developed as part of HP Wildebeest
+ (WDB), a port of GDB to HP-UX.
...
While we're at it, remove a pointless "This is a sample program for the HP WDB
debugger" note in gdb.base/average.c.
Pedro Alves [Tue, 21 Apr 2026 20:08:11 +0000 (21:08 +0100)]
Don't pretend infcalls don't set the inferior running (PR gdb/34082)
Commit 2954dd2b73 ("thread_info::executing+resumed ->
thread_info::internal_state"), caused a regression in
gdb.threads/hand-call-new-thread.exp:
...
(gdb) PASS: gdb.threads/hand-call-new-thread.exp: iter 1: no thread marked running
p new_thread ()
.../src/gdb/infrun.c:3742: internal-error: proceed: Assertion `!thread_is_in_step_over_chain (&tp)' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
----- Backtrace -----
FAIL: gdb.threads/hand-call-new-thread.exp: iter 2: gdb-command<p new_thread ()> (GDB internal error)
...
This commit fixes it.
Let's say we have three threads, 1, 2, and 3. User does:
(gdb) continue
This makes GDB switch all three threads to THREAD_RUNNING.
If some of those threads, now running, spawns a new thread, that
thread is also set to state THREAD_RUNNING. We end up with four
threads marked THREAD_RUNNING.
If e.g., threads 2 and 3 both hit a breakpoint that needs to be
stepped over (e.g., condition evals false), and there is only one
displaced-stepping slot, then one thread starts a displaced stepping
sequence, while the other is put in the step-over queue, waiting for
its turn.
Now, if meanwhile thread 1 hits a user-visible stop, GDB stops all
threads, and transitions all their states to THREAD_STOPPED. Any
thread that was still waiting for its turn in the step-over queue is
removed from the queue. That happens in the THREAD_RUNNING =>
THREAD_STOPPED transition, here:
thread_state
thread_info::set_state (thread_state state, bool suppress_notification)
{
...
switch (m_state)
{
case THREAD_STOPPED:
if (thread_is_in_step_over_chain (this))
global_thread_step_over_chain_remove (this);
The next time the user continues execution, if the breakpoint is still
inserted, proceed() sets them stepping the breakpoint again. And
again, if there is more than one thread that needs to step-over, and
there aren't enough slots, some threads may end up in the step-over
queue. Rinse, repeat.
All this works well with normal resumption commands, like continue,
step, next, etc.
The problem exposed by gdb.threads/hand-call-new-thread.exp is if you
resume execution with an infcall instead of a normal execution
command. In that case, proceed() skips transitioning (pre-existing)
threads to THREAD_RUNNING, here:
proceed (CORE_ADDR addr, enum gdb_signal siggnal)
{
...
/* Even if RESUME_PTID is a wildcard, and we end up resuming fewer
threads in RESUME_PTID are now running. Unless we're calling an
inferior function, as in that case we pretend the inferior
doesn't run at all. */
if (!cur_thr->control.in_infcall)
set_state (resume_target, resume_ptid, THREAD_RUNNING);
So later, when the call finishes for any reason (normal call finish,
or some other user-visible stop happens), and GDB transitions all
threads to THREAD_STOPPED, we hit the early return in
thread_info::set_state:
m_state = state;
switch (m_state)
{
case THREAD_STOPPED:
if (thread_is_in_step_over_chain (this))
global_thread_step_over_chain_remove (this); // NOT REACHED
break;
...
}
}
... and so if any thread had been put in the step-over queue since the
last proceed(), it will incorrectly be left still in the step-over
queue, with THREAD_STOPPED state.
If/when the user re-resumes the program again, we trip the assertion
in proceed:
(gdb) p new_thread ()
../../src/gdb/infrun.c:3742: internal-error: proceed: Assertion `!thread_is_in_step_over_chain (&tp)' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Before commit 2954dd2b73 ("thread_info::executing+resumed ->
thread_info::internal_state"), this didn't happen because
set_running_thread(..., running=false) would remove threads from the
step-over queue unconditionally, even if they were already marked
stopped.
I think the right fix is to stop pretending that infcalls don't set
the target running. I can't think of a reason we do that. It really
does run. Some thoughts:
- I added the code to skip set_running (nowadays 'set_state(...,
THREAD_RUNNING))' for infcalls back in commit 4d9d9d0423 over 10
years ago, but I honestly don't recall why. My guess is that it
must have been to keep backwards compatibility with something, and
the code has probably changed sufficiently since then making it no
longer necessary.
- infcalls are always synchronous, so the intermediate running state
can't be observed with commands.
- MI still suppresses *running
The running state could potentially be observed on frontends, with
e.g., MI's *running => *stopped transitions, but if that is a
problem, I think it should be handled by the interpreter layer not
emiting the notifications, instead of hacking the threads's core
state. I.e., make it a presentation detail.
I don't think it would be a problem for frontends to see *running
during infcalls, but in any case, MI is already suppressing such
notifications when they are caused by an infcall. See
mi_interp::on_target_resumed.
So e.g., if we let the threads transition to THREAD_RUNNING, with
MI, we still see no *running/*stopped:
I don't know if it'd be a problem for DAP, but I assume not too.
Note how the code in proceed that skips setting threads to
THREAD_RUNNING only applies to already-known threads. Any new
thread that appears while the infcall is ongoing will end up marked
THREAD_RUNNING, causing frontend notifications. This shows how
hiding the running state doesn't really hide it completely.
So this is what this commit does. It lets threads transition to
THREAD_RUNNING even during infcalls, fixing the problem described
above, as now there will be proper THREAD_RUNNING => THREAD_STOPPED
transitions when the infcall finishes.
It also tweaks the testcase to spawn 10 threads per infcall instead of
one. This makes it much more likely to reproduce the problem on my
machine. Without it, the test still passes for me, which is why I
didn't see the problem before merging 2954dd2b73.
Tested on x86-64 Linux, native and gdbserver.
Approved-By: Andrew Burgess <aburgess@redhat.com>
Change-Id: I1bdc733ac865102d3f7bf0a4f7f56e6f7d75d457
commit-id:d2abd130
Tom Tromey [Tue, 21 Apr 2026 14:25:04 +0000 (08:25 -0600)]
Call error_value_optimized_out from value_assign
value_assign will throw an exception indicating that the value is
optimized out, but it seemed better to me for it to use the utility
function error_value_optimized_out.
Simon Marchi [Thu, 23 Apr 2026 17:37:06 +0000 (13:37 -0400)]
gdb/dwarf: fix internal error in dwarf2_fetch_cfa_info for FDEs without a CFA rule
Commit a910478f65a5 ("gdb/dwarf: fix internal error when FDEs do not
describe the CFA") fixed the switch on cfa_how in dwarf2_frame_cache,
but there is a very similar switch in dwarf2_fetch_cfa_info that has
the same issue. If the unwind info does not set a rule for the CFA,
fs.regs.cfa_how is left as CFA_UNSET and we hit the default case,
triggering an internal error.
dwarf2_fetch_cfa_info is called from two places, both while compiling
a DWARF expression that contains DW_OP_call_frame_cfa:
- dwarf2_compile_expr_to_ax, when compiling to agent-expression
bytecode
- do_compile_dwarf_expr_to_c, when compiling to C source (used by
the "compile" commands).
Unlike dwarf2_frame_cache, where we can degrade gracefully by setting
undefined_retaddr, the callers of dwarf2_fetch_cfa_info need a concrete
CFA to generate code, so we can't continue. Throw an error with the
same message used a few lines above, when the FDE itself cannot be
found.
Extend the debug-frame-no-cfa.exp test to reproduce this case: add a
compile unit describing the "main" function and with a DW_AT_frame_base
that uses DW_OP_call_frame_cfa and a local variable "x" that uses
DW_OP_fbreg (the evaluation of which requires the CFA). Then, try to
translate the location of "x" to agent-expression bytecode with "maint"
agent. This hits the internal error without the corresponding fix.
Change-Id: I82349e3d9259c8f943eabee5c2fce360876feee8 Approved-by: Kevin Buettner <kevinb@redhat.com>
Tom Tromey [Thu, 23 Apr 2026 13:27:15 +0000 (07:27 -0600)]
Update gdb.ada/attr_ref_and_charlit for gnat-llvm
This patch changes the gdb.ada/attr_ref_and_charlit test to come
closer to passing with gnat-llvm. This mostly works around some
existing problems in gnat-llvm, but since the changes are harmless and
help reduce the noise in the test results, I thought it would be
better to land this.
Because this is Ada-specific, I am checking this in.
Andrew Burgess [Mon, 23 Mar 2026 22:25:17 +0000 (22:25 +0000)]
gdb/dap: add support for opening core files
This patch adds core file support to GDB's DAP interface.
Core files are supported as a GDB specific argument to 'attach', the
new argument is 'coreFile', the name of the core file to debug.
I think handling core files via attach makes the most sense; attach is
for connecting to existing processes, but these targets are (usually)
stopped as soon as GDB attaches, and that's what a core file looks
like, a target that was running, but is now stopped. It just happens
that core file targets are special in that the target cannot be
resumed again, nor can the user modify the program state (e.g. write
to memory or registers).
Prior to starting this work I took a look at what lldb does. The
documentation is not super clear, but this page seems to indicate that
lldb might also use the 'coreFile' argument to 'attach':
Like I said, it's not very clear, but search for "coreFile" and you'll
see it mentioned, just once, under the "attach" header. In order to
be compatible with lldb I used the same argument name with the same
capitalisation.
The new argument is added to the documentation and mentioned in NEWS.
I had to make some changes to testsuite/lib/dap-support.exp to support
this new feature. There's a new dap_corefile proc to handle setting
up the initial connection. This seemed cleaner that overloading
dap_attach, even though under the hood it is still an 'attach' request
that gets sent.
The new test tries to write to memory and registers with the core file
target in place, neither of these requests succeed, which is what we
want, but the exceptions are logged into the dap log file. The
dap_shutdown proc calls dap_check_log_file to check the log for
exceptions, and these two exceptions are spotted and trigger a FAIL.
To avoid this I've added a new "expected_exception_count" argument
for dap_shutdown. Now we check that we see the expected number of
exceptions. We don't check for the specific exception types right
now, but as the test is already checking that the expected requests
fail, I think we're OK.
H.J. Lu [Tue, 21 Apr 2026 19:53:20 +0000 (03:53 +0800)]
ld: Maintain the input file order
When adding a new input archive, which comes from a linker script file
and isn't referenced by any inputs, ld appends it to the input file list.
On Linux, when -lm is used with /usr/lib64/libm.a:
GROUP ( /usr/lib64/libm-2.42.a /usr/lib64/libmvec.a )
since the compiler may not add compiler builtin functions to the LTO
symbol table as it doesn't really know if builtin functions will have
real symbols. When ld extracts an element from the archive later during
LTO rescan, the final input file order is
where x.o references the builtin function, feclearexcept which is defined
in fclrexcpt.o from /usr/lib64/libm-2.42.a.
As the result, the .eh_frame section terminator in crtn.o is placed before
fclrexcpt.o and the .eh_frame section in the output isn't terminated. The
output crashes when it runs over the .eh_frame section during EH frame
registration. Insert the new input file before the current input file to
maintain the same input file order:
as the non-LTO input to properly terminate the .eh_frame section.
Add a static LTO test to reference feclearexcept which is a compiler
builtin function and isn't in the LTO symbol table when GCC is used.
It triggers the run-time crash on glibc targets of a linker script
libm.a without this fix when GCC 13 or above is used:
Also add a debug function, debug_input_files, to display the input file
chain. It is optimized out when compiler optimization is turned on.
PR ld/34088
* ldlang.c (current_input_file): Changed to the pointer to
lang_input_statement_type.
(new_afile): Insert the new input file before the current input
file to maintain the input file order.
(lang_add_input_file): Updated.
(load_symbols): Likewise.
(debug_input_files): New function.
(lang_process): Reference it.
* testsuite/ld-plugin/lto.exp: Run PR ld/34088 test.
* testsuite/ld-plugin/pr34088.c: New file.
Tom de Vries [Wed, 22 Apr 2026 21:29:09 +0000 (23:29 +0200)]
[gdb/symtab] Factor out new_symbol_file_line
I was looking at a patch [1] modifying the DW_AT_decl_file / DW_AT_call_file
handling of new_symbol, and noticed that:
- the code is fairly nested, and
- the patch adds another nesting layer.
Factor out function new_symbol_file_line out of new_symbol, handling both the
DW_AT_decl_file / DW_AT_call_file and DW_AT_call_line / DW_AT_decl_line
attributes.
Having factored out the code, simplify it using return, and modernize it using
bool and nullptr.
Approved-By: Tom Tromey <tom@tromey.com>
[1] https://sourceware.org/pipermail/gdb-patches/2026-March/226065.html
Alan Modra [Tue, 21 Apr 2026 22:32:54 +0000 (08:02 +0930)]
libctf ctf-link.c labs?
ctf-link.c has a labs that clang finds suspect when compiling for a
32-bit host, warning about the signed/unsigned comparison. (labs
return is 32-bit signed, 0xffffffffe is 32-bit unsigned on a 32-bit
host.) I think the labs is bogus, apparently there because
ctf_link_deduplicating_count_inputs returns a ssize_t rather than a
size_t to cover error status returns. However, the error status isn't
a negative count but only -1. So remove the labs.
* ctf-link.c (ctf_link_deduplicating_per_cu): Avoid 32-bit
host warning; don't use labs, cast "inputs" instead.
Style/formatting fix. Report "too many inputs" using %lu.
Tom de Vries [Tue, 21 Apr 2026 16:12:05 +0000 (18:12 +0200)]
[gdb/tui] Handle tui disable in new ui
I tried out the following scenario.
First, I start a terminal to get a tty [1].
...
$ gnome-terminal -- bash -c "tty; sleep 999999999" &
...
Then I start gdb, and start a new UI in the tty:
...
$ gdb
(gdb) new-ui console /dev/pts/3
New UI allocated
(gdb)
...
Now I have two terminals, each with a gdb prompt.
I try in the new UI to enable TUI:
...
(gdb) tui enable
❌️ Cannot enable the TUI when the interpreter is 'console'
(gdb)
...
but that is not allowed.
So instead, I enable TUI in the main UI. So far so good.
Now I disable TUI in the new UI. That leaves the new UI in this state:
...
(gdb) tui disable
<blinking cursor>
...
So, no gdb prompt.
The main UI does leave TUI, AFAICT.
If I do "print 1<enter>" in the new UI, I get the result in the main UI, which
is not supposed to happen.
Fix this by instead throwing an error if we try to disable TUI in the new UI:
...
(gdb) tui disable
❌️ Cannot enable or disablethe TUI when the interpreter is 'console'
(gdb)
...
We could try to fix this instead by allowing tui enable/disable from the new
UI, using:
...
scoped_restore restore_interpreter
= make_scoped_restore (¤t_ui, main_ui);
...
but with just that change, when doing "tui enable" in the new UI we do get a
TUI layout in the main UI, but no gdb prompt. The prompt does appear after a
new keystroke in the main UI though.
A few more TUI commands (for instance "layout asm") have the same problem:
- they work when entered in the new UI,
- they have an effect in the main UI, but
- they don't show a prompt in the main UI.
I left that problem as is, and exercised some TUI commands in a new UI in the
test-case.
Tested on x86_64-linux.
Approved-By: Andrew Burgess <aburgess@redhat.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34035
gdb, multi-target: pass a target argument to delete_exited_threads
Similar to the parent commit, delete_exited_threads is also mostly
used in the context of a particular target. Pass that target as a
parameter and skip other targets.
There seems to be two cases where we actually would want to iterate
all targets. One use is in inferior_appeared (inferior.c) and the
other is in thread_select (thread.c). To handle these cases, allow
the argument to be nullptr.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
gdb, multi-target: pass a target argument to prune_threads
The 'prune_threads' function is used by various targets; in one case
in the 'create_inferior' method, in all other cases in
'update_thread_list'. This is an indication that the function should
handle the threads that belong to the calling target and not mess with
the threads of other targets. So, while iterating the threads, ignore
those that do not belong to the current target. To do this,
prune_threads is modified to take a process_stratum_target as a
parameter.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
gdb: parameterize all_threads_safe for the target and ptid
'all_threads' and 'all_threads_safe' are two range providers to
iterate over threads. The former takes optional process target and
ptid arguments to filter the threads that are being iterated. The
latter does not. Enhance 'all_threads_safe' to also take the optional
arguments. Use the new version of the function in two places.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Andrew Burgess [Fri, 27 Mar 2026 09:52:55 +0000 (09:52 +0000)]
gdb/python: new events.corefile_changed event
Add a new Python event registry, events.corefile_changed. This event
is emitted each time the corefile within an inferior changes.
The event object has a single 'inferior' attribute which is the
gdb.Inferior object for which the core file changed. The user can
then inspect Inferior.corefile to see details about the new core file,
or this will be None if the core file was removed from the inferior.
I've updated the existing test to cover this new event.
The new test covers both the corefile_changed event, but also monitors
the exited event. This ties into the work done in the previous
commit where we use whether the inferior has exited or not as a guard
for whether core_target::exit_core_file_inferior should be called.
Unloading a core file should result in a single corefile_changed event
and a single exited event.
Andrew Burgess [Fri, 27 Mar 2026 11:29:07 +0000 (11:29 +0000)]
gdb: refactor core_target ::close and ::detach functions
This patch refactors the core_target ::close, ::detach, and
::clear_core functions so that the m_core_bfd is not cleared before
the core_target is deleted.
My motivation for this change is the get_inferior_core_bfd function.
This function checks to see if an inferior has a core_target in its
target stack, if it does then there is an assert that the
core_target's m_core_bfd will not be NULL.
Currently, this assert is mostly correct, but during a call to
target_detach, the assert stops being true. Calling target_detach
will call core_target::detach, which calls core_target::clear_core,
which sets m_core_bfd to NULL. The core_target is not unpushed from
the inferior's target stack until GDB returns from ::clear_core back
to ::detach. This means that, for a short period of time, from the
moment m_core_bfd is set to NULL in ::clear_core, until the unpush
back in ::detach, the assertion in get_inferior_core_bfd is no longer
valid.
Within this window we trigger the core_file_changed observer. If any
of the observers call get_inferior_core_bfd then the assertion will
trigger.
Currently, no observer calls get_inferior_core_bfd, the observer just
clears some caches. However, in the next commit I'd like to add a new
Python event API for when the core file is changed. User code
attached to this event can call Inferior.corefile, which is
implemented by a call to get_inferior_core_bfd, and in this case the
assert will trigger.
I could just delete the assertion, but I'd prefer to not do that. I
think by restructuring the code we can leave the assertion in place.
The first thing to understand is that a core_target is not shareable,
see process_stratum_target::is_shareable. This means that a
core_target will only appear within a single inferior's target stack.
Next there are two ways that a core_target can be removed from an
inferior's target stack. First is via target_detach, this is
triggered either by the 'detach' command, or by the 'core-file'
command without a core filename. In both these cases target_detach is
called, which calls core_target::detach, this function unpushes the
core_target from the inferior's target stack. As the core_target is
not shareable the reference count will return to zero, at which point
the core_target will be closed and deleted.
The second way that a core_target can be removed from an inferior's
target_stack is by direct replacement. If a user loads a different
process_stratum_target, e.g. 'target remote ....' then this replaces
the core_target in the target_stack. Doing this reduces the
core_target's reference count to zero, which causes the target to be
closed and deleted.
These two approaches differ in that the first calls
core_target::detach and then core_target::close, while the second
avoids calling ::detach, and immediately calls ::close.
My proposal is that we can defer calling the core_file_changed
observer until core_target::close. By this point the core_target will
have been removed from the inferior's target_stack, and so the assert
in get_inferior_core_bfd will still hold. We already call the
observer at this point for the process_stratum_target replacement
case (e.g. when a user does 'target remote ...' to replace a core file
target), this proposal would just mean that we always call the
observer at this point, rather than potentially calling it earlier in
the detach case.
This commit does this change by making a number of changes:
* The code to reset m_core_bfd to NULL, and to trigger the
core_file_changed observer, is removed from core_target::clear,
this only leaves the code relating to exiting and cleaning up
after the inferior that was created for inspecting the core file.
* To reflect this change of focus, core_target::clear_core is
renamed to core_target::exit_core_file_inferior.
* In core_target::detach, nothing really needs to change other than
calling ::exit_core_file_inferior. I have added an assert that
reflects the fact that ::detach cannot be called twice on the same
core_target (after the first call the core_target will always be
closed and deleted).
* In core_target::close the call to ::exit_core_file_inferior needs
to be conditional. As discussed above, in the replacement case,
::close can be called without first calling ::detach. But in the
target_detach case, ::detach will have already been called, and as
a result ::exit_core_file_inferior will have already been called.
* Also in core_target::close, we now unconditionally trigger the
core_target_changed observer.
This commit is a refactor, and there should be no user observable
changes.
Andrew Burgess [Sun, 29 Mar 2026 19:19:55 +0000 (20:19 +0100)]
gdb: delete some unnecessary code from core_target::detach
This commit removes some unnecessary code from core_target::detach.
When a core_target is created the core BFD (m_core_bfd) is set, and
will never be NULL. The m_core_bfd remains set until either
core_target::detach or core_target::close is called.
The core_target::close function is only called when the refcount of a
core_target reaches zero, the core_target::close function deletes the
core_target, so we know that after calling core_target::close no other
core_target member functions will be called (as the core_target will
have been deleted).
The core_target::detach function is called as a result of calling
target_detach, which is called as a result of either the 'detach'
command, or the 'core-file' command (without passing a file name).
As a core_target is not shareable (see
process_stratum_target::is_shareable), once a core_target is detached,
its reference count will eventually reduce to zero (a reference is
temporarily held in target_detach), and then it will be closed and
deleted.
What this means is that there is absolutely no way that a core_target
can ever be detached twice, not that such a thing would make much
sense, but it cannot happen.
Understanding this we can know that when core_target::detach is called
m_core_bfd will never be NULL, I've added an assert for this case.
Given this assert, if we look at core_target::clear_core, which
core_target::detach calls, we can see that exit_inferior will always
be called. If we look at exit_inferior (in inferior.c) we see that
the last two actions of that function are:
/* Clear the register cache and the frame cache. */
registers_changed ();
reinit_frame_cache ();
Which are also two of the last three actions of core_target::detach.
Clearly the calls in core_target::detach are redundant.
Just for good measure, if we look in target_detach, from where
core_target::detach will have been called, just before the function
returns we have:
The registers_changed_ptid call is slightly more restrictive, only
clearing the register cache for the target being detached, but that
should be good enough -- I think exit_inferior could probably be
changed to call registers_changed_ptid for the inferior that exited,
but that's a problem for another day.
What this all tells me is that the registers_changed call and the
reinit_frame_cache call in core_target::detach are unnecessary, and
can be deleted, which is what this patch does.
Given I was making changes in core_target::detach, I've taken this
opportunity to update an out of date comment. The comment talked
about 'this' possibly becoming dangling, however, this was never the
case as target_detach holds a reference to the target, preventing it
from being deleted until target_detach returns.
There should be no user visible changes after this commit.
Tom Tromey [Fri, 17 Apr 2026 18:34:20 +0000 (12:34 -0600)]
Remove OBJSTAT and OBJSTATS macros
The OBJSTATS macro seems pretty pointless, so I removed it. Then when
looking at the OBJSTAT macro as well, I decided to remove it and also
struct objstats.
After this patch, symbols are allocated using a template method that
automatically updates the n_syms member. This cleans up the code a
little.
Also, nothing ever set objstats::sz_strtab, so this is removed.
Tom Tromey [Wed, 15 Apr 2026 19:22:52 +0000 (13:22 -0600)]
Rename context_stack and make it private
"context_stack" has been misnamed at least since the storage was
changed to a std::vector, and arguably even since the very beginning.
This patch renames it to "lexical_context", which is a bit closer to
what is is for.
This type is also no longer used outside of buildsym itself -- callers
can now push and pop contexts, but they don't act on the context
object itself. So, the type is made private.
One benefit of this approach is that callers no longer need to be
quite as careful -- previously there was at least a possibility that a
context object pointer would be invalidated when pushing and popping
the stack.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Wed, 15 Apr 2026 19:11:31 +0000 (13:11 -0600)]
Return void from buildsym_compunit::push_context
There is one caller that uses the result of
buildsym_compunit::push_context. This patch changes this method to
return void and changes that spot to instead call a new methods on
buildsym_compunit.
This patch also removes the get_current_context_stack method in favor
of a new method that checks the exact condition needed by the one
caller.
This patch enables a subsequent cleanup; in particular now the
'context_stack' type isn't used outside of buildsym.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Wed, 15 Apr 2026 17:46:21 +0000 (11:46 -0600)]
Change pop_context to return a block
This changes buildsym_compunit::pop_context to create and return the
block, if needed. It also arranges to reset some fields in the
buildsym_compunit object to their saved values.
This also enables the removal of the set_local_using_directives
method.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Wed, 15 Apr 2026 17:35:32 +0000 (11:35 -0600)]
Remove some dead code from buildsym.c
This patch removes some code from buildsym.c that, according to the
comment, was only used for some SCO or maybe COFF thing. This code is
dead now, and it was a hack anyway and probably should never have been
allowed.
In v2 I've removed the entire block, since callers should be pairing
pushes and pops anyway.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Wed, 15 Apr 2026 17:11:29 +0000 (11:11 -0600)]
Use scoped_restore for dwarf2_cu::list_in_scope
Some functions in the DWARF reader temporarily set
dwarf2_cu::list_in_scope and then reset it when returning. This patch
changes these spots to use scoped_restore.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
gdb, testsuite: fix regression in gdb.multi/multi-{exit, kill}.exp
Commit d19862435df418a2ec78d078048f54d115c67a66 ("gdbserver:
require_running_or_break for the 'z' and 'vCont' packets") introduced
regressions in gdb.multi/multi-exit.exp and gdb.multi/multi-kill.exp.
If the remote target has multiple inferiors and is resumed with
schedule-multi on, all processes may terminate, making the target have
no threads left. The target reports termination events to GDB. GDB
processes the first event and attempts to stop all processes. For
this, it sends 'vCont;t' packets to the target. But the patch mentioned
above required the target to be in a running state, which is not true
anymore. Therefore, target responds with an error.
Fix the regression by reverting the 'require_running_or_return'
enforcement for vCont.
Jan Beulich [Mon, 20 Apr 2026 06:39:12 +0000 (08:39 +0200)]
x86/Intel: make PC-relative expressions work (again?)
Associating fixups with too complex expressions will prevent the detection
and conversion of expressions which are actually PC-relative. Such a
situation can arise when converting O_index expressions: An extra O_add
(with a constant) or O_symbol is already "too complex", let alone an
O_multiply by zero.
Jan Beulich [Mon, 20 Apr 2026 06:38:35 +0000 (08:38 +0200)]
x86/Intel: don't modify equates' expressions
Equates involving registers were mis-treated when parsing insn operand
expressions: When "pulling out" the register(s), they would have got
converted to O_constant. While other (local) parsing code was able to cope
with this, the generic part of the assembler was misled. A visible bad
effect would be that local absolute symbols would appear in the symbol
table, when really that should be register symbols (which wouldn't be put
in the symbol table at all).
Clone symbols / expressions as necessary before modifying them.
Jan Beulich [Mon, 20 Apr 2026 06:37:47 +0000 (08:37 +0200)]
gas: don't lose addend in snapshot_symbol() when hitting a local symbol
Unlike the one in PR gas/20941, input doesn't need to be entirely bogus
for a local symbol to appear here: Local symbols can be created for
various reasons. If we find one, we have to take exp.X_add_number into
account. Plus, like for "normal" symbols, we should not add in the
symbol's value if the result (in resolve_expression()) is still going to
be O_symbol: The returned value then is relative to the returned symbol.
Jan Beulich [Mon, 20 Apr 2026 06:36:54 +0000 (08:36 +0200)]
gas: don't fail due to local register symbols
The diagnostic text as well as its origin are pretty clear: This is about
global symbols. This is further supported by S_IS_LOCAL() returning true
for symbols in reg_section. Add the missing check, adjusting the testcase
that was introduced back at the time (where the sole diagnostic originally
issued was therefore wrong, while other diagnostics were missing, but got
added thanks to work done elsewhere). Further drop the bogus trailing .equ
in another testcase, which were apparently put there to avoid tripping
this or some other undue check (albeit no error surfaced there already
before the change here).
While there also fully eliminate the redundant "sname": There's "name"
already, getting set up a little earlier.
Alan Modra [Sat, 18 Apr 2026 08:07:13 +0000 (17:37 +0930)]
Don't create got in bfin_relocate_section
Commit 7a84e3daf81a created .got in relocate_section which according
to https://sourceware.org/pipermail/binutils/2008-February/055281.html
was to "fix a crash when linking incompatible object files (normal
vs. FD-PIC)". I can see how that happens by inspecting the two
check_relocs functions, and note that Bernd's change to
merge_private_bfd_data will cause a linker error on trying to link
incompatible ABI objects before relocate_section is reached. However
a user can silence the error with --no-warn-mismatch. It is far too
late to be creating sections in relocate_sections. They won't be
output.
* elf32-bfin.c (bfin_relocate_section): Do not create .got
here to avoid a segfault. Instead report an unresolvable
relocation error if .got has not already been created.
Alan Modra [Mon, 20 Apr 2026 00:31:17 +0000 (10:01 +0930)]
tc-i386.c s_insn and input_line_pointer
A comment in check_Scc_OszcOperations says:
/* No need to save/restore input_line_pointer; that's done in the
caller already. */
That isn't true always. Fix a case where input_line_pointer is not
restored and ignore_rest_of_line() accesses a wild pointer.
Alan Modra [Sun, 19 Apr 2026 22:08:06 +0000 (07:38 +0930)]
gas: don't allow single quote to go past eol
Fuzzers have found a testcase where expr() runs off the end of a strdup
buffer created in tc-i386.c check_Scc_OszcOperations.
printf '\"\000.insn EVEX {scc='\''\000' > test.s
This patch fixes the overrun, and another parsing error that has
existed since commit 219deb70ce2c. gas/testsuite/gas/mri/float.s
doesn't exercise that mri mode code path.
* expr.c (operand): Don't increment input_line_pointer past
end of line/statement when single quote appears at the end of
a line. Don't increment input_line_pointer before mri mode
':' hex float.
Sunil Dora [Tue, 24 Mar 2026 16:45:27 +0000 (09:45 -0700)]
gdb/ser-unix: add POSIX cfsetispeed/cfsetospeed support for custom baud rates
glibc 2.42 and later, and GNU Hurd, accept arbitrary baud rates
directly via cfsetispeed and cfsetospeed. This behaviour is expected
to be standardized in a future POSIX revision.
Introduce a configure-time test (HAVE_CFSETSPEED_ARBITRARY) that
detects this capability and add a new platform-agnostic helper
set_custom_baudrate_posix. The main dispatcher now prefers the POSIX
path when it is available, falling back to the existing Linux (BOTHER)
or Darwin (IOSSIOSPEED) implementations otherwise.
The HAVE_CUSTOM_BAUDRATE_SUPPORT guard is extended to also cover the
new POSIX case.
Suggested-by: Kevin Buettner <kevinb@redhat.com> Suggested-by: Maciej W. Rozycki <macro@orcam.me.uk> Signed-off-by: Sunil Dora <sunilkumar.dora@windriver.com> Reviewed-By: Eli Zaretskii <eliz@gnu.org> Approved-by: Kevin Buettner <kevinb@redhat.com>
Sunil Dora [Tue, 24 Mar 2026 16:45:26 +0000 (09:45 -0700)]
gdb/ser-unix: fix musl build failure when setting custom baud rates
On musl-based systems, <asm/termbits.h> may expose BOTHER even though
struct termios does not define c_ispeed/c_ospeed. This causes the
Linux-specific custom baud rate path to be compiled and fail to build.
Fix the problem at the macro level by requiring
HAVE_STRUCT_TERMIOS_C_OSPEED (obtained via AC_CHECK_MEMBERS) together
with BOTHER in the HAVE_CUSTOM_BAUDRATE_SUPPORT guard. This prevents
the Linux-specific code from being compiled on musl while leaving
set_custom_baudrate_linux unchanged.
This is a pure build fix with no functional or behavioural change on
any existing platform.
Suggested-by: Maciej W. Rozycki <macro@orcam.me.uk> Signed-off-by: Sunil Dora <sunilkumar.dora@windriver.com> Approved-by: Kevin Buettner <kevinb@redhat.com>
Tom de Vries [Sat, 18 Apr 2026 09:54:01 +0000 (11:54 +0200)]
[gdb/build] Fix Wunused-variable in selftests::test_enumerate
On openSUSE Leap 15.6, with gcc 7.5.0, I ran into:
...
enumerate-selftests.c: In function 'void selftests::test_enumerate()':
enumerate-selftests.c:85:22: error: \
unused variable 'i' [-Werror=unused-variable]
for (auto [i, val] : gdb::ranges::views::enumerate (vec))
^
cc1plus: all warnings being treated as errors
...
Simon Marchi [Sat, 18 Apr 2026 02:41:52 +0000 (22:41 -0400)]
gdbsupport: remove iteration_status_str
When building with g++ 8 (on Alma Linux 8), we get:
CXX ada-exp.o
In file included from /binutils-gdb/gdb/../gdbsupport/array-view.h:24,
from /binutils-gdb/gdb/../gdbsupport/common-utils.h:27,
from /binutils-gdb/gdb/../gdbsupport/common-defs.h:214,
from /binutils-gdb/gdb/defs.h:26,
from <command-line>:
/binutils-gdb/gdb/../gdbsupport/iteration-status.h: In function ‘constexpr const char* iteration_status_str(iteration_status)’:
/binutils-gdb/gdb/../gdbsupport/gdb_assert.h:43:22: error: call to non-‘constexpr’ function ‘void internal_error_loc(const char*, int, const char*, ...)’
internal_error_loc (__FILE__, __LINE__, _("%s: " message), __func__, \
~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
##__VA_ARGS__)
~~~~~~~~~~~~~~
/binutils-gdb/gdb/../gdbsupport/iteration-status.h:45:3: note: in expansion of macro ‘gdb_assert_not_reached’
gdb_assert_not_reached ("invalid iteration_status value");
^~~~~~~~~~~~~~~~~~~~~~
It turns out that iteration_status_str isn't used, it apparently was
only used transiently in my series. Remove it to fix the build failure.
Andrew Burgess [Wed, 15 Apr 2026 09:43:31 +0000 (10:43 +0100)]
gdb: don't use .text as default entry point section
We got a Fedora GDB bug report that a user tried to debug an Appimage,
and GDB would reliably crash like this:
(gdb) run
Starting program: /tmp/build/gdb/testsuite/outputs/gdb.base/solib-bad-entry-addr/solib-bad-entry-addr
../../src/gdb/symfile.c:843: internal-error: sect_index_text not initialized
A problem internal to GDB has been detected,
further debugging may prove unreliable.
----- Backtrace -----
... etc ...
The specific AppImage being debugged can be found here, I've modified
the URL with a warning marker. I make no claims about whether it is
safe to download the image, and running it is definitely at your own
risk. If you wish to, delete the warning marker to download:
https://github.com/Murmele/Gittyup/<RUN AT YOUR OWN RISK>/releases/download/gittyup_v1.4.0/Gittyup-1.4.0-x86_64.AppImage
At the point of the above crash GDB's stack is:
#9 0x000000000190c6ed in internal_error_loc (file=0x1e4a94e "../../src/gdb/symfile.c", line=838, fmt=0x1e4aa50 "sect_index_text not initialized") at ../../src/gdbsupport/errors.cc:57
#10 0x0000000000f5f5ac in init_entry_point_info (objfile=0x5a98e80) at ../../src/gdb/symfile.c:838
#11 0x0000000000f5f943 in syms_from_objfile (objfile=0x5a98e80, addrs=0x7ffd78728490, add_flags=...) at ../../src/gdb/symfile.c:962
#12 0x0000000000f5fe6d in symbol_file_add_with_addrs (abfd=..., name=0x3e76e50 "/tmp/.mount_GittyujmIBkD/usr/bin/../../home/runner/work/Gittyup/Qt/5.15.2/gcc_64/lib/./libicudata.so.56", add_flags=..., addrs=0x7ffd78728490, flags=..., parent=0x0) at ../../src/gdb/symfile.c:1071
#13 0x0000000000f601aa in symbol_file_add_from_bfd (abfd=..., name=0x3e76e50 "/tmp/.mount_GittyujmIBkD/usr/bin/../../home/runner/work/Gittyup/Qt/5.15.2/gcc_64/lib/./libicudata.so.56", add_flags=..., addrs=0x7ffd78728490, flags=..., parent=0x0) at ../../src/gdb/symfile.c:1145
#14 0x0000000000f0f2ad in solib_read_symbols (so=..., flags=...) at ../../src/gdb/solib.c:627
#15 0x0000000000f10263 in solib_add (pattern=0x0, from_tty=0, readsyms=1) at ../../src/gdb/solib.c:960
From this we can see GDB is trying to add the shared library:
1. There really is no .text section, or any executable sections,
2. there are 3 LOAD segments. This will be important later, and
3. the "Entry point address" is outside all sections, and is
non-zero.
Next we can investigate where objfile::sect_index_text is set to
something other than -1. Starting in init_objfile_sect_indices, if
the objfile has a ".text" section then sect_index_text can be set.
This case clearly doesn't apply.
Next symfile_find_segment_sections is called. This tries to match a
common case where we have either 1 or 2 LOAD segments, and assumes a
default distribution of sections to segments. However, we have 3 LOAD
segments, so these lines:
result in an early return from symfile_find_segment_sections without
sect_index_text being set.
Back in init_objfile_sect_indices, if no sections have an offset then
we set any currently unset sect_index_* values, including
sect_index_text, to point at section 0. However, in our case the
objfile is a relocatable shared library, so the sections will have an
offset, and so this final fallback case doesn't apply.
The result is that init_objfile_sect_indices never sets
sect_index_text. This worries me a little as
init_objfile_sect_indices contains this comment:
/* This is where things get really weird... We MUST have valid
indices for the various sect_index_* members or gdb will abort.
So if for example, there is no ".text" section, we have to
accommodate that. First, check for a file with the standard
one or two segments. */
Notice the emphasis on MUST in that comment, and indeed, we exit this
function without setting sect_index_text, and GDB does indeed abort.
The comment seems to imply that the following code is going to try to
figure out a suitable stand-in sect_index_text for when there is no
".text" section, but clearly I've run into a case that isn't covered.
All of this code relating to setting sect_index_text was introduced in
commit:
Which unfortunately is from a time where we didn't write useful commit
messages, so to understand the commit you need to go read the mailing
list archive, but they don't offer much more insight:
Clearly the comment in init_objfile_sect_indices would suggest that
the fix here is to figure out some "fake" value for sect_index_text,
and that would certainly avoid the problem here. But, at least for
this problem, I think there's maybe a better solution.
The original internal error is triggered by a use of SECT_OFF_TEXT in
init_entry_point_info. We have an entry point address, we try to find
the section index for the section containing the entry point, and
failing that, we assume the entry point is in the text section. This
fall-back assumption means that, if the text section has an offset
applied, then the entry point will also have that same offset
applied. But it's not clear to me why picking the text section is
going to be any more valid than any other section, especially in a
case like this where we don't even have a text section, so the
sect_index_text might itself point to some other arbitrary section.
Earlier in init_entry_point_info we already have a fall-back case
where we set entry_info::entry_point_p to false to indicate that the
objfile has no entry point, so this is always a possibility. So I
wondered about writing something like:
if (!found)
{
if (objfile->sect_index_text != -1)
ei->the_bfd_section_index = SECT_OFF_TEXT (objfile);
else
ei->entry_point_p = false;
}
If we have no text section index then we just claim the objfile has no
entry point. But I didn't like this for two reasons, first, the
comment back in init_objfile_sect_indices saying that the index should
be set, this seems to indicate that we should not be making decisions
later within GDB based on whether the index is set or not.
And second, using the text section as a fall back, when the entry
address is outside every section, just seems off. So I wondered, why
not just reject the entry point completely in this case? Which is how
I ended up with:
if (!found)
ei->entry_point_p = false;
With this patch in place I was able to start debugging the AppImage
linked above.
I created a simple test case which reproduces this issue. It's a
little contrived because it has to hit all the points required to
trigger this bug:
1. No .text section,
2. more than 2 LOAD segments, and
3. entry address outside every section.
I have no idea what caused the original shared library to take on
these characteristics, it might even be a tool issue building the
original shared library. I haven't investigated this, as I don't
think it really matters, GDB shouldn't be crashing just because the
incoming objects are a little weird.
I've attached a link to the Fedora bug in the 'Bug:' tag, but it's a
little confusing. An automated system has merged together two bug
reports. As such the overall bug report linked too is for a
completely different issue, only comments 21, 22, 23, and 24 relate to
the bug being fixed here.
Andrew Burgess [Wed, 15 Apr 2026 09:42:03 +0000 (10:42 +0100)]
gdb: int to bool in struct entry_info
Convert 'struct entry_info' to use bool for flag fields. The places
where these flags are set are updated too. I have also removed the
bit width specifier ": 1" from the flag field definitions. There is
one entry_info struct per BFD in GDB, which is not a huge number, so
forcing the bool fields to 1-bit doesn't seen necessary.
There should be no user visible changes after this commit.
Simon Marchi [Thu, 16 Apr 2026 20:16:21 +0000 (16:16 -0400)]
gdb: make iterate_over_symbols return void, rename to for_each_symbol
Nothing really uses the return value of iterate_over_symbols and
language::iterate_over_symbols. Also, all provided callback always
return true, iterating on all matching symbols. Simplify them to not
return a value and not have the "stop iterating" feature.
Rename to for_each_symbol, just to be consistent with previous patches.
Also rename symbol_found_callback_ftype to
for_each_symbol_callback_ftype for consistency.
Change-Id: I55ff3162098bb069dc1de1afca10dd9abfc05c34 Approved-By: Andrew Burgess <aburgess@redhat.com>
Simon Marchi [Thu, 16 Apr 2026 20:16:19 +0000 (16:16 -0400)]
gdb: change objfile::map_symtabs_matching_filename to find_symtab_matching_filename
The only user of objfile::map_symtabs_matching_filename uses that method
to find the first matching symtab. It would therefore be more natural
for that method to be a "find" method, returning the first symtab
matching the predicate.
Change map_symtabs_matching_filename to be
find_symtab_matching_filename, and the internal
iterate_over_one_compunit_symtab to be find_symtab_in_compunit_symtab.
This makes function find_symtab simpler.
Change-Id: Id14a95498fad243495d6eab18810d0c4ab8dbf90 Approved-By: Andrew Burgess <aburgess@redhat.com>
Simon Marchi [Fri, 17 Apr 2026 14:30:23 +0000 (10:30 -0400)]
gdb: split iterate_over_symtabs into for_each_symtab and find_symtab
Same rationale as the previous patches.
For the moment, find_symtab is only needed internally in symtab.c, so
keep it static there. Note that the interaction with
objfile.map_symtabs_matching_filename gets cleaner in a subsequent
patch.
for_each_symtab is implemented using find_symtab, because the iteration
behavior is not completely trivial.
find_symtab_callback_ftype is in the header file, because it is used
from another source file in the next patch.
Change-Id: I6ab8342151eb735327fc2e7935e7a65cede5e1dd Approved-By: Andrew Burgess <aburgess@redhat.com>
projects / thirdparty / binutils-gdb.git / log
