| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Make the inline_state::skipped_symbols a vector of 'const symbol *',
adding the const qualifier.
There's only a couple of places this leaks into the rest of GDB and in
both places its fine for the symbol to become const.
There should be no functional change after this commit.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
>From what I can see, lookup_minimal_symbol doesn't have any dependencies
on the global current state other than the single reference to
current_program_space. Add a program_space parameter and make that
current_program_space reference bubble up one level.
Change-Id: I759415e2f9c74c9627a2fe05bd44eb4147eee6fe
Reviewed-by: Keith Seitz <keiths@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Most calls to lookup_minimal_symbol don't pass a value for sfile and
objf. Make these parameters optional (have a default value of
nullptr). And since passing a value to `objf` is much more common than
passing a value to `sfile`, swap the order so `objf` comes first, to
avoid having to pass a nullptr value to `sfile` when wanting to pass a
value to `objf`.
Change-Id: I8e9cc6b942e593bec640f9dfd30f62786b0f5a27
Reviewed-by: Keith Seitz <keiths@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Make the current program space bubble up one level.
Change-Id: Ic3ad0869ca1afe41854f605a6f7eb092fca29ff8
Approved-By: Tom Tromey <tom@tromey.com>
Reviewed-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
|
|
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 some declarations related to the "quit" machinery from defs.h to
event-top.h. Most of the definitions associated to these declarations
are in event-top.c. The exceptions are `quit()` and `maybe_quit()`,
that are defined in utils.c. For consistency, move these two
definitions to event-top.c.
Include "event-top.h" in many files that use these things.
Change-Id: I6594f6df9047a9a480e7b9934275d186afb14378
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>
|
|
In commit 2aaba744467 ("[gdb] Fix "value is not available" with debug frame")
I fixed a case in frame_unwind_register_value where using "set debug frame on"
caused an "info frame" command to abort, reporting a "value is not available"
error, due to the tpidruro register being unavailable.
Subsequently, commit bbb12eb9c84 ("gdb/arm: Remove tpidruro register from
non-FreeBSD target descriptions") removed the unavailable register, which
caused a progression on test-case gdb.base/inline-frame-cycle-unwind.exp.
While investigating the progression (see PR python/31437), I found that the
"debug frame" output of the test-case (when reverting commit bbb12eb9c84)
showed a smilar problem:
...
Python Exception <class 'gdb.error'>: value is not available^M
...
that was absent without "debug frame".
Fix this likewise in fetch_lazy_register, and update the test-case to check
for the exception.
Furthermore, I realized that there's both value::entirely_available and
value::entirely_unavailable, and that commit 2aaba744467 handled the case
of !entirely_available by printing unavailable.
Instead, print:
- "unavailable" for entirely_unavailable, and
- "partly unavailable" for !entirely_unavailable && !entirely_available.
Tested on x86_64-linux and arm-linux.
|
|
This started with a Red Hat bug report which can be seen here:
https://bugzilla.redhat.com/show_bug.cgi?id=1850710
The problem reported here was using GDB on GNU/Linux for S390, the
user stepped into JIT generated code. As they enter the JIT code GDB
would report 'PC not saved', and this same message would be reported
after each step/stepi.
Additionally, the user had 'set disassemble-next-line on', and once
they entered the JIT code this output was not displayed, nor were any
'display' directives displayed.
The user is not making use of the JIT plugin API to provide debug
information. But that's OK, they aren't expecting any source level
debug here, they are happy to use 'stepi', but the missing 'display'
directives are a problem, as is the constant 'PC not saved' (error)
message.
What is happening here is that as GDB is failing to find any debug
information for the JIT generated code, it is falling back on to the
S390 prologue unwinder to try and unwind frame #0. Unfortunately,
without being able to identify the function boundaries, the S390
prologue scanner can't help much, in fact, it doesn't even suggest an
arbitrary previous $pc value (some targets that use a link-register
will, by default, assume the link-register contains the previous $pc),
instead the S390 will just say, "sorry, I have no previous $pc value".
The result of this is that when GDB tries to find frame #1 we end
throwing an error from frame_unwind_pc (the 'PC not saved' error).
This error is not caught anywhere except at the top-level interpreter
loop, and so we end up skipping all the 'display' directive handling.
While thinking about this, I wondered, could I trigger the same error
using the Python Unwinder API? What happens if a Python unwinder
claims a frame, but then fails to provide a previous $pc value?
Turns out that exactly the same thing happens, which is great, as that
means we now have a way to reproduce this bug on any target. And so
the test included with this patch does just this. I have a Python
unwinder that claims a frame, but doesn't provide any previous
register values.
I then do two tests, first I stop in the claimed frame (i.e. frame #0
is the frame that can't be unwound), I perform a few steps, and check
the backtrace. And second, I stop in a child of the problem
frame (i.e. frame #1 is the frame that can't be unwound), and from
here I check the backtrace.
While all this is going on I have a 'display' directive in place, and
each time GDB stops I check that the display directive triggers.
Additionally, when checking the backtrace, I am checking that the
backtrace finishes with the message 'Backtrace stopped: frame did not
save the PC'.
As for the fix I chose to add a call to frame_unwind_pc directly to
get_prev_frame_always_1. Calling frame_unwind_pc will cache the
unwound $pc value, so this doesn't add much additional work as
immediately after the new frame_unwind_pc call, we call
get_prev_frame_maybe_check_cycle, which actually generates the
previous frame, which will always (I think) require a call to
frame_unwind_pc anyway.
The reason for adding the frame_unwind_pc call into
get_prev_frame_always_1, is that if the frame_unwind_pc call fails we
want to set the frames 'stop_reason', and get_prev_frame_always_1
seems to be the place where this is done, so I wanted to keep the new
stop_reason setting code next to all the existing stop_reason setting
code.
Additionally, once we enter get_prev_frame_maybe_check_cycle we
actually create the previous frame, then, if it turns out that the
previous frame can't be created we need to remove the frame .. this
seemed more complex than just making the check in
get_prev_frame_always_1.
With this fix in place the original S390 bug is fixed, and also the
test added in this commit, that uses the Python API, is also fixed.
Reviewed-By: Kevin Buettner <kevinb@redhat.com>
|
|
On arm-linux, with a started hello world, running "info frame" works fine, but
when I set debug frame to on, I run into:
...
(gdb) info frame
...
[frame] frame_unwind_register_value: exit
value is not available
(gdb)
...
The problem is here in frame_unwind_register_value:
...
if (value->lazy ())
gdb_printf (&debug_file, " lazy");
else
{
int i;
gdb::array_view<const gdb_byte> buf = value->contents ();
...
where we call value->contents () while !value->entirely_available ().
Fix this by checking value->entirely_available () and printing:
...
[frame] frame_unwind_register_value: -> register=91 unavailable
...
Tested on arm-linux.
PR gdb/31369
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31369
|
|
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>
|
|
If you try to call Frame.static_link for a frame without debug info,
gdb crashes:
```
Temporary breakpoint 1, 0x000000013f821650 in main ()
(gdb) py print(gdb.selected_frame().static_link())
This application has requested the Runtime to terminate it in an unusual way.
Please contact the application's support team for more information.
```
The problem was a missing check if get_frame_block returns nullptr
inside frame_follow_static_link.
With this, it works:
```
Temporary breakpoint 1, 0x000000013f941650 in main ()
(gdb) py print(gdb.selected_frame().static_link())
None
```
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31366
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This changes inside_main_func to search only for functions.
|
|
This changes lookup_symbol and associated APIs to accept
domain_search_flags rather than a domain_enum.
Note that this introduces some new constants to Python and Guile. I
chose to break out the documentation patch for this, because the
internals here do not change until a later patch, and it seemed
simpler to patch the docs just once, rather than twice.
|
|
Remove SYMBOL_BLOCK_OPS, SYMBOL_COMPUTED_OPS and SYMBOL_REGISTER_OPS, in
favor of methods on struct symbol. More changes could be done here to
improve the design and make things safer, but I just wanted to do a
straightforward change to remove the macros for now.
Change-Id: I27adb74a28ea3c0dc9a85c2953413437cd95ad21
Reviewed-by: Kevin Buettner <kevinb@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.
|
|
Remove VALUE_REGNUM, replace it with a method on struct value. Set
`m_location.reg.regnum` directly from value::allocate_register_lazy,
which is fine because allocate_register_lazy is a static creation
function for struct value.
Change-Id: Id632502357da971617d9dce1e2eab9b56dbcf52d
|
|
Add a new variant of gdbarch_pseudo_register_write that takes a
frame_info in order to write raw registers. Use this new method when
available:
- in put_frame_register, when trying to write a pseudo register to a given frame
- in regcache::cooked_write
No implementation is migrated to use this new method (that will come in
subsequent patches), so no behavior change is expected here.
The objective is to fix writing pseudo registers to non-current
frames. See previous commit "gdb: read pseudo register through
frame" for a more detailed explanation.
Change-Id: Ie7fe364a15a4d86c2ecb09de2b4baa08c45555ac
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_bytes to take
the "next frame" instead of "this frame".
Change-Id: I27bcb26573686d99b231230823cff8db6405a788
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>
|
|
I was going to change frame_register to take the "next frame", but I
realized that doing so would make it a useless wrapper around
frame_register_unwind. So, just remove frame_register and replace uses
with frame_register_unwind.
Change-Id: I185868bc69f8e098124775d0550d069220a4678a
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 found this only by inspection: the myaddr pointer in
{get,put}_frame_register_bytes is reset to `buffer.data ()` at each
iteration. This means that we will always use the bytes at the
beginning of `buffer` to read or write to the registers, instead of
progressing in `buffer`.
Fix this by re-writing the functions to chip away the beginning of the
buffer array_view as we progress in reading or writing the data.
These bugs was introduced almost 3 years ago [1], and yet nobody
complained. I'm wondering which architecture relies on that register
"overflow" behavior (reading or writing multiple consecutive registers
with one {get,put}_frame_register_bytes calls), and in which situation.
I find these functions a bit dangerous, if a caller mis-calculates
things, it could end up silently reading or writing to the next
register, even if it's not the intent.
If I could change it, I would prefer to have functions specifically made
for that ({get,put}_frame_register_bytes_consecutive or something like
that) and make {get,put}_frame_register_bytes only able to write within
a single register (which I presume represents most of the use cases of
the current {get,put}_frame_register_bytes). If a caller mis-calculates
things and an overflow occurs while calling
{get,put}_frame_register_bytes, it would hit an assert. The problem is
knowing which callers rely on the overflow behavior and which don't.
[1] https://gitlab.com/gnutools/binutils-gdb/-/commit/bdec2917b1e94c7198ba39919f45060067952f43
Change-Id: I43bd4a9f7fa8419d388a2b20bdc57d652688ddf8
Reviewed-By: John Baldwin <jhb@FreeBSD.org>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Change most of regcache (and base classes) to use array_view when
possible, instead of raw pointers. By propagating the use of array_view
further, it enables having some runtime checks to make sure the what we
read from or write to regcaches has the expected length (such as the one
in the `copy(array_view, array_view)` function. It also integrates well
when connecting with other APIs already using gdb::array_view.
Add some overloads of the methods using raw pointers to avoid having to
change all call sites at once (which is both a lot of work and risky).
I tried to do this change in small increments, but since many of these
functions use each other, it ended up simpler to do it in one shot than
having a lot of intermediary / transient changes.
This change extends into gdbserver as well, because there is some part
of the regcache interface that is shared.
Changing the reg_buffer_common interface to use array_view caused some
build failures in nat/aarch64-scalable-linux-ptrace.c. That file
currently "takes advantage" of the fact that
reg_buffer_common::{raw_supply,raw_collect} operates on `void *`, which
IMO is dangerous. It uses raw_supply/raw_collect directly on
uint64_t's, which I guess is fine because it is expected that native
code will have the same endianness as the debugged process. To
accomodate that, add some overloads of raw_collect and raw_supply that
work on uint64_t.
This file also uses raw_collect and raw_supply on `char` pointers.
Change it to use `gdb_byte` pointers instead. Add overloads of
raw_collect and raw_supply that work on `gdb_byte *` and make an
array_view on the fly using the register's size. Those call sites could
be converted to use array_view with not much work, in which case these
overloads could be removed, but I didn't want to do it in this patch, to
avoid starting to dig in arch-specific code.
During development, I inadvertently changed reg_buffer::raw_compare's
behavior to not accept an offset equal to the register size. This
behavior (effectively comparing 0 bytes, returning true) change was
caught by the AArch64 SME core tests. Add a selftest to make sure that
this raw_compare behavior is preserved in the future.
Change-Id: I9005f04114543ddff738949e12d85a31855304c2
Reviewed-By: John Baldwin <jhb@FreeBSD.org>
|
|
When running test-case gdb.base/vfork-follow-parent.exp on powerpc64 (likewise
on s390x), I run into:
...
(gdb) PASS: gdb.base/vfork-follow-parent.exp: \
exec_file=vfork-follow-parent-exit: target-non-stop=on: non-stop=off: \
resolution_method=schedule-multiple: print unblock_parent = 1
continue^M
Continuing.^M
Reading symbols from vfork-follow-parent-exit...^M
^M
^M
Fatal signal: Segmentation fault^M
----- Backtrace -----^M
0x1027d3e7 gdb_internal_backtrace_1^M
src/gdb/bt-utils.c:122^M
0x1027d54f _Z22gdb_internal_backtracev^M
src/gdb/bt-utils.c:168^M
0x1057643f handle_fatal_signal^M
src/gdb/event-top.c:889^M
0x10576677 handle_sigsegv^M
src/gdb/event-top.c:962^M
0x3fffa7610477 ???^M
0x103f2144 for_each_block^M
src/gdb/dcache.c:199^M
0x103f235b _Z17dcache_invalidateP13dcache_struct^M
src/gdb/dcache.c:251^M
0x10bde8c7 _Z24target_dcache_invalidatev^M
src/gdb/target-dcache.c:50^M
...
or similar.
The root cause for the segmentation fault is that linux_is_uclinux gives an
incorrect result: it should always return false, given that we're running on a
regular linux system, but instead it returns first true, then false.
In more detail, the segmentation fault happens as follows:
- a program space with an address space is created
- a second program space is about to be created. maybe_new_address_space
is called, and because linux_is_uclinux returns true, maybe_new_address_space
returns false, and no new address space is created
- a second program space with the same address space is created
- a program space is deleted. Because linux_is_uclinux now returns false,
gdbarch_has_shared_address_space (current_inferior ()->arch ()) returns
false, and the address space is deleted
- when gdb uses the address space of the remaining program space, we run into
the segfault, because the address space is deleted.
Hardcoding linux_is_uclinux to false makes the test-case pass.
We leave addressing the root cause for the following commit in this series.
For now, prevent the segmentation fault by making the address space a refcounted
object.
This was already suggested here [1]:
...
A better solution might be to have the address spaces be reference counted
...
Tested on top of trunk on x86_64-linux and ppc64le-linux.
Tested on top of gdb-14-branch on ppc64-linux.
Co-Authored-By: Simon Marchi <simon.marchi@polymtl.ca>
PR gdb/30547
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30547
[1] https://sourceware.org/pipermail/gdb-patches/2023-October/202928.html
|
|
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
|
|
While looking at the regcache code, I noticed that the address space
(passed to regcache when constructing it, and available through
regcache::aspace) wasn't relevant for the regcache itself. Callers of
regcache::aspace use that method because it appears to be a convenient
way of getting the address space for a thread, if you already have the
regcache. But there is always another way to get the address space, as
the callers pretty much always know which thread they are dealing with.
The regcache code itself doesn't use the address space.
This patch removes anything related to address_space from the regcache
code, and updates callers to get it from the thread in context. This
removes a bit of unnecessary complexity from the regcache code.
The current get_thread_arch_regcache function gets an address_space for
the given thread using the target_thread_address_space function (which
calls the target_ops::thread_address_space method). This suggest that
there might have been the intention of supporting per-thread address
spaces. But digging through the history, I did not find any such case.
Maybe this method was just added because we needed a way to get an
address space from a ptid (because constructing a regcache required an
address space), and this seemed like the right way to do it, I don't
know.
The only implementations of thread_address_space and
process_stratum_target::thread_address_space and
linux_nat_target::thread_address_space, which essentially just return
the inferior's address space. And thread_address_space is only used in
the current get_thread_arch_regcache, which gets removed. So, I think
that the thread_address_space target method can be removed, and we can
assume that it's fine to use the inferior's address space everywhere.
Callers of regcache::aspace are updated to get the address space from
the relevant inferior, either using some context they already know
about, or in last resort using the current global context.
So, to summarize:
- remove everything in regcache related to address spaces
- in particular, remove get_thread_arch_regcache, and rename
get_thread_arch_aspace_regcache to get_thread_arch_regcache
- remove target_ops::thread_address_space, and
target_thread_address_space
- adjust all users of regcache::aspace to get the address space another
way
Change-Id: I04fd41b22c83fe486522af7851c75bcfb31c88c7
|
|
This moves the follow_static_link function to frame.c and exports it
for use elsewhere. The API is changed slightly to make it more
generically useful.
|
|
I noticed a comment by an include and remembered that I think these
don't really provide much value -- sometimes they are just editorial,
and sometimes they are obsolete. I think it's better to just remove
them. Tested by rebuilding.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Fix some more typos:
- distinquish -> distinguish
- actualy -> actually
- singe -> single
- frash -> frame
- chid -> child
- dissassembler -> disassembler
- uninitalized -> uninitialized
- precontidion -> precondition
- regsiters -> registers
- marge -> merge
- sate -> state
- garanteed -> guaranteed
- explictly -> explicitly
- prefices (nonstandard plural) -> prefixes
- bondary -> boundary
- formated -> formatted
- ithe -> the
- arrav -> array
- coresponding -> corresponding
- owend -> owned
- fials -> fails
- diasm -> disasm
- ture -> true
- tpye -> type
There's one code change, the name of macro SIG_CODE_BONDARY_FAULT changed to
SIG_CODE_BOUNDARY_FAULT.
Tested on x86_64-linux.
|
|
I noticed that Frame.older and Frame.newer don't document that they
return None at the ends of the stack. This patch updates the
documentation, and also fixes a somewhat related typo in a comment
that I noticed while digging into this.
Approved-By: Eli Zaretskii <eliz@gnu.org>
|
|
In commit 995a34b1772 ("Guard against frame.c destructors running before
frame-info.c's") the following problem was addressed.
The frame_info_ptr destructor:
...
~frame_info_ptr ()
{
frame_list.erase (frame_list.iterator_to (*this));
}
...
uses frame_list, which is a static member of class frame_info_ptr,
instantiated in frame-info.c:
...
intrusive_list<frame_info_ptr> frame_info_ptr::frame_list;
...
Then there's a static frame_info_pointer variable named selected_frame in
frame.c:
...
static frame_info_ptr selected_frame;
...
Because the destructor of selected_frame uses frame_list, its destructor needs
to be called before the destructor of frame_list.
But because they're in different compilation units, the initialization order and
consequently destruction order is not guarantueed.
The commit fixed this by handling the case that the destructor of frame_list
is called first, adding a check on is_linked ():
...
~frame_info_ptr ()
{
- frame_list.erase (frame_list.iterator_to (*this));
+ /* If this node has static storage, it may be deleted after
+ frame_list. Attempting to erase ourselves would then trigger
+ internal errors, so make sure we are still linked first. */
+ if (is_linked ())
+ frame_list.erase (frame_list.iterator_to (*this));
}
...
However, since then frame_list has been moved into frame.c, and
initialization/destruction order is guarantueed inside a compilation unit.
Revert aforementioned commit, and fix the destruction order problem by moving
frame_list before selected_frame.
Reverting the commit is another way of fixing the already fixed
Wdangling-pointer warning reported in PR build/30413, in a different way than
commit 9b0ccb1ebae ("Pass const frame_info_ptr reference for
skip_[language_]trampoline").
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tested on x86_64-linux.
PR build/30413
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30413
|
|
A recent patch caused my system gcc (Fedora 36, so gcc 12.2.1) to warn
about sym_addr being possibly uninitialized in frame.c. It isn't, but
the compiler can't tell. So, this patch initializes the variable. I
also fixed a formatting buglet that I missed in review.
|
|
In the case where a Fortran program has a program name of "main" and
there is also a minimal symbol called main, such as with programs built
with GCC version 4.4.7 or below, the backtrace will erroneously stop at
the minimal symbol rather than the user specified main, e.g.:
(gdb) bt
#0 bar () at .../gdb/testsuite/gdb.fortran/backtrace.f90:17
#1 0x0000000000402556 in foo () at .../gdb/testsuite/gdb.fortran/backtrace.f90:21
#2 0x0000000000402575 in main () at .../gdb/testsuite/gdb.fortran/backtrace.f90:31
#3 0x00000000004025aa in main ()
(gdb)
This patch fixes this issue by increasing the precedence of the full
symbol when the language of the current frame is Fortran.
Newer versions of GCC transform the program name to "MAIN__" in this
case, avoiding the problem.
Co-Authored-By: Maciej W. Rozycki <macro@embecosm.com>
|
|
gdb 13.1 crashes while running the rust compiler's debugger tests.
The crash has a number of causes.
First, the rust compiler still uses the C++-like _Z mangling, but with
its own twist -- some hex digits added to the end of a symbol. So,
while gdb finds the correct name of "main":
(top-gdb) p name
$13 = 0x292e0c0 "rustc_gdb_1031745::main"
It isn't found in the minsyms, because C++ demangling yields:
[99] t 0x90c0 _ZN17rustc_gdb_10317454main17h5b5be7fe16a97225E section .text rustc_gdb_1031745::main::h5b5be7fe16a97225 zko06yobckx336v
This could perhaps be fixed. I also filed a new PR to suggest
preferring the linkage name of the main program.
Next, the rust compiler emits both a DW_TAG_subprogram and a
DW_TAG_namespace for "main". This happens because the file is named
"main.rs" -- i.e., the bug is specific to the source file name. The
crash also seems to require the nested function inside of 'main', at
least for me. The namespace always is generated, but perhaps this
changes the ordering in the DWARF.
When inside_main_func looks up the main symbol, it finds the namespace
symbol rather than the function. (I filed a bug about fixing gdb's
symbol tables -- long overdue.)
Meanwhile, as I think it's important to fix this crash sooner rather
than later, this patch changes inside_main_func to check that the
symbol that is found is LOC_BLOCK. This perhaps should have been done
in the first place, anyway.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30158
|
|
This converts contained_in to be a method of block.
|
|
Tom de Vries reported [1] a regression in gdb.btrace/record_goto.exp
caused by 6d3717d4c4 ("gdb: call frame unwinders' dealloc_cache methods
through destroying the frame cache"). This issue is caught by ASan. On
a non-ASan build, it may or may not cause a crash or some other issue, I
haven't tried.
I managed to narrow it down to:
$ ./gdb -nx -q --data-directory=data-directory testsuite/outputs/gdb.btrace/record_goto/record_goto -ex "start" -ex "record btrace" -ex "next"
... and then doing repeatedly "record goto 19" and "record goto 27".
Eventually, I get:
(gdb) record goto 27
=================================================================
==1527735==ERROR: AddressSanitizer: heap-use-after-free on address 0x6210003392a8 at pc 0x55e4c26eef86 bp 0x7ffd229f24e0 sp 0x7ffd229f24d8
READ of size 8 at 0x6210003392a8 thread T0
#0 0x55e4c26eef85 in bfcache_eq /home/simark/src/binutils-gdb/gdb/record-btrace.c:1639
#1 0x55e4c37cdeff in htab_find_slot_with_hash /home/simark/src/binutils-gdb/libiberty/hashtab.c:659
#2 0x55e4c37ce24a in htab_find_slot /home/simark/src/binutils-gdb/libiberty/hashtab.c:703
#3 0x55e4c26ef0c6 in bfcache_new /home/simark/src/binutils-gdb/gdb/record-btrace.c:1653
#4 0x55e4c26f1242 in record_btrace_frame_sniffer /home/simark/src/binutils-gdb/gdb/record-btrace.c:1820
#5 0x55e4c1b926a1 in frame_unwind_try_unwinder /home/simark/src/binutils-gdb/gdb/frame-unwind.c:136
#6 0x55e4c1b930d7 in frame_unwind_find_by_frame(frame_info_ptr, void**) /home/simark/src/binutils-gdb/gdb/frame-unwind.c:196
#7 0x55e4c1bb867f in get_frame_type(frame_info_ptr) /home/simark/src/binutils-gdb/gdb/frame.c:2925
#8 0x55e4c2ae6798 in print_frame_info(frame_print_options const&, frame_info_ptr, int, print_what, int, int) /home/simark/src/binutils-gdb/gdb/stack.c:1049
#9 0x55e4c2ade3e1 in print_stack_frame(frame_info_ptr, int, print_what, int) /home/simark/src/binutils-gdb/gdb/stack.c:367
#10 0x55e4c26fda03 in record_btrace_set_replay /home/simark/src/binutils-gdb/gdb/record-btrace.c:2779
#11 0x55e4c26fddc3 in record_btrace_target::goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/record-btrace.c:2843
#12 0x55e4c2de2bb2 in target_goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/target.c:4169
#13 0x55e4c275ed98 in record_goto(char const*) /home/simark/src/binutils-gdb/gdb/record.c:372
#14 0x55e4c275edba in cmd_record_goto /home/simark/src/binutils-gdb/gdb/record.c:383
0x6210003392a8 is located 424 bytes inside of 4064-byte region [0x621000339100,0x62100033a0e0)
freed by thread T0 here:
#0 0x7f6ca34a5b6f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:123
#1 0x55e4c38a4c17 in rpl_free /home/simark/src/binutils-gdb/gnulib/import/free.c:44
#2 0x55e4c1bbd378 in xfree<void> /home/simark/src/binutils-gdb/gdb/../gdbsupport/gdb-xfree.h:37
#3 0x55e4c37d1b63 in call_freefun /home/simark/src/binutils-gdb/libiberty/obstack.c:103
#4 0x55e4c37d25a2 in _obstack_free /home/simark/src/binutils-gdb/libiberty/obstack.c:280
#5 0x55e4c1bad701 in reinit_frame_cache() /home/simark/src/binutils-gdb/gdb/frame.c:2112
#6 0x55e4c27705a3 in registers_changed_ptid(process_stratum_target*, ptid_t) /home/simark/src/binutils-gdb/gdb/regcache.c:564
#7 0x55e4c27708c7 in registers_changed_thread(thread_info*) /home/simark/src/binutils-gdb/gdb/regcache.c:573
#8 0x55e4c26fd922 in record_btrace_set_replay /home/simark/src/binutils-gdb/gdb/record-btrace.c:2772
#9 0x55e4c26fddc3 in record_btrace_target::goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/record-btrace.c:2843
#10 0x55e4c2de2bb2 in target_goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/target.c:4169
#11 0x55e4c275ed98 in record_goto(char const*) /home/simark/src/binutils-gdb/gdb/record.c:372
#12 0x55e4c275edba in cmd_record_goto /home/simark/src/binutils-gdb/gdb/record.c:383
previously allocated by thread T0 here:
#0 0x7f6ca34a5e8f in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
#1 0x55e4c0b55c60 in xmalloc /home/simark/src/binutils-gdb/gdb/alloc.c:57
#2 0x55e4c37d1a6d in call_chunkfun /home/simark/src/binutils-gdb/libiberty/obstack.c:94
#3 0x55e4c37d1c20 in _obstack_begin_worker /home/simark/src/binutils-gdb/libiberty/obstack.c:141
#4 0x55e4c37d1ed7 in _obstack_begin /home/simark/src/binutils-gdb/libiberty/obstack.c:164
#5 0x55e4c1bad728 in reinit_frame_cache() /home/simark/src/binutils-gdb/gdb/frame.c:2113
#6 0x55e4c27705a3 in registers_changed_ptid(process_stratum_target*, ptid_t) /home/simark/src/binutils-gdb/gdb/regcache.c:564
#7 0x55e4c27708c7 in registers_changed_thread(thread_info*) /home/simark/src/binutils-gdb/gdb/regcache.c:573
#8 0x55e4c26fd922 in record_btrace_set_replay /home/simark/src/binutils-gdb/gdb/record-btrace.c:2772
#9 0x55e4c26fddc3 in record_btrace_target::goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/record-btrace.c:2843
#10 0x55e4c2de2bb2 in target_goto_record(unsigned long) /home/simark/src/binutils-gdb/gdb/target.c:4169
#11 0x55e4c275ed98 in record_goto(char const*) /home/simark/src/binutils-gdb/gdb/record.c:372
#12 0x55e4c275edba in cmd_record_goto /home/simark/src/binutils-gdb/gdb/record.c:383
The problem is a stale entry in the bfcache hash table (in
record-btrace.c), left across a reinit_frame_cache. This entry points
to something that used to be allocated on the frame obstack, that has
since been wiped by reinit_frame_cache.
Before the aforementioned, unwinder deallocation functions were called
by iterating on the frame chain, starting with the sentinel frame, like
so:
/* Tear down all frame caches. */
for (frame_info *fi = sentinel_frame; fi != NULL; fi = fi->prev)
{
if (fi->prologue_cache && fi->unwind->dealloc_cache)
fi->unwind->dealloc_cache (fi, fi->prologue_cache);
if (fi->base_cache && fi->base->unwind->dealloc_cache)
fi->base->unwind->dealloc_cache (fi, fi->base_cache);
}
After that patch, we relied on the fact that all frames are (supposedly)
in the frame_stash. A deletion function was added to the frame_stash
hash table, so that dealloc functions would be called when emptying the
frame stash. There is one case, however, where a frame_info is not in
the frame stash. That is when we create the frame_info for the current
frame (level 0, unwound from the sentinel frame), but don't compute its
frame id. The computation of the frame id for that frame (and only that
frame, AFAIK) is done lazily. And putting a frame_info in the frame stash
requires knowing its id. So a frame 0 whose frame id is not computed
yet is necessarily not in the frame stash.
When replaying with btrace, record_btrace_frame_sniffer insert entries
corresponding to frames in the "bfcache" hash table. It then relies on
record_btrace_frame_dealloc_cache being called for each frame to remove
all those entries when the frames get invalidated. If a frame reinit
happens while frame 0's id is not computed (and therefore that frame is
not in frame_stash), record_btrace_frame_dealloc_cache does not get
called for it, and it leaves a stale entry in bfcache. That then leads
to a use-after-free when that entry is accessed later, which ASan
catches.
The proposed solution is to explicitly call frame_info_del on frame 0,
if it exists, and if its frame id is not computed. If its frame id is
computed, it is expected that it will be in the frame stash, so it will
be "deleted" through that.
[1] https://inbox.sourceware.org/gdb-patches/20230130200249.131155-1-simon.marchi@efficios.com/T/#mcf1340ce2906a72ec7ed535ec0c97dba11c3d977
Reported-By: Tom de Vries <tdevries@suse.de>
Tested-By: Tom de Vries <tdevries@suse.de>
Change-Id: I2351882dd511f3bbc01e4152e9db13b69b3ba384
|
|
This removes deprecated_lval_hack and the VALUE_LVAL macro, replacing
all uses with a call to value::lval.
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 the value_address and set_value_address functions to be
methods of value.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes the value_lazy and set_value_lazy functions to be methods
of value. Much of this patch was written by script.
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>
|
|
Add a new command "maint info frame-unwinders":
...
(gdb) help maint info frame-unwinders
List the frame unwinders currently in effect, starting with the highest \
priority.
...
Output 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
inline INLINE_FRAME
i386 epilogue NORMAL_FRAME
dwarf2 NORMAL_FRAME
dwarf2 signal SIGTRAMP_FRAME
i386 stack tramp NORMAL_FRAME
i386 sigtramp SIGTRAMP_FRAME
i386 prologue NORMAL_FRAME
...
Output for x86_64:
...
$ 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
i386 epilogue NORMAL_FRAME
dwarf2 NORMAL_FRAME
dwarf2 signal SIGTRAMP_FRAME
amd64 sigtramp SIGTRAMP_FRAME
amd64 prologue NORMAL_FRAME
i386 stack tramp NORMAL_FRAME
i386 sigtramp SIGTRAMP_FRAME
i386 prologue NORMAL_FRAME
...
Tested on x86_64-linux.
Reviewed-By: Tom Tromey <tom@tromey.com>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
|
|
the frame cache
The test gdb.base/frame-view.exp fails like this on AArch64:
frame^M
#0 baz (z1=hahaha, /home/simark/src/binutils-gdb/gdb/value.c:4056: internal-error: value_fetch_lazy_register: Assertion `next_frame != NULL' failed.^M
A problem internal to GDB has been detected,^M
further debugging may prove unreliable.^M
FAIL: gdb.base/frame-view.exp: with_pretty_printer=true: frame (GDB internal error)
The sequence of events leading to this is the following:
- When we create the user frame (the "select-frame view" command), we
create a sentinel frame just for our user-created frame, in
create_new_frame. This sentinel frame has the same id as the regular
sentinel frame.
- When printing the frame, after doing the "select-frame view" command,
the argument's pretty printer is invoked, which does an inferior
function call (this is the point of the test). This clears the frame
cache, including the "real" sentinel frame, which sets the
sentinel_frame global to nullptr.
- Later in the frame-printing process (when printing the second
argument), the auto-reinflation mechanism re-creates the user frame
by calling create_new_frame again, creating its own special sentinel
frame again. However, note that the "real" sentinel frame, the
sentinel_frame global, is still nullptr. If the selected frame had
been a regular frame, we would have called get_current_frame at some
point during the reinflation, which would have re-created the "real"
sentinel frame. But it's not the case when reinflating a user frame.
- Deep down the stack, something wants to fill in the unwind stop
reason for frame 0, which requires trying to unwind frame 1. This
leads us to trying to unwind the PC of frame 1:
#0 gdbarch_unwind_pc (gdbarch=0xffff8d010080, next_frame=...) at /home/simark/src/binutils-gdb/gdb/gdbarch.c:2955
#1 0x000000000134569c in dwarf2_tailcall_sniffer_first (this_frame=..., tailcall_cachep=0xffff773fcae0, entry_cfa_sp_offsetp=0xfffff7f7d450)
at /home/simark/src/binutils-gdb/gdb/dwarf2/frame-tailcall.c:390
#2 0x0000000001355d84 in dwarf2_frame_cache (this_frame=..., this_cache=0xffff773fc928) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1089
#3 0x00000000013562b0 in dwarf2_frame_unwind_stop_reason (this_frame=..., this_cache=0xffff773fc928) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1101
#4 0x0000000001990f64 in get_prev_frame_always_1 (this_frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2281
#5 0x0000000001993034 in get_prev_frame_always (this_frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:2376
#6 0x000000000199b814 in get_frame_unwind_stop_reason (frame=...) at /home/simark/src/binutils-gdb/gdb/frame.c:3051
#7 0x0000000001359cd8 in dwarf2_frame_cfa (this_frame=...) at /home/simark/src/binutils-gdb/gdb/dwarf2/frame.c:1356
#8 0x000000000132122c in dwarf_expr_context::execute_stack_op (this=0xfffff7f80170, op_ptr=0xffff8d8883ee "\217\002", op_end=0xffff8d8883ee "\217\002")
at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:2110
#9 0x0000000001317b30 in dwarf_expr_context::eval (this=0xfffff7f80170, addr=0xffff8d8883ed "\234\217\002", len=1) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1239
#10 0x000000000131d68c in dwarf_expr_context::execute_stack_op (this=0xfffff7f80170, op_ptr=0xffff8d88840e "", op_end=0xffff8d88840e "") at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1811
#11 0x0000000001317b30 in dwarf_expr_context::eval (this=0xfffff7f80170, addr=0xffff8d88840c "\221p", len=2) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1239
#12 0x0000000001314c3c in dwarf_expr_context::evaluate (this=0xfffff7f80170, addr=0xffff8d88840c "\221p", len=2, as_lval=true, per_cu=0xffff90b03700, frame=..., addr_info=0x0,
type=0xffff8f6c8400, subobj_type=0xffff8f6c8400, subobj_offset=0) at /home/simark/src/binutils-gdb/gdb/dwarf2/expr.c:1078
#13 0x000000000149f9e0 in dwarf2_evaluate_loc_desc_full (type=0xffff8f6c8400, frame=..., data=0xffff8d88840c "\221p", size=2, per_cu=0xffff90b03700, per_objfile=0xffff9070b980,
subobj_type=0xffff8f6c8400, subobj_byte_offset=0, as_lval=true) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1513
#14 0x00000000014a0100 in dwarf2_evaluate_loc_desc (type=0xffff8f6c8400, frame=..., data=0xffff8d88840c "\221p", size=2, per_cu=0xffff90b03700, per_objfile=0xffff9070b980, as_lval=true)
at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1557
#15 0x00000000014aa584 in locexpr_read_variable (symbol=0xffff8f6cd770, frame=...) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:3052
- AArch64 defines a special "prev register" function,
aarch64_dwarf2_prev_register, to handle unwinding the PC. This
function does
frame_unwind_register_unsigned (this_frame, AARCH64_LR_REGNUM);
- frame_unwind_register_unsigned ultimately creates a lazy register
value, saving the frame id of this_frame->next. this_frame is the
user-created frame, to this_frame->next is the special sentinel frame
we created for it. So the saved ID is the sentinel frame ID.
- When time comes to un-lazify the value, value_fetch_lazy_register
calls frame_find_by_id, to find the frame with the ID we saved.
- frame_find_by_id sees it's the sentinel frame ID, so returns the
sentinel_frame global, which is, if you remember, nullptr.
- We hit the `gdb_assert (next_frame != NULL)` assertion in
value_fetch_lazy_register.
The issues I see here are:
- The ID of the sentinel frame created for the user-created frame is
not distinguishable from the ID of the regular sentinel frame. So
there's no way frame_find_by_id could find the right frame, in
value_fetch_lazy_register.
- Even if they had distinguishable IDs, sentinel frames created for
user frames are not registered anywhere, so there's no easy way
frame_find_by_id could find it.
This patch addresses these two issues:
- Give sentinel frames created for user frames their own distinct IDs
- Register sentinel frames in the frame cache, so they can be found
with frame_find_by_id.
I initially had this split in two patches, but I then found that it was
easier to explain as a single patch.
Rergarding the first part of the change: with this patch, the sentinel
frames created for user frames (in create_new_frame) still have
stack_status == FID_STACK_SENTINEL, but their code_addr and stack_addr
fields are now filled with the addresses used to create the user frame.
This ensures this sentinel frame ID is different from the "target"
sentinel frame ID, as well as any other "user" sentinel frame ID. If
the user tries to create the same frame, with the same addresses,
multiple times, create_sentinel_frame just reuses the existing frame.
So we won't end up with multiple user sentinels with the same ID.
Regular "target" sentinel frames remain with code_addr and stack_addr
unset.
The concrete changes for that part are:
- Remove the sentinel_frame_id constant, since there isn't one
"sentinel frame ID" now. Add the frame_id_build_sentinel function
for building sentinel frame IDs and a is_sentinel_frame_id function
to check if a frame id represents a sentinel frame.
- Replace the sentinel_frame_id check in frame_find_by_id with a
comparison to `frame_id_build_sentinel (0, 0)`. The sentinel_frame
global is meant to contain a reference to the "target" sentinel, so
the one with addresses (0, 0).
- Add stack and code address parameters to create_sentinel_frame, to be
able to create the various types of sentinel frames.
- Adjust get_current_frame to create the regular "target" sentinel.
- Adjust create_new_frame to create a sentinel with the ID specific to
the created user frame.
- Adjust sentinel_frame_prev_register to get the sentinel frame ID from
the frame_info object, since there isn't a single "sentinel frame ID"
now.
- Change get_next_frame_sentinel_okay to check for a
sentinel-frame-id-like frame ID, rather than for sentinel_frame
specifically, since this function could be called with another
sentinel frame (and we would want the assert to catch it).
The rest of the change is about registering the sentinel frame in the
frame cache:
- Change frame_stash_add's assertion to allow sentinel frame levels
(-1).
- Make create_sentinel_frame add the frame to the frame cache.
- Change the "sentinel_frame != NULL" check in reinit_frame_cache for a
check that the frame stash is not empty. The idea is that if we only
have some user-created frames in the cache when reinit_frame_cache is
called, we probably want to emit the frames invalid annotation. The
goal of that check is to avoid unnecessary repeated annotations, I
suppose, so the "frame cache not empty" check should achieve that.
After this change, I think we could theoritically get rid of the
sentienl_frame global. That sentinel frame could always be found by
looking up `frame_id_build_sentinel (0, 0)` in the frame cache.
However, I left the global there to avoid slowing the typical case down
for nothing. I however, noted in its comment that it is an
optimization.
With this fix applied, the gdb.base/frame-view.exp now passes for me on
AArch64. value_of_register_lazy now saves the special sentinel frame ID
in the value, and value_fetch_lazy_register is able to find that
sentinel frame after the frame cache reinit and after the user-created
frame was reinflated.
Tested-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com>
Tested-By: Luis Machado <luis.machado@arm.com>
Change-Id: I8b77b3448822c8aab3e1c3dda76ec434eb62704f
|
|
frame cache
Currently, some frame resources are deallocated by iterating on the
frame chain (starting from the sentinel), calling dealloc_cache. The
problem is that user-created frames are not part of that chain, so we
never call dealloc_cache for them.
I propose to make it so the dealloc_cache callbacks are called when the
frames are removed from the frame_stash hash table, by registering a
deletion function to the hash table. This happens when
frame_stash_invalidate is called by reinit_frame_cache. This way, all
frames registered in the cache will get their unwinder's dealloc_cache
callbacks called.
Note that at the moment, the sentinel frames are not registered in the
cache, so we won't call dealloc_cache for them. However, it's just a
theoritical problem, because the sentinel frame unwinder does not
provide this callback. Also, a subsequent patch will change things so
that sentinel frames are registered to the cache.
I moved the obstack_free / obstack_init pair below the
frame_stash_invalidate call in reinit_frame_cache, because I assumed
that some dealloc_cache would need to access some data on that obstack,
so it would be better to free it after clearing the hash table.
Change-Id: If4f9b38266b458c4e2f7eb43e933090177c22190
|
