Pedro Alves [Mon, 18 May 2026 13:20:00 +0000 (14:20 +0100)]
Sync thread state after infcalls with "set unwind-on-* on" (PR gdb/34148)
Commit 519774805a1 ("Don't pretend infcalls don't set the inferior
running (PR gdb/34082)") removed the special case in proceed that
skipped set_state(THREAD_RUNNING) for infcalls. That fixed
gdb.threads/hand-call-new-thread.exp, but introduced a regression in
gdb.compile/compile.exp:
...
set unwind-on-signal on
(gdb) PASS: gdb.compile/compile.exp: set unwind-on-signal on
compile code *(volatile int *) 0 = 0;
The program being debugged received signal SIGSEGV, Segmentation fault
while in a function called from GDB. GDB has restored the context
to what it was before the call. To change this behavior use
"set unwind-on-signal off". Evaluation of the expression containing
the function (_gdb_expr) will be abandoned.
(gdb) PASS: gdb.compile/compile.exp: compile code segfault second
break 132
Breakpoint 2 at 0x555555555262: file .../compile.c, line 132.
(gdb) continue
Cannot execute this command while the selected thread is running.
(gdb) FAIL: gdb.compile/compile.exp: continue to breakpoint: break-here
The "compile code" command before the FAIL is an infcall under the
hood. That hits SIGSEGV with "set unwind-on-signal on" in effect, so
GDB unwinds and abandons the call. After that, "continue" is rejected
because the thread is still marked THREAD_RUNNING from the proceed
that started the infcall.
When an infcall is unwound due to a signal, timeout, or terminating
exception, call_thread_fsm::should_notify_stop returns false, and so
normal_stop is not called from fetch_inferior_event. normal_stop is
what would normally call finish_thread_state to sync the public thread
state back to THREAD_STOPPED. run_inferior_call has a fallback
finish_thread_state call for that purpose, but it is gated on
stop_stack_dummy == STOP_STACK_DUMMY, which is only true for
successful calls.
Before the commit mentioned above, proceed never marked an infcall's
thread as THREAD_RUNNING, so the missing RUNNING => STOPPED transition
was harmless. The old comment in infcall.c about the
finish_thread_state call claimed "If the infcall does NOT succeed,
normal_stop will have already finished the thread states", but that
was already incorrect for the unwind paths. It just happened to not
matter.
Fix this by dropping the STOP_STACK_DUMMY guard and updating the
comment to describe the actual rule: sync regardless of how the call
ended. The !was_running check is kept since it is there to exclude
the in-cond-eval case, where the thread is meant to stay marked
running. finish_thread_state is idempotent, so the call is harmless
on paths where normal_stop also ran.
Extend gdb.base/unwindonsignal.exp to exercise the "set
unwind-on-signal on" path without having to rely on the "compile code"
feature. Without the fix, the test fails like so:
info threads
Id Target Id Frame
* 1 Thread 0x7ffff7f8f740 (LWP 239019) "unwindonsignal" (running)
(gdb) FAIL: gdb.base/unwindonsignal.exp: thread is stopped
continue
Cannot execute this command while the selected thread is running.
(gdb) FAIL: gdb.base/unwindonsignal.exp: continue until exit at after unwound infcall
Similarly, extend gdb.cp/gdb2495.exp for "set
unwind-on-terminating-exception on", and gdb.base/infcall-timeout.exp
for "set unwind-on-timeout on". Both would fail without the code fix,
too.
With the fix, gdb.compile/compile.exp now passes cleanly.
Tested on x86_64-unknown-linux-gnu.
Approved-By: Andrew Burgess <aburgess@redhat.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34148
Change-Id: Idef0dcd4dd751b501869c58b752f77d4dadb6c72
Alan Modra [Tue, 26 May 2026 07:40:34 +0000 (17:10 +0930)]
aarch64 core: use of uninitialised value
Commit d0ff5ca959df adding PT_AARCH64_MEMTAG_MTE support, creates
a section that stores "p_memsz" from the program header in "rawsize"
and "p_filesz" in "size". p_memsz is the memory range, usually 32
times p_filesz. This can be a problem when bfd reads the section, for
example to display it with objdump -s, as the usual (linker) meaning
for input section "rawsize" is the original size on disk. Memory is
allocated to read the larger of "size" and "rawsize". So 32 times the
memory is allocated than what is really needed.
With fuzzed input "rawsize" can be smaller than "size" since the
header values are not sanity checked. This results in section
contents with only the first "rawsize" bytes read from disk, the rest
being uninitialised. Of course fuzzed input can also have very large
"rawsize" which might result in OOM.
One way of fixing this is to move the p_memsz value somewhere else
(output_offset would work, I think). Another might be to qualify use
of rawsize by is_linker_input, but I haven't checked over all the bfd
code using rawsize. And then there is this approach:
Pedro Alves [Thu, 21 May 2026 14:33:16 +0000 (15:33 +0100)]
Adjust gdb.base/exitsignal.exp for Cygwin
Cygwin has this feature where if the program is about to die with a
signal, and there's a debugger attached, it raises a SIGTRAP via
DebugBreak. So if you try to pass a terminating signal to the
inferior, you see that SIGTRAP first, before the process exits with
the signal. E.g.:
Thread 1 "segfault" received signal SIGSEGV, Segmentation fault.
0x0000000100401092 in main () at segfault.cc:5
5 *(volatile int *)0;
(gdb) c
Continuing.
Thread 1 "segfault" received signal SIGTRAP, Trace/breakpoint trap.
0x00007ffe99d35a13 in KERNELBASE!DebugBreak () from C:/WINDOWS/System32/KERNELBASE.dll
(gdb) bt
#0 0x00007ffe99d35a13 in KERNELBASE!DebugBreak () from C:/WINDOWS/System32/KERNELBASE.dll
#1 0x00007ffe896163b7 in break_here () at /usr/src/debug/cygwin-3.6.9-1/winsup/cygwin/dcrt0.cc:473
#2 0x00007ffe8962fe13 in try_to_debug () at /usr/src/debug/cygwin-3.6.9-1/winsup/cygwin/exceptions.cc:599
#3 exception::handle (e=0x7ffffc9b0, frame=<optimized out>, in=0x7ffffc4c0, dispatch=<optimized out>) at /usr/src/debug/cygwin-3.6.9-1/winsup/cygwin/exceptions.cc:812
#4 0x00007ffe9c5e63df in ntdll!.chkstk () from C:/WINDOWS/SYSTEM32/ntdll.dll
#5 0x00007ffe9c499497 in ntdll!RtlLocateExtendedFeature () from C:/WINDOWS/SYSTEM32/ntdll.dll
#6 0x00007ffe9c5e5d1e in ntdll!KiUserExceptionDispatcher () from C:/WINDOWS/SYSTEM32/ntdll.dll
#7 0x0000000100401092 in main () at segfault.cc:5
(gdb) c
Continuing.
...
[Inferior 1 (process 8032) exited with code 05400]
(gdb)
gdb.base/exitsignal.exp fails on Cygwin partly because it doesn't take
that into account. This commit fixes it.
In addition, the typical adjustement for the fact that all programs
are multi-threaded on Windows is also necessary.
gdb.base/exitsignal.exp still won't pass cleanly on Cygwin yet.
That'll be finally fixed in a following patch.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I2d18e2604afe3a4f80987848e2c1cd307ed43401
commit-id: 013964ce
Pedro Alves [Wed, 20 May 2026 13:48:03 +0000 (14:48 +0100)]
Fix "set cwd ..." on Cygwin, part 2
Even after the previous patch, on both native and gdbserver Cygwin, we
get:
(gdb) set cwd /cygdrive/d/cygwin-gdb/build-testsuite/outputs/gdb.base/exitsignal
(gdb) start
Temporary breakpoint 3 at 0x100401094: file /home/alves/rocm/gdb/src/gdb/testsuite/gdb.base/segv.c, line 26.
Starting program: /cygdrive/d/cygwin-gdb/build-testsuite/outputs/gdb.base/exitsignal/exitsignal.exe
❌️ Error creating process /cygdrive/d/cygwin-gdb/build-testsuite/outputs/gdb.base/exitsignal/exitsignal.exe (error 6): The handle is invalid.
(gdb)
On the native side, this is because in
windows_nat_target::create_inferior, we unconditionally convert
forward slashes to backward slashes:
and then cygwin_conv_path(CCP_POSIX_TO_WIN_W) does nothing on such
path, as the backward slashes make the path not look like a Unix-style
path.
CreateProcess then fails to CD into that directory, as that's not a
real Windows native path.
The fix is to not do the slashes replacement on Cygwin.
On the GDBserver side, we're just completely missing the
cygwin_conv_path logic. This commit adds it. The code isn't shared
with GDB because GDB uses wide chars, and GDBserver uses narrow char.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I004f2a562757a566423f6acb9aecfcc1a7f2f746
commit-id: 85aa8c22
Pedro Alves [Wed, 20 May 2026 12:14:07 +0000 (13:14 +0100)]
Fix "set cwd ..." on Cygwin, part 1
When running gdb.base/exitsignal.exp on Cygwin, we see:
(gdb) set cwd /cygdrive/d/cygwin-gdb/build-testsuite/outputs/gdb.base/exitsignal
(gdb) run
Starting program: /cygdrive/d/cygwin-gdb/build-testsuite/outputs/gdb.base/exitsignal/exitsignal
Error converting inferior cwd: 28
(gdb) FAIL: gdb.base/exitsignal.exp: runto: run to main
28 is ENOSPC. But this isn't really literally no space left, though.
cygwin_conv_path documentation mentions that error code.
According to the Cygwin API documentation for cygwin_conv_path, the
function fails with ENOSPC ("No space left on device") when the size
of the destination buffer is smaller than what is required for the
conversion. See:
If we look closely at how the buffer size argument is being passed, we
see we have two problems here:
1) Incorrectly passing down the input buffer size instead of the
output size.
The code passes strlen(inferior_cwd) as the size of the destination
buffer (infcwd). However, the target Windows path format
(e.g. "D:\cygwin-gdb\..." in my case) could be longer or shorter than
the POSIX source path ("/cygdrive/d/..."). In my specific case, the
source string is 64 characters, while the target Windows string is 61
(wide) characters (and twice as many bytes).
2) Incorrectly passing character count instead of byte count
The conversion target token is CCP_POSIX_TO_WIN_W. The _W means that
the destination buffer infcwd takes wide characters (wchar_t). The
documentation states that the size argument is in bytes, not
characters.
This commit fixes it, by passing the byte size of the destination
buffer.
Approved-By: Tom Tromey <tom@tromey.com>
Change-Id: I70af6ef394f48da35ccc2e04ef764915e09e59de
commit-id: 66c930c2
H.J. Lu [Fri, 22 May 2026 21:12:10 +0000 (05:12 +0800)]
alpha: Properly handle local weak undefined symbols
Since the local undefined TLS symbol address isn't mapped to any TLS
storage, it isn't usable. Set its value to 0 to avoid relocation overflow.
When processing TLS relocations, elf_hash_table (info)->tls_sec can be
NULL if all TLS symbols are weak, hidden and undefined. Don't assert
elf_hash_table (info)->tls_sec != NULL. Always set dtp_base and tp_base
to 0 if elf_hash_table (info)->tls_sec == NULL.
bfd/
PR ld/34165
* * elf-bfd.h (elf_link_local_undefweak_p): New function.
* elf64-alpha.c (elf64_alpha_relax_got_load): Set dtp_base and
tp_base to 0 if elf_hash_table (info)->tls_sec == NULL.
(elf64_alpha_relocate_section): Set the local undefined TLS
symbol value to 0. Don't assert elf_hash_table (info)->tls_sec
!= NULL.
hppa*64*-*-hpux*: Create dummy milli.a archive for binutils and ld tests
On cross builds, this fixes numerous test fails due to the milli.a
archive not being found. On hppa*64*-*-hpux*, this fixes four fails
due to undefined symbols in the HP milli.a archive.
The change to objcopy.exp causes the "objcopy executable (pr25662)"
test to fail. Previously, it didn't run because milli.a wasn't
found.
2026-05-24 John David Anglin <danglin@gcc.gnu.org>
binutils/ChangeLog:
* testsuite/binutils-all/objcopy.exp: Append LDFLAGS to
ldflags for milli.a archive.
* testsuite/config/default.exp: Create dummy milli.a archive
in tmpdir/hppa. Append " -Ltmpdir/hppa" to LDFLAGS.
Alan Modra [Sun, 24 May 2026 04:55:10 +0000 (14:25 +0930)]
"eqv involving dot" gas test and pdp11
This changes the eqv-dot test to make 'x' non-zero, so targets
(eg. i386-darwin) that silently ignore the value of fx_subsy in fixups
will fail the test.
When updating the test I noticed seriously odd expected output for
pdp11. Why should .long write in different byte order when needing
fixups (".long z")? That appears to be a bug in the pdp11
md_apply_fix, which reads section contents using pdp-endian but writes
them little-endian. Fix that too.
gas/
* config/tc-pdp11.c (md_apply_fix): Use md_mumber_to_chars.
* testsuite/gas/all/eqv-dot.s: Move x to second .long.
* testsuite/gas/all/eqv-dot.d: Update expected result.
* testsuite/gas/all/eqv-dot-pdp11.d: Likewise, and use
source directive.
* testsuite/gas/all/eqv-dot-pdp11.s: Delete.
* testsuite/gas/all/gas.exp (do_930509a): Support pdp11.
* testsuite/gas/all/simple-forward.d: Likewise.
Alan Modra [Sun, 24 May 2026 04:54:36 +0000 (14:24 +0930)]
mips: section .note.gnu.build-id can't be allocated in segment
After modifying test_build_id_debuglink, mips-linux fails
objcopy --only-keep-debug tmpdir/testprog tmpdir/testprog.debug
with the above error. This is due to .MIPS.abiflags and .reginfo
being made SHT_NOBITS but the .note.gnu.build-id which follows is
SHT_PROGBITS. That tickles a bug in file offset tracking.
* elf.c (assign_file_positions_for_load_sections): When
resetting "off_adjust" for sections with contents following
sections without contents, put back "off" too.
Alan Modra [Sun, 24 May 2026 04:53:03 +0000 (14:23 +0930)]
binutils test_build_id_debuglink
I saw build-id-debuglink test failures on a number of targets due to
deleting the glibc source I'd used when building the cross toolchains.
The tests failed with timeouts.
$ objdump -Sl tmpdir/testprog.strip
tmpdir/testprog.strip: file format elf64-ia64-little
I don't know where debuginfod was looking to try to find that crti.S
source, but wherever it was took too long, and we aren't trying to
test debuginfod here. So remove the extraneous source dependencies
by building with -shared -nostdlib. When making this change I also
removed the code passing extra options to target_compile via
CFLAGS_FOR_TARGET. Instead, pass extra options with additional_flags.
* testsuite/binutils-all/objdump.exp (test_build_id_debuglink):
Don't use CFLAGS_FOR_TARGET to pass extra options to
target_compile. Instead use additional_flags. Build test
with -fPIC -shared -nostdlib.
Hannes Domani [Thu, 7 May 2026 18:54:54 +0000 (20:54 +0200)]
Implement native TLS support on Windows
GCC 16 introduced native TLS variables on Windows, so this adds
debugger support for them.
The fetch_tls_load_module_address gdbarch method is used to get the
address of _tls_index of the OBJFILE, which is then forwarded as LM_ADDR
to windows_get_thread_local_address.
The TLS slot for a module can be found in
TIB->thread_local_storage[_tls_index].
Tom de Vries [Sat, 23 May 2026 08:16:57 +0000 (10:16 +0200)]
[gdb/testsuite] Improve dwarf assembly for implicit const in .debug_names
In the assembly generated for the dwarf assembly for test-case
gdb.dwarf2/debug-names-tu-dwarf5.exp, there's this bit:
...
.uleb128 0x2003 /* DW_IDX_GNU_language */
.uleb128 0x21 /* DW_FORM_flag_present */
.sleb128 0x0002 /* DW_LANG_C */
...
As it happens, 0x21 is not DW_FORM_flag_present, but DW_FORM_implicit_const,
and the following entry is the value of the attribute.
Fix the comment, and make the relation between the two entries more clear by
printing:
...
.uleb128 0x2003 /* DW_IDX_GNU_language */
.uleb128 0x21 /* DW_FORM_implicit_const */
.sleb128 0x0002 /* DW_FORM_implicit_const value: DW_LANG_C */
...
Tom de Vries [Sat, 23 May 2026 08:16:57 +0000 (10:16 +0200)]
[gdb/testsuite] Add missing type_offset in gdb.dwarf/debug-names-tu.exp
With llvm-dwarfdump we run into this warning:
...
$ llvm-dwarfdump -a -v debug-names-tu > /dev/null
warning: DWARF type unit at offset 0x00000028 has its relocated type_offset \
0x00000028 pointing inside the header
...
Fix this by adding the missing type_offset.
Likewise in gdb.dwarf2/debug-names-bad-cu-index.exp.
Approved-By: Tom Tromey <tom@tromey.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34157
Tom de Vries [Fri, 22 May 2026 13:11:56 +0000 (15:11 +0200)]
[binutils] Handle implicit const in .debug_names
I tried to use readelf to read the debug info for gdb test-case
gdb.dwarf2/debug-names-tu-dwarf5.exp, and ran into some trouble reading the
.debug_names section:
...
$ readelf -w debug-names-tu-dwarf5
...
Symbol table:
[ 1] #eddb6232 _start: <0><1> DW_TAG_subprogram DW_IDX_compile_unit=0
[ 2] #53a2ae86 struct_with_int_member:readelf: Warning: Unrecognized form: 0
readelf: Error: end of data encountered whilst reading LEB
<0x3><2> DW_TAG_structure_type DW_IDX_type_unit=0 DW_IDX_GNU_language=0 DW_IDX_type_unitreadelf: Warning: Unrecognized form: 0
readelf: Error: end of data encountered whilst reading LEB
...
The .debug_names abbreviation table contains a DW_FORM_implicit_const entry:
...
.uleb128 0x2003 /* DW_IDX_GNU_language */
.uleb128 0x21 /* DW_FORM_implicit_const */
.sleb128 0x0002 /* DW_FORM_implicit_const value: DW_LANG_C */
...
and readelf doesn't support this.
Add support for DW_FORM_implicit_const in display_debug_names, such that we
get the expected:
...
Symbol table:
[ 1] #eddb6232 _start: <0><1> DW_TAG_subprogram DW_IDX_compile_unit=0
[ 2] #53a2ae86 struct_with_int_member: <0x3><2> DW_TAG_structure_type \
DW_IDX_type_unit=0 DW_IDX_GNU_language=2
...
Jan Beulich [Fri, 22 May 2026 06:49:12 +0000 (08:49 +0200)]
x86/disasm: rework comment handling
Model this after operand handling, such that comments can be emitted in
the same order as operands. %rip-relative address comments remain
separate for now. While there correct style for the symbols associated
with immediates: These aren't "comment starts", but symbol names.
The thread_local g_complaint_interceptor pointer is unnecessary. The
complaint_interceptor constructor registers itself as the warning hook
via m_saved_warning_hook (this), so when complaint_internal dispatches
through the warning hook, it lands in complaint_interceptor::warn with
'this' already pointing at the registered interceptor. Inside warn,
g_complaint_interceptor and 'this' always refer to the same object.
Replace g_complaint_interceptor->m_complaints with m_complaints in
complaint_interceptor::warn and remove g_complaint_interceptor.
Change-Id: I75565a5f2c0e51363f36be0e3544210c10bb5491 Approved-By: Tom Tromey <tom@tromey.com>
Location is heap block of size 32 at 0x7208000004c0 allocated by main thread:
...
#11 __static_initialization_and_destruction_0 /home/simark/src/binutils-gdb/gdb/complaints.c:31 (gdb+0xf0df04) (BuildId: 5d682ab96882c738940aae3c2c67270d969f113f)
...
This points to clear_complaints touching the global counters map in the
main thread without holding a lock, while a background thread touched
the map in a worker thread while holding the lock complaint_mutex.
Fix this by holding the complaint_mutex lock in clear_complaints.
Change-Id: Id2fe442486bcdf57156aacdc9253055702c07600 Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Wed, 20 May 2026 18:19:29 +0000 (14:19 -0400)]
gdb/dwarf: track whether the all_units vector is sorted
dwarf2_find_unit performs a binary search over dwarf2_per_bfd::all_units
using std::lower_bound. Before calling std::lower_bound, we want to
make sure that all_units is properly sorted.
Track the state of whether all_units is considered sorted with a new
dwarf2_per_bfd::all_units_sorted flag, and assert it in
dwarf2_find_unit. This will help catch bugs where we call
dwarf2_find_unit on a non sorted vector, for instance what was fixed by
commit fbaef7de7701 ("gdb/dwarf: fix order of operations when reading
.debug_names").
The flag is set:
- At initialization time, because an empty vector is sorted.
- Whenever the vector is cleared, for the same reason (added a helper
method for that).
- After sorting in dwarf2_per_bfd::sort_all_unit.
It is cleared:
- Whenever a unit is appended to all_units. To keep this centralized,
add a new dwarf2_per_bfd::add_unit helper that appends the unit and
clears the flag.
- Whenever a unit's sort key (section / sect_off) is modified. That is
done in dwarf2_per_cu::set_section and dwarf2_per_cu::set_sect_off.
Note that the flag is cleared when appending a unit even if the vector
would by chance already be sorted. This is good, because it will catch
mistakes even on machines where a problem with std::lower_bound would
not manifest.
I checked that this patch would have caught the problem before commit fbaef7de7701:
Simon Marchi [Wed, 20 May 2026 18:19:28 +0000 (14:19 -0400)]
gdb/dwarf: change signatured_type_up to include deleter type
We currently have:
using dwarf2_per_cu_up = std::unique_ptr<dwarf2_per_cu, dwarf2_per_cu_deleter>;
using signatured_type_up = std::unique_ptr<signatured_type>;
Meaning that it's not possible to pass a signatured_type_up as a
dwarf2_per_cu_up, even though the target types are related (it is
possible to pass a `signatured_type *` as a `dwarf2_per_cu *`).
If we give signatured_type_up the same deleter as dwarf2_per_cu_up, then
it becomes possible to pass a signatured_type_up as a dwarf2_per_cu_up.
This lets us avoid releasing a signatured_type_up only to create a
dwarf2dwarf2_per_cu_up immediately after in some spots. The only
downside is that we can't use make_unique anymore, but it's already the
case for dwarf2_per_cu_up.
Swap the order of things in add_type_unit so that we don't need a
special holder variable.
Change-Id: Iee34e5d1711d601297f109e58cbaeccb5a0c6cde Approved-By: Tom Tromey <tom@tromey.com>
Tom Tromey [Fri, 8 May 2026 16:44:59 +0000 (10:44 -0600)]
Add test for Ada abs operator
There were no tests in the tree for the Ada abs operator, meaning that
ada_abs was never invoked during a test. This patch adds a new basic
test for this.
Note that operator overloading of 'abs' does seem to be tested.
Tom Tromey [Fri, 8 May 2026 16:36:26 +0000 (10:36 -0600)]
Test attributes with array types
While looking at code coverage, I noticed that gdb was not testing the
case where certain attributes were applied to an Ada array type. This
patch adds some new tests, improving the coverage.
gdb/amd-dbgapi-target: suppress a repeated stop request
Sending a second stop request to an AMD GPU thread before fetching the
event caused by the first request leads to an error:
wave_stop for wave_1 failed (The wave has an outstanding stop request)
Prevent sending a new stop request if there already is an outstanding
one. The fix is in amd_dbgapi_target::stop.
A regression test is included. The test uses non-stop mode and
executes the "interrupt" command twice, because in non-stop mode this
command uses the 'stop' target op, where the fix is applied.
To be able to execute two interrupt commands repeatedly, we define a
user command.
Approved-by: Lancelot Six <lancelot.six@amd.com> (amdgpu)
Abdul Basit Ijaz [Tue, 21 Apr 2026 12:05:08 +0000 (14:05 +0200)]
gdb: Handle TYPE_CODE_COMPLEX in valprint_check_validity.
When printing complex variables with partial optimization (e.g., real
part available but imaginary part optimized out), GDB incorrectly
reported the entire variable as <optimized out> instead of showing
available components.
The issue occurred because valprint_check_validity () only exempted
TYPE_CODE_UNION, TYPE_CODE_STRUCT, and TYPE_CODE_ARRAY from the
optimized-out check, but not TYPE_CODE_COMPLEX. Complex types, like
structures, can have components with different availability.
A new test gdb.dwarf2/complex-partial-optimized.exp is added to verify
that available components are printed.
Simon Marchi [Tue, 19 May 2026 20:09:10 +0000 (16:09 -0400)]
gdb/dwarf: fix order of operations when reading .debug_names
PR 34149 shows a hard to reproduce bug. The problem sometimes shows up
when reading in `.debug_names` indexes, manifesting like this:
warning: Section .debug_names has incorrect entry in CU table, ignoring .debug_names.
This is shown even if the CU table in the `.debug_names` index
accurately describes the CUs in `.debug_info`.
The bug was introduced by commit b301725d35c1 ("gdb/dwarf: read foreign
type units").
Before b301725d35c1, the steps when reading a `.debug_names` index were:
- call to create_all_units(), which filled the all_units vector by
walking `.debug_info` and `.debug_types` and then sorted it.
- calls to the build_and_check_* functions, which do some lookups in
the all_units vector, requiring it to be sorted.
- call to create_all_units(), which still fills the all_units vector by
walking `.debug_info` and `.debug_types`, but does not sort it
anymore.
- calls to the build_and_check_* functions, which do some lookups in
the all_units vector, requiring it to be sorted (oops)
- call to create_foreign_type_units_from_debug_names(), which may add
more items to the all_units vector.
- call to finalize_all_units(), to sort the all_units vector.
The sorting of all_units was taken out of create_all_units() on purpose.
Because create_foreign_type_units_from_debug_names() may add more units,
we don't want to sort the vector in create_all_units() and then sort it
again after create_foreign_type_units_from_debug_names().
The build_and_check_* functions do some lookups in the all_units vector
with dwarf2_find_unit(), which uses std::lower_bound() internally. This
happens while the all_units vector is not sorted according to the
sorting key, all_units_less_than(), which makes the search is bogus.
dwarf2_find_unit() might return "no such unit", when in fact, the given
unit exists. This leads build_and_check_cu_list_from_debug_names() to
believe that the list of CUs in the `.debug_names` index does not match
what was built from reading `.debug_info`, we get the warning shown
above, and the index gets wrongfully rejected.
For this bug to show up, it is necessary to have type units in a
`.debug_types`. The sorting key for units in the all_units vector is:
section pointer (`dwarf2_section_info *`), then the unit offset into the
section. If all units come from the same section (`.debug_info`), then
this bug does not show up because the all_units vector happens to be
sorted after create_all_units() runs. This happens for example in test
gdb.dwarf2/debug-names-bad-cu-index.exp, where we create some DWARF 4
units and then a `.debug_names` index out of them.
It must also happen for the `dwarf2_section_info *` for `.debug_types`
to be "less than" the `dwarf2_section_info *` for `.debug_info`,
otherwise the all_units also happens to be sorted after
create_all_units() runs. This entirely depends on the whims of the
memory allocator.
I was not able to reproduce it in my normal GDB dev build, but I was
able in a "thread sanitizer" build, for some reason. Not because there
is an actual threading issue, but I think because it shuffled the memory
allocations in just the right way to make the bug happen. And then, it
only happened when doing "make check", I could not get it to reproduce
when running interactively. Using -D_GLIBCXX_DEBUG was immensely
useful, because it pointed out that the "range must be sorted"
precondition for std::lower_bound() was not met, which made the problem
immediately obvious:
Error: elements in iterator range [first, last) are not partitioned by the
predicate __comp and value __val.
Knowing all this, is it easy to artificially reproduce the problem by
making create_all_units() reverse the all_units vector, like so:
// Reverse the all_units vector
std::reverse (per_objfile->per_bfd->all_units.begin (),
per_objfile->per_bfd->all_units.end ());
The fix is to reorder the operations in this way
- call create_all_units(), to fill the all_units vector by walking
`.debug_info` and `.debug_types`
- call create_foreign_type_units_from_debug_names(), which may add more
items to the all_units vector.
- call finalize_all_units(), to sort the all_units vector.
- call the build_and_check_* functions, which is fine now that
all_units is sorted.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34149
Change-Id: I315f391605af549b55341a167683d2dc6203bcea Approved-By: Tom Tromey <tom@tromey.com>
Alan Modra [Tue, 12 May 2026 00:27:16 +0000 (09:57 +0930)]
PR 30308 again
Commit 22f8905d9f38 reintroduced pr30308. This fix resolves equates
safely in the presence of symbol loops by using symbol_equated_to.
Symbols aren't prematurely modified, by copying their value
expressions before i386_intel_simplify does its work. I figure that
it is safer to do this for all symbols rather than just the particular
case of equates, so the in_equate parameter can disappear.
Tom de Vries [Wed, 20 May 2026 06:31:12 +0000 (08:31 +0200)]
[gdb/tui] Allow second tui enable if first fails
Commit fb23d7ba4a2 ("[gdb/tui] Handle error in tui_enable") handles a
particular error during tui_enable, but doesn't allow trying to enable TUI
again.
I tried to get this to work, but didn't manage because I didn't understand the
interaction between endwin and the "tui_batch_rendering defer" destructor:
- first endwin restores the shell terminal mode
- then the defer destructor calls doupdate, restoring program terminal mode.
Fix this by checking for tui_active in ~tui_batch_rendering.
Tom de Vries [Wed, 20 May 2026 06:31:12 +0000 (08:31 +0200)]
[gdb/tui] Factor out require_tui_terminal
I noticed this bit of code in tui_enable:
...
/* Check required terminal capabilities. The MinGW port of
ncurses does have them, but doesn't expose them through "cup". */
cap = tigetstr ((char *) "cup");
if (cap == NULL || cap == (char *) -1 || *cap == '\0')
{
endwin ();
delscreen (s);
error (_("Cannot enable the TUI: "
"terminal doesn't support cursor addressing [TERM=%s]"),
gdb_getenv_term ());
}
...
At this point in tui_enable, we need endwin and delscreen to clean up after
newterm, but the check can be done before newterm.
Fix this by factoring out a new function require_tui_terminal, and moving the
code there.
Andrew Burgess [Thu, 14 May 2026 16:47:01 +0000 (17:47 +0100)]
gdb/testsuite: add a test to check for Python traits static_assert
The previous two commits added a new type trait which can be used
within a static_assert to check the properties of a struct used by GDB
to implement Python objects.
The previous commit fixed a bug in GDB which this trait check exposed.
This commit adds a new test gdb.gdb/python-traits-check.exp which
checks that every struct in the Python/ directory that inherits from
PyObject, has a suitable static_assert in place.
Adding this test should mean that if someone adds a new Python object
type to GDB, and they forget to add the static_assert, then this test
should give a failure, which should remind them to add the required
static_assert. The static_assert will then check that their new
struct is compliant.
Andrew Burgess [Thu, 14 May 2026 09:42:59 +0000 (10:42 +0100)]
gdb/python: fix use of frame_info_ptr within pending_frame_object
The previous commit added a type trait which identifies types that
should not be used within Python objects, that is, types that are not
trivially default constructible. As a result of this, it was
discovered that pending_frame_object includes a frame_info_ptr field.
The problem with frame_info_ptr is that its constructor registers the
new frame_info_ptr with the global frame_list. It is by this
registration that invalidation of frame_info_ptr objects is performed.
As Python is written in C, C++ constructors are not called, so when a
pending_frame_object is created the constructor for the nested
frame_info_ptr field is never run, and the frame_info_ptr is never
registered with the global frame_list. As a result the frame_info_ptr
will never be invalidated if the frame cache is flushed, this can then
lead to problems where we make use of the 'frame_info *' within the
frame_info_ptr, even though it is no longer valid.
In this commit I change the frame_info_ptr within pending_frame_object
to a 'frame_info_ptr *' and allocate the frame_info_ptr object on the
heap, releasing the object, and resetting the point to NULL, when we
are done with it. As the pending_frame_object only needs to remain
valid for the duration of frame_unwind_python::sniff, the 'new' and
'delete' both performed within the function.
We can now check that a pending_frame_object is valid by checking if
the 'frame_info_ptr *' is NULL or not. As the frame_info_ptr is
created in a valid state, and the point is set back to NULL when we
are done with it, we no longer need to compare the frame_info_ptr
object itself against NULL.
The remaining changes in this patch are to dereference the
'frame_info_ptr *' in places where we need the actual object. In some
cases I need to move the dereference later within a function, after a
validity check, in order to avoid dereferencing a NULL pointer.
Finally, I can add the static_assert that guarantees that
pending_frame_object is now safe for allocation by Python.
I discovered this bug while looking at PR gdb/32120. That bug is
about a user's custom frame unwinder that triggers a flush of the
frame cache during the sniffer phase (the
RemoteTargetConnection.send_packet call switches thread, which
triggers the frame cache flush). While looking at that bug I noticed
that the frame_info_ptr within the pending_frame_object wasn't being
reset when the frame cache was flushed. Fixing this does not resolve
the user's issue, but I thought it was still worth tagging this commit
with the bug link.
Andrew Burgess [Thu, 14 May 2026 08:39:57 +0000 (09:39 +0100)]
gdb/python: add type traits check for all PyObject sub-classes
All of our custom Python types are created as structs, like this:
struct some_new_type : public PyObject
{
... various fields ...
};
Then instances of this struct are created by calling PyObject_New,
either directly within GDB's C++ code, or within Python when a user's
Python script creates an instance of that class.
The problem is that Python is written in C, and PyObject_New doesn't
call any constructors for `some_new_type`, nor for any of the fields
within `some_new_type`.
If `some_new_type` is Plain Old Data (POD), then this is fine. Or, to
be more C++ specific, if `some_new_type` is trivially default
constructable, then we're fine.
But if a field within `some_new_type` has a non-trivial constructor,
then we're in trouble as that constructor will never be run.
An example of a problematic field type is frame_info_ptr. The
constructor for this type registers the new object with a central
management object, recording the `this` pointer, using this type within
`some_new_type` will not work as expected; frame invalidation will not
show up within the frame_info_ptr as you might expect.
And so, this type trait exists. Whenever a struct is created to define
a new Python type we should add a line like:
This will fail if any field of `some_new_type` are unsuitable for this
use.
We don't actually check is_trivially_default_constructible here. Some
types, e.g. ui_file_style::color, have non-trivial (or no default)
constructors, but are still safe to use within `some_new_type` because
their constructors just initialise data fields; there's nothing
"special" that the constructor does that cannot be achieved by
assigning the fields after creation with PyObject_New.
What actually matters is that the type is trivially destructible
(Python won't call C++ destructors, so destructors with side effects,
like deregistering from a list, would be skipped) and trivially
copyable (Python may copy objects with memcpy). Types like
frame_info_ptr, whose constructors and destructors have side effects
such as registering with a central management object, will be caught
because they are neither trivially destructible nor trivially copyable.
Simple POD types like ui_file_style are trivially destructible and
copyable, so pass this trait.
This commit adds the new type trait, and makes use of it in all cases
but one, pending_frame_object in python/py-unwind.c, has a field of
type frame_info_ptr, which is currently broken. This will be fixed,
and the static_assert added, in the next commit.
Alan Modra [Sat, 16 May 2026 00:57:26 +0000 (10:27 +0930)]
Re: binutils/configure: look for msgpack-c.pc (in addition to msgpack.pc)
Commit 520c7eefed7f results in the following if both msgpack-c and
msgpack are missing.
checking for msgpack-c... no
checking for msgpack... no
configure: error: Package requirements (msgpack) were not met:
Package 'msgpack', required by 'virtual:world', not found
Consider adjusting the PKG_CONFIG_PATH environment variable if you
installed software in a non-standard prefix.
Alternatively, you may set the environment variables MSGPACK_CFLAGS
and MSGPACK_LIBS to avoid the need to call pkg-config.
See the pkg-config man page for more details.
make[2]: *** [Makefile:920: config.status] Error 1
make[2]: Leaving directory '/build/gas/all/binutils'
make[1]: *** [Makefile:4123: all-binutils] Error 2
binutils/
* configure.ac <msgpack>: Tell PKG_CHECK_MODULES that errors
will be handled by its caller.
* configure: Regenerate.
bfd/
* bfd-in2.h: Regenerate.
Tom Tromey [Thu, 7 May 2026 18:08:40 +0000 (12:08 -0600)]
Handle DW_AT_encoding on DW_TAG_enumeration_type
A user pointed out a problem when printing certain values from an Ada
enumeration type. Investigation showed that the problem was that some
enumeration constants were emitted using DW_FORM_data1, and were
incorrectly sign-extended by gdb.
First, this is yet another instance of a general problem with DWARF.
See https://sourceware.org/bugzilla/show_bug.cgi?id=32680 for the
analysis.
Meanwhile, it turns out that GCC implements an extension to handle
this scenario. In particular, in non-strict mode, it will emit
DW_AT_encoding using either DW_ATE_signed or DW_ATE_unsigned. This
was done back in 2017 by Pierre-Marie, in support of Ada -- but then
somehow nothing was ever implemented on the gdb side. For this see
GCC commit f76f096e ("DWARF: add DW_AT_encoding attributes for
DW_TAG_enumeration_type DIEs").
This patch adds the missing code to gdb. The included test case shows
the bug that was originally reported. I've also included the snippet
from Pierre-Marie's commit message for good measure.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32680 Approved-By: Andrew Burgess <aburgess@redhat.com>
I came across this code:
...
if (c)
return v;
else
Py_RETURN_NONE;
...
and realized I couldn't easily rewrite it into "return c ? v : ...".
Probably something like this would work:
...
return c ? v : ([] { Py_RETURN_NONE; } ());
...
but it made me wonder if we can get rid of Py_RETURN_NONE.
The only point of it seems to be to increase the reference count on the
Py_None object for older python versions.
Add a refcount-safe wrapper py_none (returning a gdbpy_ref), with the aim of
replacing all uses of Py_RETURN_NONE with "return py_none ().release ()".
Likewise for Py_RETURN_TRUE/Py_RETURN_FALSE/Py_RETURN_NOTIMPLEMENTED.
Simon Marchi [Thu, 7 May 2026 19:23:12 +0000 (15:23 -0400)]
binutils/configure: look for msgpack-c.pc (in addition to msgpack.pc)
msgpack.pc was renamed to msgpack-c.pc in this commit [1]. This means
that we now find both in the wild. For example Debian Bookworm has
msgpack.pc [2] while Debian Trixie has msgpack-c.pc [3].
Update the check in configure.ac to check for both. Nothing in the code
needs to change.
Alice Carlotti [Thu, 7 May 2026 06:08:53 +0000 (07:08 +0100)]
aarch64: Add new qualifier AARCH64_OPND_QLF_UNUSED
Replace QLF_NIL with QLF_UNUSED for qualifier sequence list padding.
This splits apart distinct qualifier meanings, and simplifies detection
of empty qualifier sequences.
Alice Carlotti [Thu, 29 Jan 2026 12:44:10 +0000 (12:44 +0000)]
aarch64: Add new qualifier ARCH64_OPND_QLF_UNKNOWN
Some uses of AARCH64_OPND_QLF_NIL actually represent an unknown operand
qualifier. The F_STRICT flag was added to disable the wild card
behaviour for most SVE instructions, but this just makes the code less
consistent and more confusing.
Add a new qualifer to indicate that the real qualifier is currently
unknown, and use this in any place where we need the wildcard behaviour
(and in a few other places where UNKNOWN is more descriptive than NIL).
For consistency (and as an extra check during development), change the
default qualifier value during parsing to AARCH64_OPND_QLF_ERR, and add
explicit qualifier assignments to each non-failure code path.
Alice Carlotti [Thu, 29 Jan 2026 12:10:43 +0000 (12:10 +0000)]
aarch64: Return QLF_ERR for error conditions
In vectype_to_qualifier and get_qualifier_from_partial_encoding, return
AARCH64_OPND_QLF_ERR instead of AARCH64_OPND_QLF_NIL to indicate an
error condition.
Alice Carlotti [Thu, 29 Jan 2026 12:14:33 +0000 (12:14 +0000)]
aarch64: Add QLF_ERR to aarch64_opnd_qualifiers
Add an entry to aarch64_opnd_qualifiers for AARCH64_OPND_QLF_ERR. This
should never be used (otherwise we would previously have had
out-of-bounds memory accesses), but it makes the array length match the
enum length, and might be slightly more robust.
Alice Carlotti [Thu, 29 Jan 2026 16:32:19 +0000 (16:32 +0000)]
aarch64: Append explicit NILs to qualifier sequences
For some opcodes, the NIL qualifier on the final operand was set
implicitly via empty initialisation of the unspecified entries in the
list of qualifier sequences. Make this assignment explicit, to avoid
depending on the numerical value of AARCH64_OPND_QLF_NIL.
Alice Carlotti [Fri, 1 May 2026 16:25:13 +0000 (17:25 +0100)]
aarch64: Make FPIMM0 qualifiers consistent
Two entries in the opcode table used QLF_S_H for the FPIMM0 operand
instead of the QLF_NIL operand used elsewhere. There is no good reason
for this inconsistency, so change them to use QLF_NIL as well.
Alice Carlotti [Fri, 30 Jan 2026 12:13:06 +0000 (12:13 +0000)]
aarch64: Fix qualifier sequences for cinc/cinv/cneg
The three-operand cinc/cinv/cneg instructions were incorrectly specified
with the same qualifier sequences as the four-operand variants, which
resulted in their third operand having an incorrect qualifier specified.
This works fine while AARCH64_OPND_QLF_NIL is treated as a wildcard, but
will break when that behaviour is removed.
Alice Carlotti [Thu, 29 Jan 2026 12:17:24 +0000 (12:17 +0000)]
aarch64: Fix ldst_lo12_determine_real_reloc_type
Add fallback handling for an invalid choice of opd0_qlf, instead of
hitting an assert when trying to use X registers in byte or half
instructions.
Additionally, simplify the code by inlining the relevant parts of
aarch64_get_expected_qualifer, and by deducing the array index directly
from the qualifier enum values (instead of looking up the element size
and computing its log).
This makes aarch64_get_expected_qualifier unused, so remove it.
Alice Carlotti [Fri, 30 Jan 2026 17:11:29 +0000 (17:11 +0000)]
aarch64: Add F_REQUIRES_SP and eliminate QLF_SP and QLF_WSP
Add a new opcode flag F_REQUIRES_SP and use that to enforce the
requirement for at least one SP operand in the mov (to/from SP) opcode.
This requirement was the only reason for the existence of the QLF_SP and
QLF_WSP qualifiers. Delete them, and remove the confusing code that
would switch between SP and non-SP qualifiers in the middle of qualifier
matching.
Alice Carlotti [Fri, 30 Jan 2026 12:04:06 +0000 (12:04 +0000)]
aarch64: Fix use of wrong zero enum value
aarch64_get_expected_qualifier was incorrectly comparing an
aarch64_opnd_qualifier_t against AARCH64_OPND_NIL instead of
AARCH64_OPND_QLF_NIL (these are the zero values of two different enums).
A previous patch incorrectly addressed the type mismatch by casting the
qualifier to (enum aarch64_opnd).
Fix it properly by removing the cast and updating the comparison to use
AARCH64_OPND_QLF_NIL.
Andrew Burgess [Fri, 8 May 2026 14:13:16 +0000 (15:13 +0100)]
gdb: use value::embedded_offset in check_pieced_synthetic_pointer
This commit builds on the previous two commits. You should go and
read them both for context.
Looking at the users of value::bits_synthetic_pointer, all callers but
two are now passing either the offset of a field within the value, or
hard-coded zero as they want to ask about the entire value.
The two exceptions are in coerce_pieced_ref (in dwarf2/expr.c) and in
value_addr (in valops.c) where we still pass value::embedded_offset.
The problem is that some of the other callers, where we currently pass
just a field offset, might also have a non-zero embedded offset,
specifically, the call in cp_print_value_fields is in this category.
In cp_print_value_fields we are printing fields from a value which is
potentially a base class contained within an instance of a derived
class, this will be represented by a non-zero embedded_offset, which
is currently not taken into account.
Additionally, the two callers that currently pass the
value::embedded_offset don't scale the embedded offset from bytes to
bits.
The failure to scale from bytes to bits is interesting, I originally
tried to create some tests that exposed this, but constantly failed.
It turns out, that in both these locations, in all code paths that I
could find, the value::embedded_offset will always be zero. For
example in coerce_pieced_ref, a reference cannot be a base class, and
so we never expect to see a non-zero embedded_offset in this case.
Similarly, in value_addr, the use of value::bits_synthetic_pointer is
within a block that only applies to reference types, which means the
embedded_offset will be zero.
Now within check_pieced_synthetic_pointer we already add the
value::offset into the bit_offset which is passed in. Remember, after
the previous two commits, the incoming bit_offset is (almost) always
the offset of a field within the value. The almost here is the two
cases mentioned above.
I propose that within check_pieced_synthetic_pointer we should take
into account both value::offset and value::embedded_offset. Then the
two cases that currently pass value::embedded_offset can be changed to
just pass zero.
With this done, and the previous two commits, we now have a consistent
model. value::bits_synthetic_pointer expects the (bit) offset of a
field within the value to check. In many cases this will be zero
meaning we want to check from the start of the value, but in some
cases it can be non-zero.
Then within value::bits_synthetic_pointer implementations, like
check_pieced_synthetic_pointer, we will take the value::offset and
value::embedded_offset into account, remembering to scale them from
bytes to bits.
I have also changed indirect_pieced_value to also take the
embedded_offset into account. I have done this for consistency rather
than necessity. I believe that the embedded_offset will always be
zero within indirect_pieced_value. The indirect_pieced_value function
is only called from value_ind (in valops.c), and only operates on
TYPE_CODE_PTR types (checked for in indirect_pieced_value). As a
TYPE_CODE_PTR cannot be the base class for a derived type, then we
don't expect to ever see a TYPE_CODE_PTR value with a non-zero
embedded_offset. But, having indirect_pieced_value take the embedded
offset into account is simple enough, and future proofs the code.
In both check_pieced_synthetic_pointer and indirect_pieced_value I
have changed uses of '8' to 'TARGET_CHAR_BIT', I was already touching
some of these lines, and I think TARGET_CHAR_BIT is clearer, but one
line in indirect_pieced_value was just updated to use TARGET_CHAR_BIT,
this is done for consistency.
The gdb.dwarf2/multi-piece-inherited-bitfield.exp test fails without
this patch, this exposes the case where the embedded_offset is
non-zero and we were previously failing to take this into account.
The gdb.dwarf2/multi-piece-primitive-field.exp test was something I
wrote while trying to exercise the coerce_pieced_ref code path some
more. It is an inheritance based version of the existing test, I was
wondering if this would result in a value with a non-zero
embedded_offset, but due to how the fields are extracted from the
aggregate prior to calling coerce_pieced_ref the embedded_offset is
always zero in this function.
Andrew Burgess [Fri, 8 May 2026 20:25:11 +0000 (21:25 +0100)]
gdb: remove embedded_offset argument that is always 0
For the background and motivation for this patch you should read the
previous commit.
The goal of this commit started as similar to the previous one; rename
the embedded_offset argument to generic_val_print_ref. To do this I
started tracing back which values are passed into this function so I
could make sure the new argument name matched the usage.
But it turns out that the only value that is ever passed to this
function is zero.
I believe that, like the last commit, the embedded_offset is actually
representing the offset of a field within a value. However, in all of
the use cases, the "field" being accessed is the entire value, hence
why we always pass 0, we are asking about the whole value starting
from the very beginning.
Given this, I couldn't bring myself to rename the argument. Let's just
remove it. It turns out that there's a bunch of functions in
valprint.c that take an argument called embedded_offset, which are
always zero.
This commit removes the argument, and updates the code to assume zero.
There should be no user visible changes after this commit.
This passes the result from value::embedded_offset as the first argument.
Though it's not clear from value.h, the number returned by
value::embedded_offset is a byte offset.
The value::bits_synthetic_pointer call can end up calling
check_pieced_synthetic_pointer, with the first argument to
value::bits_synthetic_pointer becoming the second argument to
check_pieced_synthetic_pointer. The second argument to
check_pieced_synthetic_pointer is called BIT_OFFSET, a value in bits.
Looking at how this argument is used confirms that it is expected to be
a value in bits, not bytes.
The fix should be easy, multiply the embedded offset by
TARGET_CHAR_BIT, but I think things are a little more complex than
this.
I started looking at how value::bits_synthetic_pointer is used, there
are only 7 uses, and they can be grouped as follows:
gdb/dwarf2/expr.c
gdb/valops.c
These call value::embedded_offset directly within the call to
value::bits_synthetic_pointer.
gdb/opencl-lang.c
This function is used as a wrapper that implements a
lval_funcs::check_synthetic_pointer callback, it adjusts the
arguments and then calls value::bits_synthetic_pointer. As such we'd
not expect to see embedded offset mentioned here as that would (we
assume) be handled by the caller.
gdb/valprint.c (in generic_val_print_ref)
This call passes an argument called 'embedded_offset', but the
function generic_val_print_ref is only used in one place, and the
embedded_offset is always passed as zero.
gdb/valprint.c (in valprint_check_validity)
This call also passes an argument called 'embedded_offset'. This
function is used in two places. In common_val_print the
embedded_offset is always passed as zero, but in
valprint_check_validity (in cp-valprint.c) the embedded_offset being
passed is the offset of a field within the value, not the value's
embedded_offset within some larger value.
gdb/cp-valprint.c
gdb/p-valprint.c
As with the call to valprint_check_validity in cp-valprint.c, these
two direct calls to value::bits_synthetic_pointer pass the offset of a
field within the value, rather than the value's embedded_offset.
What I see in the above is some confusion. In some places we are
passing the value::embedded_offset, while in other places we are
passing the offset of a field within the value itself.
If we consider the direct call to value::bits_synthetic_pointer in
gdb/cp-valprint.c, where a field offset is passed, then it should be
possible, that if we can create an object with a non-zero
embedded_offset (which isn't accounted for in this code path), then we
should see some bugs in GDB, and indeed, this is what I do see.
My plan for fixing this is to have the offset passed to
value::bits_synthetic_pointer always be a field offset within the value,
the value::embedded_offset will then be handled directly within the
various value::bits_synthetic_pointer implementations.
This commit is a small refactor in preparation for this fix.
I believe part of the confusion here is that we have functions that
take arguments called embedded_offset, when the value they should
accept is no longer the embedded offset.
So in this commit I propose renaming the embedded_offset argument to
field_byte_offset in valprint_check_validity. This is purely a
mechanical rename, there should be no user visible changes after this
commit.
projects / thirdparty / binutils-gdb.git / log
