Since the frame variable is now a frame_info_ptr, the issue
with the dangling frame pointer is apparently no longer there.
So remove the re-fetch code and the corresponding meanwhile
misleading comments.
Approved-By: Tom Tromey <tom@tromey.com>
This commit adds a SECURITY document to GDB. The idea behind this
document is to define what security expectations a user can reasonably
have when using GDB. In addition the document specifies which bugs
GDB developers consider a security bug, and which are just "normal"
bugs.
Discussion for the creation of this initial version can be found here:
https://inbox.sourceware.org/gdb-patches/877cmvui64.fsf@redhat.com/
Like any part of GDB, this is not intended as the absolute final
version, instead this is a living document, and this is just a
reasonable starting point from which we can iterate.
For now I've added this document as a text file but I am considering
merging this document into the manual at a later date, and having the
SECURITY.txt file just say "Read the manual"
Approved-By: Tom Tromey <tom@tromey.com>
This patch fixes a pretty funny issue on sh targets that occurred
because $pc (and similar registers) were typed as int. When $pc is in
the upper half of the address space (i.e. kernel code on sh), `x/i $pc'
would resolve to a negative value. At least in the case of a remote
target with an Xfer memory map, this leads to a spurious "cannot access
memory" error as negative addresses are out of bounds.
(gdb) x/i $pc
0x8c202c04: Cannot access memory at address 0x8c202c04
(gdb) x/i 0x8c202c04
=> 0x8c202c04 <gintctl_gint_gdb+304>: mov.l @r1,r10
The issue is fixed by specifying pointer types for pc and other pointer
registers. Code pointer registers on sh include pc, pr (return address
of a call), vbr (interrupt handler) and spc (return address after
interrupt). Data pointers include r15 (stack pointer) and gbr (base
register for a few specific addressing modes).
Change-Id: I043a058f7cbc6494f380dc0461616a9f3e0d87e0
Approved-By: Simon Marchi <simon.marchi@efficios.com>
The patch:
From f0d556d14b Mon Sep 17 00:00:00 2001
From: Tom de Vries <tdevries@suse.de>
Date: Wed, 17 Apr 2024 12:55:00 +0200
Subject: [PATCH] [gdb/testsuite] Fix end_sequence addresses
I noticed in test-case gdb.reverse/map-to-same-line.exp, that the end of main:
...
00000000004102c4 <end_of_sequence>:
4102c4: 52800000 mov w0, #0x0 // #0
4102c8: 9100c3ff add sp, sp, #0x30
4102cc: d65f03c0 ret
...
is not described by the line table:
...
<snip>
The regression failure on PowerPC is due to the change in file
dw2-lines.exp,
- DW_LNE_set_address bar_label_5
+ DW_LNE_set_address "$main_start + $main_len"
The label bar_label_5 is in function bar, not function main. The new
set address should have been $bar_start + $bar_len.
Add type annotations to ada-unicode.py, just enough to make pyright
happy:
$ pyright --version
pyright 1.1.359
$ pyright ada-unicode.py
0 errors, 0 warnings, 0 informations
Introduce a `Range` class instead of using separate variables and
tuples, to make the code and type annotations a bit cleaner.
When running ada-unicode.py, I get a diff for ada-casefold.h, but I get
the same diff before and after this patch, so that is a separate issue.
Change-Id: I0d8975a57f9fb115703178ae197dc6b6b8b4eb7a
Approved-By: Tom Tromey <tom@tromey.com>
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>
There is no corresponding definition for print_command_line.
There is already a declaration for print_command_lines in
cli/cli-script.h (the implementation is in cli/cli-script.c).
Change-Id: Ic9e67ed04703306d614383ead14e2b2b059b2a8e
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>
Factor the test for libc debug info out of gdb.base/relativedebug.exp to
a new procedure.
Also, change the "info sharedlibrary" test to explicitly detect when
libc has debug info.
Approved-by: Kevin Buettner <kevinb@redhat.com>
The 'PacketSize' attribute of the qSupported packet was
documented to be the maximum size of the packet including
the frame and checksum bytes, however this is not how it
was treated in the code. In reality, PacketSize is the
maximum size of the data in the RSP packets, not including
the framing or checksum bytes.
For instance, GDB's remote.c treats it as the maximum
number of data bytes. See remote_read_bytes_1, where the
size of the request is capped at PacketSize/2 (for
hex-encoding).
Also see gdbserver's server.cc, where the internal buffer
is sized as PBUFSIZ and PBUFSIZ-1 is used as PacketSize.
In gdbserver's case, the buffer is not used for any of the
framing or checksum characters. (I am not certain where the -1
comes from. I think it comes from back when there were no
binary packets, so packets were treated as strings with
null terminators).
It also seems like gdbservers in the wild treat it in
this way:
Embocosm doc:
https://www.embecosm.com/appnotes/ean4/embecosm-howto-rsp-server-ean4-issue-2.html#id3078000
A quick glance over openocd's gdb_server.c gdb_put_packet_inner()
function shows that the internal buffer also excludes the framing
and checksum.
Likewise, qEmu's gdbstub.c allocates PacketSize bytes for
the internal packet contents, and PacketSize+4 for the
full frame.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Pedro Alves <pedro@palves.net>
Consider the following test-case:
...
$ cat hello.c
int main()
{
printf("hello ");
#include "world.inc"
$ cat world.inc
printf("world\n");
return 0;
}
$ gcc -g hello.c
...
The line table for the compilation unit, consisting just of
function main, is translated into these two gdb line tables, one for hello.c
and one for world.inc:
...
compunit_symtab: hello.c
symtab: hello.c
INDEX LINE REL-ADDRESS UNREL-ADDRESS IS-STMT PROLOGUE-END EPILOGUE-BEGIN
0 3 0x400557 0x400557 Y
1 4 0x40055b 0x40055b Y
2 END 0x40056a 0x40056a Y
compunit_symtab: hello.c
symtab: world.inc
INDEX LINE REL-ADDRESS UNREL-ADDRESS IS-STMT PROLOGUE-END EPILOGUE-BEGIN
0 1 0x40056a 0x40056a Y
1 2 0x400574 0x400574 Y
2 3 0x400579 0x400579 Y
3 END 0x40057b 0x40057b Y
...
The epilogue of main starts at 0x400579:
...
400579: 5d pop %rbp
40057a: c3 ret
...
Now, say we have an epilogue_begin marker in the line table at 0x400579.
We won't find it using find_epilogue_using_linetable, because it does:
...
const struct symtab_and_line sal = find_pc_line (start_pc, 0);
...
which gets us the line table for hello.c.
Fix this by using "find_pc_line (end_pc - 1, 0)" instead.
Tested on x86_64-linux.
Co-Authored-By: Tom de Vries <tdevries@suse.de>
PR symtab/31622
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31622
An out of bounds array access in find_epilogue_using_linetable causes random
test failures like these:
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: $fba_value == $fn_fba
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: check frame-id matches
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: bt 2
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: up
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: $sp_value == $::main_sp
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: $fba_value == $::main_fba
FAIL: gdb.base/unwind-on-each-insn-amd64.exp: foo: instruction 6: [string equal $fid $::main_fid]
Here the read happens below the first element of the line
table, and the test failure depends on the value that is
read from there.
It also happens that std::lower_bound returns a pointer exactly at the upper
bound of the line table, also here the read value is undefined, that happens
in this test:
FAIL: gdb.dwarf2/dw2-epilogue-begin.exp: confirm watchpoint doesn't trigger
Fixes: 528b729be1 ("gdb/dwarf2: Add support for DW_LNS_set_epilogue_begin in line-table")
Co-Authored-By: Tom de Vries <tdevries@suse.de>
PR symtab/31268
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31268
With test-case gdb.threads/threadcrash.exp using host board local-remote-host
and target board remote-gdbserver-on-localhost I run into:
...
(gdb) PASS: gdb.threads/threadcrash.exp: test_gcore: continue to crash
gcore $outputs/gdb.threads/threadcrash/threadcrash.gcore^M
Failed to open '$outputs/gdb.threads/threadcrash/threadcrash.gcore' for output.^M
(gdb) FAIL: gdb.threads/threadcrash.exp: test_gcore: saving gcore
UNSUPPORTED: gdb.threads/threadcrash.exp: test_gcore: couldn't generate gcore file
...
The problem is that the gcore command tries to save a file on a remote host,
but the filename is a location on build.
Fix this by using host_standard_output_file.
Tested on x86_64-linux.
After installing glibc debuginfo, I ran into:
...
FAIL: gdb.threads/threadcrash.exp: test_live_inferior: \
$thread_count == [llength $test_list]
...
This happens because the clause:
...
-re "^\r\n${hs}main$hs$eol" {
...
which is intended to match only:
...
#1 <hex> in main () at threadcrash.c:423^M
...
also matches "remaining" in:
...
#1 <hex> in __GI___nanosleep (requested_time=<hex>, remaining=<hex>) at \
nanosleep.c:27^M
...
Fix this by checking for "in main" instead.
Tested on x86_64-linux.
When building on a system where "phony iconv" is used (NetBSD in this
case, not sure why), I get:
CXX charset.o
/home/smarchi/src/binutils-gdb/gdb/charset.c: In function 'size_t phony_iconv(int, const char**, size_t*, char**, size_t*)':
/home/smarchi/src/binutils-gdb/gdb/charset.c:140:8: error: 'extract_unsigned_integer' was not declared in this scope
= extract_unsigned_integer ((const gdb_byte *)*inbuf, 4, endian);
^~~~~~~~~~~~~~~~~~~~~~~~
/home/smarchi/src/binutils-gdb/gdb/charset.c:140:8: note: suggested alternative: 'btrace_insn_number'
= extract_unsigned_integer ((const gdb_byte *)*inbuf, 4, endian);
^~~~~~~~~~~~~~~~~~~~~~~~
btrace_insn_number
Add the necessary include.
Change-Id: I10b967584645961c86167a8395d88929a42bef03
The contents of these files was copied from defs.h and findvar. Copy
over the copyright years (1986-2024).
Change-Id: Idfb0f255fbcfda7e107e9a82804cece3d81ed5fc
Move it out of defs.h, the corresponding definition is in symfile.c.
Change-Id: I984666c3bcd213f8574e9ec91462e1d61f77f16b
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>
Change the return type of the check_quit_flag function to bool. Update
a few related spots.
Change-Id: I9d3a15d3f8651efb02c7d211f06222a592bd4184
Approved-By: Tom Tromey <tom@tromey.com>
Move them out of defs.h, to extension.h, since the implementations are
in extension.c.
Change-Id: Ie7321468bd7fecc684d70b09f72c3ee8ac75d8f4
Approved-By: Tom Tromey <tom@tromey.com>
Remove the gdbcmd.h, which is reported as unused by clangd. Add
cli/cli-cmds.h instead, to get access to `cmdlist` and friends.
Change-Id: Ic0c60d2f6d3618f1bd9fd80b95ffd7c33c692a04
When building with this clang:
$ c++ --version
FreeBSD clang version 16.0.6 (https://github.com/llvm/llvm-project.git llvmorg-16.0.6-0-g7cbf1a259152)
I see:
$ gmake
CXX dwarf2/read.o
/home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:4890:6: error: moving a temporary object prevents copy elision [-Werror,-Wpessimizing-move]
std::move (thread_storage.release_parent_map ()));
^
/home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:4890:6: note: remove std::move call here
std::move (thread_storage.release_parent_map ()));
^~~~~~~~~~~ ~
The compiler seems right, there is not need to std::move the result of
`release_parent_map ()`, it's already going to be an rvalue. Remove the
std::move.
The issue isn't FreeBSD-specific, I see it on Linux as well when
building hwith clang, I just noticed it on a FreeBSD build first.
Change-Id: I7aa20a4db56c799f20d838ad08099a01653bba19
Approved-By: Tom Tromey <tom@tromey.com>
Nothing in defs.h actually uses this. Everything that I (and the
buildbot) can compile still compiles, so I guess that all users of
array_view already include it one way or another. Worst case, if this
causes some build failure, the fix will be one #include away.
Change-Id: I981be98b0653cc18c929d85e9afd8732332efd15
Approved-By: John Baldwin <jhb@FreeBSD.org>
Nothing in defs.h actually uses this.
Add some includes for some spots using things from hashtab.h. Note that
if the GDB build doesn't use libxxhash, hashtab.h is included by
gdbsupport/common-utils.h, so all files still see hashtab.h. It puzzled
me for some time why I didn't see build failures in my build (which
didn't use libxxhash) but the buildbot gave build failures (it uses
libxxhash).
Change-Id: I8efd68decdaf579f048941c7537cd689885caa2a
Approved-By: John Baldwin <jhb@FreeBSD.org>
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>
Move it out of defs.h, adjust the includes here and there.
Change-Id: I11901fdce55d54f5e51723e123cef154cfb1bbc5
Approved-By: John Baldwin <jhb@FreeBSD.org>
Move declarations of initialize_progspace and initialize_inferiors to
progspace.h and inferior.h, respectively.
Change-Id: I62292ffda429861b9f27d8c836a56d161dfa548d
Approved-By: John Baldwin <jhb@FreeBSD.org>
runto uses a hard-coded timeout of 30s in its invocation of gdb_expect.
This is normally fine, but for very a slow system (e.g., an emulator) it
may not be enough time for GDB to reach the intended breakpoint.
gdb_expect can obtain a timeout value from user-configurable variables
when it's not given one explicitly, so use that mechanism instead since
the user will have already adjusted the timeout variable to account for
the slow system.
Approved-By: Tom Tromey <tom@tromey.com>
Fix 'val' -> 'value' typo in c-exp.y which was breaking the build.
Introduced in commit:
commit e6375bc8eb
Date: Wed Apr 17 16:17:33 2024 -0600
Remove some alloca uses
I found a couple of spots where VLAs are in use but where they can
easily be removed.
In one spot, adding 'const' is enough -- and is already done in
similar code elsewhere in the file.
In another spot, one of two arrays will be used, so making the buffer
large enough for both works.
Approved-By: John Baldwin <jhb@FreeBSD.org>
A few spots (mostly in the parsers) use alloca to ensure that a string
is terminated before passing it to a printf-like function (mostly
'error'). However, this isn't needed as the "%.*s" format can be used
instead.
This patch makes this change.
In one spot the alloca is dead code and is simply removed.
Regression tested on x86-64 Fedora 38.
Approved-By: John Baldwin <jhb@FreeBSD.org>
Add the `target_debug_printf` and `target_debug_printf_nofunc` macros
and use them when outputting debug messages depending on `targetdebug`.
I opted for `target_debug_printf_nofunc` to follow the current style
where the function name is already printed, along with the arguments.
Modify the debug printfs in the `debug_target` methods (generated by
`make-target-delegates.py`) to use `target_debug_printf_nofunc` as well.
This makes the "target" debug prints integrate nicely with the other
debug prints that use the "new" debug print system:
[infrun] proceed: enter
[infrun] follow_fork: enter
[target] -> multi-thread->record_will_replay (...)
[target] <- multi-thread->record_will_replay (-1, 0) = false
[target] -> multi-thread->supports_multi_process (...)
[target] <- multi-thread->supports_multi_process () = true
[infrun] follow_fork: exit
...
Change-Id: Ide3c8c1b8a30e6d4c353a29cba911c7192de29ac
Approved-By: Tom Tromey <tom@tromey.com>
Rename the method to `register_debug_string`.
This makes it easier to introduce `target_debug_printf` in a subsequent
patch.
Change-Id: I5bb2d49476d17940d503e66f40762e3f1e3baabc
Approved-By: Tom Tromey <tom@tromey.com>
Change the functions in target-debug.h to return string representations
in an std::string, such that they don't need to know how the printing
part is done. This also helps the following patch that makes the debug
prints in debug_target one-liners.
Update target-delegates.c (through make-target-delegates.py) to do the
printing.
Add an overload of gdb_puts to avoid using `.c_str ()`.
Change-Id: I55cbff1c1b03a3b24a81740e34c6ad41ac4f8453
Approved-By: Tom Tromey <tom@tromey.com>
clangd tells me that the gdb_signals.h include in target/waitstatus.h is
unused. This include was probably to give access to `enum gdb_signal`,
but this is in fact defined in gdb/signals.h. Change the include to
gdb/signals.h. Include gdbsupport/gdb_signals.h in some files that were
relying on the transitive include.
Change-Id: I6f4361b3d801394bf29abe8c1393aff110aa0ad6
Convert all the macros to static functions. Some macros were unused,
and now that they are functions, got flagged by the compiler, so I
omitted them.
No behavior change expected.
Change-Id: Ia88e61d95e29a0378901c71aa50df7c37d16bebe
Approved-By: Tom Tromey <tom@tromey.com>
Editing target-debug.h with clangd shows a bunch of errors. Add some
includes to fix that (make target-debug.h include what it uses).
Change-Id: I49075a171e6875fa516d6b2ce56b4a03ac7b3376
This commit documents the qIsAddressTagged packet.
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Reviewed-by: Eli Zaretskii <eliz@gnu.org>
Approved-By: Eli Zaretskii <eliz@gnu.org>
Add unit tests for testing qIsAddressTagged packet request creation and
reply checks.
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Approved-By: Luis Machado <luis.machado@arm.com>
Tested-By: Luis Machado <luis.machado@arm.com>
This commit adds a new packet, qIsAddressTagged, allowing GDB remote
targets to use it to query the stub if a given address is tagged.
Currently, the memory tagging address check is done via a read query,
where the contents of /proc/<PID>/smaps is read and the flags are
inspected for memory tagging-related flags that indicate the address is
in a memory tagged region.
This is not ideal, for example, for QEMU stub and other cases, such as
on bare-metal, where there is no notion of an OS file like 'smaps.'
Hence, the introduction of qIsAddressTagged packet allows checking
if an address is tagged in an agnostic way.
The is_address_tagged target hook in remote.c attempts to use the
qIsAddressTagged packet first for checking if an address is tagged and
if the stub does not support such a packet (reply is empty) it falls
back to using the current mechanism that reads the contents of
/proc/<PID>/smaps via vFile requests.
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Approved-By: Luis Machado <luis.machado@arm.com>
Tested-By: Luis Machado <luis.machado@arm.com>
This commit introduces a new target hook, target_is_address_tagged,
which is used instead of the gdbarch_tagged_address_p gdbarch hook in
the upper layer (printcmd.c).
This change enables easy specialization of memory tagging address
check per target in the future. As target_is_address_tagged continues
to utilize the gdbarch_tagged_address_p hook, there is no change in
behavior for all the targets that use the new target hook (i.e., the
remote.c, aarch64-linux-nat.c, and corelow.c targets).
Just the gdbarch_tagged_address_p signature is changed for convenience,
since target_is_address_tagged takes the address to be checked as a
CORE_ADDR type.
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Approved-By: Luis Machado <luis.machado@arm.com>
Tested-By: Luis Machado <luis.machado@arm.com>
In do_examine function, use passed gdbarch when checking if an address
is tagged instead of calling current_inferior()->arch() to make the code
more localized and help modularity by not calling a current_* function,
which disguises the use of a global state/variable. There is no change
in the code behavior.
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Suggested-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Approved-By: Luis Machado <luis.machado@arm.com>
Tested-By: Luis Machado <luis.machado@arm.com>
This commit removes aarch64_linux_tagged_address_p from
aarch64_linux_memtag_matches_p. aarch64_linux_tagged_address_p checks if
an address is tagged (MTE) or not.
The check is redundant because aarch64_linux_memtag_matches_p is always
called from the upper layers (i.e. from printcmd.c via gdbarch hook
gdbarch_memtag_matches_p) after either gdbarch_tagged_address_p (that
already points to aarch64_linux_tagged_address_p) has been called or
after should_validate_memtags (that calls gdbarch_tagged_address_p at
the end) has been called, so the address is already checked. Hence:
a) in print_command_1, gdbarch_memtag_matches_p is called only after
should_validate_memtags is called, which checks the address at its end;
b) in memory_tag_check_command, gdbarch_memtag_matches_p is called only
after gdbarch_tagged_address_p is called directly.
Also, because after this change the address checking only happens at the
upper layer it now allows the address checking to be specialized easily
per target, via a target hook.
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Approved-By: Luis Machado <luis.machado@arm.com>
Tested-By: Luis Machado <luis.machado@arm.com>
