Alan Modra [Sun, 23 Feb 2025 10:35:00 +0000 (21:05 +1030)]
gas: avoid dangling pointers into freed memory
The oss-fuzz gas fuzzer is quite broken in that it doesn't
reinitialise all gas and bfd static variables between runs. Since gas
naughtily modifies bfd_und_section and bfd_abs_section those bfd
statics can hold pointers into freed memory between runs.
This patch fixes oss-fuzz issue 398060144.
Maximilian Ciric [Sat, 22 Feb 2025 20:57:15 +0000 (20:57 +0000)]
MIPS objdump: Recognize o64 ABI names
Add gpr and fpr names for the o64 ABI to objdump.
With the recent addition of both EABIs, this completes support for the
standard ABI options (ABI-breaking options such as -modd-spreg or
-mabi=32 -mfp64 notwithstanding). The names have been verified against
GCC's usage of the registers. Notably, the only(?) documentation that
defines the o64 ABI at
https://gcc.gnu.org/projects/mipso64-abi.html
appears to contain a mistake w.r.t. floating-point arguments. In
particular:
> If the first and second arguments floating-point arguments to a
> function are 32-bit values, they are passed in $f12 and $f14.
As from 4.0.0 this does not happen in GCC's implementation of the ABI;
a pair of single-float arguments are still passed in $f12 and $f13, the
same as when one or both of the arguments are double-precision floats.
The registers $f12, $f13 and $f14 have been named $fa0, $fa1 and $ft10
to match the implementation.
Signed-off-by: Maximilian Ciric <max.ciric@gmail.com>
Shahab Vahedi [Wed, 12 Feb 2025 14:58:15 +0000 (15:58 +0100)]
gdb/testsuite/rocm.exp: Use system GPU(s) to detect features
gdb/testsuite/rocm.exp: Use system GPU(s) to detect features
Background
----------
This patch revisits the purpose of hcc_amdgpu_targets{} in
order to address the separation of concerns between:
- GPU targets passed to the compiler. This kind of target
is passed as an argument to flags like "--offload-arch=...",
"--targets=...", etc.
- GPU targets as in available GPU devices on the system. This
is crucial for finding which capabilities are available,
and therefore which tests should be executed or skipped.
Code change
-----------
- A new "find_amdgpu_devices{}" procedure is added. It is
responsible for listing the GPU devices that are available
on the system.
- "hcc_amdgpu_targets{}" is rewritten to use the newly added
"find_amdgpu_devices{}" when there's no environment variable
(HCC_AMDGPU_TARGET) set.
- The output of "hcc_amdgpu_targets{}" is now only used in
places that set the target for the building toolchains.
- The output of "find_amdgpu_devices{}" is used anywhere that
needs to evaluate the GPU features.
Approved-By: Lancelot Six <lancelot.six@amd.com> (amdgpu)
Change-Id: Ib11021dbe674aa40192737ede78284a1bc531513
Jan Beulich [Fri, 21 Feb 2025 09:28:46 +0000 (10:28 +0100)]
IQ2000: drop maintainer
After I found his email bouncing, Stan, via private communication which
Nick helped with, has indicated that - having retired - he won't any
longer fulfill the maintainer role here.
Jan Beulich [Fri, 21 Feb 2025 09:28:24 +0000 (10:28 +0100)]
x86: GOT is an ELF-only entity
Make md_undefined_symbol() conditional upon dealing with ELF, much like
other architectures (e.g. Arm32 and Arm64) have it. This avoids errors
in gas and even assertions in libbfd when "accidentally" e.g. a COFF-
targeting source file uses "_GLOBAL_OFFSET_TABLE_" for whatever reason.
While there also convert the final return statement to properly use
NULL.
NB: In principle 64-bit Mach-O knows GOT, too. Yet only an i?86-macho
assembler can be built right now, as per configure.tgt. Pretty clearly
adjustments to gotrel[] would also be necessary before these targets
could actually work reasonably cleanly.
Jan Beulich [Fri, 21 Feb 2025 09:26:59 +0000 (10:26 +0100)]
ix86: restrict use of GOT32X relocs
The ELF linker rejects use of this reloc type without a base register
for PIC code. Suppress its use by gas in such cases.
To keep things building for non-ELF, include the entire containing if()
in an #ifdef: All consumers of ->fx_tcbit* live in such conditionals as
well, hence there's no reason to keep the producer active.
Jan Beulich [Fri, 21 Feb 2025 09:24:50 +0000 (10:24 +0100)]
x86: widen @got{,pcrel} support to PUSH and APX IMUL
With us doing the transformation to an immediate operand for MOV and
various ALU insns, there's little reason to then not support the same
conversion for the other two insns which have respective immediate
operand forms. Unfortunately for IMUL (due to the 0F opcode prefix)
there's no suitable relocation, so the pre-APX forms cannot be marked
for relaxation in the assembler.
Jan Beulich [Fri, 21 Feb 2025 09:22:50 +0000 (10:22 +0100)]
ix86: tighten convert-load-reloc checking
Just like was done recently for x86-64 (commit 4998f9ea9d35): Even if
the assembler avoids using the relaxable relocation for inapplicable
insns, the relocation type can still appear for other reasons. Be more
thorough in the opcode checking we do, to avoid bogusly altering other
insns.
Furthermore correct an opcode mask (even if with the added condition
that's now fully benign).
Simon Marchi [Thu, 20 Feb 2025 16:38:04 +0000 (11:38 -0500)]
gdb/doc: fix sentence in save gdb-index` command doc
The part "... this command by default creates it produces a single ..."
sounds wrong. Replace with "... this command by default produces a
single ...".
Simon Marchi [Thu, 20 Feb 2025 15:13:38 +0000 (10:13 -0500)]
gdb/compile: add missing entry in bfd_link_callbacks array
clang 19 fails to build gdb with this error:
/home/simark/src/binutils-gdb/gdb/compile/compile-object-load.c:302:3: error: cannot initialize a member subobject of type 'void (*)(const char *, ...) __attribute__((noreturn))' with an lvalue of type 'void (const char *, ...)'
302 | link_callbacks_einfo, /* einfo */
| ^~~~~~~~~~~~~~~~~~~~
This illustrates that the bfd_link_callbacks array is missing an entry
for the "fatal" callback, add it.
The fatal field was added very recently, in d26161914 ("PR 32603, more
ld -w misbehaviour"). We're lucky that the new callback was marked with
the noreturn attribute and that clang checks that, otherwise this would
have gone unnoticed.
Tom Tromey [Thu, 5 Sep 2024 18:09:43 +0000 (12:09 -0600)]
Handle optional lines correctly in gdb.ada/complete.exp
While working on another series, I discovered that the existing code
in gdb.ada/complete.exp that conditionally accepts a completion does
not work correctly. The code assumes that wrapping a line in "(...)?"
will make the entire line optional, but really this will only match a
blank line.
Meanwhile, I needed this same patch for a second series I'm working
on, so I've pulled this out. As it only affects Ada, I am going to
check it in.
Tom Tromey [Wed, 19 Feb 2025 15:55:37 +0000 (08:55 -0700)]
Small get_tib_address cleanups
I noticed a non-bool-like use of target_get_tib_address in
windows-tdep.c. After fixing this I thought it would be good to
document the target method; and this also lead to some non-bool-like
commentary in remote.c. This patch fixes all of these nits.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Guinevere Larsen [Mon, 17 Feb 2025 19:43:04 +0000 (16:43 -0300)]
GDB: add stabs deprecation warning
Now that stabs is deprecated, we should probably warn our users of it
before removing support, so that they have time to react and either make
themselves heard, or fix things on their end so that they can still debug
their applications.
This commit adds a new function that emits a warning whenever GDB does
stabs reading. Since there are several places where stabs is
re-invented, this warning had to be added to many places, but I think I
managed to warn everywhere relevant without duplicating warnings.
Also, the test gdb.stabs/weird.exp explicitly checks for GDB warnings
when reading stabs, so it had to be updated to account for the
deprecation warning. It is done generically, since it will be removed in
the next release anyway.
Alan Modra [Thu, 20 Feb 2025 01:19:25 +0000 (11:49 +1030)]
PR 32721, internal error in tc-i386.c:parse_register
pr30117 showed one of the assertions added by 4d1bb7955a8b was too
strict. oss-fuzz also found the second assertion to be too strict,
with this testcase distilled from 7k of garbage source:
A=%eax%%!
Y=A
Z=A
or $6,Z
PR 32721
* config/tc-i386.c (parse_register): Move "know" into
condition. Simplify.
Tom Tromey [Sat, 8 Feb 2025 02:14:35 +0000 (19:14 -0700)]
Hoist language-finding in expand_symtabs_matching
Right now, cooked_index_functions::expand_symtabs_matching computes
the language for each component of a split name, using the language of
the corresponding entry.
Instead, I think that we want to do all the comparisons using the
final entry's language. I don't think there's a way to trigger bad
behavior here right now, but with another series I'm working on, we
end up with some entries whose language can't reliably be determined;
and in this case using the final entry's language avoids issues.
I suspect we could also dispense with the per-segment name-matcher
lookup as well.
Tom Tromey [Fri, 31 Jan 2025 18:25:45 +0000 (11:25 -0700)]
Move producer checks to dwarf2_cu
This changes the various producer-checking functions to be methods on
dwarf2_cu. It adds a few new caching members as well -- every one
that could reasonably be done this way has been converted, with the
only exception being a gdbarch hook.
Note the new asserts in the accessors. Without the earlier
prepare_one_comp_unit change, these could trigger in some modes.
Tom Tromey [Fri, 31 Jan 2025 19:25:05 +0000 (12:25 -0700)]
Clean up calls to prepare_one_comp_unit
Currently, prepare_one_comp_unit is called somewhat haphazardly: it is
mostly called when a CU is read, but some places manage to instantiate
a cutu_reader* without calling it, and some code (e.g.,
read_file_scope) calls it without really needing to.
Aside from contributing to the general confusion around CU reading,
this doesn't really cause problems in the current tree. However, it
is possible for the DWARF reader to check the CU's producer before it
is ever set -- which is certainly unintended.
Tom Tromey [Sat, 8 Feb 2025 19:43:21 +0000 (12:43 -0700)]
Clean up DW_TAG_namelist handling in new_symbol
In dwarf2/read.c:new_symbol, DW_TAG_namelist is listed in the same
part of the "switch" as other tags. However, it effectively shares no
code with these. This patch splits it into its own case.
Longer term I think new_symbol should be split up drastically.
Before this patch, current_token was a global implicitly 0-initialized. Since
it is now a class field, it is not 0-initialized by default anymore. This
patch changes this.
Change-Id: I3f00b080318a70405d881ff0abe02b2c5cb1f9d8 Approved-By: Simon Marchi <simon.marchi@efficios.com> Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Mon, 17 Feb 2025 20:54:09 +0000 (15:54 -0500)]
gdb/dwarf: add logging for CU expansion
I was trying to get an understanding of which CUs were expanded when,
and how much time it was taking. I wrote this patch to add some logging
related to that, and I think it would be useful to have upstream, to
better understand performance problems related to over-eager CU
expansion, for example.
- add DWARF_READ_SCOPED_DEBUG_START_END
- use it in process_queue, to wrap the related expansion messages
together
- add a message in maybe_queue_comp_unit when enqueuing a comp unit
- add timing information to messages in process_queue, indicating how
much time it took to expand a given symtab
- count the number of expansions done in a single call to process_queue
[dwarf-read] process_queue: start: Expanding one or more symtabs of objfile /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/dw-form-ref-addr-with-type-units/dw-form-ref-addr-with-type-units ...
[dwarf-read] process_queue: Expanding symtab of CU at offset 0xc
[dwarf-read] maybe_queue_comp_unit: Queuing CU for expansion: section offset = 0x38b, queue size = 2
[dwarf-read] process_queue: Done expanding CU at offset 0xc, took 0.001s
[dwarf-read] process_queue: Expanding symtab of CU at offset 0x38b
[dwarf-read] process_queue: Done expanding CU at offset 0x38b, took 0.000s
[dwarf-read] process_queue: Done expanding 2 symtabs.
[dwarf-read] process_queue: end: Expanding one or more symtabs of objfile /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/dw-form-ref-addr-with-type-units/dw-form-ref-addr-with-type-units ...
Change-Id: I5237d50e0c1d06be33ea83a9120b5fe1cf7ab8c2 Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Mon, 17 Feb 2025 19:59:35 +0000 (14:59 -0500)]
gdb/dwarf: set is_debug_types in signatured_type constructor
This makes it more obvious that all created signatured_type objects have
this flag set.
Also, remove an unnecessary assignment in create_cus_hash_table: when
constructing the dwarf2_per_cu_data object, is_debug_types is already
initialized to 0/false.
Change-Id: I6d28b17ac77edc040172254f6970d05ebc4a47f4 Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Mon, 17 Feb 2025 19:59:32 +0000 (14:59 -0500)]
gdb/dwarf: pass dwarf2_per_bfd to dwarf2_per_cu_data constructor
Pass a dwarf2_per_bfd to the constructor of dwarf2_per_cu_data and set
the per_bfd field there. All "real" instantiations of
dwarf2_per_cu_data must have a valid, non-nullptr dwarf2_per_bfd
backlink, this makes it a bit more obvious. The instantiations of
dwarf2_per_cu_data that receive a nullptr dwarf2_per_bfd are the ones
used to do hash map lookups and the ones used in selftests.
Remove an unnecessary assignment of per_bfd in
fill_in_sig_entry_from_dwo_entry: the per_bfd field is already set when
the signatured_type object is constructor (before that, it was set in
allocate_signatured_type).
Change-Id: Ifeebe55fdb1bc2de4de9c852033fafe8abdfde8a Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Mon, 17 Feb 2025 19:59:31 +0000 (14:59 -0500)]
gdb/dwarf: change some functions from "per objfile" to "per bfd"
I noticed that the following functions accept a "dwarf2_per_objfile",
but they can actually accept a less specific "dwarf2_per_bfd". This
makes it more obvious that the work they do is per BFD and not per
objfile.
Qwinci [Tue, 18 Feb 2025 18:47:51 +0000 (20:47 +0200)]
gdb/remote: don't error if qGetTIBAddr is unsupported
This change makes it possible to debug PE executables run in e.g. Qemu
without needing to set osabi to none, it breaks backtrace
and commands like finish if frame pointers are not present but SEH unwind info is.
Hui Li [Tue, 11 Feb 2025 12:18:28 +0000 (20:18 +0800)]
gdb: LoongArch: Extend the maximum number of hardware watchpoints
The maximum number of load/store watchpoints and fetch instruction
watchpoints is 14 each according to LoongArch Reference Manual [1],
so extend the maximum number of hardware watchpoints from 8 to 14.
A new struct user_watch_state_v2 was added into uapi in the related
kernel commit 531936dee53e ("LoongArch: Extend the maximum number of
watchpoints") [2], but there may be no struct user_watch_state_v2 in
the system header in time. Modify the struct loongarch_user_watch_state
in GDB which is same with the uapi struct user_watch_state_v2.
As far as I can tell, the only users for this struct in the userspace
are GDB and LLDB, there are no any problems of software compatibility
between the application and kernel according to the analysis.
The compatibility problem has been considered while developing and
testing. When the applications in the userspace get watchpoint state,
the length will be specified which is no bigger than the sizeof struct
user_watch_state or user_watch_state_v2, the actual length is assigned
as the minimal value of the application and kernel in the generic code
of ptrace:
For example, there are four kind of combinations, all of them work well.
(1) "older kernel + older app", the actual length is 8+(8+8+4+4)*8=200;
(2) "newer kernel + newer app", the actual length is 8+(8+8+4+4)*14=344;
(3) "older kernel + newer app", the actual length is 8+(8+8+4+4)*8=200;
(4) "newer kernel + older app", the actual length is 8+(8+8+4+4)*8=200.
BTW, LLDB also made this change in the related commit ff79d83caeee
("[LLDB][LoongArch] Extend the maximum number of watchpoints") [3]
Alan Modra [Wed, 19 Feb 2025 12:15:29 +0000 (22:45 +1030)]
binutils/dwarf.c debug_information leak
It is possible with fuzzed files to have num_debug_info_entries zero
after allocating space for debug_information, leading to multiple
allocations.
* dwarf.c (process_debug_info): Don't test num_debug_info_entries
to determine whether debug_information has been allocated,
test alloc_num_debug_info_entries.
gdbserver, remote: introduce "id_str" in the "qXfer:threads:read" XML
GDB prints the target id of a thread in various places such as the
output of the "info threads" command in the "Target Id" column or when
switching to a thread. A target can define what to print for a given
ptid by overriding the `pid_to_str` method.
The remote target is a gateway behind which one of many various
targets could be running. The remote target converts a given ptid to
a string in a uniform way, without consulting the low target at the
server-side.
In this patch we introduce a new attribute in the XML that is sent in
response to the "qXfer:threads:read" RSP packet, so that a low target
at the server side, if it wishes, can specify what to print as the
target id of a thread.
Note that the existing "name" attribute or the "extra" text provided
in the XML are not sufficient for the server-side low target to
achieve the goal. Those attributes, when present, are simply appended
to the target id by GDB.
Reviewed-By: Eli Zaretskii <eliz@gnu.org> Reviewed-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org> Approved-By: Simon Marchi <simon.marchi@efficios.com>
testsuite, mi: prevent buffer overflow in get_mi_thread_list
If there is a large number of threads in the input program, the expect
buffer in `get_mi_thread_list` would become full. Prevent this by
consuming the buffer in small pieces.
Regression-tested using the gdb.mi tests.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom de Vries [Tue, 18 Feb 2025 07:47:33 +0000 (08:47 +0100)]
[gdb/testsuite] Don't start gdb in gdb.base/gstack.exp
In test-case gdb.base/gstack.exp we start a gdb implicitly using
prepare_for_testing.
The gdb is not really used, but its spawn_id (available in variable
gdb_spawn_id) is used in a gdb_test_multiple, which is used to interact with
the gstack process.
Usually, a running gdb is cleaned up at test-case exit in gdb_finish, which
calls gdb_exit, which by default calls gdb_default_exit, which does
'send_gdb "quit\n"'.
However, this sends a quit to the host process expect is currently talking to,
defined by board_info(host,fileid), and after spawning gstack that's gstack, not
gdb.
Fix this by:
- using build_executable instead of prepare_for_testing to not spawn an unused
gdb, and
- changing the gdb_test_multiple into a gdb_expect, eliminating the implicit use
of gdb_spawn_id.
Tested on x86_64-linux.
Reviewed-By: Keith Seitz <keiths@redhat.com>
PR testsuite/32709
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32709
Alan Modra [Sun, 16 Feb 2025 22:55:30 +0000 (09:25 +1030)]
bfd_set_section_alignment errors
I noticed when making the change from "einfo" to "fatal" that the
alignment error in _bfd_elf_link_create_gnu_property_sec lacked a %P,
and then decided that a bfd_set_section_alignment that can't happen
does not merit a separate error message. elfxx-x86.c had copied the
same code, so fix that too. In fact, every bfd_set_section_alignment
call in elfxx-x86.c will always return true absent some future
programming error. This patch makes those that accompany making a
section lose their "failed to align " error and share the "failed to
create" error. Those that are changing alignment of a section created
elsewhere now abort on bfd_set_section_alignment returning false.
Alan Modra [Sun, 16 Feb 2025 13:04:55 +0000 (23:34 +1030)]
PR 32603, more ld -w misbehaviour
Commit 8d97c1a53f3d claimed to replace all einfo calls using %F with
a call to fatal. It did so only for the ld/ directory. This patch
adds a "fatal" to linker callbacks, and replaces those calls in bfd/
too.
Andrew Oates [Sun, 16 Feb 2025 15:16:25 +0000 (16:16 +0100)]
gdb: fix color_option_def compile error (clang)
color_option_def was added in commit 6447969d0 ("Add an option with a
color type."), but not used.
The color_option_def constructor passes the wrong number of arguments
to the option_def constructor. Since color_option_def is a template and
never actually instantiated, GCC does not fail to compile this. clang
generates an error (see below).
This passes nullptr to the extra_literals_ option_def ctor argument,
which matches what filename_option_def above it does.
clang's generated error:
../../gdb/cli/cli-option.h:343:7: error: no matching constructor for initialization of 'option_def'
: option_def (long_option_, var_color,
^ ~~~~~~~~~~~~~~~~~~~~~~~~
../../gdb/cli/cli-option.h:50:13: note: candidate constructor not viable: requires 8 arguments, but 7 were provided
constexpr option_def (const char *name_,
^
../../gdb/cli/cli-option.h:37:8: note: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 7 were provided
struct option_def
^
../../gdb/cli/cli-option.h:37:8: note: candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 7 were provided
Alan Modra [Sat, 15 Feb 2025 06:36:58 +0000 (17:06 +1030)]
PR32698, potential null pointer dereference in tekhex.c
PR 32698
* tekhex.c (find_chunk): Remove unnecessary casts.
(insert_byte): Check and return status from find_chunk.
(move_section_contents): Likewise.
(tekhex_get_section_contents, tekhex_set_arch_mach): Return
status from move_section_contents.
(first_phase): Check and return status from first_phase.
Alan Modra [Fri, 14 Feb 2025 12:40:29 +0000 (23:10 +1030)]
riscv disassembler leak
Commit 3f61a38b5e81 moved the disassembler subset_list from a static
variable to disassembler private_data. It is now malloc'd in
riscv_init_disasm_info so should be freed when disassemble_free_target
runs.
Anghelo Carvajal [Sat, 15 Feb 2025 01:30:58 +0000 (01:30 +0000)]
MIPS objdump: Add `eabi32` and `eabi64` ABI options
Extend gpr and fpr register names with names suitable for both EABIs.
Heavily inspired by the EABI documenation written by Eric Christopher,
which can be read at
https://sourceware.org/legacy-ml/binutils/2003-06/msg00436.html
MIPS/GAS/testsuite: Reuse n64 GPR disassembly for n32
The MIPS ABI register names are the same between n64 and n32, so remove
duplication and use n64 GPR disassembly output for the n32 test as well.
The tests were developed long before we gained output reuse support.
MIPS/GAS: Set default CPU to MIPS64r6 for 64-bit "img" configurations
Fix broken commit 070961b377b3 ("MIPS: Set r6 as default arch if vendor
is img") that sets up GAS in an inconsistent way where "img" vendor has
been used with a 64-bit configuration, such as `mips64-img-linux-gnu'.
In that case GAS is set up to use a 64-bit ABI by default combined with
the MIPS32r6 CPU, which is 32-bit.
Consequently GAS always fails to assemble even trivial input, producing
a message such as:
Assembler messages:
Error: -march=mips32r6 is not compatible with the selected ABI
.../gas/testsuite/gas/all/nop.s:2: Error: `gp=32' used with a 64-bit ABI
unless the defaults have been suitably overridden either for the ABI or
the CPU.
Set the default CPU to MIPS64r6 for 64-bit "img" vendor configurations
then and adjust the GAS testsuite accordingly, removing 1048 FAIL and 3
ERROR regression test results for the `mips64-img-linux-gnu' and
`mips64el-img-linux-gnu' targets each.
MIPS/GAS/testsuite: Support negated targets for default architecture
Add support for giving negated targets in the list of targets passed to
`mips_arch_create' for the purpose of setting the default architecture.
This is so that a subset of targets can be excluded from matching within
a broader set of targets.
Ivan Kokshaysky [Sat, 15 Feb 2025 01:30:58 +0000 (01:30 +0000)]
alpha, ld: remove -taso option
The -taso switch was quite useful 25 years ago for porting 32-bit
code with broken integer-pointer casting. Not anymore. The EF_ALPHA_32BIT
Linux support is going to be dropped in kernel v6.14 [1], NetBSD and OpenBSD
never had it, so there is no point in keeping the -taso option around.
Also remove alpha special case that uses -taso from gdb.base/dump.exp
in gdb testsuite.
Andrew Burgess [Thu, 13 Feb 2025 16:43:39 +0000 (16:43 +0000)]
gdb/testsuite: clean ups in gdb.python/py-source-styling.exp
The top comment in gdb.python/py-source-styling.exp was completely
wrong, clearly a cut&paste job from elsewhere. Write a comment that
actually reflects what the test does.
I've also moved the allow_python_tests check earlier in the file.
And I changed some 'return -1' into just 'return'. I'm not aware that
the '-1' adds any value.
I also folded a 'pass $gdb_test_name' into the preceding gdb_assert,
which I think is neater.
There is no change in what is actually being tested after this commit.
Andrew Burgess [Thu, 6 Feb 2025 15:02:37 +0000 (15:02 +0000)]
gdb/tui: use maybe_update for source centring in an extra case
I noticed that, with recent versions of GDB, when the TUI is enabled
before the inferior is started, the source code display is not as
helpful as it used to be. Here's a simple test program being
displayed using GDB 15.2, at this point the inferior has not started,
all I've done is 'tui enable':
gdb: adjust the default place of 'list' to main's prologue
I don't think the new behaviour is really a problem with that commit,
rather, when using 'tui enable' before the inferior has started GDB
ends up calling tui_source_window_base::rerender(), and then passes
through the code path which calls update_source_window_with_addr().
When using 'tui enable' after the inferior has started, GDB again
calls tui_source_window_base::rerender(), but this time has a frame,
and so takes the second code path, which centres the selected source
line, and then calls update_source_window.
The point is that the update_source_window_with_addr() path doesn't
include the logic to centre the source line.
Before the above commit this was fine as GDB's default location would
be prior to main, and so we got the "good" TUI output. After the
above commit the default location is now main's prologue, and without
the centring logic, the first line shown is main's prologue.
I propose fixing this by having update_source_window_with_addr() call
maybe_update(). This will first check if the requested line is
already visible, and if not, show the requested line with centring
applied.
It's not identical to the old behaviour, but that was never the
objective, we do however, see the context around main's prologue,
which will usually be enough to see the function name and return type,
which I think is useful.
Andrew Burgess [Thu, 6 Feb 2025 12:16:48 +0000 (12:16 +0000)]
gdb/tui: update maybe_update to take gdbarch
This is a refactor to setup for the next commit.
The maybe_update function currently takes a frame_info_ptr&, however,
it only uses this to get the frame's gdbarch.
In the next commit I want to call maybe_update when I have a gdbarch,
but no frame_info_ptr& (the inferior hasn't even started).
So, update maybe_update to take the gdbarch, and update the callers to
pass that through. Most callers already have the gdbarch to hand, but
in one place I do need to extract this from the frame_info_ptr&.
There should be no user visible changes after this commit.
Tom Tromey [Fri, 14 Feb 2025 00:45:23 +0000 (17:45 -0700)]
Handle DW_FORM_data4 in read-debug-names.c
The recent .debug_names patches caused the writer to emit
DW_FORM_data4. Unfortunately the reader did not handle this form.
This patch updates the reader to handle a few DW_FORM_data* forms.
The complaint that is there went unnoticed -- I only found this when
debugging a failure in another series. More evidence, IMO, that
complaints should be removed.
I think the reason the failure itself went unnoticed is that the
symbol table code in gdb often works by accident, and in particular in
small programs like the ones in the test suite, it's often the case
that a CU will be expanded for some other reason, causing the test to
pass without really touching the index code. The aforementioned
series is aimed at fixing this.
It would probably be good to unify the abbrev/form code to some
degree, but it's mildly a pain as some forms don't make sense here and
because we recently discovered other issues with gdb's DW_FORM_data*
handling.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Simon Marchi [Tue, 26 Nov 2024 05:32:21 +0000 (00:32 -0500)]
gdbserver: use `gdb::unordered_map`
Replace the few uses of `std::unordered_map` in gdbserver with
`gdb::unordered_map`.
The only one of these that is likely to ever see a lot of elements is
probably `process_info::m_ptid_thread_map`. It was added precisely to
improve performance when there are a lot of threads, so I guess using
`gdb::unordered_map` here won't hurt. I changed the others too, since
it's easy.
Change-Id: Ibc4ede5245551fdd7717cb349a012d05726f4363 Reviewed-By: Stephan Rohr <stephan.rohr@intel.com>
Simon Marchi [Wed, 12 Feb 2025 15:52:51 +0000 (10:52 -0500)]
gdb/dwarf: use term "shard" instead of "index"
A bit more changes as in 8e745eac7db3 ("gdb/dwarf: rename
cooked_index::m_vector to m_shards"). I think it's clearer if the term
"index" is reserved for the whole thing, while "shard" or "index shard"
are used for the parts.
Change-Id: I457bb0016a70f3f9918f4a3c3977262a7801705b Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Thu, 13 Feb 2025 20:08:29 +0000 (15:08 -0500)]
gdb/python/dap: prefix internal attributes with underscore
I'm currently reading the DAP code, and I think this would help. This
is pretty much standard Python style, we do it as some places but not
others. I think it helps readability, by saying that this attribute
isn't mean to be accessed outside the class.
A similar pass could be done for internal methods, I haven't done that.
Change-Id: I8e8789b39adafe62d14404d19f7fc75e2a364e01 Approved-By: Tom Tromey <tom@tromey.com>
Andrew Burgess [Sat, 25 Jan 2025 13:00:12 +0000 (13:00 +0000)]
gdb: only update m_last_subfile after writing a line table entry
While working on another patch which changes how we parse the line
DWARF line tables I noticed what I think is a minor bug in how we
process the line tables.
What I noticed is that my new line table parser was adding more END
markers into the parsed table than GDB's current approach. This
difference was observed when processing the debug information for
libstdc++.
Here is the line table from the new test, this is a reasonable
reproduction of the problem case that I observed in the actual debug
line table:
Contents of the .debug_line section:
dw2-skipped-line-entries-1.c:
File name Line number Starting address View Stmt
dw2-skipped-line-entries-1.c 101 0x40110a x
/tmp/dw2-skipped-line-entries-2.c:
dw2-skipped-line-entries-2.c 201 0x401114 x
/tmp/dw2-skipped-line-entries-3.c:
dw2-skipped-line-entries-3.c 301 0x40111e x
/tmp/dw2-skipped-line-entries-1.c:
dw2-skipped-line-entries-1.c 102 0x401128 x
dw2-skipped-line-entries-1.c 103 0x401128 x
dw2-skipped-line-entries-1.c 104 0x401128 x
The problem is caused by the entry for line 211. Notice that this
entry is at the same address as the previous entries. Further, the
entry for 211 is a non-statement entry, while the previous entries are
statement entries.
As the entry for line 211 is a non-statement entry, and the previous
entries at that address are statement entries in a different symtab,
it is thought that it is better to prefer the earlier entries (in
dw2-skipped-line-entries-1.c), and so the entry for line 211 will be
discarded.
As GDB parses the line table it switches between the 3 symtabs (based
on source filename) adding the relevant entries to each symtab.
Additionally, as GDB switches symtabs, it adds an END entry to the
previous symtab.
The problem then is that, for the line 211 entry, this is the only
entry in dw2-skipped-line-entries-2.c before we switch symtab again.
But the line 211 entry is discarded. This means that GDB switches
from dw2-skipped-line-entries-1.c to dw2-skipped-line-entries-2.c, and
then on to dw2-skipped-line-entries-3.c without ever adding an entry
to dw2-skipped-line-entries-2.c.
And here then is the bug. GDB updates its idea of the previous symtab
not when an entry is written into a symtab, but every time we change
symtab.
In this case, when we switch to dw2-skipped-line-entries-3.c we add
the END marker to dw2-skipped-line-entries-2.c, even though no entries
were written to dw2-skipped-line-entries-2.c. At the same time, no
END marker is ever written into dw2-skipped-line-entries-1.c as the
dw2-skipped-line-entries-2.c entry (for line 211) was discarded.
Here is the 'maint info line-table' for dw2-skipped-line-entries-1.c
before this patch:
INDEX LINE REL-ADDRESS UNREL-ADDRESS IS-STMT PROLOGUE-END EPILOGUE-BEGIN
0 101 0x000000000040110a 0x000000000040110a Y
1 END 0x0000000000401114 0x0000000000401114 Y
2 102 0x0000000000401128 0x0000000000401128 Y
3 103 0x0000000000401128 0x0000000000401128 Y
4 104 0x0000000000401128 0x0000000000401128 Y
5 104 0x000000000040113c 0x000000000040113c
6 105 0x0000000000401146 0x0000000000401146 Y
7 END 0x0000000000401150 0x0000000000401150 Y
And after this patch:
INDEX LINE REL-ADDRESS UNREL-ADDRESS IS-STMT PROLOGUE-END EPILOGUE-BEGIN
0 101 0x000000000040110a 0x000000000040110a Y
1 END 0x0000000000401114 0x0000000000401114 Y
2 102 0x0000000000401128 0x0000000000401128 Y
3 103 0x0000000000401128 0x0000000000401128 Y
4 104 0x0000000000401128 0x0000000000401128 Y
5 END 0x0000000000401132 0x0000000000401132 Y
6 104 0x000000000040113c 0x000000000040113c
7 105 0x0000000000401146 0x0000000000401146 Y
8 END 0x0000000000401150 0x0000000000401150 Y
Notice that we gained an extra entry, the END marker that was added at
position #5 in the table.
Now, does this matter? I cannot find any bugs that trigger because of
this behaviour.
So why fix it? First, the current behaviour is inconsistent, as we
switch symtabs, we usually get an END marker in the previous symtab.
But occasionally we don't. I don't like things that are inconsistent
for no good reason. And second, as I said, I want to change the line
table parsing. To do this I want to check that my new parser creates
an identical table to the current parser. But my new parser naturally
"fixes" this inconsistency, so I have two choices, do extra work to
make my new parser bug-compatible with the current one, or fix the
current one. I'd prefer to just fix the current line table parser.
There's a test that includes the above example and checks that the END
markers are put in the correct place. But as I said, I've not been
able to trigger any negative behaviour from the current solution, so
there's no test that exposes any broken behaviour.
Jan Beulich [Fri, 14 Feb 2025 09:47:50 +0000 (10:47 +0100)]
x86: drop redundant i.operands checks from output_disp()
The opcode space, major opcode, and - where applicable - opcode
extension checks fully qualify the insns we're after; operand matching
has been done far earlier, so wrong operand counts cannot occur here.
Jan Beulich [Fri, 14 Feb 2025 08:35:07 +0000 (09:35 +0100)]
x86: correct ISA-used version recording
Updating should be based solely on the current instruction. For example,
recording of VEX-encoded insns as v3 should be independent of there
being earlier AMX insns.
Further for BASELINE only a very limited set of the
GNU_PROPERTY_X86_FEATURE_2_* bits should actually be taken into account:
Most of the bits represent advanced (later) features (XSAVE, XSAVEOPT,
and XSAVEC for example being part of v3).
Jan Beulich [Fri, 14 Feb 2025 08:33:18 +0000 (09:33 +0100)]
gas: fix rs_fill_nop listing
In commit a0094f1a70e1 ("gas: make .nops output visible in listing") I
was wrongly assuming fr_fix would be zero for rs_fill_nop, when that's
only a side effect of listing_newline() inserting dummy frags, but only
when file/line did actually change from the previous invocation. This is
in particular not going to be true when the .nops directive isn't the
first statement on a line.
Jan Beulich [Fri, 14 Feb 2025 08:32:35 +0000 (09:32 +0100)]
x86/APX: make .insn extended-EVEX capable
So far tricks had to be played to use .insn to encode extended-EVEX
insns; the X4 bit couldn't be controlled at all. Extend the syntax just
enough to cover all features, taking care to reject invalid feature
combinations (albeit aiming at being as lax there as possible, to offer
users as much flexibility as we can - we don't, after all, know what
future will bring).
In a pre-existing testcase replace all but one .byte; the one that needs
to remain wants to have EVEX.U clear in a way that's neither
controllable via AVX10/256 embedded rounding (would otherwise also set
EVEX.ND), nor via the index register (EVEX.X4), as there's no memory
operand. For one of the converted instances ModR/M.mod needs correcting:
An 8-bit displacement requires that to be 1, not 2. Also adjust source
comments to better represent what the bad insns mimic.
Hau Hsu [Fri, 14 Feb 2025 02:40:53 +0000 (10:40 +0800)]
RISC-V: Make SSAMOSWAP.W available for rv64
Previously we limited SSAMOSWAP.W only available on RV32, but it should
be available on RV64 as well.
See
https://github.com/riscv/riscv-cfi/blob/main/src/cfi_backward.adoc
https://github.com/riscv/riscv-isa-manual/blob/702a3e6e843235a2a13b918ae6938b04f8974ffc/src/unpriv-cfi.adoc#L789
Alan Modra [Thu, 13 Feb 2025 01:46:26 +0000 (12:16 +1030)]
dlltool memory leaks
dlltool copies strings with strdup all over the place, seeming to take
the attitude that anything might be modified. That leads to lots of
memory leaks. Fixing the leaks by removing the strdup calls of course
means you need to take good care that strings *aren't* modified. This
isn't as easy as it sounds due to functions like xlate that have
const char* params but then manage to modify the strings. I've fixed
xlate, but if I've missed something somewhere then this patch likely
will break dlltool. Testsuite coverage of dlltool isn't good.
The leaks in defparse.y are small. It also is a little work to verify
that all the strings I'm freeing in defparse.y are in fact malloc'd,
which is no doubt why the leaks are there.
Using bfd_xalloc in make_one_lib_file and functions called from there
results in memory being freed automatically at the bfd_close in
make_one_lib_file, without any fuss.
The patch also makes use of xasprintf to replace xmalloc followed by
sprintf.
* defparse.y (opt_name2): Free incoming ID strings after
adding prefix/suffix.
* dlltool.c (struct ifunct): Constify char* fields.
(struct iheadt, struct dlist): Likewise.
(set_dll_name_from_def, def_heapsize, def_stacksize),
(def_section, assemble_file): Use xasprintf.
(def_name, def_library): Free dll_name and name.
(def_description, new_directove): Don't strdup incoming args.
(append_import): Likewise.
(def_import): Free module after appending dllext.
(run): Free temp_base.
(scan_filtered_symbols): Don't segfault on NULL strchr return.
Remove unnecessary strdup.
(scan_drectve_symbols): Likewise. Constify pointers.
Use bfd_malloc_and_get_section. Use xmemdup.
(add_excludes): Use xasprintf and xstrdup.
(gen_exp_file): Free xlate return. Constify pointer to suit
struct changes. Free copy.
(xlate): Always copy arg. Use xasprintf and xstrdup.
(make_imp_label): Add bfd arg. Use bfd_xalloc.
(gen_lib_file): Adjust to suit.
(make_one_lib_file): Likewise. Use bfd_xalloc for section data
and relocs. Simplify code calling xlate, and free xlate return.
(dll_name_list_free_contents): Flatten recursion.
(mangle_defs): Free d_export_vec.
(main): Formatting. Use xasprintf.
* resres.c (write_res_id): Free section data.
Alan Modra [Thu, 13 Feb 2025 00:29:51 +0000 (10:59 +1030)]
gas: replace bfd_alloc with notes_alloc
bfd_alloc can return NULL on out-of-memory so code needs to check the
return value and print an error. That check was missing in write.c.
notes_alloc won't return NULL, instead the underlying obstack_alloc
prints an OOM message and the process exits. This is more convenient,
and when the bfd_alloc memory is attached to the gas output bfd it is
released only slightly before the notes obstack.
* config/obj-macho.c (obj_mach_o_set_indirect_symbols): Use
notes_calloc rather than bfd_zalloc.
* write.c (set_symtab): Use notes_alloc.
Alan Modra [Wed, 12 Feb 2025 21:55:01 +0000 (08:25 +1030)]
gas obj-coff memory leaks
This patch addresses memory leaks in gas that show up when running the
testsuite on x86_64-w64-mingw32. The seh_ctx_cur, and weak sym naming
leaks can occur many times during assembly. The symbol hook and
section leaks are not so important since this memory needs to persist
until closing the output bfd.
* config/obj-coff-seh.c (do_seh_endproc): Free seh_ctx_cur and
its fields.
* config/obj-coff-seh.h (struct seh_context): Remove unused
"next" field.
* config/obj-coff.c (coff_obj_symbol_new_hook): Use notes_alloc
for aux entries.
(coff_obj_symbol_clone_hook): Likewise.
(obj_coff_def): Don't strdup name unless we need to do so
for tc_canonicalize_symbol_name. Free after making symbol.
(weak_name2altname, weak_altname2name): Return a char*.
(weak_uniquify): Use notes_concat.
(pecoff_obj_set_weak_hook, pecoff_obj_clear_weak_hook): Free name
returned by weak_name2altname.
(coff_frob_symbol): Similarly for weak_altname2name.
(obj_coff_section): Use notes_memdup0.
* symbols.h: Add include guard.
(notes_memdup0): New inline function.
Tom Tromey [Thu, 13 Feb 2025 03:43:38 +0000 (20:43 -0700)]
Remove assumption from py-symbol.exp
The current py-symbol.exp test makes an assumption about which symbol
will be returned first. I don't think gdb should really make promises
about the order in which the symbols are listed, though, and a series
I am working on changes this behavior. This patch changes the test to
merely ensure that both symbols are returned.
Approved-By: Simon Marchi <simon.marchi@efficios.com>