| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
I noticed that attempting to select a tail-call frame using 'frame
function NAME' wouldn't work:
(gdb) bt
#0 func_that_never_returns () at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/frame-selection.c:49
#1 0x0000000000401183 in func_that_tail_calls () at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/frame-selection.c:59
#2 0x00000000004011a5 in main () at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/frame-selection.c:70
(gdb) frame function func_that_tail_calls
No frame for function "func_that_tail_calls".
(gdb) up
#1 0x0000000000401183 in func_that_tail_calls () at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/frame-selection.c:59
59 func_that_never_returns ();
(gdb) disassemble
Dump of assembler code for function func_that_tail_calls:
0x000000000040117a <+0>: push %rbp
0x000000000040117b <+1>: mov %rsp,%rbp
0x000000000040117e <+4>: call 0x40116c <func_that_never_returns>
End of assembler dump.
(gdb)
The problem is that the 'function' mechanism uses get_frame_pc() and
then compares the address returned with the bounds of the function
we're looking for.
So in this case, the bounds of func_that_tail_calls are 0x40117a to
0x401183, with 0x401183 being the first address _after_ the function.
However, because func_that_tail_calls ends in a tail call, then the
get_frame_pc() is 0x401183, the first address after the function. As
a result, GDB fails to realise that frame #1 is inside the function
we're looking for, and the lookup fails.
The fix is to use get_frame_address_in_block, which will return an
adjusted address, in this case, 0x401182, which is within the function
bounds. Now the lookup works:
(gdb) frame function func_that_tail_calls
#1 0x0000000000401183 in func_that_tail_calls () at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/frame-selection.c:59
59 func_that_never_returns ();
(gdb)
I've extended the gdb.base/frame-selection.exp test to cover this
case.
|
|
Consider test-case gdb.base/return-3.exp:
...
$ gdb -q outputs/gdb.base/return-3/return-3
Reading symbols from outputs/gdb.base/return-3/return-3...
(gdb)
...
Function bar is an inlined function, and consequently we cannot return from
it:
...
(gdb) b bar
Breakpoint 1 at 0x4006ac: file return-3.c, line 25.
(gdb) r
Starting program: return-3
...
Breakpoint 1, bar () at return-3.c:25
25 c++;
(gdb) return
Can not force return from an inlined function.
(gdb)
...
However, function foo is not an inline function, and we should be able to
return from it, but we get the same error message:
...
(gdb) up
31 bar ();
(gdb) return
Can not force return from an inlined function.
(gdb)
...
Fix this by using the selected frame rather than the current frame in
return_command, such that we get instead:
...
(gdb) up
31 bar ();
(gdb) return
40 printf ("%d\n", c);
(gdb)
...
Tested on aarch64-linux.
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
PR cli/32479
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32479
|
|
It already accepts a nullptr value and a couple of places were always
calling it that way, so make it possible to omit the argument entirely.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
PR cli/32525 reports that a variable with this DWARF:
..
<2><423>: Abbrev Number: 14 (DW_TAG_variable)
<424> DW_AT_name : var1867
<42a> DW_AT_type : <0x2f8>
<42e> DW_AT_const_value : 8 byte block: 0 0 0 0 0 0 0 0
...
is not shown by info locals.
Fix this by handling LOC_CONST_BYTES in iterate_over_block_locals.
Tested on x86_64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32525
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The Linaro CI reported a regression on arm-linux in test-case
gdb.base/sigstep.exp following commit 7b46460a619 ("[gdb/symtab] Apply
workaround for PR gas/31115 a bit more") [1]:
...
(gdb) return^M
Make __default_sa_restorer return now? (y or n) n^M
Not confirmed^M
(gdb) FAIL: $exp: return from handleri: \
leave signal trampoline (got interactive prompt)
...
After installing package glibc-debuginfo and adding --with-separate-debug-dir
to the configure flags, I managed to reproduce the FAIL.
The regression seems to be a progression in the sense that the function name
for the signal trampoline is found.
After reading up on the signal trampoline [2] and the return command [3], my
understanding is that forced returning from the signal trampoline is
potentially unsafe, given that for instance the process signal mask won't be
restored.
Fix this by:
- rather than using the name, using "signal trampoline" in the query, and
- adding a warning about returning from a signal trampoline,
giving us:
...
(gdb) return^M
warning: Returning from signal trampoline does not fully restore pre-signal \
state, such as process signal mask.^M
Make signal trampoline return now? (y or n) y^M
87 dummy = 0; dummy = 0; while (!done);^M
(gdb) PASS: $exp: return from handleri: leave signal trampoline (in main)
...
Tested on x86_64-linux.
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
[1] https://linaro.atlassian.net/browse/GNU-1459
[2] https://man7.org/linux/man-pages/man2/sigreturn.2.html
[3] https://sourceware.org/gdb/current/onlinedocs/gdb.html/Returning.html
|
|
This patch adds separate styling for line numbers. That is, whenever
gdb prints a source line number, it uses this style.
v2 includes a change to ensure that %ps works in query.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Reviewed-by: Keith Seitz <keiths@redhat.com>
|
|
Some of the gdb and testsuite files double include some headers. While
all headers use include guards, it helps a bit keeping the code base
tidy.
No functional change.
Approved-by: Kevin Buettner <kevinb@redhat.com>
|
|
This is a simple find / replace from "struct bound_minimal_symbol" to
"bound_minimal_symbol", to make things shorter and more consisten
througout. In some cases, move variable declarations where first used.
Change-Id: Ica4af11c4ac528aa842bfa49a7afe8fe77a66849
Reviewed-by: Keith Seitz <keiths@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Remove some includes reported as unused by clangd. Add some includes in
other files that were previously relying on the transitive include.
Change-Id: Ibdd0a998b04d21362a20d0ca8e5267e21e2e133e
|
|
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>
|
|
These functions are implemented in top.c, move their declarations to
top.h.
Change-Id: I8893ef91d955156a6530734fefe8002d78c3e5fc
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The declaration of annotation_level is currently in defs.h, while the
definition is in stack.c. I don't really understand why that variable
would live in stack.c, it seems completely unrelated. Move it to
annotate.c, and move the declaration to annotate.h.
Change-Id: I6cf8e9bd20e83959bdf5ad58dd008b6e1187d7d8
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>
|
|
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>
|
|
Currently, if frame-filters are active, raw-values is used instead of
raw-frame-arguments to decide if a pretty-printer should be invoked for
frame arguments in a backtrace.
In this example, "super struct" is the output of the pretty-printer:
(gdb) disable frame-filter global BasicFrameFilter
(gdb) bt
#0 foo (x=42, ss=super struct = {...}) at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:47
#1 0x004016aa in main () at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:57
If no frame-filter is active, then the raw-values print option does not
affect the backtrace output:
(gdb) set print raw-values on
(gdb) bt
#0 foo (x=42, ss=super struct = {...}) at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:47
#1 0x004016aa in main () at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:57
(gdb) set print raw-values off
Instead, the raw-frame-arguments option disables the pretty-printer in the
backtrace:
(gdb) bt -raw-frame-arguments on
#0 foo (x=42, ss=...) at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:47
#1 0x004016aa in main () at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:57
But if a frame-filter is active, the same rules don't apply.
The option raw-frame-arguments is ignored, but raw-values decides if the
pretty-printer is used:
(gdb) enable frame-filter global BasicFrameFilter
(gdb) bt
#0 foo (x=42, ss=super struct = {...}) at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:47
#1 0x004016aa in main () at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:57
(gdb) set print raw-values on
(gdb) bt
#0 foo (x=42, ss=...) at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:47
#1 0x004016aa in main () at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:57
(gdb) set print raw-values off
(gdb) bt -raw-frame-arguments on
#0 foo (x=42, ss=super struct = {...}) at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:47
#1 0x004016aa in main () at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:57
So this adds the PRINT_RAW_FRAME_ARGUMENTS flag to frame_filter_flag, which
is then used in the frame-filter to override the raw flag in enumerate_args.
Then the output is the same if a frame-filter is active, the pretty-printer
for backtraces is only disabled with the raw-frame-arguments option:
(gdb) enable frame-filter global BasicFrameFilter
(gdb) bt
#0 foo (x=42, ss=super struct = {...}) at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:47
#1 0x004016aa in main () at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:57
(gdb) set print raw-values on
(gdb) bt
#0 foo (x=42, ss=super struct = {...}) at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:47
#1 0x004016aa in main () at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:57
(gdb) set print raw-values off
(gdb) bt -raw-frame-arguments on
#0 foo (x=42, ss=...) at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:47
#1 0x004016aa in main () at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:57
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This code was probably needed before we had reinflatable
frame_info_ptrs, it's not necessary anymore.
Change-Id: I5474c6081ee1e39624c9266b05dbe01351a130b5
Approved-By: Tom Tromey <tom@tromey.com>
|
|
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>
|
|
Introduce new ui_file buffering_file to temporarily collect output
written to gdb_std* output streams during print_thread, print_frame_info
and print_stop_event.
This ensures that output during these functions is not interrupted
by debuginfod progress messages.
With the addition of deferred debuginfo downloading it is possible
for download progress messages to print during these events.
Without any intervention we can end up with poorly formatted output:
(gdb) backtrace
[...]
#8 0x00007fbe8af7d7cf in pygi_invoke_c_callable (Downloading separate debug info for /lib64/libpython3.11.so.1.0
function_cache=0x561221b224d0, state=<optimized out>...
To fix this we buffer writes to gdb_std* output streams while allowing
debuginfod progress messages to skip the buffers and print to the
underlying output streams immediately. Buffered output is then written
to the output streams. This ensures that progress messages print first,
followed by uninterrupted frame/thread/stop info:
(gdb) backtrace
[...]
Downloading separate debug info for /lib64/libpython3.11.so.1.0
#8 0x00007fbe8af7d7cf in pygi_invoke_c_callable (function_cache=0x561221b224d0, state=<optimized out>...
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Change-Id: I5e96c0f38aa788462ab19a9becfe22030e913c2a
|
|
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
|
|
Since GDB now requires C++17, we don't need the internally maintained
gdb::optional implementation. This patch does the following replacing:
- gdb::optional -> std::optional
- gdb::in_place -> std::in_place
- #include "gdbsupport/gdb_optional.h" -> #include <optional>
This change has mostly been done automatically. One exception is
gdbsupport/thread-pool.* which did not use the gdb:: prefix as it
already lives in the gdb namespace.
Change-Id: I19a92fa03e89637bab136c72e34fd351524f65e9
Approved-By: Tom Tromey <tom@tromey.com>
Approved-By: Pedro Alves <pedro@palves.net>
|
|
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
|
|
I stumbled across a few spots that mention that a function
"invalidates frame" and also assignments of NULL to a frame_info_ptr.
This code isn't harmful, but is also unnecessary since the
introduction of frame_info_ptr -- nowadays frame invalidations are
handled automatically.
Regression tested on x86-64 Fedora 38.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
The author of 'mold' pointed out that with a certain shared library,
gdb would fail to find the shared library's name in 'bt'.
The function in question appeared at the end of the .so's .text
segment and ended with a call to 'abort'.
This turned out to be a classic case of calling get_frame_pc when
get_frame_address_in_block is needed -- the former will be off-by-one
for purposes of finding the enclosing function or shared library.
The included test fails without the patch on my system. However, I
imagine it can't be assumed to reliably fail. Nevertheless it seemed
worth doing.
Regression tested on x86-64 Fedora 38.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29074
Reviewed-by: Kevin Buettner <kevinb@redhat.com>
|
|
Same as previous patches, but for user_selected_context_changed.
Change-Id: I40de15be897671227d4bcf3e747f0fd595f0d5be
|
|
547ce8f00b fixed an issue where dynamic types were not being resolved
correctly prior to printing a value. The same issue was discovered when
printing the value using mi-mode, which was not covered by the fix.
Porting the fix to the mi-mode code path resolved the issue.
However, it was discovered that a later patch series, ending
2fc3b8a4cb8, independently fixed the issue in both the cli- and mi-mode
code paths, making the original fix unneeded.
This commit removes this extra frame switch and adds test coverage for
the mi-mode scenario to protect against any future divergence in this
area.
GDB built with GCC 11.
No test suite regressions detected. Compilers: GCC 12.1.0, ACfL 22.1,
Intel 22.1; Platforms: x86_64, aarch64.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
evaluate_expression is just a little wrapper for a method on
expression. Removing it also removes a lot of ugly (IMO) calls to
get().
|
|
This removes ALL_BLOCK_SYMBOLS in favor of foreach.
|
|
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 changes value_contents_eq to be a method of value. It also
converts the static function value_contents_bits_eq into a private
method.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes value_fetch_lazy to be a method of value. A few helper
functions are converted as well, to avoid problems in later patches
when the data members are all made private.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This turns allocate_optimized_out_value into a static "constructor" of
value.
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>
|
|
This is the second step of making frame_info_ptr automatic, reinflate on
demand whenever trying to obtain the wrapper frame_info pointer, either
through the get method or operator->. Make the reinflate method
private, it is used as a convenience method in those two.
Add an "is_null" method, because it is often needed to know whether the
frame_info_ptr wraps an frame_info or is empty.
Make m_ptr mutable, so that it's possible to reinflate const
frame_info_ptr objects. Whether m_ptr is nullptr or not does not change
the logical state of the object, because we re-create it on demand. I
believe this is the right use case for mutable.
Change-Id: Icb0552d0035e227f81eb3c121d8a9bb2f9d25794
Reviewed-By: Bruno Larsen <blarsen@redhat.com>
|
|
This is the first step of making frame_info_ptr automatic. Remove the
frame_info_ptr::prepare_reinflate method, move that code to the
constructor.
Change-Id: I85cdae3ab1c043c70e2702e7fb38e9a4a8a675d8
Reviewed-By: Bruno Larsen <blarsen@redhat.com>
|
|
The gdbarch "return_value" can't correctly handle variably-sized
types. The problem here is that the TYPE_LENGTH of such a type is 0,
until the type is resolved, which requires reading memory. However,
gdbarch_return_value only accepts a buffer as an out parameter.
Fixing this requires letting the implementation of the gdbarch method
resolve the type and return a value -- that is, both the contents and
the new type.
After an attempt at this, I realized I wouldn't be able to correctly
update all implementations (there are ~80) of this method. So,
instead, this patch adds a new method that falls back to the current
method, and it updates gdb to only call the new method. This way it's
possible to incrementally convert the architectures that I am able to
test.
|
|
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.
|
|
This changes the uses of value_print_options to use 'true' and 'false'
rather than integers.
|
|
I stumbled across subset_compare today, and after looking at the
callers I realized it could be removed and replaced with calls to
startswith.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
print_frame_info calls frame_info_ptr::reinflate, but not
frame_info_ptr::prepare_reinflate, add the call to prepare_reinflate.
It works right now, because all callers of print_frame_info that could
possibly lead to the pretty printers being called, and the frame_info
objects being invalidated, do call prepare_reinflate themselves. And
since the cached frame id is copied when passing a frame_info_ptr by
value, print_frame_info does have a cached frame id on entry. So
technically, this change isn't needed. But I don't think it's good for
a function to rely on its callers to have called prepare_reinflate, if
it intends to call reinflate.
Change-Id: Ie332b2d5479aef46f83fdc1120c7c83f4e84d1b0
Reviewed-By: Bruno Larsen <blarsen@redhat.com>
|
|
With the following patch applied (gdb: use frame_id_p instead of
comparing to null_frame_id in frame_info_ptr::reinflate), I would get:
$ ./gdb -q -nx --data-directory=data-directory testsuite/outputs/gdb.base/bt-selected-frame/bt-selected-frame -ex "b breakpt" -ex r -ex "bt full"
Reading symbols from testsuite/outputs/gdb.base/bt-selected-frame/bt-selected-frame...
Breakpoint 1 at 0x1131: file /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.base/bt-selected-frame.c, line 22.
Starting program: /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/bt-selected-frame/bt-selected-frame
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Breakpoint 1, breakpt () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.base/bt-selected-frame.c:22
22 }
#0 breakpt () at /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.base/bt-selected-frame.c:22
No locals.
/home/smarchi/src/binutils-gdb/gdb/frame-info.c:42: internal-error: reinflate: Assertion `frame_id_p (m_cached_id)' failed.
This is because the code in backtrace_command_1 to manually reinflate
`fi` steps overs frame_info_ptr's toes.
When calling
fi.prepare_reinflate ();
`fi` gets properly filled with the cached frame id. But when this
happens:
fi = frame_find_by_id (frame_id);
`fi` gets replaced by a brand new frame_info_ptr that doesn't have a
cached frame id. Then this is called without a cached frame id:
fi.reinflate ();
That doesn't cause any problem currently, since
- the gdb_assert in the reinflate method doesn't actually do anything
(the following patch fixes that)
- `fi.m_ptr` will always be non-nullptr, since we just got it from
frame_find_by_id, so reinflate will not do anything, it won't try to
use m_cached_id
Fix that by removing the code to manually re-fetch the frame. That
should be taken care of by frame_info_ptr::reinflate.
Note that the old code checked if we successfully re-inflated the frame
or not, and if not it did emit a warning. The equivalent in
frame_info_ptr::reinflate asserts that the frame has been successfully
re-inflated. It's not clear if / when this can happen, but if it can
happen, we'll need to find a solution to this problem globally
(everywhere a frame_info_ptr can be re-inflated), not just here. So I
propose to leave it like this, until it does become a problem.
Reviewed-By: Bruno Larsen <blarsen@redhat.com>
Change-Id: I07b783d94e2853e0a2d058fe7deaf04eddf24835
|
|
info_frame_command_core
I noticed this crash:
$ ./gdb --data-directory=data-directory -nx -q \
testsuite/outputs/gdb.python/pretty-print-call-by-hand/pretty-print-call-by-hand \
-x testsuite/outputs/gdb.python/pretty-print-call-by-hand/pretty-print-call-by-hand.py \
-ex "b g" -ex r
(gdb) info frame
Stack level 0, frame at 0x7fffffffdd80:
rip = 0x555555555160 in g
(/home/simark/src/binutils-gdb/gdb/testsuite/gdb.python/pretty-print-call-by-hand.c:41); saved rip = 0x5555555551a3
called by frame at 0x7fffffffdda0
source language c.
Arglist at 0x7fffffffdd70, args: mt=mytype is 0x555555556004 "hello world",
depth=10
Fatal signal: Segmentation fault
This is another case of frame_info being invalidated under a function's
feet. The stack trace when the frame_info get invalidated looks like:
... many frames to pretty print the arg, that eventually invalidate the frame_infos ...
#35 0x00005568d0a8ab24 in print_frame_arg (fp_opts=..., arg=0x7ffc3216bcb0) at /home/simark/src/binutils-gdb/gdb/stack.c:489
#36 0x00005568d0a8cc75 in print_frame_args (fp_opts=..., func=0x621000233210, frame=..., num=-1, stream=0x60b000000300)
at /home/simark/src/binutils-gdb/gdb/stack.c:898
#37 0x00005568d0a9536d in info_frame_command_core (fi=..., selected_frame_p=true) at /home/simark/src/binutils-gdb/gdb/stack.c:1682
print_frame_args knows that print_frame_arg can invalidate frame_info
objects, and therefore calls prepare_reinflate/reinflate. However,
info_frame_command_core has a separate frame_info_ptr instance (it is
passed by value / copy). So info_frame_command_core needs to know that
print_frame_args can invalidate frame_info objects, and therefore needs
to prepare_reinflate/reinflate as well. Add those calls, and enhance
the gdb.python/pretty-print-call-by-hand.exp test to test that command.
Reviewed-By: Bruno Larsen <blarsen@redhat.com>
Change-Id: I9edaae06d62e97ffdb30938d364437737238a960
|
