| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
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.
|
|
This commit aims to address a problem that exists with the current
approach to displaced stepping, and was identified in PR gdb/22921.
Displaced stepping is currently supported on AArch64, ARM, amd64,
i386, rs6000 (ppc), and s390. Of these, I believe there is a problem
with the current approach which will impact amd64 and ARM, and can
lead to random register corruption when the inferior makes use of
asynchronous signals and GDB is using displaced stepping.
The problem can be found in displaced_step_buffers::finish in
displaced-stepping.c, and is this; after GDB tries to perform a
displaced step, and the inferior stops, GDB classifies the stop into
one of two states, either the displaced step succeeded, or the
displaced step failed.
If the displaced step succeeded then gdbarch_displaced_step_fixup is
called, which has the job of fixing up the state of the current
inferior as if the step had not been performed in a displaced manner.
This all seems just fine.
However, if the displaced step is considered to have not completed
then GDB doesn't call gdbarch_displaced_step_fixup, instead GDB
remains in displaced_step_buffers::finish and just performs a minimal
fixup which involves adjusting the program counter back to its
original value.
The problem here is that for amd64 and ARM setting up for a displaced
step can involve changing the values in some temporary registers. If
the displaced step succeeds then this is fine; after the step the
temporary registers are restored to their original values in the
architecture specific code.
But if the displaced step does not succeed then the temporary
registers are never restored, and they retain their modified values.
In this context a temporary register is simply any register that is
not otherwise used by the instruction being stepped that the
architecture specific code considers safe to borrow for the lifetime
of the instruction being stepped.
In the bug PR gdb/22921, the amd64 instruction being stepped is
an rip-relative instruction like this:
jmp *0x2fe2(%rip)
When we displaced step this instruction we borrow a register, and
modify the instruction to something like:
jmp *0x2fe2(%rcx)
with %rcx having its value adjusted to contain the original %rip
value.
Now if the displaced step does not succeed, then %rcx will be left
with a corrupted value. Obviously corrupting any register is bad; in
the bug report this problem was spotted because %rcx is used as a
function argument register.
And finally, why might a displaced step not succeed? Asynchronous
signals provides one reason. GDB sets up for the displaced step and,
at that precise moment, the OS delivers a signal (SIGALRM in the bug
report), the signal stops the inferior at the address of the displaced
instruction. GDB cancels the displaced instruction, handles the
signal, and then tries again with the displaced step. But it is that
first cancellation of the displaced step that causes the problem; in
that case GDB (correctly) sees the displaced step as having not
completed, and so does not perform the architecture specific fixup,
leaving the register corrupted.
The reason why I think AArch64, rs600, i386, and s390 are not effected
by this problem is that I don't believe these architectures make use
of any temporary registers, so when a displaced step is not completed
successfully, the minimal fix up is sufficient.
On amd64 we use at most one temporary register.
On ARM, looking at arm_displaced_step_copy_insn_closure, we could
modify up to 16 temporary registers, and the instruction being
displaced stepped could be expanded to multiple replacement
instructions, which increases the chances of this bug triggering.
This commit only aims to address the issue on amd64 for now, though I
believe that the approach I'm proposing here might be applicable for
ARM too.
What I propose is that we always call gdbarch_displaced_step_fixup.
We will now pass an extra argument to gdbarch_displaced_step_fixup,
this a boolean that indicates whether GDB thinks the displaced step
completed successfully or not.
When this flag is false this indicates that the displaced step halted
for some "other" reason. On ARM GDB can potentially read the
inferior's program counter in order figure out how far through the
sequence of replacement instructions we got, and from that GDB can
figure out what fixup needs to be performed.
On targets like amd64 the problem is slightly easier as displaced
stepping only uses a single replacement instruction. If the displaced
step didn't complete the GDB knows that the single instruction didn't
execute.
The point is that by always calling gdbarch_displaced_step_fixup, each
architecture can now ensure that the inferior state is fixed up
correctly in all cases, not just the success case.
On amd64 this ensures that we always restore the temporary register
value, and so bug PR gdb/22921 is resolved.
In order to move all architectures to this new API, I have moved the
minimal roll-back version of the code inside the architecture specific
fixup functions for AArch64, rs600, s390, and ARM. For all of these
except ARM I think this is good enough, as no temporaries are used all
that's needed is the program counter restore anyway.
For ARM the minimal code is no worse than what we had before, though I
do consider this architecture's displaced-stepping broken.
I've updated the gdb.arch/amd64-disp-step.exp test to cover the
'jmpq*' instruction that was causing problems in the original bug, and
also added support for testing the displaced step in the presence of
asynchronous signal delivery.
I've also added two new tests (for amd64 and i386) that check that GDB
can correctly handle displaced stepping over a single instruction that
branches to itself. I added these tests after a first version of this
patch relied too much on checking the program-counter value in order
to see if the displaced instruction had executed. This works fine in
almost all cases, but when an instruction branches to itself a pure
program counter check is not sufficient. The new tests expose this
problem.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=22921
Approved-By: Pedro Alves <pedro@palves.net>
|
|
Make find_thread_ptid (the overload that takes a process_stratum_target)
a method of process_stratum_target.
Change-Id: Ib190a925a83c6b93e9c585dc7c6ab65efbdd8629
Reviewed-By: Tom Tromey <tom@tromey.com>
|
|
There were reports of gdb throwing internal errors when calling
inferior_thread ()/get_current_regcache () on a system with
Pointer Authentication enabled.
In such cases, gdb produces the following backtrace:
../../../repos/binutils-gdb/gdb/thread.c:86: internal-error: inferior_thread: Assertion `current_thread_ != nullptr' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
----- Backtrace -----
0xaaaae04a571f gdb_internal_backtrace_1
../../../repos/binutils-gdb/gdb/bt-utils.c:122
0xaaaae04a57f3 _Z22gdb_internal_backtracev
../../../repos/binutils-gdb/gdb/bt-utils.c:168
0xaaaae0b52ccf internal_vproblem
../../../repos/binutils-gdb/gdb/utils.c:401
0xaaaae0b5310b _Z15internal_verrorPKciS0_St9__va_list
../../../repos/binutils-gdb/gdb/utils.c:481
0xaaaae0e24b8f _Z18internal_error_locPKciS0_z
../../../repos/binutils-gdb/gdbsupport/errors.cc:58
0xaaaae0a88983 _Z15inferior_threadv
../../../repos/binutils-gdb/gdb/thread.c:86
0xaaaae0956c87 _Z20get_current_regcachev
../../../repos/binutils-gdb/gdb/regcache.c:428
0xaaaae035223f aarch64_remove_non_address_bits
../../../repos/binutils-gdb/gdb/aarch64-tdep.c:3572
0xaaaae03e8abb _Z31gdbarch_remove_non_address_bitsP7gdbarchm
../../../repos/binutils-gdb/gdb/gdbarch.c:3109
0xaaaae0a692d7 memory_xfer_partial
../../../repos/binutils-gdb/gdb/target.c:1620
0xaaaae0a695e3 _Z19target_xfer_partialP10target_ops13target_objectPKcPhPKhmmPm
../../../repos/binutils-gdb/gdb/target.c:1684
0xaaaae0a69e9f target_read_partial
../../../repos/binutils-gdb/gdb/target.c:1937
0xaaaae0a69fdf _Z11target_readP10target_ops13target_objectPKcPhml
../../../repos/binutils-gdb/gdb/target.c:1977
0xaaaae0a69937 _Z18target_read_memorymPhl
../../../repos/binutils-gdb/gdb/target.c:1773
0xaaaae08be523 ps_xfer_memory
../../../repos/binutils-gdb/gdb/proc-service.c:90
0xaaaae08be6db ps_pdread
../../../repos/binutils-gdb/gdb/proc-service.c:124
0x40001ed7c3b3 _td_fetch_value
/build/glibc-RIFKjK/glibc-2.31/nptl_db/fetch-value.c:115
0x40001ed791ef td_ta_map_lwp2thr
/build/glibc-RIFKjK/glibc-2.31/nptl_db/td_ta_map_lwp2thr.c:194
0xaaaae07f4473 thread_from_lwp
../../../repos/binutils-gdb/gdb/linux-thread-db.c:413
0xaaaae07f6d6f _ZN16thread_db_target4waitE6ptid_tP17target_waitstatus10enum_flagsI16target_wait_flagE
../../../repos/binutils-gdb/gdb/linux-thread-db.c:1420
0xaaaae0a6b33b _Z11target_wait6ptid_tP17target_waitstatus10enum_flagsI16target_wait_flagE
../../../repos/binutils-gdb/gdb/target.c:2586
0xaaaae0789cf7 do_target_wait_1
../../../repos/binutils-gdb/gdb/infrun.c:3825
0xaaaae0789e6f operator()
../../../repos/binutils-gdb/gdb/infrun.c:3884
0xaaaae078a167 do_target_wait
../../../repos/binutils-gdb/gdb/infrun.c:3903
0xaaaae078b0af _Z20fetch_inferior_eventv
../../../repos/binutils-gdb/gdb/infrun.c:4314
0xaaaae076652f _Z22inferior_event_handler19inferior_event_type
../../../repos/binutils-gdb/gdb/inf-loop.c:41
0xaaaae07dc68b handle_target_event
../../../repos/binutils-gdb/gdb/linux-nat.c:4206
0xaaaae0e25fbb handle_file_event
../../../repos/binutils-gdb/gdbsupport/event-loop.cc:573
0xaaaae0e264f3 gdb_wait_for_event
../../../repos/binutils-gdb/gdbsupport/event-loop.cc:694
0xaaaae0e24f9b _Z16gdb_do_one_eventi
../../../repos/binutils-gdb/gdbsupport/event-loop.cc:217
0xaaaae080f033 start_event_loop
../../../repos/binutils-gdb/gdb/main.c:411
0xaaaae080f1b7 captured_command_loop
../../../repos/binutils-gdb/gdb/main.c:475
0xaaaae0810b97 captured_main
../../../repos/binutils-gdb/gdb/main.c:1318
0xaaaae0810c1b _Z8gdb_mainP18captured_main_args
../../../repos/binutils-gdb/gdb/main.c:1337
0xaaaae0338453 main
../../../repos/binutils-gdb/gdb/gdb.c:32
---------------------
../../../repos/binutils-gdb/gdb/thread.c:86: internal-error: inferior_thread: Assertion `current_thread_ != nullptr' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n)
We also see failures across the testsuite if the tests get executed on a target
that has native support for the pointer authentication feature. But
gdb.base/break.exp and gdb.base/access-mem-running.exp are two examples of
tests that run into errors and internal errors.
This issue started after commit d88cb738e6a7a7179dfaff8af78d69250c852af1, which
enabled more broad use of pointer authentication masks to remove non-address
bits of pointers, but wasn't immediately detected because systems with native
support for pointer authentication are not that common yet.
The above crash happens because gdb is in the middle of handling an event,
and do_target_wait_1 calls switch_to_inferior_no_thread, nullifying the
current thread. This means a call to inferior_thread () will assert, and
attempting to call get_current_regcache () will also call inferior_thread (),
resulting in an assertion as well.
target_has_registers was one function that seemed useful for detecting these
types of situation where we don't have a register cache. The problem with that
is the inconsistent state of inferior_ptid, which is used by
target_has_registers.
Despite the call to switch_to_no_thread in switch_to_inferior_no_thread from
do_target_wait_1 in the backtrace above clearing inferior_ptid, the call to
ps_xfer_memory sets inferior_ptid momentarily before reading memory:
static ps_err_e
ps_xfer_memory (const struct ps_prochandle *ph, psaddr_t addr,
gdb_byte *buf, size_t len, int write)
{
scoped_restore_current_inferior restore_inferior;
set_current_inferior (ph->thread->inf);
scoped_restore_current_program_space restore_current_progspace;
set_current_program_space (ph->thread->inf->pspace);
scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
inferior_ptid = ph->thread->ptid;
CORE_ADDR core_addr = ps_addr_to_core_addr (addr);
int ret;
if (write)
ret = target_write_memory (core_addr, buf, len);
else
ret = target_read_memory (core_addr, buf, len);
return (ret == 0 ? PS_OK : PS_ERR);
}
Maybe this shouldn't happen, or maybe it is just an unfortunate state to be
in. But this prevents the use of target_has_registers to guard against the
lack of registers, since, although current_thread_ is still nullptr,
inferior_ptid is valid and is not null_ptid.
There is another crash scenario after we kill a previously active inferior, in
which case the gdbarch will still say we support pointer authentication but we
will also have no current thread (inferior_thread () will assert etc).
If the target has support for pointer authentication, gdb needs to use
a couple (or 4, for bare-metal) mask registers to mask off some bits of
pointers, and for that it needs to access the registers.
At some points, like the one from the backtrace above, there is no active
thread/current regcache because gdb is in the middle of doing event handling
and switching between threads.
Simon suggested the use of inferior_ptid to fetch the register cache, as
opposed to relying on the current register cache. Though we need to make sure
inferior_ptid is valid (not null_ptid), I think this works nicely.
With inferior_ptid, we can do safety checks along the way, making sure we have
a thread to fetch a register cache from and checking if the thread is actually
stopped or running.
The following patch implements this idea with safety checks to make sure we
don't run into assertions or errors. If any of the checks fail, we fallback to
using a default mask to remove non-address bits of a pointer.
I discussed with Pedro the possibility of caching the mask register values
(which are per-process and can change mid-execution), but there isn't a good
spot to cache those values. Besides, the mask registers can change constantly
for bare-metal debugging when switching between exception levels.
In some cases, it is just not possible to get access to these mask registers,
like the case where threads are running. In those cases, using a default mask
to remove the non-address bits should be enough.
This can happen when we let threads run in the background and then we attempt
to access a memory address (now that gdb is capable of reading memory even
with threads running). Thus gdb will attempt to remove non-address bits
of that memory access, will attempt to access registers, running into errors.
Regression-tested on aarch64-linux Ubuntu 20.04.
|
|
metal/kernel mode addresses
At the moment GDB only handles pointer authentication (pauth) for userspace
addresses and if we're debugging a Linux-hosted program.
The Linux Kernel can be configured to use pauth instructions for some
additional security hardening, but GDB doesn't handle this well.
To overcome this limitation, GDB needs a couple things:
1 - The target needs to advertise pauth support.
2 - The hook to remove non-address bits from a pointer needs to be registered
in aarch64-tdep.c as opposed to aarch64-linux-tdep.c.
There is a patch for QEMU that addresses the first point, and it makes
QEMU's gdbstub expose a couple more pauth mask registers, so overall we will
have up to 4 pauth masks (2 masks or 4 masks):
pauth_dmask
pauth_cmask
pauth_dmask_high
pauth_cmask_high
pauth_dmask and pauth_cmask are the masks used to remove pauth signatures
from userspace addresses. pauth_dmask_high and pauth_cmask_high masks are used
to remove pauth signatures from kernel addresses.
The second point is easily addressed by moving code around.
When debugging a Linux Kernel built with pauth with an unpatched GDB, we get
the following backtrace:
#0 __fput (file=0xffff0000c17a6400) at /repos/linux/fs/file_table.c:296
#1 0xffff8000082bd1f0 in ____fput (work=<optimized out>) at /repos/linux/fs/file_table.c:348
#2 0x30008000080ade30 [PAC] in ?? ()
#3 0x30d48000080ade30 in ?? ()
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
With a patched GDB, we get something a lot more meaningful:
#0 __fput (file=0xffff0000c1bcfa00) at /repos/linux/fs/file_table.c:296
#1 0xffff8000082bd1f0 in ____fput (work=<optimized out>) at /repos/linux/fs/file_table.c:348
#2 0xffff8000080ade30 [PAC] in task_work_run () at /repos/linux/kernel/task_work.c:179
#3 0xffff80000801db90 [PAC] in resume_user_mode_work (regs=0xffff80000a96beb0) at /repos/linux/include/linux/resume_user_mode.h:49
#4 do_notify_resume (regs=regs@entry=0xffff80000a96beb0, thread_flags=4) at /repos/linux/arch/arm64/kernel/signal.c:1127
#5 0xffff800008fb9974 [PAC] in prepare_exit_to_user_mode (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:137
#6 exit_to_user_mode (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:142
#7 el0_svc (regs=0xffff80000a96beb0) at /repos/linux/arch/arm64/kernel/entry-common.c:638
#8 0xffff800008fb9d34 [PAC] in el0t_64_sync_handler (regs=<optimized out>) at /repos/linux/arch/arm64/kernel/entry-common.c:655
#9 0xffff800008011548 [PAC] in el0t_64_sync () at /repos/linux/arch/arm64/kernel/entry.S:586
Backtrace stopped: Cannot access memory at address 0xffff80000a96c0c8
|
|
This introduces the set_lval method on value, one step toward removing
deprecated_lval_hack. Ultimately I think the goal should be for some
of these set_* methods to be replaced with constructors; but I haven't
done this, as the series is already too long. Other 'deprecated'
methods can probably be handled the same way.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This patch turns a grab bag of value functions to methods of value.
These are done together because their implementations are
interrelated.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This turns many functions that are related to optimized-out or
availability-checking to be methods of value. The static function
value_entirely_covered_by_range_vector is also converted to be a
private method.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This turns the remaining value_contents functions -- value_contents,
value_contents_all, value_contents_for_printing, and
value_contents_for_printing_const -- into methods of value. It also
converts the static functions require_not_optimized_out and
require_available to be private methods.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This turns value_contents_raw, value_contents_writeable, and
value_contents_all_raw into methods on value. The remaining functions
will be changed later in the series; they were a bit trickier and so I
didn't include them in this patch.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes allocate_value to be a static "constructor" of value.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes value_type to be a method of value. Much of this patch
was written by script.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Consider the test-case test.c, compiled without debug info:
...
void
foo (const char *s)
{
}
int
main (void)
{
foo ("foo");
return 0;
}
...
Disassembly of foo:
...
0000000000400564 <foo>:
400564: d10043ff sub sp, sp, #0x10
400568: f90007e0 str x0, [sp, #8]
40056c: d503201f nop
400570: 910043ff add sp, sp, #0x10
400574: d65f03c0 ret
...
Now, let's do "info frame" at each insn in foo, as well as printing $sp
and $x29 (and strip the output of info frame to the first line, for brevity):
...
$ gdb -q a.out
Reading symbols from a.out...
(gdb) b *foo
Breakpoint 1 at 0x400564
(gdb) r
Starting program: a.out
Breakpoint 1, 0x0000000000400564 in foo ()
(gdb) display /x $sp
1: /x $sp = 0xfffffffff3a0
(gdb) display /x $x29
2: /x $x29 = 0xfffffffff3a0
(gdb) info frame
Stack level 0, frame at 0xfffffffff3a0:
(gdb) si
0x0000000000400568 in foo ()
1: /x $sp = 0xfffffffff390
2: /x $x29 = 0xfffffffff3a0
(gdb) info frame
Stack level 0, frame at 0xfffffffff3a0:
(gdb) si
0x000000000040056c in foo ()
1: /x $sp = 0xfffffffff390
2: /x $x29 = 0xfffffffff3a0
(gdb) info frame
Stack level 0, frame at 0xfffffffff3a0:
(gdb) si
0x0000000000400570 in foo ()
1: /x $sp = 0xfffffffff390
2: /x $x29 = 0xfffffffff3a0
(gdb) info frame
Stack level 0, frame at 0xfffffffff3a0:
(gdb) si
0x0000000000400574 in foo ()
1: /x $sp = 0xfffffffff3a0
2: /x $x29 = 0xfffffffff3a0
(gdb) info frame
Stack level 0, frame at 0xfffffffff3b0:
pc = 0x400574 in foo; saved pc = 0x40058c
(gdb) si
0x000000000040058c in main ()
1: /x $sp = 0xfffffffff3a0
2: /x $x29 = 0xfffffffff3a0
...
The "frame at" bit lists 0xfffffffff3a0 except at the last insn, where it
lists 0xfffffffff3b0.
The frame address is calculated here in aarch64_make_prologue_cache_1:
...
unwound_fp = get_frame_register_unsigned (this_frame, cache->framereg);
if (unwound_fp == 0)
return;
cache->prev_sp = unwound_fp + cache->framesize;
...
For insns after the prologue, we have cache->framereg == sp and
cache->framesize == 16, so unwound_fp + cache->framesize gives the wrong
answer once sp has been restored to entry value by the before-last insn.
Fix this by detecting the situation that the sp has been restored.
This fixes PRs tdep/30010 and tdep/30011.
This also fixes the aarch64 FAILs in gdb.reverse/solib-precsave.exp and
gdb.reverse/solib-reverse.exp I reported in PR gdb/PR29721.
Tested on aarch64-linux.
PR tdep/30010
PR tdep/30011
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30010
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30011
|
|
This helps resolve some cyclic include problem later in the series.
The only language-related thing frame.h needs is enum language, and that
is in defs.h.
Doing so reveals that a bunch of files were relying on frame.h to
include language.h, so fix the fallouts here and there.
Change-Id: I178a7efec1953c2d088adb58483bade1f349b705
Reviewed-By: Bruno Larsen <blarsen@redhat.com>
|
|
In aarch64-tdep.c we find these register aliases:
...
{
/* 64-bit register names. */
{"fp", AARCH64_FP_REGNUM},
{"lr", AARCH64_LR_REGNUM},
{"sp", AARCH64_SP_REGNUM},
...
The sp alias is superfluous, because the canonical name of x31 is already sp.
The fp alias is superfluous, because it's already taken by the default meaning
of fp, assigned here in _initialize_frame_reg:
...
user_reg_add_builtin ("fp", value_of_builtin_frame_fp_reg, NULL);
...
Fix this by removing the fp and sp aliases.
While we're at it, add an x31 alias for sp.
Approved-By: Luis Machado <luis.machado@arm.com>
Tested on aarch64-linux.
PR tdep/30012
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30012
|
|
It's currently not clear how the ownership of gdbarch_tdep objects
works. In fact, nothing ever takes ownership of it. This is mostly
fine because we never free gdbarch objects, and thus we never free
gdbarch_tdep objects. There is an exception to that however: when
initialization fails, we do free the gdbarch object that is not going to
be used, and we free the tdep too. Currently, i386 and s390 do it.
To make things clearer, change gdbarch_alloc so that it takes ownership
of the tdep. The tdep is thus automatically freed if the gdbarch is
freed.
Change all gdbarch initialization functions to pass a new gdbarch_tdep
object to gdbarch_alloc and then retrieve a non-owning reference from
the gdbarch object.
Before this patch, the xtensa architecture had a single global instance
of xtensa_gdbarch_tdep. Since we need to pass a dynamically allocated
gdbarch_tdep_base instance to gdbarch_alloc, remove this global
instance, and dynamically allocate one as needed, like we do for all
other architectures. Make the `rmap` array externally visible and
rename it to the less collision-prone `xtensa_rmap` name.
Change-Id: Id3d70493ef80ce4bdff701c57636f4c79ed8aea2
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
This patch updates the gdbarch_return_value_as_value implementations
to work correctly with variably-sized return types.
|
|
This converts a few selected architectures to use
gdbarch_return_value_as_value rather than gdbarch_return_value. The
architectures are just the ones that I am able to test. This patch
should not introduce any behavior changes.
|
|
This commit is the result of running the gdb/copyright.py script,
which automated the update of the copyright year range for all
source files managed by the GDB project to be updated to include
year 2023.
|
|
With the AArch64 Scalable Matrix Extension we have a new TPIDR2 register, and
it will be added to the existing NT_ARM_TLS register set. Kernel patches are
being reviewed here:
https://lore.kernel.org/linux-arm-kernel/20220818170111.351889-1-broonie@kernel.org/
From GDB's perspective, we handle it in a similar way to the existing TPIDR
register. But we need to consider cases of systems that only have TPIDR and
systems that have both TPIDR and TPIDR2.
With that in mind, the following patch adds the required code to support
TPIDR2 and turns the org.gnu.gdb.aarch64.tls feature into a
dynamically-generated target description as opposed to a static target
description containing only TPIDR.
That means we can remove the gdb/features/aarch64-tls.xml file and replace the
existing gdb/features/aarch64-tls.c auto-generated file with a new file that
dynamically generates the target description containing either TPIDR alone or
TPIDR and TPIDR2.
In the future, when *BSD's start to support this register, they can just
enable it as is being done for the AArch64 Linux target.
The core file read/write code has been updated to support TPIDR2 as well.
On GDBserver's side, there is a small change to the find_regno function to
expose a non-throwing version of it.
It always seemed strange to me how find_regno causes the whole operation to
abort if it doesn't find a particular register name. The patch moves code
from find_regno into find_regno_no_throw and makes find_regno call
find_regno_no_throw instead.
This allows us to do register name lookups to find a particular register
number without risking erroring out if nothing is found.
The patch also adjusts the feature detection code for aarch64-fbsd, since
the infrastructure is shared amongst all aarch64 targets. I haven't added
code to support TPIDR2 in aarch64-fbsd though, as I'm not sure when/if
that will happen.
|
|
The missing newline causes testsuite issues because the gdb prompt gets output
to an unexpected location.
|
|
PR tdep/28796
As reported, we are using some memory read routines that don't handle read
errors gracefully. Convert those to use the safe_* versions if available.
This allows the code to handle those read errors in a more sensible way.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28796
|
|
Currently, every internal_error call must be passed __FILE__/__LINE__
explicitly, like:
internal_error (__FILE__, __LINE__, "foo %d", var);
The need to pass in explicit __FILE__/__LINE__ is there probably
because the function predates widespread and portable variadic macros
availability. We can use variadic macros nowadays, and in fact, we
already use them in several places, including the related
gdb_assert_not_reached.
So this patch renames the internal_error function to something else,
and then reimplements internal_error as a variadic macro that expands
__FILE__/__LINE__ itself.
The result is that we now should call internal_error like so:
internal_error ("foo %d", var);
Likewise for internal_warning.
The patch adjusts all calls sites. 99% of the adjustments were done
with a perl/sed script.
The non-mechanical changes are in gdbsupport/errors.h,
gdbsupport/gdb_assert.h, and gdb/gdbarch.py.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Change-Id: Ia6f372c11550ca876829e8fd85048f4502bdcf06
|
|
This changes GDB to use frame_info_ptr instead of frame_info *
The substitution was done with multiple sequential `sed` commands:
sed 's/^struct frame_info;/class frame_info_ptr;/'
sed 's/struct frame_info \*/frame_info_ptr /g' - which left some
issues in a few files, that were manually fixed.
sed 's/\<frame_info \*/frame_info_ptr /g'
sed 's/frame_info_ptr $/frame_info_ptr/g' - used to remove whitespace
problems.
The changed files were then manually checked and some 'sed' changes
undone, some constructors and some gets were added, according to what
made sense, and what Tromey originally did
Co-Authored-By: Bruno Larsen <blarsen@redhat.com>
Approved-by: Tom Tomey <tom@tromey.com>
|
|
registers
The aarch64 port handles W registers as aliases of X registers. This is
incorrect because X registers are 64-bit and W registers are 32-bit.
This patch teaches GDB how to handle W registers as pseudo-registers of
32-bit, the bottom half of the X registers.
Testcase included.
|
|
additional registers
When using AArch64 GDB with the QEMU debugging stub (in user mode), we get
additional system registers that GDB doesn't particularly care about, so
it doesn't number those explicitly.
But given the pseudo-register numbers are above the number of real registers,
we need to setup/account for the real registers first before going ahead and
numbering the pseudo-registers. This has to happen at the end of
aarch64_gdbarch_init, after the call to tdesc_use_registers, as that
updates the total number of real registers.
This is in preparation to supporting pointer authentication for bare metal
aarch64 (QEMU).
|
|
Remove the macro, replace all uses with calls to type::length.
Change-Id: Ib9bdc954576860b21190886534c99103d6a47afb
|
|
Remove the macro, replace all uses by calls to type::target_type.
Change-Id: Ie51d3e1e22f94130176d6abd723255282bb6d1ed
|
|
As reported by Tom in https://sourceware.org/pipermail/gdb-patches/2022-August/191357.html,
the aarch64 prologue analyzer considers the adrp instruction in the
gdb.dwarf2/dw2-dir-file-name.exp testcase to be part of a prologue.
The function has no prologue though, and it only loads the volatile variable
from memory. GDB should not skip any instructions in this case.
Doing some archaeology, it seems handling for adr/adrp in prologues was
included with the original aarch64 port. It might've been an oversight.
In the particular case of gdb.dwarf2/dw2-dir-file-name.exp, the analyzer skips
a couple instructions and leaves us in a nice spot where the address to the
variable "v" is already in w0. But no prologues exists.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29481
|
|
When the inferior program changes the SVE length, GDB can stop tracking
some registers as it obtains the new gdbarch that corresponds to the
updated length:
Breakpoint 1, do_sve_ioctl_test () at sve-ioctls.c:44
44 res = prctl(PR_SVE_SET_VL, i, 0, 0, 0, 0);
(gdb) print i
$2 = 32
(gdb) info registers
⋮
[ snip registers x0 to x30 ]
⋮
sp 0xffffffffeff0 0xffffffffeff0
pc 0xaaaaaaaaa8ac 0xaaaaaaaaa8ac <do_sve_ioctl_test+112>
cpsr 0x60000000 [ EL=0 BTYPE=0 C Z ]
fpsr 0x0 0
fpcr 0x0 0
vg 0x8 8
tpidr 0xfffff7fcb320 0xfffff7fcb320
(gdb) next
45 if (res < 0) {
(gdb) info registers
⋮
[ snip registers x0 to x30 ]
⋮
sp 0xffffffffeff0 0xffffffffeff0
pc 0xaaaaaaaaa8cc 0xaaaaaaaaa8cc <do_sve_ioctl_test+144>
cpsr 0x200000 [ EL=0 BTYPE=0 SS ]
fpsr 0x0 0
fpcr 0x0 0
vg 0x4 4
(gdb)
Notice that register tpidr disappeared when vg (which holds the vector
length) changed from 8 to 4. The tpidr register is provided by the
org.gnu.gdb.aarch64.tls feature.
This happens because the code that searches for a new gdbarch to match the
new vector length in aarch64_linux_nat_target::thread_architecture doesn't
take into account the features present in the target description associated
with the previous gdbarch. This patch makes it do that.
Since the id member of struct gdbarch_info is now unused, it's removed.
|
|
On aarch64, I run into:
...
FAIL: gdb.base/large-frame.exp: optimize=-O0: backtrace
...
The problem is that the architecture-specific prologue analyzer fails to
handle the first two insns in the prologue properly:
...
0000000000400610 <func>:
400610: d2880210 mov x16, #0x4010
400614: cb3063ff sub sp, sp, x16
400618: a9007bfd stp x29, x30, [sp]
40061c: 910003fd mov x29, sp
400620: 910043a0 add x0, x29, #0x10
400624: 97fffff0 bl 4005e4 <blah>
...
so we get:
...
$ gdb -q -batch ./outputs/gdb.base/large-frame/large-frame-O0 -ex "b func"
Breakpoint 1 at 0x400614
...
Fix this by:
- fixing the support for the first insn to extract the immediate operand, and
- adding support for the second insn,
such that we have:
...
Breakpoint 1 at 0x400624
...
Note that we're overshooting by one insn (0x400620 is the first insn after the
prologue), but that's a pre-existing problem.
Tested on aarch64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29408
|
|
I built GDB for all targets on a x86-64/GNU-Linux system, and
then (accidentally) passed GDB a RISC-V binary, and asked GDB to "run"
the binary on the native target. I got this error:
(gdb) show architecture
The target architecture is set to "auto" (currently "i386").
(gdb) file /tmp/hello.rv32.exe
Reading symbols from /tmp/hello.rv32.exe...
(gdb) show architecture
The target architecture is set to "auto" (currently "riscv:rv32").
(gdb) run
Starting program: /tmp/hello.rv32.exe
../../src/gdb/i387-tdep.c:596: internal-error: i387_supply_fxsave: Assertion `tdep->st0_regnum >= I386_ST0_REGNUM' failed.
What's going on here is this; initially the architecture is i386, this
is based on the default architecture, which is set based on the native
target. After loading the RISC-V executable the architecture of the
current inferior is updated based on the architecture of the
executable.
When we "run", GDB does a fork & exec, with the inferior being
controlled through ptrace. GDB sees an initial stop from the inferior
as soon as the inferior comes to life. In response to this stop GDB
ends up calling save_stop_reason (linux-nat.c), which ends up trying
to read register from the inferior, to do this we end up calling
target_ops::fetch_registers, which, for the x86-64 native target,
calls amd64_linux_nat_target::fetch_registers.
After this I eventually end up in i387_supply_fxsave, different x86
based targets will end in different functions to fetch registers, but
it doesn't really matter which function we end up in, the problem is
this line, which is repeated in many places:
i386_gdbarch_tdep *tdep = (i386_gdbarch_tdep *) gdbarch_tdep (arch);
The problem here is that the ARCH in this line comes from the current
inferior, which, as we discussed above, will be a RISC-V gdbarch, the
tdep field will actually be of type riscv_gdbarch_tdep, not
i386_gdbarch_tdep. After this cast we are relying on undefined
behaviour, in my case I happen to trigger an assert, but this might
not always be the case.
The thing I tried that exposed this problem was of course, trying to
start an executable of the wrong architecture on a native target. I
don't think that the correct solution for this problem is to detect,
at the point of cast, that the gdbarch_tdep object is of the wrong
type, but, I did wonder, is there a way that we could protect
ourselves from incorrectly casting the gdbarch_tdep object?
I think that there is something we can do here, and this commit is the
first step in that direction, though no actual check is added by this
commit.
This commit can be split into two parts:
(1) In gdbarch.h and arch-utils.c. In these files I have modified
gdbarch_tdep (the function) so that it now takes a template argument,
like this:
template<typename TDepType>
static inline TDepType *
gdbarch_tdep (struct gdbarch *gdbarch)
{
struct gdbarch_tdep *tdep = gdbarch_tdep_1 (gdbarch);
return static_cast<TDepType *> (tdep);
}
After this change we are no better protected, but the cast is now
done within the gdbarch_tdep function rather than at the call sites,
this leads to the second, much larger change in this commit,
(2) Everywhere gdbarch_tdep is called, we make changes like this:
- i386_gdbarch_tdep *tdep = (i386_gdbarch_tdep *) gdbarch_tdep (arch);
+ i386_gdbarch_tdep *tdep = gdbarch_tdep<i386_gdbarch_tdep> (arch);
There should be no functional change after this commit.
In the next commit I will build on this change to add an assertion in
gdbarch_tdep that checks we are casting to the correct type.
|
|
"struct insn_decode_record_t" is defined in multiple .c files, causing
ODR warnings. This patch renames the types, and removes the use of
"typedef" here -- this is a C-ism that's no longer needed.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=22395
|
|
This small patch adds support for TYPE_CODE_DECFLOAT in
aapcs_is_vfp_call_or_return_candidate_1 and pass_in_v_vfp_candidate,
so that GDB for AArch64 knows how to pass DFP parameters and how to
read DFP results when calling a function.
Tested on aarch64-linux-gnu, with a GCC with DFP support in the PATH,
all of GDB's DFP tests pass.
|
|
Replace the sve bool member of aarch64_features with a vq member that
holds the vector quotient. It is zero if SVE is not present.
Add std::hash<> specialization and operator== so that aarch64_features
can be used as a key with std::unordered_map<>.
Change the various functions that create or lookup aarch64 target
descriptions to accept a const aarch64_features object rather than a
growing number of arguments.
Replace the multi-dimension tdesc_aarch64_list arrays used to cache
target descriptions with unordered_maps indexed by aarch64_feature.
|
|
This will allow the unwind info to explicitly specify a different value
for the return address from the link register.
Such usage, although uncommon, is valid and useful for signal frames.
It is also supported by aadwarf64 from ARM (Note 9 in [1]).
Ref https://sourceware.org/pipermail/gdb/2022-May/050091.html
[1] https://github.com/ARM-software/abi-aa/blob/2022Q1/aadwarf64/aadwarf64.rst#dwarf-register-names
Signed-off-by: Luis Machado <luis.machado@arm.com>
|
|
The AARCH64_DWARF_PAUTH_DMASK and AARCH64_DWARF_PAUTH_CMASK DWARF registers
never made their way into the aadwarf64. The following patch removes these
constants and their use.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26295
|
|
The aadwarf64 [1] names this register RA_SIGN_STATE, so update the code to use
the same name.
[1] https://github.com/ARM-software/abi-aa/blob/main/aadwarf64/aadwarf64.rst
|
|
|
|
There's a set of 7 default register groups. If we don't add any
gdbarch specific register groups during gdbarch initialisation, then
when we iterate over the register groups using reggroup_next and
reggroup_prev we will make use of these 7 default groups. See the use
of default_groups in gdb/reggroups.c for details on this.
However, if the gdbarch adds its own groups during gdbarch
initialisation, then these groups will be used in preference to the
default groups.
A problem arises though if the particular architecture makes use of
the target description mechanism. If the default target
description(s) (i.e. those internal to GDB that are used when the user
doesn't provide their own) don't mention any additional register
groups then the default register groups will be used.
But if the target description does mention additional groups then the
default groups are not used, and instead, the groups from the target
description are used.
The problem with this is that what usually happens is that the target
description will mention additional groups, e.g. groups for special
registers. Most architectures that use target descriptions work
around this by adding all (or most) of the default register groups in
all cases. See i386_add_reggroups, aarch64_add_reggroups,
riscv_add_reggroups, xtensa_add_reggroups, and others.
In this patch, my suggestion is that we should just add the default
register groups for every architecture, always. This change is in
gdb/reggroups.c.
All the remaining changes are me updating the various architectures to
not add the default groups themselves.
So, where will this change be visible to the user? I think the
following commands will possibly change:
* info registers / info all-registers:
The user can provide a register group to these commands. For example,
on csky, we previously never added the 'vector' group. Now, as a
default group, this will be available, but (presumably) will not
contain any registers. I don't think this is necessarily a bad
thing, there's something to be said for having some consistent
defaults available. There are other architectures that didn't add
all 7 of the defaults, which will now have gained additional groups.
* maint print reggroups
This prints the set of all available groups. As a maintenance
command I'm less concerned with the output changing here.
Obviously, for the architectures that didn't previously add all the
defaults, this list just got bigger.
* maint print register-groups
This prints all the registers, and the groups they are in. If the
defaults were not previously being added then a register (obviously)
can't appear in one of the default groups. Now the groups are
available then registers might be in more groups than previously.
However, this is again a maintenance command, so I'm less concerned
about this changing.
|
|
Change gdbarch_register_reggroup_p to take a 'const struct reggroup *'
argument. This requires a change to the gdb/gdbarch-components.py
script, regeneration of gdbarch.{c,h}, and then updates to all the
architectures that implement this method.
There should be no user visible changes after this commit.
|
|
It is better to rename floatformats_ia64_quad to floatformats_ieee_quad
to reflect the reality, and then we can clean up the related code.
As Tom Tromey said [1]:
These files are maintained in gcc and then imported into the
binutils-gdb repository, so any changes to them will have to
be proposed there first.
the related changes have been merged into gcc master now [2], it is time
to do it for gdb.
[1] https://sourceware.org/pipermail/gdb-patches/2022-March/186569.html
[2] https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b2dff6b2d9d6
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
|
|
Now that filtered and unfiltered output can be treated identically, we
can unify the printf family of functions. This is done under the name
"gdb_printf". Most of this patch was written by script.
|
|
The internal AdaCore testsuite has a test that checks that an
out-of-scope watchpoint is deleted. This fails on some aarch64
configurations, reporting an extra stop:
(gdb) continue
Continuing.
Thread 3 hit Watchpoint 2: result
Old value = 64
New value = 0
0x0000000040021648 in pck.get_val (seed=0, off_by_one=false) at [...]/pck.adb:13
13 end Get_Val;
I believe what is happening here is that the variable is stored at:
<efa> DW_AT_location : 2 byte block: 91 7c (DW_OP_fbreg: -4)
and the extra stop is reported just before a return, when the ldp
instruction is executed:
0x0000000040021644 <+204>: ldp x29, x30, [sp], #48
0x0000000040021648 <+208>: ret
This instruction modifies the frame base calculation, and so the test
picks up whatever memory is pointed to in the callee frame.
Implementing the gdbarch hook gdbarch_stack_frame_destroyed_p fixes
this problem.
As usual with this sort of patch, it has passed internal testing, but
I don't have a good way to try it with dejagnu. So, I don't know
whether some existing test covers this. I suspect there must be one,
but it's also worth noting that this test passes for aarch64 in some
configurations -- I don't know what causes one to fail and another to
succeed.
|
|
When running gdb.cp/non-trivial-retval.exp, the following shows up for
both aarch64-linux and armhf-linux:
Breakpoint 3, f1 (i1=23, i2=100) at src/gdb/testsuite/gdb.cp/non-trivial-retval.cc:35
35 A a;
(gdb) finish
Run till exit from #0 f1 (i1=23, i2=100) at src/gdb/testsuite/gdb.cp/non-trivial-retval.cc:35
main () at /src/gdb/testsuite/gdb.cp/non-trivial-retval.cc:163
163 B b = f2 (i1, i2);
Value returned is $6 = {a = -11952}
(gdb)
The return value should be {a = 123} instead. This happens because the
backends don't extract the return value from the correct location. GDB should
fetch a pointer to the memory location from X8 for aarch64 and r0 for armhf.
With the patch, gdb.cp/non-trivial-retval.exp has full passes on
aarch64-linux and armhf-linux on Ubuntu 20.04/18.04.
The problem only shows up with the "finish" command. The "call" command
works correctly and displays the correct return value.
This is also related to PR gdb/28681
(https://sourceware.org/bugzilla/show_bug.cgi?id=28681) and fixes FAIL's in
gdb.ada/mi_var_array.exp.
A new testcase is provided, and it exercises GDB's ability to "finish" a
function that returns a large struct (> 16 bytes) and display the
contents of this struct correctly. This has always been incorrect for
these backends, but no testcase exercised this particular scenario.
|
|
aarch64-tdep.c defines the following macro:
#define MEM_ALLOC(MEMS, LENGTH, RECORD_BUF) \
do \
{ \
unsigned int mem_len = LENGTH; \
if (mem_len) \
{ \
MEMS = XNEWVEC (struct aarch64_mem_r, mem_len); \
memcpy(&MEMS->len, &RECORD_BUF[0], \
sizeof(struct aarch64_mem_r) * LENGTH); \
} \
} \
while (0)
This is simlpy allocating a new array and copying it. However, for
the destination address, it is actually copying into the first member
of the first element of the array (`&MEMS->len"). This elicits a
warning with GCC 12:
../../binutils-gdb/gdb/aarch64-tdep.c: In function ‘int aarch64_process_record(gdbarch*, regcache*, CORE_ADDR)’:
../../binutils-gdb/gdb/aarch64-tdep.c:3711:23: error: writing 16 bytes into a region of size 8 [-Werror=stringop-overflow=]
3711 | memcpy(&MEMS->len, &RECORD_BUF[0], \
| ^
../../binutils-gdb/gdb/aarch64-tdep.c:4394:3: note: in expansion of macro ‘MEM_ALLOC’
4394 | MEM_ALLOC (aarch64_insn_r->aarch64_mems, aarch64_insn_r->mem_rec_count,
| ^~~~~~~~~
../../binutils-gdb/gdb/aarch64-tdep.c:3721:12: note: destination object ‘aarch64_mem_r::len’ of size 8
3721 | uint64_t len; /* Record length. */
| ^~~
The simple fix is to reference the array, `MEMS' as the destination of the copy.
Tested by rebuilding.
# Please enter the commit message for your changes. Lines starting
# with '#' will be kept; you may remove them yourself if you want to.
# An empty message aborts the commit.
#
# Date: Tue Jan 25 08:28:32 2022 -0800
#
# On branch master
# Your branch is ahead of 'origin/master' by 1 commit.
# (use "git push" to publish your local commits)
#
# Changes to be committed:
# modified: aarch64-tdep.c
#
|
|
This changes gdbarch dumping to use filtered output. This seems a bit
better to me, both on the principle that this is an ordinary command,
and because the output can be voluminous, so it may be nice to stop in
the middle.
|
|
This commit brings all the changes made by running gdb/copyright.py
as per GDB's Start of New Year Procedure.
For the avoidance of doubt, all changes in this commits were
performed by the script.
|
|
The process record code often emits unfiltered output. In some cases,
this output ought to go to gdb_stderr (but see below). In other
cases, the output is guarded by a logging variable and so ought to go
to gdb_stdlog. This patch makes these changes.
Note that in many cases, the output to stderr is followed by a
"return -1", which is how process record indicates an error. It seems
to me that calling error here would be preferable, because, in many
cases, that's all the caller does when it sees a -1. However, I
haven't made this change.
This is part of PR gdb/7233.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7233
|
|
Consider the following code:
type FP1_Type is delta 0.1 range -1.0 .. +1.0; -- Ordinary
function Call_FP1 (F : FP1_Type) return FP1_Type is
begin
return F;
end Call_FP1;
When the default in GCC is to generate proper DWARF info for fixed point
types, then in gdb, printing the result of a call to call_fp1 with a
decimal parameter leads to:
(gdb) p call_fp1(0.5)
$1 = 0
The displayed value is wrong, and we actually expected:
(gdb) p call_fp1(0.5)
$1 = 0.5
What happened is that our fixed point type parameter got promoted to a
32bit integer because we detected that the length of that object was less
than 4 bytes. The compiler does not perform this promotion and therefore
GDB should not either.
This patch fixes the behavior described above.
|
