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.
Approved-By: Tom Tromey <tom@tromey.com>
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.
Approved-By: Tom Tromey <tom@tromey.com>
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'.
Approved-By: Tom Tromey <tom@tromey.com>
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.
Tested on aarch64-linux.
PR testsuite/31618
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31618
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"
...
Tested on aarch64-linux.
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".
Tested on aarch64-linux.
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".
Tested on aarch64-linux.
PR testsuite/31592
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31592
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.
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.
Bug: https://sourceware.org/PR31595
The commit ed32754a8c ("[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.
Tested on x86_64-linux.
PR testsuite/31633
https://sourceware.org/bugzilla/show_bug.cgi?id=31633
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.
This also allow us to simplify the python script.
Tested on x86_64-linux.
Also skip the archive if the symbol isn't referenced by a regular object.
bfd/
PR ld/31644
* elflink.c (elf_link_add_archive_symbols): Also skip the archive
if the symbol isn't referenced by a regular object.
ld/
PR ld/31644
* testsuite/ld-plugin/lto.exp: Run PR ld/31644 tests.
* testsuite/ld-plugin/pr31644a.c: New test.
* testsuite/ld-plugin/pr31644b.c: Likewise.
* testsuite/ld-plugin/pr31644c.c: Likewise.
While the patch that Nick reverted in commit 3f6a060c75 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.
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.
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.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31264
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 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.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30846
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.
Co-Authored-By: Tom de Vries <tdevries@suse.de>
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.
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.
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)".
Tested on x86_64-linux.
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>
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.
* gdb/linux-nat.c (_initialize_linux_nat):
* gdb/riscv-tdep.c (riscv_insn):
* gdb/top.c (quit_force):
* gdb/infcmd.c (_initialize_infcmd): Remove excess whitespace.
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)
...
Likewise for DAP.
Tested on x86_64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31631
Fix a memory leak in md_assemble where copy may be cleared and may be
the same as copy:
if (copy && !mnem_suffix)
{
line = copy;
copy = NULL;
no_match:
* config/tc-i386.c (md_assemble): Properly free the xstrdup
memory.
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.
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.
Approved-By: John Baldwin <jhb@FreeBSD.org>
commit e8cd90f0 ("Rewrite gdb_bfd_error_handler") broke the clang
build.
The problem here is that print_error_callback isn't marked as being
printf-like, but it calls string_file::vprintf, triggering:
../../binutils-gdb/gdb/gdb_bfd.c:1202:18: error: format string is not a string literal [-Werror,-Wformat-nonliteral]
This patch applies the attribute to this function.
It also removes the attribute from gdb_bfd_error_handler, because that
function is no longer really printf-like.
The mingw build currently issues a warning:
./../../src/gdb/utils.h:378:56: warning: ignoring attributes on template argument 'void(const char*, va_list)' {aka 'void(const char*, char*)'} [-Wignored-attributes]
This patch fixes the problem as suggested by Simon:
https://sourceware.org/pipermail/gdb-patches/2024-April/207908.html
...that is, by changing the warning interceptor to a class with a
single 'warn' method.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
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>
There was apparently a confusion which cpu model uses
compressed JAL and which ADDIW. Fixed that in execute_c,
case MATCH_C_JAL | MATCH_C_ADDIW.
Fixes 3224e32fb8 ("sim: riscv: Add support for compressed integer instructions")
Approved-By: Andrew Burgess <aburgess@redhat.com>
Various gcc test cases fail due to the stack
alignment of 16 bytes is expected by gcc,
causing issues mostly with vararg functions,
e.g.
FAIL: gcc.c-torture/execute/nest-align-1.c -O0 execution test
FAIL: gcc.c-torture/execute/nest-stdar-1.c -O0 execution test
FAIL: gcc.c-torture/execute/va-arg-12.c -O0 execution test
FAIL: gcc.c-torture/execute/va-arg-15.c -O0 execution test
FAIL: gcc.c-torture/execute/va-arg-16.c -O0 execution test
FAIL: gcc.c-torture/execute/va-arg-17.c -O0 execution test
FAIL: gcc.c-torture/execute/va-arg-20.c -O0 execution test
FAIL: gcc.c-torture/execute/va-arg-26.c -O0 execution test
...
Approved-By: Andrew Burgess <aburgess@redhat.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.
Approved-By: Andrew Burgess <aburgess@redhat.com>
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.
