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>
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
2fc3b8a4cb, 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>
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 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 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.
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
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>
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.
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
Add a getter and a setter for a symtab's language. Remove the
corresponding macro and adjust all callers.
Change-Id: I9f4d840b11c19f80f39bac1bce020fdd1739e11f
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.
This commit brings all the changes made by running gdb/copyright.py
as per GDB's Start of New Year Procedure.
For the avoidance of doubt, all changes in this commits were
performed by the script.
The bug fixed by this [1] patch was caused by an out-of-bounds access to
a value's content. The code gets the value's content (just a pointer)
and then indexes it with a non-sensical index.
This made me think of changing functions that return value contents to
return array_views instead of a plain pointer. This has the advantage
that when GDB is built with _GLIBCXX_DEBUG, accesses to the array_view
are checked, making bugs more apparent / easier to find.
This patch changes the return types of these functions, and updates
callers to call .data() on the result, meaning it's not changing
anything in practice. Additional work will be needed (which can be done
little by little) to make callers propagate the use of array_view and
reap the benefits.
[1] https://sourceware.org/pipermail/gdb-patches/2021-September/182306.html
Change-Id: I5151f888f169e1c36abe2cbc57620110673816f3
