Age | Commit message (Collapse) | Author | Files | Lines |
|
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
|
|
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
|
|
Currently, despite having a smart pointer for frame_infos, GDB may
attempt to use an invalidated frame_info_ptr, which would cause internal
errors to happen. One such example has been documented as PR
python/28856, that happened when printing frame arguments calls an
inferior function.
To avoid failures, the smart wrapper was changed to also cache the frame
id, so the pointer can be reinflated later. For this to work, the
frame-id stuff had to be moved to their own .h file, which is included
by frame-info.h.
Frame_id caching is done explicitly using the prepare_reinflate method.
Caching is done manually so that only the pointers that need to be saved
will be, and reinflating has to be done manually using the reinflate
method because the get method and the -> operator must not change
the internals of the class. Finally, attempting to reinflate when the
pointer is being invalidated causes the following assertion errors:
check_ptrace_stopped_lwp_gone: assertion `lp->stopped` failed.
get_frame_pc: Assertion `frame->next != NULL` failed.
As for performance concerns, my personal testing with `time make
chec-perf GDB_PERFTEST_MODE=run` showed an actual reduction of around
10% of time running.
This commit also adds a testcase that exercises the python/28856 bug with
7 different triggers, run, continue, step, backtrace, finish, up and down.
Some of them can seem to be testing the same thing twice, but since this
test relies on stale pointers, there is always a chance that GDB got lucky
when testing, so better to test extra.
Regression tested on x86_64, using both gcc and clang.
Approved-by: Tom Tomey <tom@tromey.com>
|
|
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>
|
|
This replaces frame_id_eq with operator== and operator!=. I wrote
this for a version of this series that I later abandoned; but since it
simplifies the code, I left this patch in.
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
|
|
I noticed that solib_name_from_address returned a non-const string,
but it's more appropriate to return const. This patch implements
this. Tested by rebuilding.
|
|
Replace with equivalent methods.
Change-Id: I334a319909a50b5cc5570a45c38c70e10dc00630
|
|
Replace with equivalent methods.
Change-Id: I31ec00f5bf85335c8b23d306ca0fe0b84d489101
|
|
Remove all macros related to getting and setting some symbol value:
#define SYMBOL_VALUE(symbol) (symbol)->value.ivalue
#define SYMBOL_VALUE_ADDRESS(symbol) \
#define SET_SYMBOL_VALUE_ADDRESS(symbol, new_value) \
#define SYMBOL_VALUE_BYTES(symbol) (symbol)->value.bytes
#define SYMBOL_VALUE_COMMON_BLOCK(symbol) (symbol)->value.common_block
#define SYMBOL_BLOCK_VALUE(symbol) (symbol)->value.block
#define SYMBOL_VALUE_CHAIN(symbol) (symbol)->value.chain
#define MSYMBOL_VALUE(symbol) (symbol)->value.ivalue
#define MSYMBOL_VALUE_RAW_ADDRESS(symbol) ((symbol)->value.address + 0)
#define MSYMBOL_VALUE_ADDRESS(objfile, symbol) \
#define BMSYMBOL_VALUE_ADDRESS(symbol) \
#define SET_MSYMBOL_VALUE_ADDRESS(symbol, new_value) \
#define MSYMBOL_VALUE_BYTES(symbol) (symbol)->value.bytes
#define MSYMBOL_BLOCK_VALUE(symbol) (symbol)->value.block
Replace them with equivalent methods on the appropriate objects.
Change-Id: Iafdab3b8eefc6dc2fd895aa955bf64fafc59ed50
|
|
This patch removes gdb's dbx mode. Regression tested on x86-64 Fedora
34.
|
|
Now that filtered and unfiltered output can be treated identically, we
can unify the printf family of functions. This is done under the name
"gdb_printf". Most of this patch was written by script.
|
|
Now that filtered and unfiltered output can be treated identically, we
can unify the puts family of functions. This is done under the name
"gdb_puts". Most of this patch was written by script.
|
|
GDB notifies users about user selected thread changes somewhat
inconsistently as mentioned on gdb-patches mailing list here:
https://sourceware.org/pipermail/gdb-patches/2022-February/185989.html
Consider GDB debugging a multi-threaded inferior with both CLI and GDB/MI
interfaces connected to separate terminals.
Assuming inferior is stopped and thread 1 is selected, when a thread
2 is selected using '-thread-select 2' command on GDB/MI terminal:
-thread-select 2
^done,new-thread-id="2",frame={level="0",addr="0x00005555555551cd",func="child_sub_function",args=[],file="/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.mi/user-selected-context-sync.c",fullname="/home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c",line="30",arch="i386:x86-64"}
(gdb)
and on CLI terminal we get the notification (as expected):
[Switching to thread 2 (Thread 0x7ffff7daa640 (LWP 389659))]
#0 child_sub_function () at /home/uuu/gdb/gdb/testsuite/gdb.mi/user-selected-context-sync.c:30
30 volatile int dummy = 0;
However, now that thread 2 is selected, if thread 1 is selected
using 'thread-select --thread 1 1' command on GDB/MI terminal
terminal:
-thread-select --thread 1 1
^done,new-thread-id="1",frame={level="0",addr="0x0000555555555294",func="main",args=[],file="/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.mi/user-selected-context-sync.c",fullname="/home/jv/Projects/gdb/users_jv_patches/gdb/testsuite/gdb.mi/user-selected-context-sync.c",line="66",arch="i386:x86-64"}
(gdb)
but no notification is printed on CLI terminal, despite the fact
that user selected thread has changed.
The problem is that when `-thread-select --thread 1 1` is executed
then thread is switched to thread 1 before mi_cmd_thread_select () is
called, therefore the condition "inferior_ptid != previous_ptid"
there does not hold.
To address this problem, we have to move notification logic up to
mi_cmd_execute () where --thread option is processed and notify
user selected contents observers there if context changes.
However, this in itself breaks GDB/MI because it would cause context
notification to be sent on MI channel. This is because by the time
we notify, MI notification suppression is already restored (done in
mi_command::invoke(). Therefore we had to lift notification suppression
logic also up to mi_cmd_execute (). This change in made distinction
between mi_command::invoke() and mi_command::do_invoke() unnecessary
as all mi_command::invoke() did (after the change) was to call
do_invoke(). So this patches removes do_invoke() and moves the command
execution logic directly to invoke().
With this change, all gdb.mi tests pass, tested on x86_64-linux.
Co-authored-by: Andrew Burgess <aburgess@redhat.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=20631
|
|
This changes iterate_over_block_local_vars and
iterate_over_block_arg_vars to take a gdb::function_view rather than a
function pointer and a user-data. In one spot, this allows us to
remove a helper structure and helper function. In another spot, this
looked more complicated, so I changed the helper function to be an
"operator()" -- also a simplification, just not as big.
|
|
It is possible for a compiler to optimize a function in a such ways that
the function does not follow the calling convention of the target. In
such situation, the compiler can use the DW_AT_calling_convention
attribute with the value DW_CC_nocall to tell the debugger that it is
unsafe to call the function. The DWARF5 standard states, in 3.3.1.1:
> If the value of the calling convention attribute is the constant
> DW_CC_nocall, the subroutine does not obey standard calling
> conventions, and it may not be safe for the debugger to call this
> subroutine.
Non standard calling convention can affect GDB's assumptions in multiple
ways, including how arguments are passed to the function, how values are
returned, and so on. For this reason, it is unsafe for GDB to try to do
the following operations on a function with marked with DW_CC_nocall:
- call / print an expression requiring the function to be evaluated,
- inspect the value a function returns using the 'finish' command,
- force the value returned by a function using the 'return' command.
This patch ensures that if a command which relies on GDB's knowledge of
the target's calling convention is used on a function marked nocall, GDB
prints an appropriate message to the user and does not proceed with the
operation which is unreliable.
Note that it is still possible for someone to use a vendor specific
value for the DW_AT_calling_convention attribute for example to indicate
the use of an alternative calling convention. This commit does not
prevent this, and target dependent code can be adjusted if one wanted to
support multiple calling conventions.
Tested on x86_64-Linux, with no regression observed.
Change-Id: I72970dae68234cb83edbc0cf71aa3d6002a4a540
|
|
Add a getter and a setter for a symbol's type. Remove the corresponding
macro and adjust all callers.
Change-Id: Ie1a137744c5bfe1df4d4f9ae5541c5299577c8de
|
|
Add a getter and a setter for whether a symbol is an argument. Remove
the corresponding macro and adjust all callers.
Change-Id: I71b4f0465f3dfd2ed8b9e140bd3f7d5eb8d9ee81
|
|
Add a getter and a setter for a symbol's domain. Remove the
corresponding macro and adjust all callers.
Change-Id: I54465b50ac89739c663859a726aef8cdc6e4b8f3
|
|
Change-Id: I83211d5a47efc0564386e5b5ea4a29c00b1fd46a
|
|
Add a getter and a setter for a symtab's language. Remove the
corresponding macro and adjust all callers.
Change-Id: I9f4d840b11c19f80f39bac1bce020fdd1739e11f
|
|
This changes all existing calls to wrap_here to call the method on the
appropriate ui_file instead. The choice of ui_file is determined by
context.
|
|
I think it only really makes sense to call wrap_here with an argument
consisting solely of spaces. Given this, it seemed better to me that
the argument be an int, rather than a string. This patch is the
result. Much of it was written by a script.
|
|
In an earlier version of the pager rewrite series, it was important to
audit unfiltered output calls to see which were truly necessary.
This is no longer necessary, but it still seems like a decent cleanup
to change calls to avoid explicitly passing gdb_stdout. That is,
rather than using something like fprintf_unfiltered with gdb_stdout,
the code ought to use plain printf_unfiltered instead.
This patch makes this change. I went ahead and converted all the
_filtered calls I could find, as well, for the same clarity.
|