Tom de Vries reported that some of the test for the vmov[u|a]p[s|d] were
failing. In my machine xmm3 was consistently set to 0x54, but apparently
that is different depending on the system. This commit zeroes out xmm3
at the start of the test instead.
While debugging the test failures, I also noticed an issue where the
recording wasn't saving all the required memory. That happened because
vmovs[s|d] shares its opcode with vmovap[s|d], meaning they seem to
share code paths, but the latter encodes memory modification size on
VEX.L whereas the former encodes in VEX.pp. So this commit fixed that,
and made the relevant tests more robust and complete.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32561
Approved-By: Guinevere Larsen <guinevere@redhat.com>
Frame unwinders have historically been a structure populated with
callback pointers, so that architectures (or other specific unwinders)
could install their own way to handle the inferior. However, since
moving to C++, we could use polymorphism to get the same functionality
in a more readable way. Polymorphism also makes it simpler to add new
functionality to all frame unwinders, since all that's required is
adding it to the base class.
As part of the changes to add support to disabling frame unwinders,
this commit makes the first baby step in using polymorphism for the
frame unwinders, by making frame_unwind a virtual class, and adds a
couple of new classes. The main class added is frame_unwind_legacy,
which works the same as the previous structs, using function pointers
as callbacks. This class was added to allow the transition to happen
piecemeal. New unwinders should instead follow the lead of the other
classes implemented.
2 of the others, frame_unwind_python and frame_unwind_trampoline, were added
because it seemed simpler at the moment to do that instead of reworking
the dynamic allocation to work with the legacy class, and can be used as
an example to future implementations.
Finally, the cygwin unwinder was converted to a class since it was most
of the way there already.
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
A future patch will add a way to disable certain unwinders based on
different characteristics. This patch aims to make it more convenient
to disable related unwinders in bulk, such as architecture specific
ones, by identifying all unwinders by which part of the code adds it.
The classes, and explanations, are as follows:
* GDB: An internal unwinder, added by GDB core, such as the unwinder
for dummy frames;
* EXTENSION: Unwinders added by extension languages;
* DEBUGINFO: Unwinders installed by the debug info reader;
* ARCH: Unwinders installed by the architecture specific code.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
This commit adds support for the following types of instructions
relating to floating poitn values: add, mul, sub, min, div, max.
These are supported with packed or single values, and single or double
precision.
Some of the instructions had opcode clashes, however, considering the
mechanics of recording the registers is the same on both instructions,
this is just marked with a comment.
Approved-By: Guinevere Larsen <guinevere@redhat.com>
This commit adds support for the AVX instructions vunpck[l|h][ps|pd]
instructions, which was pretty straightforward.
This commit also fixes a mistake in the test, where "record stop" was
used after the recording was already stopped, if it failed during
vpunpck_test recording. It also improved the documentation at the start
of the relevant .c function.
Approved-By: Guinevere Larsen <guinevere@redhat.com>
This commit updates GDB's record-full to be able to record vmov[ss|sd]
and vmov [u|a] [ps|pd] AVX instructions, and tests for them.
Unlike the vmovdq[u|a] instructions, the aligned and unalgined versions
of vmov?[ps|pd] have different opcodes. The mechanics of recording them
is the same, but the aligned version has opcodes 0x28 and 0x29, while
the unaligned has the same opcode as vmov[ss|sd] instruction, 0x10 and
0x11.
Approved-By: Guinevere Larsen <guinevere@redhat.com>
Eli mentioned [1] that given that we use US English spelling in our
documentation, we should use "behavior" instead of "behaviour".
In wikipedia-common-misspellings.txt there's a rule:
...
behavour->behavior, behaviour
...
which leaves this as a choice.
Add an overriding rule to hardcode the choice to common-misspellings.txt:
...
behavour->behavior
...
and add a rule to rewrite behaviour into behavior:
...
behaviour->behavior
...
and re-run spellcheck.sh on gdb*.
Tested on x86_64-linux.
[1] https://sourceware.org/pipermail/gdb-patches/2024-November/213371.html
This commit adds recording support for the AVX instruction vpor, and the
AVX2 extension. Since the encoding of vpor and vpxor are the same, and
their semantics are basically the same, modulo the mathematical
operation, they are handled by the same switch case block.
This also updates the vpxor function, to test vpor and vpxor, and
updates the name to vpor_xor_test to better reflect what it does.
Approved-By: Tom Tromey <tom@tromey.com>
This commit adds support for recording the AVX instruction vpmovmskb,
and tests to the relevant file. The test didn't really support checking
general purpose registers, so this commit also adds a proc to
gdb.reverse/i386-avx-reverse.exp, which can be used to test them
Approved-By: Tom Tromey <tom@tromey.com>
This commit adds support to recording instructions of the form
VPCMPEQ[B|W|D]. They are all encoded in the same way and only
differentiated by the opcode, so they are all processed together. This
commit also updates the test to (quite exhaustively) test the new
instruction.
Approved-By: Tom Tromey <tom@tromey.com>
This commit adds support for recording the instruction vpxor,
introduced in the AVX extension, and extended in AVX2 to use 256 bit
registers. The test gdb.reverse/i386-avx-reverse.exp has been extended
to test this instruction as well.
Approved-By: Tom Tromey <tom@tromey.com>
This commit adds recording support for the AVX instruction vzeroupper,
which zeroes the high bits of ymm registers 0..15. In the programmer's
manual, it is explicitly states that ymm registers 16..31 won't be
affected if present, so we only need to record the first 16 registers.
We record ymm_h registers since only the higher bits are touched, and
that reduces the memory footprint of the instruction.
This instruction is tested differently as we want to confirm we're only
saving the relevant registers, and we want to ensure we're saving
all of them, so it makes use of "maint print record-instruction" to see
exactly what was recorded.
Approved-By: Tom Tromey <tom@tromey.com>
This commit adds support for the instructions VMOVDQU and VMOVDQA, used
to move values to/from 256 bit registers. Unfortunately, the
programmer's manual is very incomplete (if not wrong) about these
instructions, so the logic had to be reverse engineered from how gcc
actually encodes the instruction.
This commit also changes the memory regions from the test to store 256
bits, so its easier to test the instructions and that we're recording
ymm registers correctly.
Approved-By: Tom Tromey <tom@tromey.com>
This commit adds recording support to all AVX and AVX2 instructions
of the form vpbroadcast. GDB is not yet concerned about AVX512 in
recording mode, so for now we only support the AVX2 registers and
instructions.
This commit also updates the gdb.reverse/i386-avx-reverse.exp to test
broadcast instructions.
Approved-By: Tom Tromey <tom@tromey.com>
This commit adds support to recording instructions to unpack high
or low data from XMM registers, identified by the mnemonics in the
form: VPUNPCK [L|H] [BW|WD|DQ|QDQ].
All these instructions are encoded the exact same way, and only affect
the destination register, making them trivial to implement together.
It also updates the test gdb.reverse/i386-avx-reverse.exp to test these
new instructions. The test always uses ymm because the vpunpck
instructions overwrite the high bits, so we have to be able to record
the full ymm register, not just the output size.
Approved-By: Tom Tromey <tom@tromey.com>
This commit adds support to the x86_64 AVX instructions vmovd and vmovq.
The programmers manuals for Intel and AMD describe these 2 instructions
as being almost the same, but my local testing, using gcc 13.2 on Fedora
39, showed several differences and inconsistencies.
The instruction is supposed to always use the 3-byte VEX prefix, but I
could only find 2-byte versions. The instructions aren't differentiated
by the VEX.w bit, but by opcodes and VEX.pp.
This patch adds a test with many different uses for both vmovd and
vmovq. It also updates the test gdb.reverse/step-precsave.exp to
reference the generic "missing avx support" bug open in the bug tracker
(17346), instead of pointing to one that specifically calls out to
vmovd instructions.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23188
Approved-By: Tom Tromey <tom@tromey.com>
This patch introduces the information needed to properly identify the
VEX prefix, used to signal an AVX and AVX2 instruction, and introduces
a helper function to handle all AVX instruction, instead of adding to
the 3000 line long recording function.
This new function will temporarily set the current thread as "not
executing" so that it can read from pseudo registers as we record, since
most AVX/AVX2 instructions would benefit from recording ymm registers.
The new helper also handles unsupported instructions so that the largest
part of the i386_process_record doesn't have to be shifted by 2 spaces,
which made an unreadably big patch file.
The only expected difference to the end user added by this patch is a
small change to the unsupported message. This patch also updates the
test gdb.reverse/step-precsave.exp, by recognizing the new output.
As a note for the future, we don't handle xmm16-31 and ymm16-31 because
those require the EVEX prefix, meaning avx512 support.
Approved-By: Tom Tromey <tom@tromey.com>
GDB deprecated the commands "show/set mpx bound" in GDB 15.1, as Intel
listed Intel(R) Memory Protection Extensions (MPX) as removed in 2019.
MPX is also deprecated in gcc (since v9.1), the linux kernel (since v5.6)
and glibc (since v2.35). Let's now remove MPX support in GDB completely.
This includes the removal of:
- MPX functionality including register support
- deprecated mpx commands
- i386 and amd64 implementation of the hooks report_signal_info and
get_siginfo_type
- tests
- and pretty printer.
We keep MPX register numbers to not break compatibility with old gdbservers.
Approved-By: Felix Willgerodt <felix.willgerodt@intel.com>
This is a simple find / replace from "struct bound_minimal_symbol" to
"bound_minimal_symbol", to make things shorter and more consisten
througout. In some cases, move variable declarations where first used.
Change-Id: Ica4af11c4ac528aa842bfa49a7afe8fe77a66849
Reviewed-by: Keith Seitz <keiths@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
Most files including gdbcmd.h currently rely on it to access things
actually declared in cli/cli-cmds.h (setlist, showlist, etc). To make
things easy, replace all includes of gdbcmd.h with includes of
cli/cli-cmds.h. This might lead to some unused includes of
cli/cli-cmds.h, but it's harmless, and much faster than going through
the 170 or so files by hand.
Change-Id: I11f884d4d616c12c05f395c98bbc2892950fb00f
Approved-By: Tom Tromey <tom@tromey.com>
Move the declarations out of defs.h, and the implementations out of
findvar.c.
I opted for a new file, because this functionality of converting
integers to bytes and vice-versa seems a bit to generic to live in
findvar.c.
Change-Id: I524858fca33901ee2150c582bac16042148d2251
Approved-By: John Baldwin <jhb@FreeBSD.org>
Now that defs.h, server.h and common-defs.h are included via the
`-include` option, it is no longer necessary for source files to include
them. Remove all the inclusions of these files I could find. Update
the generation scripts where relevant.
Change-Id: Ia026cff269c1b7ae7386dd3619bc9bb6a5332837
Approved-By: Pedro Alves <pedro@palves.net>
This patch deprecates the MPX commands "show/set mpx bound".
Intel listed Intel(R) Memory Protection Extensions (MPX) as removed
in 2019. Following gcc v9.1, the linux kernel v5.6 and glibc v2.35,
deprecate MPX in GDB.
We currently pass frames to function by value, as `frame_info_ptr`.
This is somewhat expensive:
- the size of `frame_info_ptr` is 64 bytes, which is a bit big to pass
by value
- the constructors and destructor link/unlink the object in the global
`frame_info_ptr::frame_list` list. This is an `intrusive_list`, so
it's not so bad: it's just assigning a few points, there's no memory
allocation as if it was `std::list`, but still it's useless to do
that over and over.
As suggested by Tom Tromey, change many function signatures to accept
`const frame_info_ptr &` instead of `frame_info_ptr`.
Some functions reassign their `frame_info_ptr` parameter, like:
void
the_func (frame_info_ptr frame)
{
for (; frame != nullptr; frame = get_prev_frame (frame))
{
...
}
}
I wondered what to do about them, do I leave them as-is or change them
(and need to introduce a separate local variable that can be
re-assigned). I opted for the later for consistency. It might not be
clear why some functions take `const frame_info_ptr &` while others take
`frame_info_ptr`. Also, if a function took a `frame_info_ptr` because
it did re-assign its parameter, I doubt that we would think to change it
to `const frame_info_ptr &` should the implementation change such that
it doesn't need to take `frame_info_ptr` anymore. It seems better to
have a simple rule and apply it everywhere.
Change-Id: I59d10addef687d157f82ccf4d54f5dde9a963fd0
Approved-By: Andrew Burgess <aburgess@redhat.com>
This commit is the result of the following actions:
- Running gdb/copyright.py to update all of the copyright headers to
include 2024,
- Manually updating a few files the copyright.py script told me to
update, these files had copyright headers embedded within the
file,
- Regenerating gdbsupport/Makefile.in to refresh it's copyright
date,
- Using grep to find other files that still mentioned 2023. If
these files were updated last year from 2022 to 2023 then I've
updated them this year to 2024.
I'm sure I've probably missed some dates. Feel free to fix them up as
you spot them.
Make i386 and amd64 use the new gdbarch_pseudo_register_write. This
fixes writing to pseudo registers in non-current frames for those
architectures.
Change-Id: I4977e8fe12d2cef116f8834c34cdf6fec618554f
Reviewed-By: John Baldwin <jhb@FreeBSD.org>
The next patch introduces a new variant of gdbarch_pseudo_register_write
that takes a frame instead of a regcache for implementations to write
raw registers. Rename to old one to make it clear it's deprecated.
Change-Id: If8872c89c6f8a1edfcab983eb064248fd5ff3115
Reviewed-By: John Baldwin <jhb@FreeBSD.org>
Change gdbarch_pseudo_register_read_value to take a frame instead of a
regcache. The frame (and formerly the regcache) is used to read raw
registers needed to make up the pseudo register value. The problem with
using the regcache is that it always provides raw register values for
the current frame (frame 0).
Let's say the user wants to read the ebx register on amd64. ebx is a pseudo
register, obtained by reading the bottom half (bottom 4 bytes) of the
rbx register, which is a raw register. If the currently selected frame
is frame 0, it works fine:
(gdb) frame 0
#0 break_here_asm () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S:36
36 in /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S
(gdb) p/x $ebx
$1 = 0x24252627
(gdb) p/x $rbx
$2 = 0x2021222324252627
But if the user is looking at another frame, and the raw register behind
the pseudo register has been saved at some point in the call stack, then
we get a wrong answer:
(gdb) frame 1
#1 0x000055555555517d in caller () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S:56
56 in /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.arch/amd64-pseudo-unwind-asm.S
(gdb) p/x $ebx
$3 = 0x24252627
(gdb) p/x $rbx
$4 = 0x1011121314151617
Here, the value of ebx was computed using the value of rbx in frame 0
(through the regcache), it should have been computed using the value of
rbx in frame 1.
In other to make this work properly, make the following changes:
- Make dwarf2_frame_prev_register return nullptr if it doesn't know how
to unwind a register and that register is a pseudo register.
Previously, it returned `frame_unwind_got_register`, meaning, in our
example, "the value of ebx in frame 1 is the same as the value of ebx
in frame 0", which is obviously false. Return nullptr as a way to
say "I don't know".
- In frame_unwind_register_value, when prev_register (for instance
dwarf2_frame_prev_register) returns nullptr, and we are trying to
read a pseudo register, try to get the register value through
gdbarch_pseudo_register_read_value or gdbarch_pseudo_register_read.
If using gdbarch_pseudo_register_read, the behavior is known to be
broken. Implementations should be migrated to use
gdbarch_pseudo_register_read_value to fix that.
- Change gdbarch_pseudo_register_read_value to take a frame_info
instead of a regcache, update implementations (aarch64, amd64, i386).
In i386-tdep.c, I made a copy of i386_mmx_regnum_to_fp_regnum that
uses a frame instead of a regcache. The version using the regcache
is still used by i386_pseudo_register_write. It will get removed in
a subsequent patch.
- Add some helpers in value.{c,h} to implement the common cases of
pseudo registers: taking part of a raw register and concatenating
multiple raw registers.
- Update readable_regcache::{cooked_read,cooked_read_value} to pass the
current frame to gdbarch_pseudo_register_read_value. Passing the
current frame will give the same behavior as before: for frame 0, raw
registers will be read from the current thread's regcache.
Notes:
- I do not plan on changing gdbarch_pseudo_register_read to receive a
frame instead of a regcache. That method is considered deprecated.
Instead, we should be working on migrating implementations to use
gdbarch_pseudo_register_read_value instead.
- In frame_unwind_register_value, we still ask the unwinder to try to
unwind pseudo register values. It's apparently possible for the
debug info to provide information about [1] pseudo registers, so we
want to try that first, before falling back to computing them
ourselves.
[1] https://inbox.sourceware.org/gdb-patches/20180528174715.A954AD804AD@oc3748833570.ibm.com/
Change-Id: Id6ef1c64e19090a183dec050e4034d8c2394e7ca
Reviewed-by: John Baldwin <jhb@FreeBSD.org>
Similar to the previous patches, change get_frame_register_bytes to take
the "next frame" instead of "this frame".
Change-Id: Ie8f35042bfa6e93565fcefaee71b6b3903f0fe9f
Reviewed-By: John Baldwin <jhb@FreeBSD.org>
Similar to the previous patches, change put_frame_register to take the
"next frame" instead of "this frame".
Change-Id: I062fd4663b8f54f0fc7bbf39c860b7341363821b
Reviewed-By: John Baldwin <jhb@FreeBSD.org>
Change put_frame_register to take an array_view instead of a raw
pointer.
Add an assertion to verify that the number of bytes we try to write
matches the length of the register.
Change-Id: Ib75a9c8a12b47e203097621643eaa2c1830591ae
Reviewed-By: John Baldwin <jhb@FreeBSD.org>
I think that i386 k registers are raw registers, and therefore shouldn't
be handled in the various functions handling pseudo registers.
What tipped me off is the code in i386_pseudo_register_read_into_value:
else if (i386_k_regnum_p (gdbarch, regnum))
{
regnum -= tdep->k0_regnum;
/* Extract (always little endian). */
status = regcache->raw_read (tdep->k0_regnum + regnum, raw_buf);
We take regnum (the pseudo register number we want to read), subtract
k0_regnum, add k0_regnum, and pass the result to raw_read. So we would
end up calling raw_read with the same regnum as the function received
which is supposedly a pseudo register number.
Other hints are:
- The command `maint print raw-registers` shows the k registers.
- Printing $k0 doesn't cause i386_pseudo_register_read_into_value to be
called.
- There's code in i387-tdep.c to save/restore the k registers.
Remove handling of the k registers from:
- i386_pseudo_register_read_into_value
- i386_pseudo_register_write
- i386_ax_pseudo_register_collect
Change-Id: Ic97956ed59af6099fef6d36a0b61464172694562
Reviewed-by: John Baldwin <jhb@FreeBSD.org>
This patch adds support for process recording of the instruction rdtscp in
x86 architecture.
Debugging applications with "record full" fail to record with the error
message "Process record does not support instruction 0xf01f9".
Approved-by: Guinevere Larsen <blarsen@redhat.com>
This changes gdb to use the C++17 [[fallthrough]] attribute rather
than special comments.
This was mostly done by script, but I neglected a few spellings and so
also fixed it up by hand.
I suspect this fixes the bug mentioned below, by switching to a
standard approach that, presumably, clang supports.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23159
Approved-By: John Baldwin <jhb@FreeBSD.org>
Approved-By: Luis Machado <luis.machado@arm.com>
Approved-By: Pedro Alves <pedro@palves.net>
If a target provides a target description including registers from the
XSAVE extended region, but does not provide an XSAVE layout, use a
fallback XSAVE layout based on the included registers. This fallback
layout matches GDB's behavior in earlier releases which assumes the
layout from Intel CPUs.
This fallback layout is currently only used for remote targets since
native targets which support XSAVE provide an explicit layout derived
from CPUID.
PR gdb/30912
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30912
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Remove get_current_regcache, inlining the call to get_thread_regcache in
callers. When possible, pass the right thread_info object known from
the local context. Otherwise, fall back to passing `inferior_thread ()`.
This makes the reference to global context bubble up one level, a small
step towards the long term goal of reducing the number of references to
global context (or rather, moving those references as close as possible
to the top of the call tree).
No behavior change expected.
Change-Id: Ifa6980c88825d803ea586546b6b4c633c33be8d6
This function is just a wrapper around the current inferior's gdbarch.
I find that having that wrapper just obscures where the arch is coming
from, and that it's often used as "I don't know which arch to use so
I'll use this magical target_gdbarch function that gets me an arch" when
the arch should in fact come from something in the context (a thread,
objfile, symbol, etc). I think that removing it and inlining
`current_inferior ()->arch ()` everywhere will make it a bit clearer
where that arch comes from and will trigger people into reflecting
whether this is the right place to get the arch or not.
Change-Id: I79f14b4e4934c88f91ca3a3155f5fc3ea2fadf6b
Reviewed-By: John Baldwin <jhb@FreeBSD.org>
Approved-By: Andrew Burgess <aburgess@redhat.com>
This structure is fetched from the current target in i386_gdbarch_init
via a new "fetch_x86_xsave_layout" target method.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Fix grammar in some comments and docs:
- machines that doesn't -> machines that don't
- its a -> it's a
- its the -> it's the
- if does its not -> if it does it's not
- one more instructions if doesn't match ->
one more instruction if it doesn't match
- it's own -> its own
- it's first -> its first
- it's pointer -> its pointer
I also came across "it's performance" in gdb/stubs/*-stub.c in the HP public
domain notice, I've left that alone.
Tested on x86_64-linux.
This changes field_is_static to be a method on struct field, and
updates all the callers. Most of this patch was written by script.
Regression tested on x86-64 Fedora 36.
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>
It was pointed out during review of another patch that the function
displaced_step_dump_bytes really isn't specific to displaced stepping,
and should really get a more generic name and move into gdbsupport/.
This commit does just that. The function is renamed to
bytes_to_string and is moved into gdbsupport/common-utils.{cc,h}. The
function implementation doesn't really change. Much...
... I have updated the function to take an array view, which makes it
slightly easier to call in a couple of places where we already have a
gdb::bytes_vector. I've then added an inline wrapper to convert a raw
pointer and length into an array view, which is used in places where
we don't easily have a gdb::bytes_vector (or similar).
Updated all users of displaced_step_dump_bytes.
There should be no user visible changes after this commit.
Finally, I ended up having to add an include of gdb_assert.h into
array-view.h. When I include array-view.h into common-utils.h I ran
into build problems because array-view.h calls gdb_assert.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
For amd64 the current frame-unwinders are:
...
$ gdb -q -batch -ex "set arch i386:x86-64" -ex "maint info frame-unwinders"
The target architecture is set to "i386:x86-64".
dummy DUMMY_FRAME
dwarf2 tailcall TAILCALL_FRAME
inline INLINE_FRAME
python NORMAL_FRAME
amd64 epilogue NORMAL_FRAME
dwarf2 NORMAL_FRAME
dwarf2 signal SIGTRAMP_FRAME
amd64 sigtramp SIGTRAMP_FRAME
amd64 prologue NORMAL_FRAME
...
For a -g0 -fasynchronous-unwind-tables exec (without .debug_info but with
.eh_frame section), we'd like to start using the dwarf2 unwinder instead of
the "amd64 epilogue" unwinder, by returning true in
compunit_epilogue_unwind_valid for cust == nullptr.
But we'd run into the following problem for a -g0
-fno-asynchronous-unwind-tables (without .debug_info and .eh_frame section)
exec:
- the "amd64 epilogue" unwinder would not run
(because compunit_epilogue_unwind_valid () == true)
- the dwarf2 unwinder would also not run
(because there's no .eh_frame info).
Fix this by:
- renaming the "amd64 epilogue" unwinder to "amd64 epilogue override", and
- adding a fallback "amd64 epilogue" after the dwarf unwinders,
while making sure that only one of the two is active. Likewise for i386. NFC.
For amd64, this results in this change:
...
$ gdb -q -batch -ex "set arch i386:x86-64" -ex "maint info frame-unwinders"
The target architecture is set to "i386:x86-64".
dummy DUMMY_FRAME
dwarf2 tailcall TAILCALL_FRAME
inline INLINE_FRAME
python NORMAL_FRAME
-amd64 epilogue NORMAL_FRAME
+amd64 epilogue override NORMAL_FRAME
dwarf2 NORMAL_FRAME
dwarf2 signal SIGTRAMP_FRAME
+amd64 epilogue NORMAL_FRAME
amd64 sigtramp SIGTRAMP_FRAME
amd64 prologue NORMAL_FRAME
...
And for i386:
...
$ gdb -q -batch -ex "set arch i386" -ex "maint info frame-unwinders"
The target architecture is set to "i386".
dummy DUMMY_FRAME
dwarf2 tailcall TAILCALL_FRAME
iline INLINE_FRAME
-i386 epilogue NORMAL_FRAME
+i386 epilogue override NORMAL_FRAME
dwarf2 NORMAL_FRAME
dwarf2 signal SIGTRAMP_FRAME
+i386 epilogue NORMAL_FRAME
i386 stack tramp NORMAL_FRAME
i386 sigtramp SIGTRAMP_FRAME
i386 prologue NORMAL_FRAME
...
The use of compunit_epilogue_unwind_valid in both amd64_stack_frame_destroyed_p
and i386_stack_frame_destroyed_p is problematic, in the sense that the
functions no longer match their documented behaviour.
Fix this by moving the use of compunit_epilogue_unwind_valid to
amd64_epilogue_frame_sniffer and i386_epilogue_frame_sniffer. No functional
changes.
Factor out compunit_epilogue_unwind_valid from both
amd64_stack_frame_destroyed_p and i386_stack_frame_destroyed_p. No functional
changes.
Also add a comment in the new function about the assumption that in absence of
producer information, epilogue unwind info is invalid.
Approved-By: Tom Tromey <tom@tromey.com>
I fixed this a while ago for ifx, one of the two Intel compilers, in
8d624a9d80.
Apparently I missed that the older ifort Intel compiler actually emits
slightly different debug info again:
0x0000007a: DW_TAG_base_type
DW_AT_byte_size (0x20)
DW_AT_encoding (DW_ATE_complex_float)
DW_AT_name ("COMPLEX(16)")
0x00000081: DW_TAG_base_type
DW_AT_byte_size (0x10)
DW_AT_encoding (DW_ATE_float)
DW_AT_name ("REAL(16)")
This fixes two failures in gdb.fortran/complex.exp with ifort.
Approved-By: Tom Tromey <tom@tromey.com>
