This patch resolves the performance issue reported in pr/29738 by
caching the values for the stack pointers for the inner frame. By
doing so, the impact can be reduced to checking the state and
returning the appropriate value.
gdb: dwarf2 generic implementation for caching function data
When there is no dwarf2 data for a register, a function can be called
to provide the value of this register. In some situations, it might
not be trivial to determine the value to return and it would cause a
performance bottleneck to do the computation each time.
This patch allows the called function to have a "cache" object that it
can use to store some metadata between calls to reduce the performance
impact of the complex logic.
The cache object is unique for each function and frame, so if there are
more than one function pointer stored in the dwarf2_frame_cache->reg
array, then the appropriate pointer will be supplied (the type is not
known by the dwarf2 implementation).
dwarf2_frame_get_fn_data can be used to retrieve the function unique
cache object.
dwarf2_frame_allocate_fn_data can be used to allocate and retrieve the
function unique cache object.
Tom Tromey [Fri, 27 Jan 2023 18:19:16 +0000 (11:19 -0700)]
Fix comparator bug in cooked index
Simon pointed out that the cooked index template-matching patch
introduced a failure in libstdc++ debug mode. In particular, the new
code violates the assumption of std::lower_bound and std::upper_bound
that the range is sorted with respect to the comparison.
When I first debugged this, I thought the problem was unfixable as-is
and that a second layer of filtering would have to be done. However,
on irc, Simon pointed out that it could perhaps be solved if the
comparison function were assured that one operand always came from the
index, with the other always being the search string.
This patch implements this idea.
First, a new mode is introduced: a sorting mode for
cooked_index_entry::compare. In this mode, strings are compared
case-insensitively, but we're careful to always sort '<' before any
other printable character. This way, two names like "func" and
"func<param>" will be sorted next to each other -- i.e., "func1" will
not be seen between them. This is important when searching.
Second, the compare function is changed to work in a strcmp-like way.
This makes it easier to test and (IMO) understand.
Third, the compare function is modified so that in non-sorting modes,
the index entry is always the first argument. This allows consistency
in compares.
I regression tested this in libstdc++ debug mode on x86-64 Fedora 36.
It fixes the crash that Simon saw.
This is v2. I believe it addresses the review comments, except for
the 'enum class' change, as I mentioned in email on the list.
Tom Tromey [Wed, 14 Dec 2022 21:37:41 +0000 (14:37 -0700)]
Fix parameter-less template regression in new DWARF reader
PR c++/29896 points out a regression in the new DWARF reader. It does
not properly handle a case like "break fn", where "fn" is a template
function.
This happens because the new index uses strncasecmp to compare.
However, to make this work correctly, we need a custom function that
ignores template parameters.
This patch adds a custom comparison function and fixes the bug. A new
test case is included.
Tom Tromey [Thu, 15 Dec 2022 16:38:05 +0000 (09:38 -0700)]
Move hash_entry and eq_entry into cooked_index::do_finalize
I was briefly confused by the hash_entry and eq_entry functions in the
cooked index. They are only needed in a single method, and that
method already has a couple of local lambdas for a different hash
table. So, it seemed cleaner to move these there as well.
Tom Tromey [Tue, 13 Dec 2022 19:03:34 +0000 (12:03 -0700)]
Avoid submitting empty tasks in parallel_for_each
I found that parallel_for_each would submit empty tasks to the thread
pool. For example, this can happen if the number of tasks is smaller
than the number of available threads. In the DWARF reader, this
resulted in the cooked index containing empty sub-indices. This patch
arranges to instead shrink the result vector and process the trailing
entries in the calling thread.
Tom Tromey [Mon, 9 Jan 2023 14:43:29 +0000 (07:43 -0700)]
Set _WIN32_WINNT in common.m4 configure check
GCC recently added support for the Windows thread model, enabling
libstdc++ to support Windows natively. However, this supporrt
requires a version of Windows later than the minimum version that is
supported by GDB.
PR build/29966 points out that the GDB configure test for std::thread
does not work in this situation, because _WIN32_WINNT is not defined
in test program, and so <thread> seems to be fine.
This patch is an attempt to fix the problem, by using the same setting
for _WIN32_WINNT at configure time as is used at build time.
I don't have access to one of the older systems so I don't think I can
truly test this. I did do a mingw cross build, though. I'm going to
ask the bug reporter to test it.
Andrew Burgess [Tue, 3 Jan 2023 10:53:09 +0000 (11:53 +0100)]
[gdb] Fix segfault during inferior call to ifunc
With a simple test-case:
...
$ cat test.c
char *p = "a";
int main (void) {
return strlen (p);
}
$ gcc -g test.c
...
we run into this segfault:
...
$ gdb -q -batch a.out -ex start -ex "p strlen (p)"
Temporary breakpoint 1 at 0x1151: file test.c, line 4.
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Temporary breakpoint 1, main () at test.c:4
4 return strlen (p);
Fatal signal: Segmentation fault
...
The strlen is an ifunc, and consequently during the call to
call_function_by_hand_dummy for "p strlen (p)" another call
to call_function_by_hand_dummy is used to resolve the ifunc.
This invalidates the get_current_frame () result in the outer call.
Fix this by using prepare_reinflate and reinflate.
Note that this series (
https://inbox.sourceware.org/gdb-patches/20221214033441.499512-1-simon.marchi@polymtl.ca/ )
should address this problem, but this patch is a simpler fix which is easy to
backport.
Tested on x86_64-linux.
Co-Authored-By: Tom de Vries <tdevries@suse.de>
PR gdb/29941
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29941
Jonas Hoerberg [Thu, 22 Dec 2022 15:22:17 +0000 (15:22 +0000)]
Fix target remote pipe command for MinGW
The cced7cacecad104fff0 ("gdb: preserve `|` in connection details string")
commit added '|' detection and removal to ser-pipe.c, but missed to add it
to ser-mingw.c.
This results in the error message below for MinGW hosts:
error starting child process '| <executable> <args>': CreateProcess: No such file or directory
This commit add the missing '|' detection and removal to ser-mingw.c.
... by hand as instructed by the gdb/copyright.py script.
The update by hand is needed because the copyright headers
to update are actually nested inside those files, rather
than located at the start of the file.
Joel Brobecker [Sun, 1 Jan 2023 13:07:37 +0000 (17:07 +0400)]
Update copyright year range in header of all files managed by GDB
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.
Joel Brobecker [Sun, 1 Jan 2023 13:07:32 +0000 (17:07 +0400)]
gdb/copyright.py: Adjust following rename of sim/ppc/ppc-instructions...
... to sim/ppc/powerpc.igen
This file is in the NOT_FSF_LIST because this file has a copyright
which is not assigned to the FSF. Since the file got renamed,
the corresponding entry in NOT_FSF_LIST needs to be renamed as well.
Joel Brobecker [Sun, 1 Jan 2023 13:07:20 +0000 (17:07 +0400)]
Update copyright year in help message of gdb, gdbserver, gdbreplay
This commit updates the copyright year displayed by gdb, gdbserver
and gdbreplay's help message from 2022 to 2023, as per our Start
of New Year procedure. The corresponding source files' copyright
header are also updated accordingly.
Tom Tromey [Tue, 27 Dec 2022 23:34:44 +0000 (16:34 -0700)]
Fix "set debug timestamp"
PR cli/29945 points out that "set debug timestamp 1" stopped working
-- this is a regression due to commit b8043d27 ("Remove a ui-related
memory leak").
This patch fixes the bug and adds a regression test.
I think this should probably be backported to the gdb 13 branch.
Eli Zaretskii [Thu, 22 Dec 2022 10:42:24 +0000 (12:42 +0200)]
Fix MinGW build using mingw.org's MinGW
This allows to build GDB even though the default value of
_WIN32_WINNT is lower than the one needed to expose some
new APIs used here, and leave the test for their actual
support to run time.
* gdb/nat/windows-nat.c (EXTENDED_STARTUPINFO_PRESENT): Define if
not defined.
(create_process_wrapper): Use 'gdb_lpproc_thread_attribute_list'
instead of 'PPROC_THREAD_ATTRIBUTE_LIST' (which might not be defined
at compile time). This fixes compilation error using mingw.org's
MinGW.
Jan Vrany [Mon, 19 Dec 2022 11:24:36 +0000 (11:24 +0000)]
gdb: fix command lookup in execute_command ()
Commit b5661ff2 ("gdb: fix possible use-after-free when
executing commands") used lookup_cmd_exact () to lookup
command again after its execution to avoid possible
use-after-free error.
However this change broke test gdb.base/define.exp which
defines a post-hook for subcommand ("target testsuite").
In this case, lookup_cmd_exact () returned NULL because
there's no command 'testsuite' in top-level commands.
This commit fixes this case by looking up the command again
using the original command line via lookup_cmd ().
Alan Modra [Fri, 16 Dec 2022 11:07:29 +0000 (21:37 +1030)]
bfd_get_relocated_section_contents allow NULL data buffer
This patch removes the bfd_malloc in default_indirect_link_order and
bfd_simple_get_relocated_section_contents, pushing the allocation down
to bfd_get_relocated_section_contents. The idea is to make use of the
allocation done with sanity checking in bfd_get_full_section_contents,
which is called by bfd_generic_get_relocated_section_contents.
Doing this exposed a bug in bfd_get_full_section_contents. With
relaxation it is possible that an input section rawsize is different
to the section size. In that case we want to use the larger of
rawsize (the on-disk size for input sections) and size.
Alan Modra [Thu, 15 Dec 2022 12:37:10 +0000 (23:07 +1030)]
Re: The problem with warning in elf_object_p
Commit 5aa0f10c424e added a per_xvec_warn array to provide support for
warnings from elf_object_p (and a later patch for warnings from
pe_bfd_object_p) to be cached and then only printed if the target
matches. It was quite limited in the style of message supported, only
one message could be printed, and didn't really meet the stated aim of
only warning when a target matches: There are many other errors and
warnings that can be emitted by functions called from elf_object_p.
So this patch extends the error handler functions to support printing
to a string buffer, extends per_xvec_warn to support multiple errors/
warnings, and hooks this all into bfd_check_format_matches. If
bfd_check_format_matches succeeds then any errors/warnings are printed
for the matching target. If bfd_check_format_matches fails either due
to no match or to multiple matches and only one target vector produced
errors, then those errors are printed.
* bfd.c (MAX_ARGS): Define, use throughout.
(print_func): New typedef.
(_bfd_doprnt): Add new print param. Replace calls to fprintf
with print.
(PRINT_TYPE): Similarly.
(error_handler_fprintf): Renamed from error_handler_internal.
Use _bfd_get_error_program_name. Add fprintf arg. Move code
setting up args..
(_bfd_doprnt_scan): ..to here. Add ap param.
(struct buf_stream): New.
(err_sprintf): New function.
(error_handler_bfd): New static variable.
(error_handler_sprintf): New function.
(_bfd_set_error_handler_caching): New function.
(_bfd_get_error_program_name): New function.
* elfcode.h (elf_swap_shdr_in): Use _bfd_error_handler in
warning messages.
(elf_object_p): Likewise.
* format.c (print_warnmsg): New function.
(clear_warnmsg): Rewrite.
(null_error_handler): New function.
(bfd_check_format_matches): Ignore warnings from recursive calls
checking first element of an archive. Use caching error handler
otherwise. Print warnings on successful match, or when only one
target has emitted warnings/errors.
* peicode.h (pe_bfd_object_p): Use _bfd_error_handler in
warning messages.
* targets.c (per_xvec_warn): Change type of array elements.
(struct per_xvec_message): New.
(_bfd_per_xvec_warn): Rewrite.
* Makefile.am (LIBBFD_H_FILES): Add bfd.c.
* Makefile.in: Regenerate.
* bfd-in2.h: Regenerate.
* libbfd.h: Regenerate.
Indu Bhagat [Sat, 17 Dec 2022 06:02:07 +0000 (22:02 -0800)]
gas: sframe: testsuite: add testcase for .cfi_negate_ra_state
Add a new test to check that .cfi_negate_ra_state on aarch64 is handled
well (a non-empty SFrame section with valid SFrame FREs is generated).
ChangeLog:
* testsuite/gas/cfi-sframe/cfi-sframe-aarch64-2.d: New test.
* testsuite/gas/cfi-sframe/cfi-sframe-aarch64-2.s: Likewise.
* testsuite/gas/cfi-sframe/cfi-sframe.exp: Adjust the list
accordingly.
Indu Bhagat [Sat, 17 Dec 2022 06:02:02 +0000 (22:02 -0800)]
objdump/readelf: sframe: emit marker for FREs with mangled RA
In the textual dump of the SFrame section, when an SFrame FRE recovers a
mangled RA, use string "[s]" in the output to indicate that the return
address is a signed (mangled) one.
ChangeLog:
* libsframe/sframe-dump.c (dump_sframe_func_with_fres): Postfix
with "[s]" if RA is signed with authorization code.
Indu Bhagat [Sat, 17 Dec 2022 06:01:40 +0000 (22:01 -0800)]
sframe.h: add support for .cfi_negate_ra_state
Use the last remaining bit in the 'SFrame FRE info' word to store whether
the RA is signed/unsigned with PAC authorization code: this bit is named
as the "mangled RA" bit. This bit is still unused for x86-64.
The behaviour of the mangled-RA info bit in SFrame format closely
follows the behaviour of DW_CFA_AARCH64_negate_ra_state in DWARF. During
unwinding, whenever an SFrame FRE with non-zero "mangled RA" bit is
encountered, it means the upper bits of the return address contain Pointer
Authentication code. The unwinder, hence, must use appropriate means to
restore LR correctly in such cases.
include/ChangeLog:
* sframe.h (SFRAME_V1_FRE_INFO_UPDATE_MANGLED_RA_P): New macro.
(SFRAME_V1_FRE_MANGLED_RA_P): Likewise.
Pedro Alves [Fri, 16 Dec 2022 12:04:54 +0000 (12:04 +0000)]
Delay checking whether /proc/pid/mem is writable (PR gdb/29907)
As of 1bcb0708f229 ("gdb/linux-nat: Check whether /proc/pid/mem is
writable"), GDB checks if /proc/pid/mem is writable. This is done
early at GDB startup, in order to get a consistent warning, instead of
a warning that depends on whenever GDB writes to inferior memory.
PR gdb/29907 points out that some build systems (like QEMU's,
apparently) may call 'gdb --version' to check GDB's presence & its
version on the system, and that Gentoo's build process has sandboxing
which blocks the /proc/pid/mem access and thus GDB warns, which
results in build fails.
To help with that, this patch delays the /proc/pid/mem check until we
start or attach to an inferior. Ends up potentially emiting a warning
close where we already emit other ptrace- and /proc- related warnings,
which just Feels Right.
Tom de Vries [Fri, 16 Dec 2022 14:28:27 +0000 (15:28 +0100)]
[gdb/testsuite] Fix race in gdb.threads/detach-step-over.exp
Once in a while I run into:
...
FAIL: gdb.threads/detach-step-over.exp: \
breakpoint-condition-evaluation=host: target-non-stop=off: non-stop=off: \
displaced=off: iter 1: all threads running
...
In can easily reproduce this by doing:
...
# Wait a bit, to give time for the threads to hit the
# breakpoint.
- sleep 1
return true
...
Fix this by counting the running threads in a loop, effectively allowing 10
seconds (instead of 1) for the threads to start running, but only sleeping if
needed.
Andrew Burgess [Mon, 12 Dec 2022 14:05:22 +0000 (14:05 +0000)]
gdb: fix crash when getting the value of a label symbol
When the source program contains a goto label, it turns out it's
actually pretty hard for a user to find out more about that label.
For example:
(gdb) p some_label
No symbol "some_label" in current context.
(gdb) disassemble some_label
No symbol "some_label" in current context.
(gdb) x/10i some_label
No symbol "some_label" in current context.
(gdb) break some_label
Breakpoint 2 at 0x401135: file /tmp/py-label-symbol-value.c, line 35.
In all cases, some_label is a goto label within the current frame.
Only placing a breakpoint on the label worked.
This all seems a little strange to me, it feels like asking about a
goto label would not be an unreasonable thing for a user to do.
This commit doesn't fix any of the above issues, I mention them just
to provide a little context for why the following issue has probably
not been seen before.
It turns out there is one way a user can access the symbol for a goto
label, through the Python API:
The problem is that label symbols are created using the
builtin_core_addr type, which is a pure integer type.
When GDB tries to fetch the value of a label symbol then we end up in
findvar.c, in the function language_defn::read_var_value, in the
LOC_LABEL case. From here store_typed_address is called to store the
address of the label into a value object with builtin_core_addr type.
The problem is that store_typed_address requires that the destination
type be a pointer or reference, which the builtin_core_addr type is
not.
Now it's not clear what type a goto label address should have, but
GCC has an extension that allows users to take the address of a goto
label (using &&), in that case the result is of type 'void *'.
I propose that when we convert the CORE_ADDR value to a GDB value
object, we use builtin_func_ptr type instead of builtin_core_addr,
this means the result will be of type 'void (*) ()'. The benefit of
this approach is that when gdbarch_address_to_pointer is called the
target type will be correctly identified as a pointer to code, which
should mean any architecture specific adjustments are done correctly.
We can then cast the new value to 'void *' type with a call to
value_cast_pointer, this should not change the values bit
representation, but will just update the type.
After this asking for the value of a label symbol works just fine:
Simon Marchi [Thu, 20 Oct 2022 01:56:47 +0000 (21:56 -0400)]
gdbsupport: add string_xml_appendf
Add a version of buffer_xml_printf (defined in gdbsupport/buffer.{c,h})
that appends to an std::string, rather than a struct buffer. Call it
"string" rather than "buffer" since it operates on an std::string rather
than a buffer. And call it "appendf" rather than "printf", since it
appends to and does not replace the string's content. This mirrors
string_appendf.
Place the new version in gdbsupport/xml-utils.h.
The code is a direct copy of buffer_xml_printf. The old version is
going to disappear at some point, which is why I didn't do any effort to
share code.
Change-Id: I30e030627ab4970fd0b9eba3b7e8cec78fa561ba Approved-By: Pedro Alves <pedro@palves.net>
Jan Vrany [Fri, 16 Dec 2022 11:38:28 +0000 (11:38 +0000)]
gdb/testsuite: add test for Python commands redefining itself
This commit adds a test that creates a Python command that redefines
itself during its execution. This is to test use-after-free in
execute_command ().
This test needs run with ASan enabled in order to fail when it
should.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Luis Machado [Tue, 24 May 2022 22:31:09 +0000 (23:31 +0100)]
[aarch64] Fix removal of non-address bits for PAuth
PR gdb/28947
The address_significant gdbarch setting was introduced as a way to remove
non-address bits from pointers, and it is specified by a constant. This
constant represents the number of address bits in a pointer.
Right now AArch64 is the only architecture that uses it, and 56 was a
correct option so far.
But if we are using Pointer Authentication (PAuth), we might use up to 2 bytes
from the address space to store the required information. We could also have
cases where we're using both PAuth and MTE.
We could adjust the constant to 48 to cover those cases, but this doesn't
cover the case where GDB needs to sign-extend kernel addresses after removal
of the non-address bits.
This has worked so far because bit 55 is used to select between kernel-space
and user-space addresses. But trying to clear a range of bits crossing the
bit 55 boundary requires the hook to be smarter.
The following patch renames the gdbarch hook from significant_addr_bit to
remove_non_address_bits and passes a pointer as opposed to the number of
bits. The hook is now responsible for removing the required non-address bits
and sign-extending the address if needed.
While at it, make GDB and GDBServer share some more code for aarch64 and add a
new arch-specific testcase gdb.arch/aarch64-non-address-bits.exp.