Alan Modra [Sat, 31 May 2025 04:19:24 +0000 (13:49 +0930)]
weakref gas internal error
This horrible testcase (cleaned up from oss-fuzz)
r=x*2
x=r-r
.weakref r,x
r=r-5
triggers resolve_symbol_value "gas_assert (final_val == 0)" in weakref
handling.
Rui Ueyama [Sat, 31 May 2025 12:16:34 +0000 (21:16 +0900)]
PR 33033, Support compressed debug sections larger than 4 GiB
z_stream's avail_in and avail_out are defined as "unsigned int", so it
cannot decode an entire compressed stream in one pass if the stream is
larger than 4 GiB. The simplest solution to this problem is to use zlib's
convenient uncompress2() function, which handles the details for us.
Tom Tromey [Fri, 30 May 2025 13:04:03 +0000 (07:04 -0600)]
Define TLS in bfd.c if not already defined
If configure decides that thread-local storage isn't available, it
does not define "TLS". However, this is used unconditionally in a
definition. So, define it if it isn't already defined.
Alan Modra [Fri, 30 May 2025 22:43:20 +0000 (08:13 +0930)]
PR 33020 segv in _bfd_elf_strtab_offset
The PR fuzzer testcase creates a SHT_NOBITS .debug_info section, then
triggers a bug in --compress-debug-sections=zlib whereby sh_name is
set to -1 in elf_fake_sections as a flag to indicate the name is not
set yet (may change to zdebug_*), but the section never hits the debug
compression code in assign_file_positions_for_non_load_sections that
is responsible for setting sh_name.
PR 33020
* elf.c (_bfd_elf_init_reloc_shdr): Rename delay_st_name_p
param to delay_sh_name_p.
(elf_fake_sections): Rename delay_st_name_p to delay_sh_name_p.
Don't set delay_sh_name_p for no contents debug sections.
Tom Tromey [Fri, 30 May 2025 20:07:57 +0000 (14:07 -0600)]
Remove some Rust expression helpers
When I did the big expression conversion, I left some helper functions
lying around, primarily because the conversion was already quite large
and I didn't want to add on.
This patch removes a couple such helpers, turning them into methods on
the appropriate operation objects.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Wed, 16 Oct 2024 15:58:14 +0000 (09:58 -0600)]
Require Python 3.4
I believe we previously agreed that the minimum supported Python
version should be 3.4. This patch makes this change, harmonizing the
documentation (which was inconsistent about the minimum version) and
the code.
New in v2: rebased, and removed a pre-3.4 workaround from __init__.py.
Reviewed-By: Eli Zaretskii <eliz@gnu.org> Approved-by: Kevin Buettner <kevinb@redhat.com> Acked-By: Tom de Vries <tdevries@suse.de>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31870
Alan Modra [Thu, 29 May 2025 10:40:28 +0000 (20:10 +0930)]
elf symbol size
This changes elf_obj_sy.size from being malloc'd to being on the notes
obstack. That means no code needs to free these expressions, which in
turn means that the size expression can be shared when cloning
symbols. Nothing modifies the size expressions except when resolving.
In all cases I could see, if the size changes the entire expression is
replaced.
The patch also extracts code from elf_copy_symbol_attributes into a
separate function for use by riscv and aarch64.
* config/obj-elf.c (elf_obj_symbol_clone_hook): Delete.
(elf_copy_symbol_size): New function, extracted and modified from..
(elf_copy_symbol_attributes): ..here.
(obj_elf_size): Don't free size and use notes_alloc.
(elf_frob_symbol): Don't free size.
(elf_format_ops): Zero symbol_clone_hook.
* config/obj-elf.h (elf_obj_symbol_clone_hook): Delete.
(obj_symbol_clone_hook): Don't define.
(elf_copy_symbol_size): Declare.
* config/tc-aarch64.c (aarch64_elf_copy_symbol_attributes): Delete.
* config/tc-aarch64.h (OBJ_COPY_SYMBOL_ATTRIBUTES): Define as
elf_copy_symbol_size.
* config/tc-alpha.c (s_alpha_end): notes_alloc symbol size exp.
* config/tc-ia64.c (dot_endp): Likewise.
* config/tc-kvx.c (kvx_endp): Likewise.
* config/tc-mips.c (s_mips_end): Likewise.
* config/tc-riscv.c (riscv_elf_copy_symbol_attributes): Delete.
* config/tc-riscv.h (OBJ_COPY_SYMBOL_ATTRIBUTES): Define as
elf_copy_symbol_size.
Alan Modra [Thu, 29 May 2025 03:20:45 +0000 (12:50 +0930)]
gas symbol_remove
If a symbol is at the start of the symbol chain then symbol_rootP
points at the symbol and symbol->x->previous is NULL. Testing either
condition is sufficient, there is no need to test both. Similarly for
the symbol at the end of the symbol chain.
I'm testing the previous/next pointer because it's most likely that
the symbol is not at the start/finish of the chain and thus we need to
use that pointer.
* symbols.c (symbol_remove): Tidy list handling.
(symbol_append, symbol_clone, symbol_insert): Likewise.
Alan Modra [Tue, 27 May 2025 01:10:59 +0000 (10:40 +0930)]
Reduce rs_align_code memory for small alignments
On x86, MAX_MEM_FOR_RS_ALIGN_CODE is 35, when the most common
alignment is 2**3 or 2**4, where the max memory required for the
alignment nops is 7 and 15 bytes respectively. So there is some
memory wasted since commit 83d94ae428b1. It's not a large amount,
especially considering that frag overhead on x86_46 is 144 bytes,
but even so I'd rather not be blamed for increasing gas memory usage.
So to reduce the memory we'd like to take the alignment into
consideration when initialising an rs_align_code frag. The only
difficulty here is start_bundle making an rs_align_code frag with an
alignment of zero initially, then later increasing the alignment. We
change that to use the bundle alignment when setting up the frag. I
think that is sufficient as bundle_align_p2 can't change in the middle
of a start_bundle/finish_bundle sequence.
I haven't modified any targets other than x86 in this patch. Most
won't benefit much due to using fairly small MAX_MEM_FOR_RS_ALIGN_CODE.
* read.c (start_bundle): Create rs_align_code frag with
bundle_align_p2 alignment, then set to zero alignment.
(finish_bundle): Adjust comment.
* frags.c (MAX_MEM_FOR_RS_ALIGN_CODE): Pass p2align and max
to macro.
* config/tc-i386.h (HANDLE_ALIGN): Assert that max_bytes is
sufficient for nop padding.
(max_mem_for_rs_align_code): New inline function.
(MAX_MEM_FOR_RS_ALIGN_CODE): Use it.
* config/tc-aarch64.h: Adjust MAX_MEM_FOR_RS_ALIGN_CODE.
* config/tc-alpha.h: Likewise.
* config/tc-arc.h: Likewise.
* config/tc-arm.h: Likewise.
* config/tc-epiphany.h: Likewise.
* config/tc-frv.h: Likewise.
* config/tc-ia64.h: Likewise.
* config/tc-kvx.h: Likewise.
* config/tc-loongarch.h: Likewise.
* config/tc-m32r.h: Likewise.
* config/tc-metag.h: Likewise.
* config/tc-mips.h: Likewise.
* config/tc-nds32.h: Likewise.
* config/tc-ppc.h: Likewise.
* config/tc-riscv.h: Likewise.
* config/tc-rl78.h: Likewise.
* config/tc-rx.h: Likewise.
* config/tc-score.h: Likewise.
* config/tc-sh.h: Likewise.
* config/tc-sparc.h: Likewise.
* config/tc-spu.h: Likewise.
* config/tc-tilegx.h: Likewise.
* config/tc-tilepro.h: Likewise.
* config/tc-visium.h: Likewise.
* config/tc-xtensa.h: Likewise.
gdb: update corner case when canonicalizing riscv syscall names
The script syscalls/riscv-canonicalize-syscall-gen.py has been recently
introduced to help support record-full in riscv systems. However, it
was developed before commit 432eca4113d5748ad284a068873455f9962b44fe,
which made the GDB enum more consistent, which forced the python script
to have a corner case for the "gdb_old_mmap" case.
Since the aforementioned commit has already been merged, we need to
update the special case for the mmap syscall. A special case is still
needed because the script would expect that the glibc sources call the
syscall "old_mmap", or that gdb call it "gdb_sys_mmap", neither of which
happens unfortunately.
This commit doesn't change the .c file because it was already fixed by a
different commit, 65ab41b7d5c612b6000b28f4c50bb256b2a9e22b, which was
pushed as obvious to fix the build issues.
Jorenar [Sun, 25 May 2025 15:40:25 +0000 (17:40 +0200)]
gdb/dap: fix completion request for empty strings
When DAP completion requests receives empty string to complete,
the script crashes due trying to access element -1 from list
being a result of `text.splitlines()` (which for `text == ""`
evaluates into empty list).
This patch adds simple check if `text` is populated, and when it
is not, skips transformations and assigns correct result directly.
Pedro Alves [Sat, 17 May 2025 21:30:56 +0000 (22:30 +0100)]
Fix build when RUSAGE_THREAD is not available & add warning
Building current GDB on Cygwin, fails like so:
/home/pedro/gdb/src/gdbsupport/run-time-clock.cc: In function ‘void get_run_time(user_cpu_time_clock::time_point&, system_cpu_time_clock::time_point&, run_time_scope ’:
/home/pedro/gdb/src/gdbsupport/run-time-clock.cc:52:13: error: ‘RUSAGE_THREAD’ was not declared in this scope; did you mean ‘SIGEV_THREAD’?
52 | who = RUSAGE_THREAD;
| ^~~~~~~~~~~~~
| SIGEV_THREAD
Cygwin does not implement RUSAGE_THREAD. Googling around, I see
Cygwin is not alone, other platforms don't support it either. For
example, here is someone suggesting an alternative for darwin/macos:
https://stackoverflow.com/questions/5652463/equivalent-to-rusage-thread-darwin
Fix this by falling back to process scope if thread scope can't be
supported. I chose this instead of returning zero usage or some other
constant, because if gdb is built without threading support, then
process-scope run time usage is the right info to return.
But instead of falling back silently, print a warning (just once),
like so:
(gdb) maint set per-command time on
⚠️ warning: per-thread run time information not available on this platform
... so that developers on other platforms at least have a hint
upfront.
This new warning also shows on platforms that don't have getrusage in
the first place, but does not show if the build doesn't support
threading at all.
New tests are added to gdb.base/maint.exp, to expect the warning, and
also to ensure other "mt per-command" sub commands don't trigger the
new warning.
Change-Id: Ie01b916b62f87006f855e31594a5ac7cf09e4c02 Approved-By: Simon Marchi <simon.marchi@efficios.com> Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Mon, 26 May 2025 20:26:15 +0000 (16:26 -0400)]
gdb/solib: make implementation of solib_ops::open_symbol_file_object optional
The only solib implementation that implements open_symbol_file_object is
SVR4. All others just return 0. Make it optional, to avoid having
these empty functions.
Simon Marchi [Mon, 26 May 2025 20:26:13 +0000 (16:26 -0400)]
gdb/solib: move solist.h content to solib.h
I don't think that the file solist.h is useful. It would make sense to
have `struct solib` in solib.h. And then, all that would remain is
`struct solib_ops` and some solib-related function declarations. So,
move it all to solib.h.
Tom de Vries [Wed, 28 May 2025 20:17:19 +0000 (22:17 +0200)]
[gdb/symtab] Note errors in process_skeletonless_type_units
With a hello world a.out, and using the compiler flags from target board
dwarf5-fission-debug-types:
...
$ gcc -gdwarf-5 -fdebug-types-section -gsplit-dwarf ~/data/hello.c
...
I run into:
...
$ gdb -q -batch a.out
terminate called after throwing an instance of 'gdb_exception_error'
...
What happens is that an error is thrown due to invalid dwarf, but the error is
not caught, causing gdb to terminate.
In a way, this is a duplicate of PR32861, in the sense that we no longer run
into this after:
- applying the proposed patch (work around compiler bug), or
- using gcc 9 or newer (compiler bug fixed).
But in this case, the failure mode is worse than in PR32861.
Fix this by catching the error in
cooked_index_worker_debug_info::process_skeletonless_type_units.
With the patch, we get instead:
...
$ gdb -q -batch a.out
Offset from DW_FORM_GNU_str_index or DW_FORM_strx pointing outside of \
.debug_str.dwo section in CU at offset 0x0 [in module a.out]
...
While we're at it, absorb the common use of
cooked_index_worker_result::note_error:
...
try
{
...
}
catch (gdb_exception &exc)
{
(...).note_error (std::move (exc));
}
...
into the method and rename it to catch_error, resulting in more compact code
for the fix:
...
(...).catch_error ([&] ()
{
...
});
...
While we're at it, also use it in
cooked_index_worker_debug_info::process_units which looks like it needs the
same fix.
Call restore_original_signal_state after GDB forks.
When I run GDB under Valgrind, GDB seems to segfault
displaying:
Fatal signal: Segmentation fault
----- Backtrace -----
0x2803f7 ???
0x3c9696 ???
0x3c9899 ???
0x55f8fcf ???
0x486c000 ???
---------------------
A fatal error internal to GDB has been detected, further
debugging is not possible. GDB will now terminate.
This is a bug, please report it. For instructions, see:
<https://www.gnu.org/software/gdb/bugs/>.
warning: linux_ptrace_test_ret_to_nx: PC 0x5821c09d is neither near return address 0x486c000 nor is the return instruction 0x4f8f4a!
but then, acts like nothing happened and excutes normally. This is
because it's the child from linux_ptrace_test_ret_to_nx that
segfaults and parent GDB carries on normally. Restore the original
signal states to not to print confusing backtrace. After restoring,
only such warning is displayed:
warning: linux_ptrace_test_ret_to_nx: WSTOPSIG 19 is neither SIGTRAP nor SIGSEGV!
gdb/testsuite: Clarify -lbl option in gdb_test_multiple
I was a bit confused about the -lbl option in gdb_test_multiple, and needed
to read its implementation to determine that it would be useful for my
needs. Explicitly mention what the option does and why it's useful to
hopefully help other developers.
Reviewed-By: Keith Seitz <keiths@redhat.com> Approved-By: Andrew Burgess <aburgess@redhat.com>
gdb/testsuite: Fix flakiness in gdb.base/default.exp
The Linaro CI runs the GDB testsuite using the read1 tool, which
significantly increases the time it takes DejaGNU to read inferior output.
On top of that sometimes the test machine has higher than normal load,
which causes tests to run even slower.
Because gdb.base/default.exp tests some verbose commands such as "info
set", it sometimes times out while waiting for the complete command
output when running in the Linaro CI environment.
Fix this problem by consuming each line of output from the most verbose
commands with gdb_test_multiple's -lbl (line-by-line) option — which
causes DejaGNU to reset the timeout after each match — and also by
breaking up regular expressions that try to match more than 2000
characters (the default Expect buffer size) in one go into multiple
matches.
Some tests use the same regular expression, so I created a procedure for
them. This is the case for "i" / "info", "info set" / "show", and "set
print" tests.
The tests for "show print" don't actually test their output, so this
patch improves them by checking some lines of the output.
Reviewed-By: Keith Seitz <keiths@redhat.com> Approved-By: Andrew Burgess <aburgess@redhat.com>
Alan Modra [Mon, 26 May 2025 02:16:05 +0000 (11:46 +0930)]
ALPHA_R_OP_STORE
In commit db4ab410dec3 I rewrote OP_STORE handling to support writing
near the end of a section. The rewrite had some bugs, fixed in commit 3e02c4891dcb. However I wasn't entirely happy with the code writing
the bitfield:
- it doesn't support 64-bit fields with a bit offset,
- the code is duplicated and inelegant,
- the stack ought to be popped whenever seeing one of these relocs,
even if the reloc can't be applied.
This patch fixes all of the above.
In addition, it is clear from the OP_STORE description in the ABI that
a 64-bit field is encoded as 0 in r_size, so I've decoded that in
alpha_ecoff_swap_reloc_in. The aborts there are not appropriate as
they can be triggered by user input (fuzzed object files). Also,
stack underflow wasn't checked in alpha_relocate_section.
* coff-alpha.c (alpha_ecoff_swap_reloc_in): Replace aborts
with asserts. Decode ALPHA_R_OP_STORE r_size of zero.
(write_bit_field): New function.
(alpha_ecoff_get_relocated_section_contents): Use it.
(alpha_relocate_section): Here too. Catch stack underflow.
Jens Remus [Mon, 26 May 2025 18:02:47 +0000 (11:02 -0700)]
libsframe: handle SFrame FRE start/end IP offsets as unsigned
The SFrame FRE start address (fre_start_addr) is defined as unsigned
32-bit integer, as it is an offset from SFrame FDE function start
address (sfde_func_start_address) and functions only grow upwards
(towards higher addresses).
The SFrame FRE start IP offset is a synonym to the SFrame FRE start
address. The SFrame FRE end IP offset is either the value of the
subsequent FDE start address minus one, if that exists, or the FDE
function size minus one otherwise. Both should therefore be handled
as unsigned 32-bit integer.
In libsframe the "lookup PC" (pc) and SFrame FDE function start address
(sfde_func_start_address) are both signed integers, as they are actually
offsets from the SFrame section. The unsigned FDE start/end IP offsets
may therefore only be safely compared against the offset of the lookup
PC from FDE function start address if the FDE function start address is
lower or equal to the lookup PC, as this guarantees the offset to be
always positive:
If the FDE function start address is lower or equal than the lookup PC,
which both are signed offsets from SFrame section, then the function
start address is also lower or equal to the PC, which are both unsigned:
sfde_func_start_address <= lookup_pc
func_start_addr - sframe_addr <= pc - sframe_addr
func_start_addr <= pc
With that the offset of the lookup PC from FDE function start address
(lookup_pc - sfde_func_start_address) must always be positive, if
FDE function start address is lower or equal to the lookup PC:
lookup_pc - sfde_func_start_address
= pc - sframe_addr - (func_start_addr - sframe_addr)
= pc - func_start_addr
libsframe/
* sframe.c (sframe_find_fre): Define and handle start_ip_offset
and end_ip_offset as unsigned (same as FRE fre_start_addr).
(sframe_fre_check_range_p): Likewise. Define PC offset (from
function start address) as unsigned.
Jens Remus [Mon, 26 May 2025 18:02:29 +0000 (11:02 -0700)]
libsframe: stop search for SFrame FRE if its start IP is greater than PC
The SFrame FREs for an SFrame FDE are sorted on their start address.
Therefore the linear search for a matching SFrame FRE can be stopped,
if its start address is greater than the searched for PC.
libsframe/
* sframe.c (sframe_find_fre): Stop search if FRE's start IP is
greater than PC.
Jens Remus [Mon, 26 May 2025 18:01:14 +0000 (11:01 -0700)]
libsframe: correct binary search for SFrame FDE
sframe_get_funcdesc_with_addr_internal erroneously returns the last FDE,
if its function start address is lower than the searched for address.
Simplify the binary search for a SFrame FDE for a given address. Only
return an FDE, if the searched for address is within the bounds of the
FDE function start address and function size.
libsframe/
* sframe.c (sframe_get_funcdesc_with_addr_internal): Correct
binary search for SFrame FDE.
libsframe/testsuite/
* libsframe.find/plt-findfre-1.c: Add test for out of range
PLT6.
Indu Bhagat [Mon, 26 May 2025 17:54:06 +0000 (10:54 -0700)]
libsframe: fix issue finding FRE in PCMASK type SFrame FDEs
SFrame FDEs of type SFRAME_FDE_TYPE_PCMASK are used for repetitive code
patterns, e.g., pltN entries. For SFrame FDEs of type
SFRAME_FDE_TYPE_PCMASK, sframe_fre_check_range_p erroneously tested the
given PC instead of the masked PC offset from function start address.
Therefore it only worked correctly by chance, e.g., if the function start
address was aligned on the repetition block size.
For regular SFrame FDEs the PC offset from function start address must
be within a SFrame FRE's start IP offset and end IP offset. For SFrame
FDEs of type SFRAME_FDE_TYPE_PCMASK, the masked PC offset must be within
that range.
SFrame FRE start/end IP offsets are relative to the SFrame FDE function
start address. For regular SFrame FDEs, the PC offset from function
start address must be within a SFrame FRE's start IP offset and end IP
offset. For SFRAME_FDE_TYPE_PCMASK type FDEs, the masked PC offset must
be within that range.
Exercise the testcase for a variety of placements; without the fix some
of these tests will fail. Also, make the testcase itself easier to
follow by adding appropriate vars where applicable.
libsframe/
* sframe.c (sframe_fre_check_range_p): Fix logic for
SFRAME_FDE_TYPE_PCMASK type FDE.
libsframe/testsuite/
* libsframe.find/plt-findfre-1.c: Adjust the test for a variety
of placements of .sframe and .plt.
if (uia < uib)
return true;
if (uia > uib)
return false;
...
we compare pointers to struct program_space, which gives unstable sorting
results.
The assumption is that this doesn't matter, but as PR32202 demonstrates,
sometimes it does.
While PR32202 is fixed elsewhere, it seems like a good idea to stabilize this
comparison, because it comes at a small cost and possibly prevents
hard-to-reproduce user-visible ordering issues.
Fix this by comparing the program space IDs instead of the pointers.
Likewise in compare_msymbols.
Tested on x86_64-linux.
Reviewed-By: Guinevere Larsen <guinevere@redhat.com> Approved-By: Andrew Burgess <aburgess@redhat.com>
Tom de Vries [Mon, 26 May 2025 13:15:31 +0000 (15:15 +0200)]
[gdb/breakpoints] Stabilize info breakpoints output
With test-case gdb.multi/pending-bp-del-inferior.exp, occasionally I run into:
...
(gdb) info breakpoints^M
Num Type Disp Enb Address What^M
3 dprintf keep y <MULTIPLE> ^M
printf "in foo"^M
3.1 y 0x004004dc in foo at $c:21 inf 2^M
3.2 y 0x004004dc in foo at $c:21 inf 1^M
(gdb) FAIL: $exp: bp_pending=false: info breakpoints before inferior removal
...
The FAIL happens because the test-case expects:
- breakpoint location 3.1 to be in inferior 1, and
- breakpoint location 3.2 to be in inferior 2
but it's the other way around.
I managed to reproduce this with a trigger patch in
compare_symbols from gdb/linespec.c:
...
uia = (uintptr_t) a.symbol->symtab ()->compunit ()->objfile ()->pspace ();
uib = (uintptr_t) b.symbol->symtab ()->compunit ()->objfile ()->pspace ();
- if (uia < uib)
+ if (uia > uib)
return true;
- if (uia > uib)
+ if (uia < uib)
return false;
...
The order enforced by compare_symbols shows up in the "info breakpoints"
output because breakpoint::add_location doesn't enforce an ordering for equal
addresses:
...
auto ub = std::upper_bound (m_locations.begin (), m_locations.end (),
loc,
[] (const bp_location &left,
const bp_location &right)
{ return left.address < right.address; });
m_locations.insert (ub, loc);
...
Fix this by using new function bp_location_is_less_than
(forwarding to bp_location_ptr_is_less_than) in breakpoint::add_location.
Tom de Vries [Mon, 26 May 2025 13:15:31 +0000 (15:15 +0200)]
[gdb/breakpoints] Rename bp_location_is_less_than to bp_location_ptr_is_less_than
In breakpoint.c, we have:
...
/* A comparison function for bp_location AP and BP being interfaced to
std::sort. Sort elements primarily by their ADDRESS (no matter what
bl_address_is_meaningful says), secondarily by ordering first
permanent elements and tertiarily just ensuring the array is sorted
stable way despite std::sort being an unstable algorithm. */
static int
bp_location_is_less_than (const bp_location *a, const bp_location *b)
...
There are few problems here:
- the return type is int. While std::sort allows this, because int is
convertible to bool, it's clearer to use bool directly,
- it's not abundantly clear from either function name or comment that we can
use this to sort std::vector<bp_location *> but not
std::vector<bp_location>, and
- the comment mentions AP and BP, but there are no such parameters.
Fix this by:
- changing the return type to bool,
- renaming the function to bp_location_ptr_is_less_than and mentioning
std::vector<bp_location *> in the comment, and
- updating the comment to use the correct parameter names.
Tested on x86_64-linux.
Reviewed-By: Guinevere Larsen <guinevere@redhat.com> Approved-By: Andrew Burgess <aburgess@redhat.com>
Janne Ramstedt [Sun, 25 May 2025 17:17:20 +0000 (20:17 +0300)]
alpha, bfd: Fixes for ALPHA_R_OP_STORE
ALPHA_R_OP_STORE copies one byte too many and also will cause out of
range error when it tries to copy from the end of section. Since
"endbyte" is already rounded to next full byte, there is enough bits
to copy and the additional "+ 1" is erroneous in bytes count. I also
believe size is incorrectly decreased.
Simon Marchi [Fri, 23 May 2025 16:30:52 +0000 (12:30 -0400)]
gdb/solib-svr4: check that solib is SVR4 in tls_maybe_fill_slot and tls_maybe_erase_slot
Functions tls_maybe_fill_slot and tls_maybe_erase_slot blindly assume
that the passe solibs come from solib-svr4. This is not always the
case, because they are called even on the systems where the solib
implementation isn't solib-svr4. Add some checks to return early in
that case.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32990
Change-Id: I0a281e1f4826aa1914460c2213f0fae1bdc9af7c Tested-By: Hannes Domani <ssbssa@yahoo.de> Approved-By: Andrew Burgess <aburgess@redhat.com>
Tom de Vries [Sat, 24 May 2025 08:27:12 +0000 (10:27 +0200)]
[gdb/build] Fix unused var in lookup_dwo_unit_in_dwp
On x86_64-linux, with gcc 7.5.0 I ran into a build breaker:
...
gdb/dwarf2/read.c: In function ‘dwo_unit* lookup_dwo_unit_in_dwp()’:
gdb/dwarf2/read.c:7403:22: error: unused variable ‘inserted’ \
[-Werror=unused-variable]
auto [it, inserted] = dwo_unit_set.emplace (std::move (dwo_unit));
^
...
Simon Marchi [Fri, 23 May 2025 18:00:29 +0000 (14:00 -0400)]
gdb: include <mutex> in dwarf2/read.h
The buildbot pointed out this compilation failure on AlmaLinux, with g++
8.5.0, which is not seen on more recent systems:
CXX gdbtypes.o
In file included from ../../binutils-gdb/gdb/gdbtypes.c:39:
../../binutils-gdb/gdb/dwarf2/read.h:639:8: error: ‘mutex’ in namespace ‘std’ does not name a type
std::mutex dwo_files_lock;
^~~~~
../../binutils-gdb/gdb/dwarf2/read.h:639:3: note: ‘std::mutex’ is defined in header ‘<mutex>’; did you forget to ‘#include <mutex>’?
../../binutils-gdb/gdb/dwarf2/read.h:35:1:
+#include <mutex>
Tom de Vries [Fri, 23 May 2025 16:54:43 +0000 (18:54 +0200)]
[gdb] Make make-init-c more robust
In commit 2711e4754fc ("Ensure cooked_index_entry self-tests are run"), we
rewrite the function definition of _initialize_dwarf2_entry into a normal
form that allows the make-init-c script to detect it:
...
void _initialize_dwarf2_entry ();
-void _initialize_dwarf2_entry ()
+void
+_initialize_dwarf2_entry ()
...
Update make-init-c to also detect the "void _initialize_dwarf2_entry ()"
variant.
Tested on x86_64-linux, by reverting commit 2711e4754fc, rebuilding and
checking that build/gdb/init.c doesn't change.
Simon Marchi [Mon, 12 May 2025 19:09:45 +0000 (15:09 -0400)]
gdb/dwarf: split dwo_lock in more granular locks
The dwo_lock mutex is used to synchronize access to some dwo/dwp-related
data structures, such as dwarf2_per_bfd::dwo_files and
dwp_file::loaded_{cus,tus}. Right now the scope of the lock is kind of
coarse. It is taken in the top-level lookup_dwo_unit function, and held
while the thread:
- looks up an existing dwo_file in the per-bfd hash table for the given
id/signature
- if there's no existing dwo_file, attempt to find a .dwo file, open
it, build the list of units it contains
- if a new dwo_file was created, insert it in the per-bfd hash table
- look up the desired unit in the dwo_file
And something similar for the dwp code path. This means that two
indexing thread can't read in two dwo files simultaneously. This isn't
ideal in terms of parallelism.
This patch breaks this lock into 3 more fine grained locks:
- one lock to access dwarf2_per_bfd::dwo_files
- one lock to access dwp_file::loaded_{cus,tus}
- one lock in try_open_dwop_file, where we do two operations that
aren't thread safe (bfd_check_format and gdb_bfd_record_inclusion)
Unfortunately I don't see a clear speedup on my computer with 8 threads.
But the change shouldn't hurt, in theory, and hopefully this can be a
piece that helps in making GDB scale better on machines with many cores
(if we ever bump the max number of worker threads).
This patch uses "double-checked locking" to avoid holding the lock(s)
for the whole duration of reading in dwo files. The idea is, when
looking up a dwo with a given name:
- with the lock held, check for an existing dwo_file with that name in
dwarf2_per_bfd::dwo_files, if found return it
- if not found, drop the lock, load the dwo file and create a dwo_file
describing it
- with the lock held, attempt to insert the new dwo_file in
dwarf2_per_bfd::dwo_files. If an entry exists, it means another
thread simultaneously created an equivalent dwo_file, but won the
race. Drop the new dwo_file and use the existing one. The new
dwo_file is automatically deleted, because it is help by a unique_ptr
and the insertion into the unordered_set fails.
Note that it shouldn't normally happen for two threads to look up a dwo
file with the same name, since different units will point to different
dwo files. But it were to happen, we handle it. This way of doing
things allows two threads to read in two different dwo files
simulatenously, which in theory should help get better parallelism. The
same technique is used for dwp_file::loaded_{cus,tus}.
I have some local CI jobs that run the fission and fission-dwp boards,
and I haven't seen regressions. In addition to the regular testing, I
ran a few tests using those boards on a ThreadSanitizer build of GDB.
Change-Id: I625c98b0aa97b47d5ee59fe22a137ad0eafc8c25 Reviewed-By: Andrew Burgess <aburgess@redhat.com>
Simon Marchi [Mon, 12 May 2025 19:09:44 +0000 (15:09 -0400)]
gdb/dwarf: allocate DWP dwarf2_section_info with new
For the same reason as explained in the previous patch (allocations on
obstacks aren't thread-safe), change the allocation of
dwarf2_section_info object for dwo files within dwp files to use "new".
The dwo_file::section object is not always owned by the dwo_file, so
introduce a new "dwo_file::section_holder" object that is only set when
the dwo_file owns the dwarf2_section_info.
Change-Id: I74c4608573c7a435bf3dadb83f96a805d21798a2 Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Mon, 12 May 2025 19:09:43 +0000 (15:09 -0400)]
gdb/dwarf: allocate dwo_unit with new
The following patch reduces the duration where the dwo_lock mutex is
taken. One operation that is not thread safe is the allocation on
dwo_units on the per_bfd obstack:
Tom Tromey [Thu, 8 May 2025 20:04:05 +0000 (14:04 -0600)]
Handle an argument-less operator in the C++ name parser
While debugging a new failure in my long-suffering "search-in-psyms"
series, I found that the C++ name canonicalizer did not handle a case
like "some_name::operator new []". This should remove the space,
resulting in "some_name::operator new[]" -- but does not.
This happens because the parser requires an operator to be followed by
argument types. That is, it's expected.
However, it seems to me that we do want to be able to canonicalize a
name like this. It will appear in the DWARF as a DW_AT_name, and
furthermore it could be entered by the user.
This patch fixes this problem by changing the grammar to supply the
"()" itself, then removing the trailing "()" when changing to string
form (in the functions that matter).
This isn't ideal -- it might miss a very obscure case involving the
gdb extension of providing fully-qualified names for function-local
statics -- but it improves the situation at least.
It's possible a better solution might be to rewrite the name
canonicalizer. I was wondering if this could perhaps be done without
reference to the grammar -- just by examining the tokens. However,
that's much more involved.
Let me know what you think.
Regression tested on x86-64 Fedora 40.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32939 Reviewed-By: Keith Seitz <keiths@redhat.com>
Nick Alcock [Mon, 12 May 2025 11:31:00 +0000 (12:31 +0100)]
libctf: archive, open: when opening, always set errp to something
ctf_arc_import_parent, called by the cached-opening machinery used by
ctf_archive_next and archive-wide lookup functions like
ctf_arc_lookup_symbol, has an err-pointer parameter like all other opening
functions. Unfortunately it unconditionally initializes it whenever
provided, even if there was no error, which can lead to its being
initialized to an uninitialized value. This is not technically an
API-contract violation, since we don't define what happens to the error
value except when an error happens, but it is still unpleasant.
Initialize it only when there is an actual error, so we never initialize it
to an uninitialized value.
While we're at it, improve all the opening pathways: on success, set errp to
0, rather than leaving it what it was, reducing the likelihood of
uninitialized error param returns in callers too. (This is inconsistent
with the treatment of ctf_errno(), but the err value being a parameter
passed in from outside makes the divergence acceptable: in open functions,
you're never going to be overwriting some old error value someone might want
to keep around across multiple calls, some of which are successful and some
of which are not.)
Soup up existing tests to verify all this.
Thanks to Bruce McCulloch for the original patch, and Stephen Brennan for
the report.
libctf/
PR libctf/32903
* ctf-archive.c (ctf_arc_open_internal): Zero errp on success.
(ctf_dict_open_sections): Zero errp at the start.
(ctf_arc_import_parent): Intialize err.
* ctf-open.c (ctf_bufopen): Zero errp at the start.
* testsuite/libctf-lookup/add-to-opened.c: Make sure one-element
archive opens update errp.
* testsuite/libctf-writable/ctf-compressed.c: Make sure real archive
opens update errp.
when doing load store switching it wrongly adjusts the address of the
R_SH_USES reloc and not the actual offset from that instruction. This is
an issue if the pc-relative function call relaxation gets done in a
later pass wich will result in overriding the wrong instruction.
Alan Modra [Mon, 19 May 2025 06:37:20 +0000 (16:07 +0930)]
rs_fill_nop and md_generate_nops
Make rs_fill_nop behave like rs_fill in using a repeat count
(fr_offset) to emit fr_var length repeated nop patterns. Besides
being more elegant, this reduces memory required for large .nops
directives.
* as.h (rs_fill_nop): Update comment.
* config/tc-i386.c (i386_generate_nops): Handle rs_fill_nop as
for rs_align_code.
* config/tc-i386.h (MAX_MEM_FOR_RS_SPACE_NOP): Define.
* listing.c (calc_hex): Handle rs_fill_nop as for rs_fill.
* read.c (MAX_MEM_FOR_RS_SPACE_NOP): Define.
(s_nops): Use MAX_MEM_FOR_RS_SPACE_NOP setting up frag.
* write.c (write_contents): Call md_generate_nops for rs_fill_nop
before the fr_fix part is written, so that rs_fill_nop can be
handled as for rs_fill.
Alan Modra [Sat, 17 May 2025 05:41:14 +0000 (15:11 +0930)]
tidy target HANDLE_ALIGN
avr, kvx, metag, mn10300, nds32, v850, visium, and wasm32 targets
defined HANDLE_ALIGN without defining MAX_MEM_FOR_RS_ALIGN_CODE. This
can result in a rather large chunk of memory being allocated. Fix
that by a combination of changing the default allocation to one byte
and/or defining a target MAX_MEM_FOR_RS_ALIGN_CODE.
arm wanted to write out the entire set of nops, and limited allowed
code alignment to 64 bytes to prevent large memory allocations.
Fix that by making use of the fact that rs_align_code frags repeat
fr_var bytes at fr_literal + fr_fix to fill out the required area.
Fix metag, nds32 and kvx too, which it seems copied either arm or x86
in similarly not making use of repeating patterns.
It's worth mentioning that my tidy to kvx changed the order of nop
bundles, placing a short bundle first rather than last.
epiphany was totally broken in that uninitialised data was written out
for any alignment requiring more than three bytes of fill.
ppc created a new frag to handle a branch over a large number of nops.
This saves 4 bytes per rs_align_code frag, and most times the branch
isn't used so it is generally a win for memory usage, but I figured
the extra code complexity wasn't worth it. So that code of mine goes.
visium copied the same scheme, so that goes too.
This leaves x86 as the only target making large allocations for
alignment frags.
* frags.c (MAX_MEM_FOR_RS_ALIGN_CODE): Default to 1.
* config/tc-aarch64.c (aarch64_handle_align): Remove always true
condition.
* config/tc-aarch64.h (MAX_MEM_FOR_RS_ALIGN_CODE): Move to be
adjacent to HANDLE_ALIGN define.
* config/tc-arm.c (arm_handle_align): Allow alignment of more
than MAX_MEM_FOR_RS_ALIGN_CODE bytes. Just write one repeat
of nop pattern to frag.
(arm_frag_align_code): Delete function.
* config/tc-arm.h (MAX_MEM_ALIGNMENT_BYTES): Don't define.
(MAX_MEM_FOR_RS_ALIGN_CODE): Set to 7.
(md_do_align): Don't define.
(arm_frag_align_code): Don't declare.
* config/tc-epiphany.c (epiphany_handle_align): Correct frag
so that nop_pattern repeats rather than random data.
* config/tc-epiphany.h (MAX_MEM_FOR_RS_ALIGN_CODE): Define.
* config/tc-kvx.c (kvx_make_nops): Merge into..
(kvx_handle_align): ..here. Put short nop bundle first,
followed by repeated full nop bundle.
* config/tc-kvx.h (MAX_MEM_FOR_RS_ALIGN_CODE): Define.
* config/tc-m32c.h (HANDLE_ALIGN, MAX_MEM_FOR_RS_ALIGN_CODE):
Don't define.
* config/tc-metag.c (metag_handle_align): Just write one
repeat of nop pattern to frag.
* config/tc-metag.h (MAX_MEM_FOR_RS_ALIGN_CODE): Define.
* config/tc-nds32.c (nds32_handle_align): Just write one
repeat of nop pattern to frag.
* config/tc-nds32.h (MAX_MEM_FOR_RS_ALIGN_CODE): Define.
* config/tc-ppc.c (ppc_handle_align): Don't make a new frag
for branch.
* config/tc-ppc.h (MAX_MEM_FOR_RS_ALIGN_CODE): Increase to 8.
* config/tc-visium.c (visium_handle_align): Don't make a new
frag for branch.
* config/tc-visium.h (MAX_MEM_FOR_RS_ALIGN_CODE): Define.
* config/tc-wasm32.h (HANDLE_ALIGN): Don't define.
* testsuite/gas/epiphany/nop.d,
* testsuite/gas/epiphany/nop.s: New test.
* testsuite/gas/epiphany/allinsn.exp: Run it.
* testsuite/gas/kvx/nop-align.d: Adjust.
Andrew Burgess [Wed, 21 May 2025 09:16:08 +0000 (10:16 +0100)]
gdb: reorder checks in validate_exec_file
While reviewing another patch I was briefly confused by a call to
target_pid_to_exec_file coming from validate_exec_file while attaching
to a process when I had not previously set an executable.
The current order of actions in validate_exec_file is:
1. Get name of current executable.
2. Get name of executable from the current inferior.
3. If either of (1) or (2) return NULL, then there's nothing to
check, early return.
I think it would be cleaner if we instead did this:
1. Get name of current executable.
3. If (1) returned NULL then there's nothing to check, early return.
3. Get name of executable from the current inferior.
4. If (3) returned NULL then there's nothing to check, early return.
This does mean there's an extra step, but I don't think the code is
any more complex really, and we now avoid trying to extract the name
of the executable from the current inferior unless we really need it.
This avoids the target_pid_to_exec_file call that I was seeing, which
for remote targets does avoid a packet round trip (not that I'm
selling this as an "optimisation", just noting the change).
There should be no user visible changes after this commit.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom Tromey [Thu, 22 May 2025 16:46:50 +0000 (10:46 -0600)]
Ensure cooked_index_entry self-tests are run
While looking at code coverage for gdb, I noticed that the
cooked_index_entry self-tests were not run. I tracked this down to a
formatting error in cooked-index-entry.c.
I suspect it might be better to use a macro to define these
initialization functions. That would probably remove the possibility
for this kind of error.
gprofng: fix 32892 source line level information not available with "-g -O2"
gprofng did not read the .debug_rnglists section for dwarf-5.
Another problem was that gprofng ignored DW_AT_abstract_origin
As a result, gprofng skiped Dwarf for all functions declared as:
<1><e18b>: Abbrev Number: 43 (DW_TAG_subprogram)
<e18c> DW_AT_abstract_origin: <0xe168>
<e190> DW_AT_linkage_name: _ZN10Bool_ArrayD2Ev
gprofng/ChangeLog
2025-05-19 Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
PR 32892
* src/Dwarf.cc: Read the .debug_rnglists section.
Support DW_AT_abstract_origin.
* src/Dwarf.h: Likewise.
* src/DwarfLib.cc: Likewise.
* src/DwarfLib.h: Likewise.
* src/LoadObject.cc (dump_functions): Print mangled names for aliases.
* src/Stabs.cc (fixSymtabAlias): Set 'alias' correctly.
* src/Symbol.cc (find_symbols): Add argument where to collect symbols.
* src/Symbol.h: Likewise.
* NEWS: Mention new extensions.
* config/tc-riscv.c (enum riscv_csr_class): New CSR class.
(riscv_csr_address): Add support for Ssccfg.
* testsuite/gas/riscv/csr-version-1p10.d: New test for Ssccfg CSR.
* testsuite/gas/riscv/csr-version-1p10.l: New warning for Ssccfg CSR.
* testsuite/gas/riscv/csr-version-1p11.d: New test for Ssccfg CSR.
* testsuite/gas/riscv/csr-version-1p11.l: New warning for Ssccfg CSR.
* testsuite/gas/riscv/csr-version-1p12.d: New test for Ssccfg CSR.
* testsuite/gas/riscv/csr-version-1p12.l: New warning for Ssccfg CSR.
* testsuite/gas/riscv/csr-version-1p13.d: New test for Ssccfg CSR.
* testsuite/gas/riscv/csr-version-1p13.l: New warning for Ssccfg CSR.
* testsuite/gas/riscv/csr.s: New Ssccfg CSR.
* testsuite/gas/riscv/imply.d: New imply check.
* testsuite/gas/riscv/imply.s: New implies.
* testsuite/gas/riscv/march-help.l: New helping info.
include/ChangeLog:
* opcode/riscv-opc.h (CSR_SCOUNTINHIBIT): New CSR address.
(DECLARE_CSR): Add Ssccfg CSR.
Alan Modra [Wed, 21 May 2025 08:27:04 +0000 (17:57 +0930)]
ubsan: integer overflow in tc-i386.c:offset_in_range
or $9223372036854775808,%eax
runtime error: negation of -9223372036854775808 cannot be represented
in type 'offsetT' (aka 'long'); cast to an unsigned type to negate
this value to itself
* config/tc-i386.c (offset_in_range): Avoid signed overflow.
Tom Tromey [Thu, 10 Apr 2025 15:01:06 +0000 (09:01 -0600)]
Minor spelling fixes in gdb directory
I ran codespell on the gdb directory and fixed a number of minor
problems. In a couple cases I replaced a "gdb spelling" (e.g.,
"readin") with an English one ("reading") where it seemed harmless.
Tom de Vries [Tue, 20 May 2025 09:10:29 +0000 (11:10 +0200)]
[gdb/testsuite] Fix gdb.dwarf2/dw-form-strx-out-of-bounds.exp with make-check-all.sh
I forgot to run test-case gdb.dwarf2/dw-form-strx-out-of-bounds.exp with
make-check-all.sh, and consequently failed to notice that it fails with for
instance target board fission-dwp.
The test-case does:
...
source $srcdir/$subdir/dw-form-strx.exp.tcl
...
and in that tcl file, prepare_for_testing fails, so a -1 is returned, but
that is ignored by the source command.
Fix this by using require, but rather that testing the result of the source
command, communicate success by setting a global variable
prepare_for_testing_done.
Likewise in gdb.dwarf2/dw-form-strx.exp.
Also, the test-case gdb.dwarf2/dw-form-strx-out-of-bounds.exp fails for target
board readnow, because the DWARF error occurs during a different command than
expected.
Fix this by just skipping the test-case in that case.
Tested on x86_64-linux.
Reported-by: Simon Marchi <simark@simark.ca> Approved-By: Tom Tromey <tom@tromey.com>
projects / thirdparty / binutils-gdb.git / log
