Alan Modra [Thu, 18 Apr 2024 23:34:16 +0000 (09:04 +0930)]
mmix disassemble memory leak
It's a once-off and of no consequence, but fix it anyway. The mmix
caonoicalize_syms array is an array of pointers. Freeing it won't
lose symbol names.
Tom de Vries [Thu, 18 Apr 2024 15:02:13 +0000 (17:02 +0200)]
[gdb/testsuite] Use find_gnatmake instead of gdb_find_gnatmake
On SLE-11, with an older dejagnu version, I ran into:
...
Running gdb.ada/mi_prot.exp ...
UNRESOLVED: gdb.ada/mi_prot.exp: \
testcase aborted due to invalid command name: gdb_find_gnatmake
ERROR: tcl error sourcing gdb.ada/mi_prot.exp.
ERROR: invalid command name "gdb_find_gnatmake"
while executing
"::gdb_tcl_unknown gdb_find_gnatmake"
("uplevel" body line 1)
invoked from within
"uplevel 1 ::gdb_tcl_unknown $args"
(procedure "::unknown" line 5)
invoked from within
"gdb_find_gnatmake"
(procedure "gnatmake_version_at_least" line 2)
invoked from within
...
Proc gdb_find_gnatmake is actually a backup for find_gnatmake:
...
if {[info procs find_gnatmake] == ""} {
rename gdb_find_gnatmake find_gnatmake
...
so gnatmake_version_at_least should use find_gnatmake instead.
For a recent dejagnu with find_gnatmake, gdb_find_gnatmake is kept, and we
don't run into this error.
For an older dejagnu without find_gnatmake, gdb_find_gnatmake is renamed to
find_gnatmake, and we do run into the error.
It's confusing that we're using the gdb_ prefix for gdb_find_gnatmake, it
seems something legitimate to use. Maybe we should use future_ or gdb_future_
prefix instead to make this more clear, but I've left that alone for now.
Fix this by:
- triggering the same error with a recent dejagnu by removing
gdb_find_gnatmake unless used (and likewise for other procs in future.exp),
and
- using find_gnatmake in gnatmake_version_at_least.
Tested on x86_64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
PR testsuite/31647
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31647
Tom de Vries [Thu, 18 Apr 2024 15:01:09 +0000 (17:01 +0200)]
[gdb/testsuite] Use allocator_may_return_null=1 in two test-cases
Simon reported [1] that recent commit 06e967dbc9b ("[gdb/python] Throw
MemoryError in inferior.read_memory if malloc fails") introduced
AddressSanitizer allocation-size-too-big errors in the two test-cases
affected by this commit.
Fix this by suppressing the error in the two test-cases using
allocator_may_return_null=1.
Tested on aarch64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
[1] https://sourceware.org/pipermail/gdb-patches/2024-April/208171.html
Will Hawkins [Thu, 18 Apr 2024 11:28:52 +0000 (13:28 +0200)]
Add DW_TAG_compile_unit DIE to Dummy CUs
Dummy CUs help detect errors and are very helpful. However, the DWARF
spec seems to indicate the CUs need a DW_TAG_compile_unit in addition to
the header. This patch adds that.
Alan Modra [Wed, 17 Apr 2024 08:59:19 +0000 (18:29 +0930)]
alpha_vms_get_section_contents vs. fuzzed files
This patch is in response to an oss-fuzz report regarding
use-of-uninitialized-value in bfd_is_section_compressed_info from
section contents provided by alpha_vms_get_section_contents. That
hole is covered by using bfd_zalloc rather than bfd_alloc.
The rest of the patch is mostly a tidy. In a function returning
section contents, I tend to prefer a test on the section properties
over a test on file properties. That's why I've changed the file
flags test to one on section filepos and flags before calling
_bfd_generic_get_section_contents. Also, fuzzed objects can easily
have sections with file backing in relocatable objects, or sections
without file backing in images. Possible confusion is avoided by
testing each section.
Note that we are always going to run into out-of-memory with fuzzed
alpha-vms object files due to sections with contents via ETIR records.
eg. ETIR__C_STO_IMMR stores a number of bytes repeatedly, with a
32-bit repeat count. So section contents can be very large from a
relatively small file. I'm inclined to think that an out-of-memory
error is fine for such files.
* vms-alpha.c (alpha_vms_get_section_contents): Handle sections
with non-zero filepos or without SEC_HAS_CONTENTS via
_bfd_generic_get_section_contents. Zero memory allocated for
sections filled by ETIR records.
Alan Modra [Wed, 17 Apr 2024 08:46:55 +0000 (18:16 +0930)]
Tidy objdump opb expressions
I don't think any of these can overflow, but since all of the
expressions I'm editing here are inside a while loop with condition
addr_offset < stop_offset, this change makes it more obvious that they
can't overflow.
* objdump.c (disassemble_bytes): Calculate octet expressions
involving both addr_offset and stop_offset by first
subtracting addr_offset from stop_offset.
Pedro Alves [Wed, 17 Apr 2024 17:57:01 +0000 (18:57 +0100)]
gdb/Windows: Fix detach while running
While testing a WIP Cygwin GDB that supports non-stop, I noticed that
gdb.threads/attach-non-stop.exp exposes that this:
(gdb) attach PID&
...
(gdb) detach
... hangs.
And it turns out that it hangs in all-stop as well. This commits
fixes that.
After "attach &", the target is set running, we've called
ContinueDebugEvent and the process_thread thread is waiting for
WaitForDebugEvent events. It is the equivalent of "attach; c&".
In windows_nat_target::detach, the first thing we do is
unconditionally call windows_continue (for ContinueDebugEvent), which
blocks in do_synchronously, until the process_thread sees an event out
of WaitForDebugEvent. Unless the inferior happens to run into a
breakpoint, etc., then this hangs indefinitely.
If we've already called ContinueDebugEvent earlier, then we shouldn't
be calling it again in ::detach.
Still in windows_nat_target::detach, we have an interesting issue that
ends up being the bulk of the patch -- only the process_thread thread
can call DebugActiveProcessStop, but if it is blocked in
WaitForDebugEvent, we need to somehow force it to break out of it.
The only way to do that, is to force the inferior to do something that
causes WaitForDebugEvent to return some event.
This patch uses CreateRemoteThread to do it, which results in
WaitForDebugEvent reporting CREATE_THREAD_DEBUG_EVENT. We then
terminate the injected thread before it has a chance to run any
userspace code.
Note that Win32 functions like DebugBreakProcess and
GenerateConsoleCtrlEvent would also inject a new thread in the
inferior. I first used DebugBreakProcess, but that is actually more
complicated to use, because we'd have to be sure to consume the
breakpoint event before detaching, otherwise the inferior would likely
die due a breakpoint exception being raised with no debugger around to
intercept it.
See the new break_out_process_thread method.
So the fix has two parts:
- Keep track of whether we've called ContinueDebugEvent and the
process_thread thread is waiting for events, or whether
WaitForDebugEvent already returned an event.
- In windows_nat_target::detach, if the process_thread thread is
waiting for events, unblock out of its WaitForDebugEvent, before
proceeding with the actual detach.
New test included. Passes cleanly on GNU/Linux native and gdbserver,
and also passes cleanly on Cygwin and MinGW, with the fix. Before the
fix, it would hang and fail with a timeout.
Tested-By: Hannes Domani <ssbssa@yahoo.de> Reviewed-By: Tom Tromey <tom@tromey.com>
Change-Id: Ifb91c58c08af1a9bcbafecedc93dfce001040905
It's been over 9 years (since commit faf09f0119da) since Linux GDB and
GDBserver started relying on SIGTRAP si_code to tell whether a
breakpoint triggered, which is important for non-stop mode. When that
then-new code was added, I had left the then-old code as fallback, in
case some architectured still needed it. Given AFAIK there haven't
been complaints since, this commit finally removes the fallback code,
along with USE_SIGTRAP_SIGINFO.
Tom Tromey [Wed, 17 Apr 2024 13:42:28 +0000 (07:42 -0600)]
Use section name in DWARF error message
A bug points out that a certain error message in read_str_index uses a
hard-coded section name. This patch changes it to use
dwarf2_section_info::get_name instead, like the other errors in the
function.
No test because it didn't seem worthwhile.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31639 Approved-By: Simon Marchi <simon.marchi@efficios.com>
Simon Marchi [Fri, 12 Apr 2024 17:51:54 +0000 (13:51 -0400)]
gdbsupport, gdbserver, gdb: use -Wno-vla-cxx-extension
When building with clang 18, I see:
CXX aarch64-linux-tdep.o
/home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1299:26: error: variable length arrays in C++ are a Clang extension [-Werror,-Wvla-cxx-extension]
1299 | gdb_byte za_zeroed[za_bytes];
| ^~~~~~~~
/home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1299:26: note: read of non-const variable 'za_bytes' is not allowed in a constant expression
/home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1282:10: note: declared here
1282 | size_t za_bytes = std::pow (sve_vl_from_vg (svg), 2);
| ^
Since we are using VLAs right now, that warning doesn't make sense for
us. add `-Wno-vla-cxx-extension` to the list of warning flags we try to
enable. If we ever choose to disallow VLAs, we can remove that flag.
Andrew Burgess [Mon, 15 Apr 2024 13:02:15 +0000 (14:02 +0100)]
gdb/record: minor clean, remove some unneeded arguments
I spotted that the two functions:
record_full_open_1
record_full_core_open_1
both took two arguments, neither of which are used.
I stumbled onto this while reviewing how filename_completer is used.
The 'record full restore' command uses filename_completer and invokes
the cmd_record_full_restore function.
The cmd_record_full_restore function calls core_file_command and then
record_full_open, which then calls one of the above functions.
As 'record full restore' takes a filename, this is passed to
cmd_record_full_restore, which forwards the filename to both
core_file_command and record_full_open. However, record_full_open
never actually uses the filename that is passed in.
The record_full_open function is also used for 'target record-full'.
I propose that record_full_open should no longer expect to see any
user supplied arguments passed in (it doesn't use any). In fact, I've
added a check that if we do get any user supplied arguments we'll
throw an error.
Now that we know record_full_open isn't being passed any user
arguments we can stop passing the arguments to record_full_open_1 and
record_full_core_open_1, this will make no user visible difference as
these arguments were not used.
It is possible that a user was previously doing:
(gdb) target record-full blah blah blah
And this previously would work fine, the 'blah blah blah' was
ignored. Now this will give an error. Other than this case there
should be no user visible changes after this commit.
Andrew Burgess [Mon, 15 Apr 2024 13:10:05 +0000 (14:10 +0100)]
gdb/record: add an assert in cmd_record_start
The 'record' command is both a prefix command AND an alias for 'target
record-full'. As it is a prefix command, if a user types:
(gdb) record blah
Then GDB will look for, and try to invoke the 'blah' sub-command.
This will either succeed (if blah is found) or throw an error (if blah
is not found).
As such, the only way to invoke the 'record' command is like:
(gdb) record
It is impossible to pass arguments to the 'record' command. As we
know this is true then we can assert this in cmd_record_start.
I added this assert because initially I was going to try forwarding
ARGS from cmd_record_start to the 'target record-full' command, but
then I realised passing arguments to 'record' was impossible.
There should be no user visible changes after this commit.
Andrew Burgess [Mon, 15 Apr 2024 12:48:34 +0000 (13:48 +0100)]
gdb/record: remove unnecessary use of filename_completer
Spotted that the 'record' command has its completer set to
filename_completer. The problem is that 'record' is a prefix command,
as such, its completer is hard-coded to complete on sub-commands. The
attempt to use filename_completer is irrelevant.
The 'record' command is itself a command though, that is, a user can
do this:
(gdb) record
which is really just an alias for:
(gdb) target record-full
Nowhere does cmd_record_start (which is called when 'record' is used)
expect a filename, and 'target record-full' doesn't expect a filename
either.
So lets just drop the line which sets filename_completer as the
completer for 'record'.
Tom de Vries [Wed, 17 Apr 2024 10:55:00 +0000 (12:55 +0200)]
[gdb/testsuite] Require DW_LNE_end_sequence
The dwarf standard requires that every line number program sequence ends
with a DW_LNE_end_sequence instruction.
Enforce this in the dwarf assembler for the last sequence in a line number
program (we have no means to enforce this for earlier sequences), and fix a
few test-case that don't have it.
Tom de Vries [Wed, 17 Apr 2024 10:55:00 +0000 (12:55 +0200)]
[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:
...
File name Line number Starting address View Stmt
...
map-to-same-line.c 54 0x4102ac x
map-to-same-line.c - 0x4102c4
...
Fix this by ending the line table at $main_end.
Likewise in a few other test-cases, found using:
...
$ find gdb/testsuite/ -type f \
| xargs grep -B1 DW_LNE_end_sequence \
| grep set_address \
| egrep -v "_end|_len"
...
Tom de Vries [Wed, 17 Apr 2024 10:55:00 +0000 (12:55 +0200)]
[gdb/testsuite] Require address update for DW_LNS_copy
No address update before a DW_LNS_copy might mean an incorrect dwarf
assembly test-case.
Try to catch such incorrect dwarf assembly test-cases by:
- requiring an explicit address update for each DW_LNS_copy, and
- handling the cases where an update is indeed not needed, by adding
"DW_LNS_advance_pc 0".
Tom de Vries [Wed, 17 Apr 2024 10:55:00 +0000 (12:55 +0200)]
[gdb/testsuite] Require address update for DW_LNE_end_sequence
With test-case gdb.dwarf2/dw2-epilogue-begin.exp, we have an end_sequence
entry with the same address as the line entry before it:
...
File name Line number Starting address View Stmt
dw2-epilogue-begin.c 44 0x4101e8 x
dw2-epilogue-begin.c 47 0x4101ec x
dw2-epilogue-begin.c - 0x4101ec
...
and consequently the line entry is removed by gdb:
...
INDEX LINE REL-ADDRESS UNREL-ADDRESS IS-STMT PRO EPI
0 20 0x00000000004101a8 0x00000000004101a8 Y Y Y
1 27 0x00000000004101b0 0x00000000004101b0 Y
2 32 0x00000000004101b8 0x00000000004101b8 Y Y
3 34 0x00000000004101c0 0x00000000004101c0 Y Y
4 35 0x00000000004101c8 0x00000000004101c8 Y
5 40 0x00000000004101d4 0x00000000004101d4 Y Y
6 44 0x00000000004101e8 0x00000000004101e8 Y
7 END 0x00000000004101ec 0x00000000004101ec Y
...
This is a common mistake in dwarf assembly test-cases.
Fix this by:
- requiring an address update for each DW_LNE_end_sequence, and
- fixing the test-cases where that triggers an error.
I also encountered the error in test-case gdb.dwarf2/dw2-bad-elf.exp, and in
this case I worked around it using "DW_LNS_advance_pc 0".
In AIX, if in the main program the global variables are unused then the linker optimises
these variables and the dwarf will not have proper address to the same. Hence we cannot access these
variables.
This patch is a fix to the same so that all the test case of max-depth can passs in AIX as well.
aarch64: Remove asserts from operand qualifier decoders [PR31595]
Given that the disassembler should never abort when decoding
(potentially random) data, assertion statements in the
`get_*reg_qualifier_from_value' function family prove problematic.
Consider the random 32-bit word W, encoded in a data segment and
encountered on execution of `objdump -D <obj_name>'.
If:
(W & ~opcode_mask) == valid instruction
Then before `print_insn_aarch64_word' has a chance to report the
instruction as potentially undefined, an attempt will be made to have
the qualifiers for the instruction's register operands (if any)
decoded. If the relevant bits do not map onto a valid qualifier for
the matched instruction-like word, an abort will be triggered and the
execution of objdump aborted.
As this scenario is perfectly feasible and, in light of the fact that
objdump must successfully decode all sections of a given object file,
it is not appropriate to assert in this family of functions.
Therefore, we add a new pseudo-qualifier `AARCH64_OPND_QLF_ERR' for
handling invalid qualifier-associated values and re-purpose the
assertion conditions in qualifier-retrieving functions to be the
predicate guarding the returning of the calculated qualifier type.
If the predicate fails, we return this new qualifier and allow the
caller to handle the error as appropriate.
As these functions are called either from within
`aarch64_extract_operand' or `do_special_decoding', both of which are
expected to return non-zero values, it suffices that callers return
zero upon encountering `AARCH64_OPND_QLF_ERR'.
Ar present the error presented in the hypothetical scenario has been
encountered in `get_sreg_qualifier_from_value', but the change is made
to the whole family to keep the interface consistent.
Tom de Vries [Wed, 17 Apr 2024 09:45:02 +0000 (11:45 +0200)]
[gdb/testsuite] Fix gdbserver pid in gdb.server/server-kill-python.exp
The commit ed32754a8c7 ("[gdb/testsuite] Fix gdb.server/multi-ui-errors.exp for
remote target") intended to addresss the problem that this command:
...
set gdbserver_pid [exp_pid -i $server_spawn_id]
...
does not return the pid of the gdbserver for remote target, but rather the one
of the ssh client session.
To fix this, it added another way of getting the gdbserver_pid.
For the trivial case of non-remote target, the PID found by either method
should be identical, but if we compare those by adding
"puts [exec ps -p $gdbserver_pid]" we get:
...
PID TTY TIME CMD
31711 pts/8 00:00:00 gdbserver
PID TTY TIME CMD
31718 pts/8 00:00:00 server-kill-pyt
...
The problem is that while the gdbserver PID is supposed to be read from the
result of "gdb.execute ('p server_pid')" in the python script, instead it's
taken from:
...
Process server-kill-python created; pid = 31718^M
...
Fix this by moving the printing of the gdbserver PID out of the python script.
Also double-check the two methods against each other, in the cases that they
should match.
In test-case gdb.server/server-kill-python.exp we have:
...
if {[gdb_spawn_with_cmdline_opts \
"-quiet -iex \"set height 0\" -iex \"set width 0\" -ex \"source $host_file1\""] != 0} {
fail "spawn"
return
}
...
I reproduced the problem by reverting the fix at the commit adding both the
fix and the test-case, and the reproduced the same problem using:
...
(gdb) source $host_file1
...
so there doesn't seem to be a specific need to source the python file using
"-ex".
Simplify the test-case by sourcing the python file using send_gdb.
Alan Modra [Tue, 16 Apr 2024 22:36:05 +0000 (08:06 +0930)]
ARC e_flags vs. objcopy
While the patch that Nick reverted in commit 3f6a060c7543 was in the
source, "FAIL: objcopy executable (pr25662)" was seen on ARC. The
failure was triggered by the .ARC.attributes section being removed by
the linker script. When a file lacking this section is copied by
objcopy, e_flags from the input is copied to the output (in this case
the value 0x406), but arc_elf_final_write_processing then logical-ors
in 0x300 when Tag_ARC_ABI_osver is not found.
* elf32-arc.c (arc_elf_final_write_processing): Don't ignore
existing e_flags for objcopy.
Alan Modra [Mon, 8 Apr 2024 23:23:35 +0000 (08:53 +0930)]
libctf warnings
Seen with every compiler I have if using -fno-inline:
home/alan/src/binutils-gdb/libctf/ctf-create.c: In function ‘ctf_add_encoded’:
/home/alan/src/binutils-gdb/libctf/ctf-create.c:555:3: warning: ‘encoding’ may be used uninitialized [-Wmaybe-uninitialized]
555 | memcpy (dtd->dtd_vlen, &encoding, sizeof (encoding));
Seen with gcc-4.9 and probably others at lower optimisation levels:
home/alan/src/binutils-gdb/libctf/ctf-serialize.c: In function 'symtypetab_density':
/home/alan/src/binutils-gdb/libctf/ctf-serialize.c:211:18: warning: 'sym' may be used uninitialized in this function [-Wmaybe-uninitialized]
if (*max < sym->st_symidx)
Seen with gcc-4.5 and probably others at lower optimisation levels:
/home/alan/src/binutils-gdb/libctf/ctf-types.c:1649:21: warning: 'tp' may be used uninitialized in this function
/home/alan/src/binutils-gdb/libctf/ctf-link.c:765:16: warning: 'parent_i' may be used uninitialized in this function
Also with gcc-4.5:
In file included from /home/alan/src/binutils-gdb/libctf/ctf-endian.h:25:0,
from /home/alan/src/binutils-gdb/libctf/ctf-archive.c:24:
/home/alan/src/binutils-gdb/libctf/swap.h:70:0: warning: "_Static_assert" redefined
/usr/include/sys/cdefs.h:568:0: note: this is the location of the previous definition
* swap.h (_Static_assert): Don't define if already defined.
* ctf-serialize.c (symtypetab_density): Merge two
CTF_SYMTYPETAB_FORCE_INDEXED blocks.
* ctf-create.c (ctf_add_encoded): Avoid "encoding" may be used
uninitialized warning.
* ctf-link.c (ctf_link_deduplicating_open_inputs): Avoid
"parent_i" may be used uninitialized warning.
* ctf-types.c (ctf_type_rvisit): Avoid "tp" may be used
uninitialized warning.
Tom Tromey [Sat, 23 Mar 2024 21:19:20 +0000 (15:19 -0600)]
Avoid cache race in bfd_check_format_matches
Running the gdb test suite with the thread sanitizer enabled shows a
race when bfd_check_format_matches and bfd_cache_close_all are called
simultaneously on different threads.
This patch fixes this race by having bfd_check_format_matches
temporarily remove the BFD from the file descriptor cache -- leaving
it open while format-checking proceeds.
In this setup, the BFD client is responsible for closing the BFD again
on the "checking" thread, should that be desired. gdb does this by
calling bfd_cache_close in the relevant worker thread.
An earlier version of this patch omitted the "possibly_cached" helper
function. However, this ran into crashes in the binutils test suite
involving the archive-checking abort in bfd_cache_lookup_worker. I do
not understand the purpose of this check, so I've simply had the new
function work around it. I couldn't find any comments explaining this
situation, either. I suspect that there may still be races related to
this case, but I don't think I have access to the platforms where gdb
deals with archives.
Tom Tromey [Tue, 13 Feb 2024 01:06:56 +0000 (18:06 -0700)]
Thread-safety improvements for bfd_check_format_matches
A gdb bug found that bfd_check_format_matches has some data races when
called from multiple threads.
In particular, it changes the BFD error handler, which is a global.
It also has a local static variable ("in_check_format") that is used
for recursion detection. And, finally, it may emit warnings to the
per-xvec warning array, which is a global.
This patch removes all the races here.
The first part of patch is to change _bfd_error_handler to directly
handle the needs of bfd_check_format_matches. This way, the error
handler does not need to be changed.
This change lets us use the new per-thread global
(error_handler_messages, replacing error_handler_bfd) to also remove
the need for in_check_format -- a single variable suffices.
Finally, the global per-xvec array is replaced with a new type that
holds the error messages. The outermost such type is stack-allocated
in bfd_check_format_matches.
I tested this using the binutils test suite. I also built gdb with
thread sanitizer and ran the test case that was noted as failing.
Finally, Alan sent me the test file that caused the addition of the
xvec warning code in the first place, and I confirmed that "nm-new"
has the same behavior on this file both before and after this patch.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31264 Co-Authored-By: Alan Modra <amodra@gmail.com>
Tom Tromey [Sat, 13 Jan 2024 01:29:52 +0000 (18:29 -0700)]
Correctly handle DIE parent computations
Tom de Vries pointed out that the combination of sharding,
multi-threading, and per-CU "racing" means that sometimes a cross-CU
DIE reference might not be correctly resolved. However, it's
important to handle this correctly, due to some unfortunate aspects of
DWARF.
This patch implements this by arranging to preserve each worker's DIE
map through the end of index finalization. The extra data is
discarded when finalization is done. This approach also allows the
parent name resolution to be sharded, by integrating it into the
existing entry finalization loop.
In an earlier review, I remarked that addrmap couldn't be used here.
However, I was mistaken. A *mutable* addrmap cannot be used, as those
are based on splay trees and restructure the tree even during lookups
(and thus aren't thread-safe). A fixed addrmap, on the other hand, is
just a vector and is thread-safe.
Tom Tromey [Fri, 12 Jan 2024 03:07:06 +0000 (20:07 -0700)]
Introduce class parent_map for DIE range map
This changes the DIE range map from a raw addrmap to a custom class.
A new type is used to represent the ranges, in an attempt to gain a
little type safety as well.
Note that the new code includes a map-of-maps type. This is not used
yet, but will be used in the next patch.
Tom Tromey [Sun, 14 Jan 2024 18:51:38 +0000 (11:51 -0700)]
Add move operators for addrmap
A subsequent patch needs to move an addrmap. This patch adds the
necessary support. It also changes addrmap_fixed to take a 'const'
addrmap_mutable. This is fine according to the contract of
addrmap_mutable; but it did require a compensating const_cast in the
implementation.
Tom Tromey [Sat, 13 Jan 2024 01:01:00 +0000 (18:01 -0700)]
Change handling of DW_TAG_enumeration_type in DWARF scanner
Currently the DWARF scanner will enter enumeration constants into the
same namespace as the DW_TAG_enumeration_type itself. This is the
right thing to do, but the implementation may result in strange
entries being added to the addrmap that maps DIE ranges to entries.
This came up when debugging an earlier version of this series; and
while I don't think this should impact the current series, it seems
better to clean this up anyway.
In the new code, rather than pass the "wrong" scope down through
recursive calls to the scanner, the correct scope is always passed,
and then the parent handling is done when creating the enumerator
entry.
Tom de Vries [Mon, 11 Dec 2023 14:41:26 +0000 (15:41 +0100)]
[gdb/symtab] Refactor condition in scan_attributes
In scan_attributes there's code:
...
if (new_reader->cu == reader->cu
&& new_info_ptr > watermark_ptr
&& *parent_entry == nullptr)
...
else if (*parent_entry == nullptr)
...
...
that uses the "*parent_entry == nullptr" condition twice.
Make this somewhat more readable by factoring out the condition:
...
if (*parent_entry == nullptr)
{
if (new_reader->cu == reader->cu
&& new_info_ptr > watermark_ptr)
...
else
...
}
...
This also allows us to factor out "form_addr (origin_offset, origin_is_dwz)".
Nick Clifton [Tue, 16 Apr 2024 16:54:13 +0000 (17:54 +0100)]
Fix test for sections with different VMA<->LMA relationships so that it only applies to allocated sections, and only sections in the same segment are checked.
Simon Marchi [Tue, 16 Apr 2024 15:52:24 +0000 (11:52 -0400)]
gdb/make-target-delegates.py: don't handle "void" in parse_argtypes
I suppose this was needed when we had `void` in declarations of methods
with no parameters. If so, we no longer need it. There are no changes
in the generated file.
Change-Id: I0a2b398408aa129634e2d73097a038f7f80db4b4 Approved-By: John Baldwin <jhb@FreeBSD.org>
Eli Zaretskii [Tue, 16 Apr 2024 16:13:39 +0000 (19:13 +0300)]
Remove excess whitespace from doc strings of some commands
I've noticed that doc strings of some commands, like "set cwd"
and "set inferior-tty", have some excess whitespace, which
makes them display with unexpected indentation, at least in a
Windows command prompt window. This patch fixes that.
Tom de Vries [Tue, 16 Apr 2024 13:53:47 +0000 (15:53 +0200)]
[gdb/python] Throw MemoryError in inferior.read_memory if malloc fails
PR python/31631 reports a gdb internal error when doing:
...
(gdb) python gdb.selected_inferior().read_memory (0, 0xffffffffffffffff)
utils.c:709: internal-error: virtual memory exhausted.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
...
Fix this by throwing a python MemoryError, such that we have instead:
...
(gdb) python gdb.selected_inferior().read_memory (0, 0xffffffffffffffff)
Python Exception <class 'MemoryError'>:
Error occurred in Python.
(gdb)
...
Tom Tromey [Mon, 15 Apr 2024 18:52:05 +0000 (12:52 -0600)]
Fix crash in gdb_rl_callback_handler
commit bdcd50f9 ("Strip trailing newlines from input string")
introduced a crash in eof-exit.exp. This patch fixes the problem by
adding a NULL check in the appropriate spot.
Regression tested on x86-64 Fedora 38. I'm checking this in.
Tom Tromey [Wed, 10 Apr 2024 17:48:13 +0000 (11:48 -0600)]
Remove 'copy_names' parameter from add_using_directive
I noticed that add_using_directive's 'copy_names' parameter is only
used by a single caller. This patch removes the parameter and changes
that caller to copy the names itself. I chose to use intern here
since I suspect the names may well be repeated in a given objfile.
When mapping sections to segments ensure that we do not add sections whose VMA->LMA relationship does not match the relationship of earlier sections in the segment.
Tom Tromey [Thu, 4 Apr 2024 14:40:38 +0000 (08:40 -0600)]
Strip trailing newlines from input string
A co-worker noticed a strange situation where "target remote" would
fail due to a trailing newline in the address part of the command.
Eventually he tracked this down to the fact that he was pasting the
command into the terminal, and due to bracketed paste mode, the
newline was being preserved by readline.
It seems to me that we basically never want a trailing newline on a
gdb command, so this patch removes it when handling the readline
result.
Co-Authored-By: Kévin Le Gouguec <legouguec@adacore.com> Approved-By: Luis Machado <luis.machado@arm.com> Tested-By: Luis Machado <luis.machado@arm.com>
The uncompressed EBREAK instruction does not work
correctly this way, and the comment saying that
GDB expects us to step over EBREAK is just wrong.
The PC was always 4 bytes too high, which skips one
instruction at break and step over commands, and
causes complete chaos. The compressed EBREAK was
already implemented correctly.
Tested by using gdb's "target sim" and single-stepping.
Lulu Cai [Thu, 21 Mar 2024 07:16:05 +0000 (15:16 +0800)]
LoongArch: ld:Report an error when seeing an unrecognized relocation
If we generate an object file using an assembler with the new
relocations added, and then linking those files with an older
linker, the link will still complete and the linked file will
be generated.
In this case we should report an error instead of continuing
the linking process.
Pedro Alves [Tue, 2 May 2023 14:04:28 +0000 (15:04 +0100)]
Fix setting watchpoints when current thread is running
Currently, when the current thread is running, you can print global
variables. However, if you try to set a watchpoint on the same
globals, GDB errors out, complaining that the selected thread is
running. Like so:
(gdb) c&
Continuing.
(gdb) p global
$1 = 1098377287
(gdb) watch global
Selected thread is running.
This patch makes setting the watchpoint work. You'll now get:
(gdb) c&
Continuing.
(gdb) [New Thread 0x7ffff7d6e640 (LWP 434993)]
[New Thread 0x7ffff756d640 (LWP 434994)]
p global
$1 = 88168
(gdb) watch global
Hardware watchpoint 2: global
(gdb) [Switching to Thread 0x7ffff7d6e640 (LWP 434993)]
Thread 2 "function0" hit Hardware watchpoint 2: global
Old value = 185420
New value = 185423
int_return () at threads.c:39
39 }
The problem is that update_watchpoint calls get_selected_frame
unconditionally. We can skip it if the watchpoint expression is only
watching globals.
This adds a testcase that exercises both all-stop and non-stop, and
also software and hardware watchpoints. It is kfailed for software
watchpoints, as those require another fix not handled by this patch
(the sw watchpoint doesn't fire because GDB doesn't force the
running-free thread to switch to single-stepping).
Pedro Alves [Mon, 25 Mar 2024 15:17:02 +0000 (15:17 +0000)]
New testcase gdb.threads/leader-exit-attach.exp (PR threads/8153)
Add a new testcase for exercising attaching to a process after its
main thread has exited.
This is not possible on Linux, the kernel does not allow attaching to
a zombie task, so the test is kfailed there. It is possible however
on Windows at least, and was the scenario addressed by the Windows
backend fix in
https://sourceware.org/legacy-ml/gdb-patches/2003-12/msg00479.html,
nowadays PR threads/8153, back in 2003.
Passes cleanly on Cygwin.
KFAILed on GNU/Linux native and gdbserver.
Pedro Alves [Tue, 5 Mar 2024 16:03:15 +0000 (16:03 +0000)]
Cygwin/testsuite: Avoid infinite hang
On Cygwin, the gdb.base/fork-no-detach-follow-child-dlopen.exp
testcase hits a sequence of cascading FAILs:
(gdb) run
Starting program: ..../gdb.base/fork-no-detach-follow-child-dlopen/fork-no-detach-follow-child-dlopen
[New Thread 12672.0x318c]
[New Thread 12672.0x2844]
[New Thread 12672.0x714]
FAIL: gdb.base/fork-no-detach-follow-child-dlopen.exp: runto: run to add (timeout)
frame
FAIL: gdb.base/fork-no-detach-follow-child-dlopen.exp: frame (timeout)
list
FAIL: gdb.base/fork-no-detach-follow-child-dlopen.exp: list (timeout)
And the test program never makes progress.
... and at this point, Cygwin is completely stuck. I can't run any
other Cygwin program.
However, if we run the test program outside DejaGnu, we see something
different:
(gdb) b add
Function "add" not defined.
Make breakpoint pending on future shared library load? (y or [n]) y
Breakpoint 1 (add) pending.
(gdb) r
Starting program: ..../gdb.base/fork-no-detach-follow-child-dlopen/fork-no-detach-follow-child-dlopen
[New Thread 10968.0x834]
[New Thread 10968.0x29a4]
[New Thread 10968.0x16b8]
[New Thread 10968.0xf9c]
[Switching to Thread 10968.0x16b8]
Thread 4 "sig" hit Breakpoint 1.2, pending_signals::add (pack=..., this=0x7ffa1e748a40 <sigq>) at /usr/src/debug/cygwin-3.4.9-1/winsup/cygwin/sigproc.cc:1304
1304 se = sigs + pack.si.si_signo;
(gdb)
Ah, the test wanted to run to a global "add" function, but managed to
stop at an internal Cygwin method called "add". And stopping there
deadlocks everything Cygwin in the system. (I believe some
cygwin1.dll mechanisms use cross-process synchronization or
communication, we're probably blocking something like that.)
Fix this by using "break -q". The tests FAIL because we don't support
follow-fork for Cygwin, but at least we no longer deadlock the
machine.
Approved-by: Kevin Buettner <kevinb@redhat.com>
Change-Id: I7181d8481c2ae1024b0d73e3bb194f9a4f0a7eb9
Andrew Burgess [Mon, 8 Apr 2024 11:34:39 +0000 (12:34 +0100)]
gdb/data-directory: silence output from mkinstalldirs script
After my recent changes the data-directory build now uses
silent-rules.mk to reduce the output.
One problem that remains was the use of mkinstalldirs by stamp-python
and stamp-guile for creating some directories, the mkinstalldirs
prints some messages, so we're left with output like this:
I was looking at adding a --silent option to the mkinstalldirs script,
however, when I took a look at the automake package (which is where
mkinstalldirs comes from) it turns out that mkinstalldirs is
deprecated, at the advice is to use 'install-sh -d' instead.
Just like we carry mkinstalldirs in the top-level directory, we also
carry install-sh, and a version of install-sh which supports the -d
flag.
And best of all, 'install-sh -d' doesn't appear to print any of the
information messages to stdout that mkinstalldirs does, so if we
switch to use that, we get a quieter build.
There should be no changes in what is built after this commit
Tom de Vries [Thu, 11 Apr 2024 11:43:52 +0000 (13:43 +0200)]
[gdb/testsuite] Fix gdb.threads/access-mem-running-thread-exit.exp with clang
When running test-case gdb.threads/access-mem-running-thread-exit.exp with
clang, we run into:
...
(gdb) print global_var = 555^M
No symbol "global_var" in current context.^M
(gdb) FAIL: gdb.threads/access-mem-running-thread-exit.exp: all-stop: \
access mem (write to global_var, inf=2, iter=1)
...
The problem is that clang removes the unused variable.
Fix this in the same way as done in commit b4f767131f7
("Fix gdb.base/align-*.exp and Clang + LTO and AIX GCC"), by incrementing the
variable.
H.J. Lu [Tue, 9 Apr 2024 23:04:16 +0000 (16:04 -0700)]
gas: Fix a CFI label name memory leak in scfi.c
CFI label name can be freed only after use.
* scfi.c (handle_scfi_dot_cfi): Free CFI label name after use.
* scfidw2gen.c (scfi_process_cfi_label): Add a comment. Remove
TODO on freeing CFI label name.
Alan Modra [Thu, 11 Apr 2024 02:26:50 +0000 (11:56 +0930)]
Remove bfdwin.c
In commit b86d3af60ffc and 0ab0435fe672 I fixed SIGBUS errors found by
oss-fuzz now that --with-mmap defaults to enabled. It turns out there
are further problems with the aout mmap code: aout_read_minisymbols
returns the external symbol array, which is later freed by nm.c. If
the array is mmaped you can't free it. Now this could be fixed by
making aout minisymbols an array of pointers, but I figure there's not
much point in expending effort on that. So delete the aout mmap
support along with bfdwin.c and get_section_contents_in_window.
Seen on mmix.
mmix +FAIL: ld-misc/defsym1
mmix +FAIL: sysroot-prefix common plain -Lpath, quoted
mmix +FAIL: sysroot-prefix common plain -Lpath, unquoted
mmix +FAIL: sysroot-prefix common full-path, quoted
mmix +FAIL: sysroot-prefix common full-path, unquoted
mmix +FAIL: sysroot-prefix common plain =-prefixed with empty, quoted
mmix +FAIL: sysroot-prefix common plain =-prefixed with empty, unquoted
mmix +FAIL: sysroot-prefix common plain $SYSROOT-prefixed with empty, quoted
mmix +FAIL: sysroot-prefix common plain $SYSROOT-prefixed with empty, unquoted
mmix +FAIL: sysroot-prefix common plain =-prefixed -Lpath, quoted
mmix +FAIL: sysroot-prefix common plain =-prefixed -Lpath, unquoted
mmix +FAIL: sysroot-prefix common plain $SYSROOT-prefixed -Lpath, quoted
mmix +FAIL: sysroot-prefix common plain $SYSROOT-prefixed -Lpath, unquoted
mmix +FAIL: sysroot-prefix common full-path =-prefixed without, quoted
mmix +FAIL: sysroot-prefix common full-path =-prefixed without, unquoted
mmix +FAIL: sysroot-prefix common full-path $SYSROOT-prefixed without, quoted
mmix +FAIL: sysroot-prefix common full-path $SYSROOT-prefixed without, unquoted
==3746597==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6070000007a0 at pc 0x56d87b0d1a40 bp 0x7fffb1629bf0 sp 0x7fffb1629be0
READ of size 8 at 0x6070000007a0 thread T0
#0 0x56d87b0d1a3f in elf_link_add_to_first_hash /home/alan/src/binutils-gdb/bfd/elflink.c:4312
mmix uses bfd_link_generic_hash_table.
* elflink.c (_bfd_elf_archive_symbol_lookup): Dont use first_hash
unless the hash table is bfd_link_elf_hash_table.
(elf_link_add_archive_symbols): Likewise.
H.J. Lu [Mon, 1 Apr 2024 17:03:11 +0000 (10:03 -0700)]
x86-64: Use long NOPs for Intel Core processors
Use long NOPs for Intel Core processors since they are faster than
multiple NOPs. Don't use them for 64-bit processors by default since
Intel Atom processors can only decode 4 prefixes in 1 cycle.
* config/tc-i386.c (alt64_9): New.
(alt64_10): Likewise.
(alt64_11): Likewise.
(alt64_12): Likewise.
(alt64_13): Likewise.
(alt64_14): Likewise.
(alt64_15): Likewise.
(alt64_patt): Likewise.
(i386_generate_nops): Use alt64_patt for Intel Core processors
in 64-bit mode.
* testsuite/gas/i386/x86-64-nops-1-core2.d: Expect long NOPs.
* testsuite/gas/i386/x86-64-nops-4-core2.d: Likewise.
* testsuite/gas/i386/ilp32/x86-64-nops-1-core2.d: Replace
../x86-64-nops-1.d with ../x86-64-nops-1-core2.d.
* testsuite/gas/i386/ilp32/x86-64-nops-4-core2.d: Replace
../x86-64-nops-4.d with ../x86-64-nops-4-core2.d.
H.J. Lu [Wed, 10 Apr 2024 01:41:59 +0000 (18:41 -0700)]
elf: Fix a memory leak in _bfd_elf_add_dynamic_entry
Normally, the section contents is allocated by bfd_alloc which is freed
when the object is closed. But the .dynamic section contents is allocated
by bfd_realloc, which should be freed by calling free. Add a dynamic
field to elf_link_hash_table for the .dynamic section and free its
contents in _bfd_elf_link_hash_table_free.
* elf-bfd.h (elf_link_hash_table): Add dynamic.
* elflink.c (_bfd_elf_link_create_dynamic_sections): Set the
dynamic field in elf_link_hash_table.
(_bfd_elf_add_dynamic_entry): Use hash_table->dynamic.
(_bfd_elf_strip_zero_sized_dynamic_sections): Likewise.
(bfd_elf_add_dt_needed_tag): Likewise.
(elf_finalize_dynstr): Likewise.
(_bfd_elf_link_hash_table_free): Free htab->dynamic->contents.
(bfd_elf_final_link): Use htab->dynamic.
* elfxx-x86.c (_bfd_x86_elf_finish_dynamic_sections): Use
htab->elf.dynamic.
There are two state propagation functions in SCFI machinery - forward
and backward flow. The patch addresses two issues:
- In forward_flow_scfi_state (), the state being compared in forward flow
must be that at the exit of a prev bb and that at the entry of the
next bb. The variable holding the state to be compared was
previously (erroneously) stale.
- In cmp_scfi_state (), the assumption that two different control
flows, leading to the same basic block, cannot have a mismatched
notion of CFA base register, is not true. Remove the assertion and
instead return err if mismatch.
Fixing these issues helps correctly synthesize CFI, when previously
SCFI was erroring out for an otherwise valid input asm.
gas/
* scfi.c (cmp_scfi_state): Remove assertion and return mismatch
in return value as applicable.
(forward_flow_scfi_state): Update state object to be the same as
the exit state of the prev bb before comparing.
gas/testsuite/
* gas/scfi/x86_64/scfi-x86-64.exp: Add new test.
* gas/scfi/x86_64/scfi-cfg-5.d: New test.
* gas/scfi/x86_64/scfi-cfg-5.l: New test.
* gas/scfi/x86_64/scfi-cfg-5.s: New test.
A GCFG (ginsn control flow graph) is created for SCFI purposes in GAS.
The existing GCFG creation process was ignoring some paths.
add_bb_at_ginsn () is a recursive function which should return the root
of the added basic blocks. This property was being violated in some
traversals, e.g., where a taken path involving a sequence of a few basic
blocks eventually culminated in a GINSN_TYPE_RETURN instruction. This
patch fixes the issue by keeping an explicit variable root_bb to
memorize the bb to be returned.
Next, find_or_make_bb () must either create or find the bb with the
first ginsn as the provided ginsn. Add a few assertions to ensure
health of the cfg creation process.
Note that the testcase, in its current shape, is not fit for catching
regressions for the issue at hand. Although the testcase does exercise
the updated code path, the testcase passes even without the current fix,
because the added edge in this specific testcase does not alter the
synthesized CFI. (The missing edge is the fallthrough edge of the
conditional branch "jne .L13" in the testcase.)
Using a manual gcfg_print (), one can see the missing edge without the
fix. Lets keep the testcase for now, until there is a better way to
test the GCFG for this issue (e.g., either by dumping the GCFG in
textual format, or a case when the missing edge does cause wrong
synthesized CFI).
gas/
* ginsn.c (bb_add_edge): Fix a code comment.
(find_bb): Likewise.
(find_or_make_bb): Add new assertions to ensure health of cfg
creation process.
(add_bb_at_ginsn): Keep reference to the root_bb and return it.
gas/testsuite/
* gas/scfi/x86_64/scfi-x86-64.exp: Add new test.
* gas/scfi/x86_64/scfi-cfg-4.d: New test.
* gas/scfi/x86_64/scfi-cfg-4.l: New test.
* gas/scfi/x86_64/scfi-cfg-4.s: New test.
Direct leak of 4096 byte(s) in 1 object(s) allocated from:
#0 0x7cdd3d0defdf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
#1 0x5750c7f6d72b in main /home/alan/build/gas-san/all/bfd/conftest.c:239
Direct leak of 4096 byte(s) in 1 object(s) allocated from:
#0 0x7cdd3d0defdf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
#1 0x5750c7f6d2e1 in main /home/alan/build/gas-san/all/bfd/conftest.c:190
SUMMARY: AddressSanitizer: 8192 byte(s) leaked in 2 allocation(s).
Replace AC_FUNC_MMAP with GCC_AC_FUNC_MMAP to avoid the sanitizer
configure check failure.
Direct leak of 4096 byte(s) in 1 object(s) allocated from:
#0 0x7cdd3d0defdf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
#1 0x5750c7f6d72b in main /home/alan/build/gas-san/all/bfd/conftest.c:239
Direct leak of 4096 byte(s) in 1 object(s) allocated from:
#0 0x7cdd3d0defdf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
#1 0x5750c7f6d2e1 in main /home/alan/build/gas-san/all/bfd/conftest.c:190
SUMMARY: AddressSanitizer: 8192 byte(s) leaked in 2 allocation(s).
Define GCC_AC_FUNC_MMAP with export ASAN_OPTIONS=detect_leaks=0 to avoid
the sanitizer configure check failure.
config/
* mmap.m4 (GCC_AC_FUNC_MMAP): New.
* no-executables.m4 (AC_FUNC_MMAP): Renamed to GCC_AC_FUNC_MMAP.
Change AC_FUNC_MMAP to GCC_AC_FUNC_MMAP.
Alan Modra [Tue, 9 Apr 2024 23:57:44 +0000 (09:27 +0930)]
Re: ld testsuite: Append NOSANITIZE_CFLAGS to CFLAGS_FOR_TARGET
Don't use CC_FOR_TARGET in the bootstrap test, a silly idea aiming at
consistency that made things worse. The objects being linked were
built using $CC, so $CC should be used to link.
* testsuite/ld-bootstrap/bootstrap.exp: Revert last change.
projects / thirdparty / binutils-gdb.git / log
