| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
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>
|
|
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>
|
|
Add XMM16-XMM31 and K0-K1 DWARF register number mapping to
amd64_dwarf_regmap.
Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
Approved-By: John Baldwin <jhb@FreeBSD.org>
|
|
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>
|
|
Reformat amd64_dwarf_reg_to_regnum:
@@ -254,8 +254,7 @@ amd64_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
if (reg >= 0 && reg < amd64_dwarf_regmap_len)
regnum = amd64_dwarf_regmap[reg];
- if (ymm0_regnum >= 0
- && i386_xmm_regnum_p (gdbarch, regnum))
+ if (ymm0_regnum >= 0 && i386_xmm_regnum_p (gdbarch, regnum))
regnum += ymm0_regnum - I387_XMM0_REGNUM (tdep);
to remove the misaligned line.
|
|
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.
|
|
This commit adds a mechanism for GDB to detect the linetable opcode
DW_LNS_set_epilogue_begin. This opcode is set by compilers to indicate
that a certain instruction marks the point where the frame is destroyed.
While the standard allows for multiple points marked with epilogue_begin
in the same function, for performance reasons, the function that
searches for the epilogue address will only find the last address that
sets this flag for a given block.
This commit also changes amd64_stack_frame_destroyed_p_1 to attempt to
use the epilogue begin directly, and only if an epilogue can't be found
will it attempt heuristics based on the current instruction.
Finally, this commit also changes the dwarf assembler to be able to emit
epilogue-begin instructions, to make it easier to test this patch
Approved-By: Tom Tromey <tom@tromey.com>
|
|
gdbarches usually register functions to check when a frame is destroyed
which is used with software watchpoints, since the expression of the
watchpoint is no longer vlaid at this point. On amd64, this wasn't done
anymore because GCC started using CFA for variable locations instead.
However, clang doesn't use the CFA and instead relies on specifying when
an epilogue has started, meaning software watchpoints get a spurious hit
when a frame is destroyed. This patch re-adds the code to register the
function that detects when a frame is destroyed, but only uses this when
the producer is LLVM, so gcc code isn't affected. The logic that
identifies the epilogue has been factored out into the new function
amd64_stack_frame_destroyed_p_1, so the frame sniffer can call it
directly, and its behavior isn't changed.
This can also remove the XFAIL added to gdb.python/pq-watchpoint tests
that handled this exact flaw in clang.
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
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>
|
|
gdbarch_deprecated_pseudo_register_write
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>
|
|
Replace with a new equivalent "is_packed" method on struct field.
Change-Id: I78647be3d408b40b63becb6b6f0fca211bede51c
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Replace with type::field + field::bitsize.
Change-Id: I2a24755a33683e4a2775a6d2a7b7a9ae7362e43a
Approved-By: Tom Tromey <tom@tromey.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.
|
|
Fix a few typos:
- implemention -> implementation
- convertion(s) -> conversion(s)
- backlashes -> backslashes
- signoring -> ignoring
- (un)ambigious -> (un)ambiguous
- occured -> occurred
- hidding -> hiding
- temporarilly -> temporarily
- immediatelly -> immediately
- sillyness -> silliness
- similiar -> similar
- porkuser -> pokeuser
- thats -> that
- alway -> always
- supercede -> supersede
- accomodate -> accommodate
- aquire -> acquire
- priveleged -> privileged
- priviliged -> privileged
- priviledges -> privileges
- privilige -> privilege
- recieve -> receive
- (p)refered -> (p)referred
- succesfully -> successfully
- successfuly -> successfully
- responsability -> responsibility
- wether -> whether
- wich -> which
- disasbleable -> disableable
- descriminant -> discriminant
- construcstor -> constructor
- underlaying -> underlying
- underyling -> underlying
- structureal -> structural
- appearences -> appearances
- terciarily -> tertiarily
- resgisters -> registers
- reacheable -> reachable
- likelyhood -> likelihood
- intepreter -> interpreter
- disassemly -> disassembly
- covnersion -> conversion
- conviently -> conveniently
- atttribute -> attribute
- struction -> struct
- resonable -> reasonable
- popupated -> populated
- namespaxe -> namespace
- intialize -> initialize
- identifer(s) -> identifier(s)
- expection -> exception
- exectuted -> executed
- dungerous -> dangerous
- dissapear -> disappear
- completly -> completely
- (inter)changable -> (inter)changeable
- beakpoint -> breakpoint
- automativ -> automatic
- alocating -> allocating
- agressive -> aggressive
- writting -> writing
- reguires -> requires
- registed -> registered
- recuding -> reducing
- opeartor -> operator
- ommitted -> omitted
- modifing -> modifying
- intances -> instances
- imbedded -> embedded
- gdbaarch -> gdbarch
- exection -> execution
- direcive -> directive
- demanged -> demangled
- decidely -> decidedly
- argments -> arguments
- agrument -> argument
- amespace -> namespace
- targtet -> target
- supress(ed) -> suppress(ed)
- startum -> stratum
- squence -> sequence
- prompty -> prompt
- overlow -> overflow
- memember -> member
- languge -> language
- geneate -> generate
- funcion -> function
- exising -> existing
- dinking -> syncing
- destroh -> destroy
- clenaed -> cleaned
- changep -> changedp (name of variable)
- arround -> around
- aproach -> approach
- whould -> would
- symobl -> symbol
- recuse -> recurse
- outter -> outer
- freeds -> frees
- contex -> context
Tested on x86_64-linux.
Reviewed-By: Tom Tromey <tom@tromey.com>
|
|
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>
|
|
Replace spaces with tabs in a bunch of places.
Change-Id: If0f87180f1d13028dc178e5a8af7882a067868b0
|
|
Replace a couple of uses of xmalloc and alloc with a gdb::byte_vector
local variable instead.
There should be no user visible changes after this commit.
Reviewed-By: Tom Tromey <tom@tromey.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>
|
|
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 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>
|
|
A few tdep files include block.h but do not need to. This patch
removes the inclusions. I checked that this worked correctly by
examining the resulting .Po file to make sure that block.h was not
being included by some other route.
|
|
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>
|
|
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.
|
|
amd64-tdep.c could crash when 'finish'ing from a function whose return
type had variable length. In this situation, the value will be passed
by reference, and this patch avoids the crash.
(Note that this does not fully fix the bug reported, but it does fix
the crash, so it seems worthwhile to land independently.)
|
|
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.
|
|
When GDB is stopped at a ret instruction and no debug information is
available for unwinding, GDB defaults to the amd64 epilogue unwinder, to
be able to generate a decent backtrace. However, when calculating the
frame id, the epilogue unwinder generates information as if the return
instruction was the whole frame.
This was an issue especially when attempting to reverse debug, as GDB
would place a step_resume_breakpoint from the epilogue of a function if
we were to attempt to skip that function, and this breakpoint should
ideally have the current function's frame_id to avoid other problems
such as PR record/16678.
This commit changes the frame_id calculation for the amd64 epilogue,
so that it is always the same as the dwarf2 unwinder's frame_id.
It also adds a test to confirm that the frame_id will be the same,
regardless of using the epilogue unwinder or not, thanks to Andrew
Burgess.
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
|
|
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>
|
|
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
|
|
When building with clang 15, I got this,
CXX amd64-tdep.o
amd64-tdep.c:1410:13: error: variable 'insn' set but not used[-Werror,-Wunused-but-set-variable]
gdb_byte *insn = insn_details->raw_insn + modrm_offset;
^
1 error generated.
The function that uses this variable has been removed in this commit,
commit 870f88f7551b0f2d6aaaa36fb684b5ff8f468107
Date: Mon Apr 18 13:16:27 2016 -0400
remove trivialy unused variables
Fix this by removing unused variable.
Tested by rebuilding on x86_64-linux with clang 15 and gcc 12.
|
|
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.
|
|
Add a getter and a setter for a symtab's compunit_symtab. Remove the
corresponding macro and adjust all callers.
For brevity, I chose the name "compunit" instead of "compunit_symtab"
the the field, getter and setter names. Since we are already in symtab
context, the _symtab suffix seems redundant.
Change-Id: I4b9b731c96e3594f7733e75af1e3d01bc0e4fe92
|
|
Add a getter and a setter for a compunit_symtab's epilogue unwind valid flag.
Remove the corresponding macro and adjust all callers.
Change-Id: If3b68629d987767da9be7041a95d96dc34367a9a
|
|
Add a getter and a setter for a compunit_symtab's producer. Remove the
corresponding macro and adjust all callers.
Change-Id: Ia1d6d8a0e247a08a21af23819d71e49b37d8931b
|
|
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.
|
|
Fixes PR gdb/28681. It was observed that after using the `finish`
command an incorrect value was displayed in some cases. Specifically,
this behaviour was observed on an x86-64 target.
Consider this test program:
struct A
{
int i;
A ()
{ this->i = 0; }
A (const A& a)
{ this->i = a.i; }
};
A
func (int i)
{
A a;
a.i = i;
return a;
}
int
main ()
{
A a = func (3);
return a.i;
}
And this GDB session:
$ gdb -q ex.x
Reading symbols from ex.x...
(gdb) b func
Breakpoint 1 at 0x401115: file ex.cc, line 14.
(gdb) r
Starting program: /home/andrew/tmp/ex.x
Breakpoint 1, func (i=3) at ex.cc:14
14 A a;
(gdb) finish
Run till exit from #0 func (i=3) at ex.cc:14
main () at ex.cc:23
23 return a.i;
Value returned is $1 = {
i = -19044
}
(gdb) p a
$2 = {
i = 3
}
(gdb)
Notice how after the `finish` the contents of $1 are junk, but, when I
immediately ask for the value of `a`, I get back the correct value.
The problem here is that after the finish command GDB calls the
function amd64_return_value to figure out where the return value can
be found (on x86-64 targets anyway).
This function makes the wrong choice for the struct A in our case, as
sizeof(A) <= 8, then amd64_return_value decides that A will be
returned in a register. GDB then reads the return value register an
interprets the contents as an instance of A.
Unfortunately, A is not trivially copyable (due to its copy
constructor), and the sys-v specification for argument and return
value passing, says that any non-trivial C++ object should have space
allocated for it by the caller, and the address of this space is
passed to the callee as a hidden first argument. The callee should
then return the address of this space as the return value.
And so, the register that GDB is treating as containing an instance of
A, actually contains the address of an instance of A (in this case on
the stack), this is why GDB shows the incorrect result.
The call stack within GDB for where we actually go wrong is this:
amd64_return_value
amd64_classify
amd64_classify_aggregate
And it is in amd64_classify_aggregate that we should be classifying
the type as AMD64_MEMORY, instead of as AMD64_INTEGER as we currently
do (via a call to amd64_classify_aggregate_field).
At the top of amd64_classify_aggregate we already have this logic:
if (TYPE_LENGTH (type) > 16 || amd64_has_unaligned_fields (type))
{
theclass[0] = theclass[1] = AMD64_MEMORY;
return;
}
Which handles some easy cases where we know a struct will be placed
into memory, that is (a) the struct is more than 16-bytes in size,
or (b) the struct has any unaligned fields.
All we need then, is to add a check here to see if the struct is
trivially copyable. If it is not then we know the struct will be
passed in memory.
I originally structured the code like this:
if (TYPE_LENGTH (type) > 16
|| amd64_has_unaligned_fields (type)
|| !language_pass_by_reference (type).trivially_copyable)
{
theclass[0] = theclass[1] = AMD64_MEMORY;
return;
}
This solved the example from the bug, and my small example above. So
then I started adding some more extensive tests to the GDB testsuite,
and I ran into a problem. I hit this error:
gdbtypes.h:676: internal-error: loc_bitpos: Assertion `m_loc_kind == FIELD_LOC_KIND_BITPOS' failed.
This problem is triggered from:
amd64_classify_aggregate
amd64_has_unaligned_fields
field::loc_bitpos
Inside the unaligned field check we try to get the bit position of
each field. Unfortunately, in some cases the field location is not
FIELD_LOC_KIND_BITPOS, but is FIELD_LOC_KIND_DWARF_BLOCK.
An example that shows this bug is:
struct B
{
short j;
};
struct A : virtual public B
{
short i;
A ()
{ this->i = 0; }
A (const A& a)
{ this->i = a.i; }
};
A
func (int i)
{
A a;
a.i = i;
return a;
}
int
main ()
{
A a = func (3);
return a.i;
}
It is the virtual base class, B, that causes the problem. The base
class is represented, within GDB, as a field within A. However, the
location type for this field is a DWARF_BLOCK.
I spent a little time trying to figure out how to convert the
DWARF_BLOCK to a BITPOS, however, I realised that, in this case at
least, conversion is not needed.
The C++ standard says that a class is not trivially copyable if it has
any virtual base classes. And so, in this case, even if I could
figure out the BITPOS for the virtual base class fields, I know for
sure that I would immediately fail the trivially_copyable check. So,
lets just reorder the checks in amd64_classify_aggregate to:
if (TYPE_LENGTH (type) > 16
|| !language_pass_by_reference (type).trivially_copyable
|| amd64_has_unaligned_fields (type))
{
theclass[0] = theclass[1] = AMD64_MEMORY;
return;
}
Now, if we have a class with virtual bases we will fail quicker, and
avoid the unaligned fields check completely.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28681
|
|
I would like to be able to use non-trivial types in gdbarch_tdep types.
This is not possible at the moment (in theory), because of the one
definition rule.
To allow it, rename all gdbarch_tdep types to <arch>_gdbarch_tdep, and
make them inherit from a gdbarch_tdep base class. The inheritance is
necessary to be able to pass pointers to all these <arch>_gdbarch_tdep
objects to gdbarch_alloc, which takes a pointer to gdbarch_tdep.
These objects are never deleted through a base class pointer, so I
didn't include a virtual destructor. In the future, if gdbarch objects
deletable, I could imagine that the gdbarch_tdep objects could become
owned by the gdbarch objects, and then it would become useful to have a
virtual destructor (so that the gdbarch object can delete the owned
gdbarch_tdep object). But that's not necessary right now.
It turns out that RISC-V already has a gdbarch_tdep that is
non-default-constructible, so that provides a good motivation for this
change.
Most changes are fairly straightforward, mostly needing to add some
casts all over the place. There is however the xtensa architecture,
doing its own little weird thing to define its gdbarch_tdep. I did my
best to adapt it, but I can't test those changes.
Change-Id: Ic001903f91ddd106bd6ca09a79dabe8df2d69f3b
|
