Pedro Alves [Fri, 5 May 2023 14:41:03 +0000 (15:41 +0100)]
Windows gdb: Watchpoints and internal vs external stops
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.
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.
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_= 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 that infrun is responsible for
stopping all threads when needed, instead of the backend always doing
that.
No displaced stepping, but that's "just" a missed optimization to be
done later.
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 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.
Pedro Alves [Wed, 17 May 2023 13:34:53 +0000 (14:34 +0100)]
Windows gdb/gdbserver: Eliminate struct pending_stop
After the previous patch, struct pending_stop only contains one field.
So move that field into the windows_thread_info structure directly,
and eliminate struct pending_stop.
Pedro Alves [Mon, 22 May 2023 17:17:54 +0000 (18:17 +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.
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.
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.
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.
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.
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", "off" or "auto" expected.
(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
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 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.
Pedro Alves [Thu, 21 Oct 2021 17:16:58 +0000 (18:16 +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.
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.
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.
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).
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.
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.
Pedro Alves [Thu, 4 May 2023 12:23:18 +0000 (13:23 +0100)]
Windows gdb/gdbserver: Check whether DBG_REPLY_LATER is available
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, ..."
On Linux, we check which ptrace options are supported by the running
kernel by forking gdb and then the parent gdb debugging the child gdb
with PTRACE_ME, and then trying to set the ptrace options.
Doing something like that on Windows would be more complicated,
because #1 - we can't just fork, we have to start some executable, and
the only executable we know we can start, probably, is gdb itself.
And that's a large program, so take time to be started. And then we'd
have to implement a WaitForDebugEvent loop to start up the process,
and then finally try ContinueDebugEvent(DBG_REPLY_LATER). It seems a
lot simpler to just check the Windows version. Unlike on Linux, we
don't have to worry about kernel feature backports.
Windows has a number of functions you can use to check the OS version,
like GetVersion/GetVersionEx, or the Version Helper functions like
IsWindows10OrGreater, VerifyVersionInfo, etc., however, as explained by
... and other pages, "Applications not manifested for Windows 8.1 or
Windows 10 will return the Windows 8 OS version value (6.2)."
"Manifested" here means that the application is linked with an xml
manifest as detailed at:
https://learn.microsoft.com/en-us/windows/win32/sysinfo/targeting-your-application-at-windows-8-1
I have actually tried doing that with windres, but I wasn't able to
make it work. I am probably missing something.
However, I found out that we can find the Windows major/minor/build in
the KUSER_SHARED_DATA structure, which defines the layout of a data
area that the kernel places at a pre-set address for sharing with
user-mode software:
The Windows major/minor/build version retrieved using that method
bypasses the manifest stuff, it actually gets you the real OS version
numbers. That is what this patch is using.
Pedro Alves [Fri, 28 Apr 2023 18:11:31 +0000 (19:11 +0100)]
Fix Windows sharing_input_terminal hang
After running a number of programs under gdb and detaching them, I
typed run in gdb, and got a hang, here:
(top-gdb) bt
#0 sharing_input_terminal (pid=4672) at /home/pedro/rocm/gdb/src/gdb/mingw-hdep.c:388
#1 0x00007ff71a2d8678 in sharing_input_terminal (inf=0x23bf23dafb0) at /home/pedro/rocm/gdb/src/gdb/inflow.c:269
#2 0x00007ff71a2d887b in child_terminal_save_inferior (self=0x23bf23de060) at /home/pedro/rocm/gdb/src/gdb/inflow.c:423
#3 0x00007ff71a2c80c0 in inf_child_target::terminal_save_inferior (this=0x23bf23de060) at /home/pedro/rocm/gdb/src/gdb/inf-child.c:111
#4 0x00007ff71a429c0f in target_terminal_is_ours_kind (desired_state=target_terminal_state::is_ours_for_output) at /home/pedro/rocm/gdb/src/gdb/target.c:1037
#5 0x00007ff71a429e02 in target_terminal::ours_for_output () at /home/pedro/rocm/gdb/src/gdb/target.c:1094
#6 0x00007ff71a2ccc8e in post_create_inferior (from_tty=0) at /home/pedro/rocm/gdb/src/gdb/infcmd.c:245
#7 0x00007ff71a2cd431 in run_command_1 (args=0x0, from_tty=0, run_how=RUN_NORMAL) at /home/pedro/rocm/gdb/src/gdb/infcmd.c:502
#8 0x00007ff71a2cd58b in run_command (args=0x0, from_tty=0) at /home/pedro/rocm/gdb/src/gdb/infcmd.c:527
The problem is that the loop around GetConsoleProcessList looped
forever, because there were exactly 10 processes to return.
GetConsoleProcessList's documentation says:
If the buffer is too small to hold all the valid process identifiers,
the return value is the required number of array elements. The
function will have stored no identifiers in the buffer. In this
situation, use the return value to allocate a buffer that is large
enough to store the entire list and call the function again.
In this case, the buffer wasn't too small, it was exactly the right
size, so we should have broken out of the loop. We didn't due to a
"<" check that should have been "<=". That is fixed by this patch.
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.
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.
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.
Pedro Alves [Mon, 8 May 2023 20:36:28 +0000 (21:36 +0100)]
Windows gdb/gdbserver: Eliminate thread_rec(INVALIDATE_CONTEXT) calls
Replace them 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.
Pedro Alves [Mon, 8 May 2023 19:51:44 +0000 (20:51 +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.
Pedro Alves [Mon, 8 May 2023 18:40:50 +0000 (19:40 +0100)]
Windows gdb: Eliminate Cygwin-specific have_saved_context / saved_context
I don't see the point of having have_saved_context / saved_context,
when windows_per_inferior::handle_output_debug_string can simply write
directly to th->Context. That's what this patch does.
Pedro Alves [Tue, 2 May 2023 19:42:35 +0000 (20:42 +0100)]
Windows gdb: Eliminate global current_process.dr[8] global
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 xx 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.
Pedro Alves [Tue, 2 May 2023 14:04:28 +0000 (15:04 +0100)]
Fix setting watchpoints when current thread is running
Currently, when the current thread is running, you can print global
variables. However, if you try to set a watchpoint on the same
globals, GDB errors out, complaining that the selected thread is
running. Like so:
(gdb) c&
Continuing.
(gdb) p global
$1 = 1098377287
(gdb) watch global
Selected thread is running.
This patch makes setting the watchpoint work. You'll now get:
(gdb) c&
Continuing.
(gdb) [New Thread 0x7ffff7d6e640 (LWP 434993)]
[New Thread 0x7ffff756d640 (LWP 434994)]
p global
$1 = 88168
(gdb) watch global
Hardware watchpoint 2: global
(gdb) [Switching to Thread 0x7ffff7d6e640 (LWP 434993)]
Thread 2 "function0" hit Hardware watchpoint 2: global
Old value = 185420
New value = 185423
int_return () at threads.c:39
39 }
The problem is that update_watchpoint calls get_selected_frame
unconditionally. We can skip it if the watchpoint expression is only
watching globals.
This adds a testcase that exercises both all-stop and non-stop, and
also software and hardware watchpoints. It is kfailed for software
watchpoints, as those require another fix not handled by this patch
(the sw watchpoint doesn't fire because GDB doesn't force the
running-free thread to switch to single-stepping).
Pedro Alves [Wed, 17 May 2023 17:59:15 +0000 (18:59 +0100)]
Fix windows_nat_target::fake_create_process ptid
During development of this series, I managed to introduce a bug that
let to fake_create_process being called. That then resulted in GDB
crashes later on, because fake_create_process added a thread with an
incorrect ptid for this target. It is putting dwThreadId in the tid
field of the ptid instead of on the lwp field. This is fixed by this
patch.
I do however wonder why nobody has seen it this long. The
fake_create_process code was added for
https://sourceware.org/bugzilla/show_bug.cgi?id=8153 and Chris said
back then, in:
https://sourceware.org/legacy-ml/gdb-patches/2003-12/msg00479.html
"If the main thread has exited, there is never any create process
notification when attaching to a process. That confuses all sorts of
things."
I don't see any missing CREATE_PROCESS_DEBUG_EVENT when I try that
scenario on Windows 10. I would have to try on Windows XP (the oldest
version of Windows we claim to support), but I don't have such a
machine handy. Do note however, that GDBserver has never gotten an
equivalent to fake_create_process.
Kito Cheng [Mon, 17 Apr 2023 12:16:33 +0000 (20:16 +0800)]
RISC-V: Cache the latest mapping symbol and its boundary.
This issue was reported from https://github.com/riscv-collab/riscv-gnu-toolchain/issues/1188
Current flow:
1) Scan any mapping symbol less than this instruciton.
2) If not found, did a backward search.
The flow seems not big issue, let run an example here:
$x:
0x0 a <--- Found at step 1
0x4 b <--- Not found in step 1, but found at step 2
0x8 c <--- Not found in step 1, but found at step 2
$d
0x12 .word 1234 <-- Found at step 1
The instruciton didn't have the same address with mapping symbol will
still did backward search again and again.
So the new flow is:
1) Use the last mapping symbol status if the address is still within the range
of the current mapping symbol.
2) Scan any mapping symbol less than this instruciton.
3) If not found, did a backward search.
4) If a proper mapping symbol is found in either step 2 or 3, find its boundary,
and cache that.
Use the same example to run the new flow again:
$x:
0x0 a <--- Found at step 2, the boundary is 0x12
0x4 b <--- Cache hit at step 1, within the boundary.
0x8 c <--- Cache hit at step 1, within the boundary.
$d
0x12 .word 1234 <-- Found at step 2, the boundary is the end of section.
The disassemble time of the test cases has been reduced from ~20 minutes to ~4
seconds.
opcode/ChangeLog
PR 30282
* riscv-dis.c (last_map_symbol_boundary): New.
(last_map_state): New.
(last_map_section): New.
(riscv_search_mapping_symbol): Cache the result of latest
mapping symbol.
Our library does not set the default version for symbols.
This is correct because we don't know which libC will be used.
gcc and g++ links differently the version symbols when the default version is
not set. c-linker is using our pthread_create@GLIBC_2.34 and c++-linker is using
our pthread_create@GLIBC_2.0 by default.
The current implementation of the interposed functions is:
If we are in our pthread_create@GLIBC_<NN>,
we use dlvsym (dlflag, "pthread_create", "GLIBC_<NN>") to find and call
the same function from libC.
In the test from PR 30360, pthread_create@GLIBC_2.0 is not in the current libC.
We need to call the default version symbol from libC.
gprofng/ChangeLog
2023-04-16 Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
PR gprofng/30360
* libcollector/iotrace.c: Find and call a default libC version symbol.
* libcollector/dispatcher.c: Likewise.
* libcollector/iotrace.c: Likewise.
* libcollector/linetrace.c: Likewise.
* libcollector/mmaptrace.c: Likewise.
* libcollector/synctrace.c: Likewise.
* libcollector/collector.h (REAL_DCL): Remove an unused argument.
This patch addresses bugzilla 29521:
Bug 29521 - [docs] man pages are not in the release tarball
The dependence on help2man to create the man pages has been eliminated.
All man pages are now written in Texinfo. Texi2pod and pod2man are used
to generate the man pages from the source.
The user guide has been significantly expanded. It also includes all
the man pages. These are formatted appropriately in the INFO, PDF, and
HTML formats.
The index in the user guide has been enhanced to include an overview
of all options and commands that have been documented so far.
The work on the documentation has not been completed, but this is
a significant step forward.
gprofng/ChangeLog
2023-04-15 Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
PR gprofng/29521
* doc/Makefile.am: Build documentation.
* doc/gprofng.texi: Update documentation.
* doc/version.texi: Likewise.
* src/Makefile.am: Move the man pages generation to doc/Makefile.am.
* gp-display-html/Makefile.am: Likewise.
* doc/gp-archive.texi: New file.
* doc/gp-collect-app.texi: New file.
* doc/gp-display-html.texi: New file.
* doc/gp-display-src.texi: New file.
* doc/gp-display-text.texi: New file.
* doc/gp-macros.texi: New file.
* doc/gprofng_ug.texi: New file.
* doc/Makefile.in: Rebuild.
* gp-display-html/Makefile.in: Rebuild.
* src/Makefile.in" Rebuild.
Tom Tromey [Mon, 17 Apr 2023 19:36:51 +0000 (13:36 -0600)]
Remove some unnecessary casts from ada-lang.c
I noticed some unnecessary casts to LONGEST in ada-lang.c. This patch
removes the ones I think are very clearly not needed. I'm checking
this in as obvious.
Simon Marchi [Mon, 3 Apr 2023 18:52:08 +0000 (14:52 -0400)]
gdb/amdgpu: add follow fork and exec support
Prior to this patch, it's not possible for GDB to debug GPU code in fork
children or after an exec. The amd-dbgapi target attaches to processes
when an inferior appears due to a "run" or "attach" command, but not
after a fork or exec. This patch adds support for that, such that it's
possible to for an inferior to fork and for GDB to debug the GPU code in
the child.
To achieve that, use the inferior_forked and inferior_execd observers.
In the case of fork, we have nothing to do if `child_inf` is nullptr,
meaning that GDB won't debug the child. We also don't attach if the
inferior has vforked. We are already attached to the parent's address
space, which is shared with the child, so trying to attach would cause
problems. And anyway, the inferior can't do anything other than exec or
exit, it certainly won't start GPU kernels before exec'ing.
In the case of exec, we detach from the exec'ing inferior and attach to
the following inferior. This works regardless of whether they are the
same or not. If they are the same, meaning the execution continues in
the existing inferior, we need to do a detach/attach anyway, as
amd-dbgapi needs to be aware of the new address space created by the
exec.
Note that we use observers and not target_ops::follow_{fork,exec} here.
When the amd-dbgapi target is compiled in, it will attach (in the
amd_dbgapi_process_attach sense, not the ptrace sense) to native
inferiors when they appear, but won't push itself on the inferior's
target stack just yet. It only pushes itself if the inferior
initializes the ROCm runtime. So, if a non-GPU-using inferior calls
fork, an amd_dbgapi_target::follow_fork method would not get called.
Same for exec. A previous version of the code had the amd-dbgapi target
pushed all the time, in which case we could use the target methods. But
we prefer having the target pushed only when necessary, it's less
intrusive when doing native debugging that doesn't involve the GPU.
Change-Id: I5819c151c371120da8bab2fa9cbfa8769ba1d6f9 Reviewed-By: Pedro Alves <pedro@palves.net>
Simon Marchi [Mon, 3 Apr 2023 18:52:07 +0000 (14:52 -0400)]
gdb: switch to right inferior in fetch_inferior_event
The problem explained and fixed in the previous patch could have also
been fixed by this patch. But I think it's good change anyhow, that
could prevent future bugs, so here it is.
fetch_inferior_event switches to an arbitrary (in practice, the first) inferior
of the process target of the inferior used to fetch the event. The idea is
that the event handling code will need to do some target calls, so we want to
switch to an inferior that has target target.
However, you can have two inferiors that share a process target, but with one
inferior having an additional target on top:
inf 1 inf 2
----- -----
another target
process target process target
exec exec
Let's say inferior 2 is selected by do_target_wait and returns an event that is
really synthetized by "another target". This "another target" could be a
thread or record stratum target (in the case explained by the previous patch,
it was the arch stratum target, but it's because the amd-dbgapi abuses the arch
layer). fetch_inferior_event will then switch to the first inferior with
"process target", so inferior 1. handle_signal_stop then tries to fetch the
thread's registers:
This will try to get the thread's register by calling into the current target
stack, the stack of inferior 1. This is problematic because "another target"
might have a special fetch_registers implementation.
I think it would be a good idea to switch to the inferior for which the
even was reported, not just some inferior of the same process target.
This will ensure that any target call done before we eventually call
context_switch will be done on the full target stack that reported the
event.
Not all events are associated to an inferior though. For instance,
TARGET_WAITKIND_NO_RESUMED. In those cases, some targets return
null_ptid, some return minus_one_ptid (ideally the expected return value
should be clearly defined / documented). So, if the ptid returned is
either of these, switch to an arbitrary inferior with that process
target, as before.
Change-Id: I1ffc8c1095125ab591d0dc79ea40025b1d7454af Reviewed-By: Pedro Alves <pedro@palves.net>
Simon Marchi [Mon, 3 Apr 2023 18:52:06 +0000 (14:52 -0400)]
gdb: make regcache::raw_update switch to right inferior
With the following patch, which teaches the amd-dbgapi target to handle
inferiors that fork, we end up with target stacks in the following
state, when an inferior that does not use the GPU forks an inferior that
eventually uses the GPU.
When a GPU thread from inferior 2 hits a breakpoint, the following
sequence of events would happen, if it was not for the current patch.
- we start with inferior 1 as current
- do_target_wait_1 makes inferior 2 current, does a target_wait, which
returns a stop event for an amd-dbgapi wave (thread).
- do_target_wait's scoped_restore_current_thread restores inferior 1 as
current
- fetch_inferior_event calls switch_to_target_no_thread with linux-nat
as the process target, since linux-nat is officially the process
target of inferior 2. This makes inferior 1 the current inferior, as
it's the first inferior with that target.
- In handle_signal_stop, we have:
regcache_read_pc executes while inferior 1 is still the current one
(because it's before the `context_switch`). This is a problem,
because the regcache is for a ptid managed by the amd-dbgapi target
(e.g. (12345, 1, 1)), a ptid that does not make sense for the
linux-nat target. The fetch_registers target call goes directly
to the linux-nat target, which gets confused.
- We would then get an error like:
Couldn't get extended state status: No such process.
... since linux-nat tries to do a ptrace call on tid 1.
GDB should switch to the inferior the ptid belongs to before doing the
target call to fetch registers, to make sure the call hits the right
target stack (it should be handled by the amd-dbgapi target in this
case). In fact the following patch does this change, and it would be
enough to fix this specific problem.
However, I propose to change regcache to make it switch to the right
inferior, if needed, before doing target calls. That makes the
interface as a whole more independent of the global context.
My first attempt at doing this was to find an inferior using the process
stratum target and the ptid that regcache already knows about:
However, this caused some failures in fork-related tests and gdbserver
boards. When we detach a fork child, we may create a regcache for the
child, but there is no corresponding inferior. For instance, to restore
the PC after a displaced step over the fork syscall. So
find_inferior_ptid would return nullptr, and
switch_to_inferior_no_thread would hit a failed assertion.
So, this patch adds to regcache the information "the inferior to switch
to to makes target calls". In typical cases, it will be the inferior
that matches the regcache's ptid. But in some cases, like the detached
fork child one, it will be another inferior (in this example, it will be
the fork parent inferior).
The problem that we witnessed was in regcache::raw_update specifically,
but I looked for other regcache methods doing target calls, and added
the same inferior switching code to raw_write too.
In the regcache constructor and in get_thread_arch_aspace_regcache,
"inf_for_target_calls" replaces the process_stratum_target parameter.
We suppose that the process stratum target that would be passed
otherwise is the same that is in inf_for_target_calls's target stack, so
we don't need to pass both in parallel. The process stratum target is
still used as a key in the `target_pid_ptid_regcache_map` map, but
that's it.
There is one spot that needs to be updated outside of the regcache code,
which is the path that handles the "restore PC after a displaced step in
a fork child we're about to detach" case mentioned above.
regcache_test_data needs to be changed to include full-fledged mock
contexts (because there now needs to be inferiors, not just targets).
Change-Id: Id088569ce106e1f194d9ae7240ff436f11c5e123 Reviewed-By: Pedro Alves <pedro@palves.net>
Simon Marchi [Mon, 3 Apr 2023 18:52:05 +0000 (14:52 -0400)]
gdb: add maybe_switch_inferior function
Add the maybe_switch_inferior function, which ensures that the given
inferior is the current one. Return an instantiated
scoped_restore_current_thread object only we actually needed to switch
inferior.
Returning a scoped_restore_current_thread requires it to be
move-constructible, so give it a move constructor.
Change-Id: I1231037102ed6166f2530399e8257ad937fb0569 Reviewed-By: Pedro Alves <pedro@palves.net>
Simon Marchi [Mon, 3 Apr 2023 18:52:04 +0000 (14:52 -0400)]
gdb: remove regcache::target
The regcache class takes a process_stratum_target and then exposes it
through regcache::target. But it doesn't use it itself, suggesting it
doesn't really make sense to put it there. The only user of
regcache::target is record_btrace_target::fetch_registers, but it might
as well just get it from the current target stack. This simplifies a
little bit a patch later in this series.
Change-Id: I8878d875805681c77f469ac1a2bf3a508559a62d Reviewed-By: Pedro Alves <pedro@palves.net>
Simon Marchi [Mon, 3 Apr 2023 18:52:03 +0000 (14:52 -0400)]
gdb: add inferior_forked observable
In the upcoming patch to support fork in the amd-dbgapi target, the
amd-dbgapi target will need to be notified of fork events through an
observer, to attach itself (attach in the amd-dbgapi sense, not ptrace
sense) to the new inferior / process.
The reason that this can't be done through target_ops::follow_fork is
that the amd-dbgapi target isn't pushed on the inferior's target stack
right away. It attaches itself to the process and only pushes itself on
its target stack if and when the inferior initializes the ROCm runtime.
If an inferior that is not using the ROCm runtime forks, we want to be
notified of it, so we can attach to the child, and catch if the child
starts using the ROCm runtime.
So, add a new observable and notify it in follow_fork_inferior. It will
be used later in this series.
Change-Id: I67fced5a9cba6d5da72b9c7ea1c8397644ca1d54 Reviewed-By: Pedro Alves <pedro@palves.net>
Simon Marchi [Mon, 3 Apr 2023 18:52:02 +0000 (14:52 -0400)]
gdb: pass execing and following inferior to inferior_execd observers
The upcoming patch to support exec in the amd-dbgapi target needs to
detach amd-dbgapi from the inferior doing the exec and attach amd-dbgapi
to the inferior continuing the execution. They may or may not be the
same, depending on the `set follow-exec-mode` setting. But even if they
are the same, we need to do the detach / attach dance.
With the current observable signature, the observers only receive the
inferior in which execution continues (the "following" inferior).
Change the signature to pass both inferiors, and update all existing
observers.
Change-Id: I259d1ea09f70f43be739378d6023796f2fce2659 Reviewed-By: Pedro Alves <pedro@palves.net>
Tom Tromey [Mon, 27 Mar 2023 20:35:17 +0000 (14:35 -0600)]
Additions to gdb_mpz
In preparation for adding more 128-bit support to gdb, a few additions
to gdb_mpz are needed.
First, this adds a new 'as_integer_truncate' method. This method
works like 'as_integer' but does not require the value to fit in the
target type -- it just truncates.
Second, gdb_mpz::export_bits is changed to handle the somewhat unusual
situation of zero-length types. This can happen for a Rust '()' type;
but I think other languages have zero-bit integer types as well.
Nick Clifton [Mon, 17 Apr 2023 16:19:21 +0000 (17:19 +0100)]
Make the .rsrc section read only.
PR 30142
* peXXigen.c (_bfd_XXi_swap_scnhdr_out): Do not force the .rsrc section to be writeable.
* rescoff.c (write_coff_file): Add the SEC_READONLY flag to the .rsrc section.
Tom de Vries [Mon, 17 Apr 2023 16:09:32 +0000 (18:09 +0200)]
[gdb/symtab] Handle empty file name in .debug_line section
With DWARF 5, it's possible to produce an empty file name in the File Name
Table of the .debug_line section:
...
The File Name Table (offset 0x112, lines 1, columns 2):
Entry Dir Name
0 1 (indirect line string, offset: 0x2d):
...
Currently, when gdb reads an exec containing such debug info, it segfaults:
...
Thread 1 "gdb" received signal SIGSEGV, Segmentation fault.
0x000000000072cd38 in dwarf2_start_subfile (cu=0x2badc50, fe=..., lh=...) at \
gdb/dwarf2/read.c:18716
18716 if (!IS_ABSOLUTE_PATH (filename) && dirname != NULL)
...
because read_direct_string transforms "" into a nullptr, and we end up
dereferencing the nullptr.
Note that the behaviour of read_direct_string has been present since repo
creation.
Fix this in read_formatted_entries, by transforming nullptr filenames in to ""
filenames.
Tested on x86_64-linux.
Reviewed-By: Tom Tromey <tom@tromey.com>
PR symtab/30357
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30357
[gdb/testsuite] Fix unbalanced quotes in mi_expect_stop argument
Introduced the use of {"} in mi-support.exp. There is absolutely
nothing wrong with this in any way. However, this is causing my
editor to get the syntax highlighting of this file wrong after this
point.
Maybe the real answer is to use a better editor, or fix my current
editor.... but I'm hoping I can instead take the lazy approach of just
changing {"} to "\"", which is handled fine, and means exactly the
same as far as I understand it.
There should be no change in what is tested after this commit.
Luis Machado [Mon, 3 Apr 2023 09:43:34 +0000 (10:43 +0100)]
pauth: Create new feature string for pauth to prevent crashing older gdb's
Older gdb's (9, 10, 11 and 12) have a bug that causes them to crash whenever
a target reports the pauth feature string in the target description and also
provide additional register outside of gdb's known and expected feature
strings.
This was fixed in gdb 13 onwards, but that means we're stuck with gdb's out
there that will crash on connection to the above targets.
QEMU has postponed inclusion of the pauth feature string in version 8, and
instead we agreed to use a new feature name to prevent crashing those older
gdb's.
Initially there was a plan to backport a trivial fix all the way to gdb 9, but
given QEMU's choice, this is no longer needed.
This new feature string is org.gnu.gdb.aarch64.pauth_v2, and should be used
by all targets going forward, except native linux gdb and gdbserver, for
backwards compatibility with older gdb's/gdbserver's.
gdb/gdbserver will still emit the old feature string for Linux since it doesn't
report additional system registers and thus doesn't cause a crash of older
gdb's. We can revisit this in the future once the problematic gdb's are likely
no longer in use.
I've added some documentation to explain the situation.
Hui Li [Mon, 6 Mar 2023 03:55:15 +0000 (11:55 +0800)]
gdb/testsuite: Skip dump ihex for 64-bit address in gdb.base/dump.exp
(1) Description of problem
In the current code, when execute the following test on LoongArch:
$make check-gdb TESTS="gdb.base/dump.exp"
```
FAIL: gdb.base/dump.exp: dump array as value, intel hex
FAIL: gdb.base/dump.exp: dump struct as value, intel hex
FAIL: gdb.base/dump.exp: dump array as memory, ihex
FAIL: gdb.base/dump.exp: dump struct as memory, ihex
```
These tests passed on the X86_64,
(2) Root cause
On LoongArch, variable intarray address 0x120008068 out of range for IHEX,
so dump ihex test failed.
gdb.base/dump.exp has the following code to check 64-bit address
```
# Check the address of a variable. If it is bigger than 32-bit,
# assume our target has 64-bit addresses that are not supported by SREC,
# IHEX and TEKHEX. We skip those tests then.
set max_32bit_address "0xffffffff"
set data_address [get_hexadecimal_valueof "&intarray" 0x100000000]
if {${data_address} > ${max_32bit_address}} {
set is64bitonly "yes"
}
```
We check the "&intarray" on different target as follow:
On LoongArch:
(gdb) print /x &intarray
$1 = 0x120008068
```
The variable address difference here is due to the link script
of linker.
```
On X86_64:
$ld --verbose
...
PROVIDE (__executable_start = SEGMENT_START("text-segment", 0x400000));
. = SEGMENT_START("text-segment", 0x400000) + SIZEOF_HEADERS;
On LoongArch:
$ld --verbose
...
PROVIDE (__executable_start = SEGMENT_START("text-segment", 0x120000000));
. = SEGMENT_START("text-segment", 0x120000000) + SIZEOF_HEADERS;
```
(3) How to fix
Because 64-bit variable address out of range for IHEX, it's not an
functional problem for LoongArch. Refer to the handling of 64-bit
targets in this testsuite, use the "is64bitonly" flag to skip those
tests for the target has 64-bit addresses.
Signed-off-by: Hui Li <lihui@loongson.cn> Approved-By: Tom Tromey <tom@tromey.com> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
Tom Tromey [Tue, 6 Dec 2022 19:07:12 +0000 (12:07 -0700)]
Avoid double-free with debuginfod
PR gdb/29257 points out a possible double free when debuginfod is in
use. Aside from some ugly warts in the symbol code (an ongoing
issue), the underlying issue in this particular case is that elfread.c
seems to assume that symfile_bfd_open will return NULL on error,
whereas in reality it throws an exception. As this code isn't
prepared for an exception, bad things result.
This patch fixes the problem by introducing a non-throwing variant of
symfile_bfd_open and using it in the affected places.
All the tests are designed for a little-endian ARC system. Thus,
update the arc predicate in arc.exp, improve the matching pattern for
linker relaxation test, and add linker scripts to nps-1x tests.
The double store/loads instructions (e.g. STD/LDD) are not baseline
ARC ISA. The same holds for some short instructions. Update the
tests to use base ARC ISA.
Alan Modra [Thu, 13 Apr 2023 05:33:16 +0000 (15:03 +0930)]
Preserve a few more bfd fields in check_format_matches
AOUT and COFF targets set symcount and start_address in their object_p
functions. If these are used anywhere then it would pay to save and
restore them so that a successful match gets the values expected
rather than that for a later unsuccessful target match.
* format.c (struct bfd_preserve): Move some fields. Add
symcount, read_only and start_address.
(bfd_preserve_save): Save..
(bfd_preserve_restore): ..and restore..
(bfd_reinit): ..and zero new fields.
Alan Modra [Thu, 13 Apr 2023 02:14:41 +0000 (11:44 +0930)]
Re: pe_ILF_object_p and bfd_check_format_matches
The last patch wasn't quite correct. bfd_preserve_restore also needs
to handle an in-memory to file backed transition, seen in a testcase
ILF object matching both pei-arm-little and pei-arm-wince-little.
There the first match is saved in preserve_match, and restored at the
end of the bfd_check_format_matches loop making the bfd in-memory. On
finding more than one match the function wants to restore the bfd back
to its original state with another bfd_preserve_restore call before
exiting with a bfd_error_file_ambiguously_recognized error.
It is also not correct to restore abfd->iostream unless the iovec
changes. abfd->iostream is a FILE* when using cache_iovec, and if
the file has been closed and reopened the iostream may have changed.
* format.c (io_reinit): New function.
(bfd_reinit, bfd_preserve_restore): Use it.
Tom de Vries [Wed, 12 Apr 2023 22:18:12 +0000 (00:18 +0200)]
[gdb/tui] Revert workaround in tui_source_window::show_line_number
The m_digits member of tui_source_window is documented as having semantics:
...
/* How many digits to use when formatting the line number. This
includes the trailing space. */
...
The commit 1b6d4bb2232 ("Redraw both spaces between line numbers and source
code") started printing two trailing spaces instead:
...
- xsnprintf (text, sizeof (text), "%*d ", m_digits - 1, lineno);
+ xsnprintf (text, sizeof (text), "%*d ", m_digits - 1, lineno);
...
Now that PR30325 is fixed, this no longer has any effect.
Fix this by reverting to the original behaviour: print one trailing space
char.
Tom de Vries [Wed, 12 Apr 2023 22:18:12 +0000 (00:18 +0200)]
[gdb/tui] Fix left margin in disassembly window
With a hello world a.out, and maint set tui-left-margin-verbose on, we have
this disassembly window:
...
┌───────────────────────────────────────────────────────────┐
│___ 0x555555555149 <main> endbr64 │
│___ 0x55555555514d <main+4> push %rbp │
│___ 0x55555555514e <main+5> mov %rsp,%rbp │
│B+> 0x555555555151 <main+8> lea 0xeac(%rip),%rax│
│___ 0x555555555158 <main+15> mov %rax,%rdi │
...
Note the space between "B+>" and 0x555555555151. The space shows that a bit
of the left margin is not written, which is a problem because that location is
showing a character previously written, which happens to be a space, but also
may be something else, for instance a '[' as reported in PR tui/30325.
The problem is caused by confusion about the meaning of:
...
#define TUI_EXECINFO_SIZE 4
...
There's the meaning of defining the size of this zero-terminated char array:
...
char element[TUI_EXECINFO_SIZE];
...
which is used to print the "B+>" bit, which is 3 chars wide.
And there's the meaning of defining part of the size of the left margin:
...
int left_margin () const
{ return 1 + TUI_EXECINFO_SIZE + extra_margin (); }
...
where it represents 4 chars.
The discrepancy between the two causes the space between "B+>" and
"0x555555555151".
Fix this by redefining TUI_EXECINFO_SIZE to 3, and using:
...
char element[TUI_EXECINFO_SIZE + 1];
...
such that we have:
...
|B+>0x555555555151 <main+8> lea 0xeac(%rip),%rax │
...
This changes the layout of the disassembly window back to what it was before
commit 9e820dec13e ("Use a curses pad for source and disassembly windows"),
the commit that introduced the PR30325 regression.
This also changes the source window from:
...
│___000005__{ |
...
to:
...
│___000005_{ |
...
The TUI has two types of windows derived from tui_source_window_base:
- tui_source_window (the source window), and
- tui_disasm_window (the disassembly window).
The two windows share a common concept: the left margin.
With a hello world a.out, we can see the source window:
...
┌─/home/vries/hello.c───────────────────────────────────────┐
│ 5 { │
│B+> 6 printf ("hello\n"); │
│ 7 return 0; │
│ 8 } │
│ 9 │
│
...
where the left margin is the part holding "B+>" and the line number, and the
disassembly window:
...
┌───────────────────────────────────────────────────────────┐
│ 0x555555555149 <main> endbr64 │
│ 0x55555555514d <main+4> push %rbp │
│ 0x55555555514e <main+5> mov %rsp,%rbp │
│B+> 0x555555555151 <main+8> lea 0xeac(%rip),%rax│
│ 0x555555555158 <main+15> mov %rax,%rdi │
...
where the left margin is just the bit holding "B+>".
Because the left margin contains some spaces, it's not clear where it starts
and ends, making it harder to observe problems related to it.
Add a new maintenance command "maint set tui-left-margin-verbose", that when
set to on replaces the spaces in the left margin with either '_' or '0',
giving us this for the source window:
...
┌─/home/vries/hello.c───────────────────────────────────────┐
│___000005__{ │
│B+>000006__ printf ("hello\n"); │
│___000007__ return 0; │
│___000008__} │
...
and this for the disassembly window:
...
┌───────────────────────────────────────────────────────────┐
│___ 0x555555555149 <main> endbr64 │
│___ 0x55555555514d <main+4> push %rbp │
│___ 0x55555555514e <main+5> mov %rsp,%rbp │
│B+> 0x555555555151 <main+8> lea 0xeac(%rip),%rax│
│___ 0x555555555158 <main+15> mov %rax,%rdi │
...
Note the space between "B+>" and 0x555555555151. The space shows that a bit
of the left margin is not written, a problem reported as PR tui/30325.
Specifically, PR tui/30325 is about the fact that the '[' character from the
string "[ No Assembly Available ]" ends up in that same spot:
...
│B+>[0x555555555151 <main+8> lea 0xeac(%rip),%rax│
...
which only happens for certain window widths.
The new command allows us to spot the problem with any window width.
Likewise, when we revert the fix from commit 1b6d4bb2232 ("Redraw both spaces
between line numbers and source code"), we have:
...
┌─/home/vries/hello.c───────────────────────────────────────┐
│___000005_ { │
│B+>000006_ printf ("hello\n"); │
│___000007_ return 0; │
│___000008_ } │
...
showing a similar problem at the space between '_' and '{'.
Tested on x86_64-linux.
Reviewed-By: Eli Zaretskii <eliz@gnu.org> Approved-By: Tom Tromey <tom@tromey.com>
YunQiang Su [Wed, 12 Apr 2023 12:25:46 +0000 (13:25 +0100)]
MIPS: make mipsisa32 and mipsisa64 link more systematic
Introduce `static const struct mips_mach_extension mips_mach_32_64[]`
and `mips_mach_extends_32_64 (unsigned long base, unsigned long extension)`,
to make mipsisa32 and mipsisa64 interlink more systemtic.
Normally, the ISA mipsisa64rN has two subset: mipsisa64r(N-1) and
mipsisa32rN. `mips_mach_extensions` can hold only mipsisa64r(N-1),
so we need to introduce a new instruction `mips_mach_32_64`, which holds the pair 32vs64.
Note: R6 is not compatible with pre-R6.
bfd/ChangeLog:
* elfxx-mips.c (mips_mach_extends_p): make mipsisa32 and
mipsisa64 interlink more systematic.
(mips_mach_32_64): new struct added.
(mips_mach_extends_32_64): new function added.
Alan Modra [Wed, 12 Apr 2023 01:30:42 +0000 (11:00 +0930)]
PR30326, uninitialised value in objdump compare_relocs
This is a fuzzing PR, with a testcase involving a SHF_ALLOC and
SHF_COMPRESSED SHT_RELA section, ie. a compressed dynamic reloc
section. BFD doesn't handle compressed relocation sections, with most
of the code reading relocs using sh_size (often no bfd section is
created) but in the case of SHF_ALLOC dynamic relocs we had some code
using the bfd section size. This led to a mismatch, sh_size is
compressed, size is uncompressed, and from that some uninitialised
memory. Consistently using sh_size is enough to fix this PR, but I've
also added tests to exclude SHF_COMPRESSED reloc sections from
consideration.
PR 30362
* elf.c (bfd_section_from_shdr): Exclude reloc sections with
SHF_COMPRESSED flag from normal reloc processing.
(_bfd_elf_get_dynamic_reloc_upper_bound): Similarly exclude
SHF_COMPRESSED sections from consideration. Use sh_size when
sizing to match slurp_relocs.
(_bfd_elf_canonicalize_dynamic_reloc): Likewise.
(_bfd_elf_get_synthetic_symtab): Use NUM_SHDR_ENTRIES to size
plt relocs.
* elf32-arm.c (elf32_arm_get_synthetic_symtab): Likewise.
* elf32-ppc.c (ppc_elf_get_synthetic_symtab): Likewise.
* elf64-ppc.c (ppc64_elf_get_synthetic_symtab): Likewise.
* elfxx-mips.c (_bfd_mips_elf_get_synthetic_symtab): Likewise.
Alan Modra [Tue, 11 Apr 2023 12:41:26 +0000 (22:11 +0930)]
pe_ILF_object_p and bfd_check_format_matches
If pe_ILF_object_p succeeds, pe_ILF_build_a_bfd will have changed the
bfd from being file backed to in-memory. This can have unfortunate
results for targets checked by bfd_check_format_matches after that
point as they will be matching against the created in-memory image
rather than the file. bfd_preserve_restore also has a problem if it
flips the BFD_IN_MEMORY flag, because the flag affects iostream
meaning and should be set if using _bfd_memory_iovec. To fix these
problems, save and restore iostream and iovec along with flags, and
modify bfd_reinit to make the bfd file backed again. Restoring the
iovec and iostream allows the hack in bfd_reinit keeping BFD_IN_MEMORY
(part of BFD_FLAGS_SAVED) to be removed.
One more detail: If restoring from file backed to in-memory then the
bfd needs to be forcibly removed from the cache lru list, since after
the bfd becomes in-memory a bfd_close will delete the bfd's memory
leaving the lru list pointing into freed memory.
* cache.c (bfd_cache_init): Clear BFD_CLOSED_BY_CACHE here..
(bfd_cache_lookup_worker): ..rather than here.
(bfd_cache_close): Comment.
* format.c (struct bfd_preserve): Add iovec and iostream fields.
(bfd_preserve_save): Save them..
(bfd_preserve_restore): ..and restore them, calling
bfd_cache_close if the iovec differs.
(bfd_reinit): Add preserve param. If the bfd has been flipped
to in-memory, reopen the file. Restore flags.
* peicode.h (pe_ILF_cleanup): New function.
(pe_ILF_object_p): Return it.
* bfd.c (BFD_FLAGS_SAVED): Delete.
* bfd-in2.h: Regenerate.
The bfd_elf_hash loop is taken straight from the sysV document, but it
is poorly optimized. This refactoring removes about 5 x86 insns from
the 15 insn loop.
1) The if (..) is meaningless -- we're xoring with that value, and of
course xor 0 is a nop. On x86 (at least) we actually compute the xor'd
value and then cmov. Removing the if test removes the cmov.
2) The 'h ^ g' to clear the top 4 bits is not needed, as those 4 bits
will be shifted out in the next iteration. All we need to do is sink
a mask of those 4 bits out of the loop.
3) anding with 0xf0 after shifting by 24 bits can allow betterin
encoding on RISC ISAs than masking with '0xf0 << 24' before shifting.
RISC ISAs often require materializing larger constants.
bfd/
* elf.c (bfd_elf_hash): Refactor to optimize loop.
(bfd_elf_gnu_hash): Refactor to use 32-bit type.
gdb, doc: correct argument description for info connections/inferiors
It said for 'info inferiors' and 'info connections' that the argument
could be 'a space separated list of inferior numbers' which is correct
but incomplete. In fact the arguments can be any space separated
combination of numbers and (ascending) ranges.
The beginning of the section now describes the ID list as a new keyword.
Co-Authored-By: Christina Schimpe <christina.schimpe@intel.com>
projects / thirdparty / binutils-gdb.git / log
