In a target without SVE support, the V registers have a size of 16 bytes,
otherwise they may have a size bigger than 16 bytes (depending on the current
vector length for the Z registers, as they overlap the V registers).
In aarch64-tdep.c:aarch64_store_return_value, the code is laid
out in a way that allocates the buffer with the size of the register, but
only updates the amount of bytes for the particular type we're returning.
This may cause a situation where we have a register size of 32 bytes but
are returning a floating point value of 8 bytes. The temporary buffer
will therefore have 32 bytes, but we'll only update 8 bytes of it.
When we write the entire register back, it will have potentially 24 bytes
of garbage in it.
Fix this by first reading the original contents of the register and then
overriding only the bytes that we need for the return value.
Tested on aarch64-linux Ubuntu 22.04/20.04.
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
This is a patch in preparation to upcoming patches enabling SME support. It
attempts to simplify the gdb/gdbserver shared interface used to read/write
SVE registers.
Where the current code makes use of unique_ptr, allocating a new buffer by
hand and passing a buffer around, this patch makes that code use
gdb::byte_vector and passes a reference to this byte vector to the functions,
allowing the functions to have ready access to the size of the buffer.
It also shares a bit more code between gdb and gdbserver, in particular around
handling of ptrace get/set requests for SVE.
I think gdbserver could be refactored to handle register reads/writes more
like gdb's native layer as opposed to letting the generic linux-low layer do
the ptrace calls. This is not very flexible and assumes one size for the
responses. If you have something like NT_ARM_SVE, where you can have either
FPSIMD or SVE contents, it doesn't work that well.
I didn't want to change that interface right now as it is a bit too much work
and touches all the targets, some of which I can't easily test.
Hence the reason why the buffer the generic linux-now passes down to
linux-aarch64-low is unused or ignored.
No user-visible changes should happen as part of this refactor other than a
slightly reworded warning message.
While doing the refactor, I also noticed what seems to be a mistake in checking
if the register cache contains active (non-zero) SVE data.
For instance, the original code did something like this in
aarch64_sve_regs_copy_from_reg_buf:
has_sve_state |= reg_buf->raw_compare (AARCH64_SVE_Z0_REGNUM + i
reg, sizeof (__int128_t));
"reg" is a zeroed-out buffer that we compare the Z register contents
past the first 128 bits. The problem here is that raw_compare returns
1 if the contents compare the same, which means has_sve_state will be
true. But if we compared the Z register contents to 0, it means we
*do not* have SVE state, and therefore has_sve_state should be false.
The consequence of this mistake is that we convert the initial
FPSIMD-formatted data we get from ptrace for the NT_ARM_SVE register
set to a SVE-formatted one.
In the end, this doesn't cause user-visible differences because the
values of both the Z and V registers will still be the same. But the
logic is not correct.
I used the opportunity to fix this, and it gets tested later on by
the additional SME tests.
I do plan on submitting some SVE-specific tests to make sure we have
a bit more coverage in GDB's testsuite.
Regression-tested on aarch64-linux Ubuntu 22.04/20.04.
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
In preparation to the SME support patches, rename the SVE-specific files to
something a bit more meaningful that can be shared with the SME code.
In this case, I've renamed the "sve" in the names to "scalable".
No functional changes.
Regression-tested on aarch64-linux Ubuntu 22.04/20.04.
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
I noticed we don't handle register reads/writes in the best way for native
AArch64 Linux. Some other registers are fetched/stored even if upper level
code told us to fetch a particular register number.
Fix this by being more strict about which registers we touch when
reading/writing them in the native AArch64 Linux layer.
There should be no user-visible changes due to this patch.
Regression-tested on aarch64-linux Ubuntu 22.04/20.04.
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
This patch moves instances of system register definitions, represented
by the SYSREG macro, out of their original place in `aarch64-opc.c'
and into a dedicated .def file, `aarch64-sys-regs.def'.
System register entries in this new file are ordered alphabetically by
name. This choice is made to enable the use of fast search algorithms
such as binary search when validating register names.
The SYSREG macro, defined as SYSREG (name, encoding, flags, features)
is kept as is and used in the def file, but all other SR_* macros
which previously served as indirections to SYSREG are removed.
opcodes/ChangeLog:
* aarch64-opc.c (SR_CORE): Macro definition and uses deleted.
(SR_FEAT): Likewise.
(SR_FEAT2): Likewise.
(SR_V8_1_A): Likewise.
(SR_V8_4_A): Likewise.
(SR_V8A): Likewise.
(SR_V8R): Likewise.
(SR_V8_1A): Likewise.
(SR_V8_2A): Likewise.
(SR_V8_3A): Likewise.
(SR_V8_4A): Likewise.
(SR_V8_6A): Likewise.
(SR_V8_7A): Likewise.
(SR_V8_8A): Likewise.
(SR_GIC): Likewise.
(SR_AMU): Likewise.
(SR_LOR): Likewise.
(SR_PAN): Likewise.
(SR_RAS): Likewise.
(SR_RNG): Likewise.
(SR_SME): Likewise.
(SR_SSBS): Likewise.
(SR_SVE): Likewise.
(SR_ID_PFR2): Likewise.
(SR_PROFILE): Likewise.
(SR_MEMTAG): Likewise.
(SR_SCXTNUM): Likewise.
(SR_EXPAND_ELx): Likewise.
(SR_EXPAND_EL12): Likewise.
* opcodes/aarch64-sys-regs.def: New.
This patch adds a mechanism for system register name alias detection
to register-matching mechanisms.
A new `F_REG_ALIAS' flag is added to the set of register flags and
used to label which entries in aarch64_sys_regs[] correspond to
aliases (and thus which CPENC values are non-unique in this array).
Where this is used is, for example, in `aarch64_print_operand' where,
in the case of system register decoding, the aarch64_sys_regs[] array
is iterated through until a match in CPENC value is made and the first
match accepted. If insufficient care is given in the ordering of
system registers in this array, the alias is encountered before the
"real" register and used incorrectly as the register name in the
disassembled output.
With this flag and the new `aarch64_sys_reg_alias_p' test, search
candidates corresponding to aliases can be conveniently skipped over.
One concrete example of where this is useful is with the
`trcextinselr0' system register. It was initially placed in the
system register list before `trcextinselr', in contrast to a more
natural alphabetical order.
include/ChangeLog:
* opcode/aarch64.h: add `aarch64_sys_reg_alias_p' prototype.
opcodes/ChangeLog:
* aarch64-opc.c (aarch64_sys_reg_alias_p): New.
(aarch64_print_operand): add aarch64_sys_reg_alias_p check.
(aarch64_sys_regs): Add F_REG_ALIAS flag to "trcextinselr"
entry.
* aarch64-opc.h (F_REG_ALIAS): New.
When creating a core file from within GDB we include a NT_GDB_TDESC
that includes the target description of the architecture in use.
For architectures with dynamic architectures (e.g. AArch64 with
sve/sme) the original architecture, calculated from the original
target description, might not match the per-thread architecture.
In the general case, where each thread has a different architecture,
then we really need a separate NT_GDB_TDESC for each thread, however,
there's currently no way to read in multiple NT_GDB_TDESC.
This commit is a step towards per-thread NT_GDB_TDESC. In this commit
I have updated the function that writes the NT_GDB_TDESC to accept a
gdbarch (rather than calling target_gdbarch() to find a gdbarch), and
I now pass in the gdbarch of the signalled thread.
In many cases (though NOT all) targets with dynamic architectures
really only use a single architecture, even when there are multiple
threads, so in the common case, this should ensure that GDB emits an
architecture that is more likely to be correct.
Additional work will be needed in order to support corefiles with
truly per-thread architectures, but that will need to be done in the
future.
Add a 64-bit traditional MIPS dump variant for the `readelf -S bintest'
test from binutils-all/readelf.exp, using a filename suffix according to
the rules set there, removing:
FAIL: readelf -S bintest
regressions with `mips64-linux-gnuabi64', `mips64el-linux-gnuabi64',
`mips64-openbsd', and `mips64el-openbsd' targets, which default to the
n64 ABI and consequently produce a section layout that is different from
what the generic dump pattern covers.
Co-Authored-By: Maciej W. Rozycki <macro@orcam.me.uk>
binutils/
* testsuite/binutils-all/readelf.s-64-tmips: New test variant.
Reformat a Python file with black after this commit:
commit 59912fb2d22f8a4bb0862487f12a5cc65b6a013f
Date: Tue Sep 19 11:45:36 2023 +0100
gdb: add Python events for program space addition and removal
There should be no functional change with this commit.
Yesterday I pushed a patch to fix a small oversight in the DAP code
that caused an instructionReference to be an array instead of a
string.
This patch adds a test case for that regression. This required
exposing the TON form of the response -- something I mentioned might
be necessary when this code was changed to return just the Tcl form.
I tested this by backing out yesterday's bug fix and verifying that a
failure is seen.
Following on Tom de Vries' work in the other array-printers, this
patch changes val_print_packed_array_elements to also avoid allocating
too many values when printing an Ada packed array.
I see this failure:
FAIL: gdb.base/jit-reader-simple.exp: standalone: change addr: initial run: maint info breakpoints shows jit breakpoint
The jit breakpoint expected by the test is there, it's just that the
number of spaces doesn't match what the test expects, after "jit
events":
-2 jit events keep y 0x0000555555555119 <__jit_debug_register_code> inf 1
Fix that by relaxing the regex a bit.
Change-Id: Ia3b04e6d5978399d940fd1a590f95f15275ca7ac
This setting controls whether GDB may attempt to download ELF/DWARF
sections from debuginfod servers.
This setting is enabled by default if gdb is built with debuginfod
section download support (requires libdebuginfod 0.188).
Also update debuginfod command help text and gdb.texinfo with
information on section downloading and the new command.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Andrew Burgess <aburgess@redhat.com>
Add new function debuginfod_section_query. This function queries
debuginfod servers for an individual ELF/DWARF section associated with
a given build-id.
Approved-By: Andrew Burgess <aburgess@redhat.com>
When running test-case gdb.ada/import.exp with gcc 7, most test fail:
...
FAIL: gdb.ada/import.exp: print imported_var_ada
FAIL: gdb.ada/import.exp: print local_imported_var
FAIL: gdb.ada/import.exp: print pkg.imported_var_ada
FAIL: gdb.ada/import.exp: print pkg.exported_var_ada
FAIL: gdb.ada/import.exp: print exported_var_ada
FAIL: gdb.ada/import.exp: gdb_breakpoint: set breakpoint at pkg.imported_func_ada
FAIL: gdb.ada/import.exp: gdb_breakpoint: set breakpoint at imported_func_ada
FAIL: gdb.ada/import.exp: gdb_breakpoint: set breakpoint at local_imported_func
...
When running with gcc 8 or 9, only 2 tests fail:
...
FAIL: gdb.ada/import.exp: gdb_breakpoint: set breakpoint at pkg.imported_func_ada
FAIL: gdb.ada/import.exp: gdb_breakpoint: set breakpoint at imported_func_ada
...
The test-case passes fully with gcc 10, 11, 12 and 13.
Debug info for pragma import seems to not have been supported before gcc 8, so
require that version.
The two FAILs with gcc 8 and 9 seem to be due to problems in debug info. Add
an xfail for these.
Tested on x86_64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
With gcc 13.2.1, I run into a cluster of fails:
...
FAIL: gdb.ada/str_binop_equal.exp: print my_str = "ABCD"
FAIL: gdb.ada/widewide.exp: print my_wws = " helo"
FAIL: gdb.ada/widewide.exp: print my_ws = "wide"
...
The problem is that the debug info contains information about function
ada.strings.maps."=", and gdb uses it to implement the comparison.
The function is supposed to compare two char sets, not strings, so gdb
shouldn't use it. This is PR ada/30908.
I don't see the same problem with gcc 7.5.0, because the exec doesn't contain
the debug info for the function, because the corresponding object is not
linked in. Adter adding "with Ada.Strings.Maps; use Ada.Strings.Maps;" to
gdb.ada/widewide/foo.adb I run into the same problem with gcc 7.5.0.
Add KFAILs for the PR.
Tested on x86_64-linux:
- openSUSE Leap 15.4 (using gcc 7.5.0), and
- openSUSE Tumbleweed (using gcc 13.2.1).
Approved-By: Tom Tromey <tom@tromey.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30908
Initially I just wanted a Python event for when GDB removes a program
space, I'm writing a Python extension that caches information for each
program space, and need to know when I should discard entries for a
particular program space.
But, it seemed easy enough to also add an event for when GDB adds a
new program space, so I went ahead and added both new events.
Of course, we don't currently have an observable for program space
addition or removal, so I first needed to add these. After that it's
pretty simple to add two new Python events and have these trigger.
The two new event registries are:
events.new_progspace
events.free_progspace
These emit NewProgspaceEvent and FreeProgspaceEvent objects
respectively, each of these new event types has a 'progspace'
attribute that contains the relevant gdb.Progspace object.
There's a couple of things to be mindful of.
First, it is not possible to catch the NewProgspaceEvent for the very
first program space, the one that is created when GDB first starts, as
this program space is created before any Python scripts are sourced.
In order to allow this event to be caught we would need to defer
creating the first program space, and as a consequence the first
inferior, until some later time. But, existing scripts could easily
depend on there being an initial inferior, so I really don't think we
should change that -- and so, we end up with the consequence that we
can't catch the event for the first program space.
The second, I think minor, issue, is that GDB doesn't clean up its
program spaces upon exit -- or at least, they are not cleaned up
before Python is shut down. As a result, any program spaces in use at
the time GDB exits don't generate a FreeProgspaceEvent. I'm not
particularly worried about this for my use case, I'm using the event
to ensure that a cache doesn't hold stale entries within a single GDB
session. It's also easy enough to add a Python at-exit callback which
can do any final cleanup if needed.
Finally, when testing, I did hit a slightly weird issue with some of
the remote boards (e.g. remote-stdio-gdbserver). As a consequence of
this issue I see some output like this in the gdb.log:
(gdb) PASS: gdb.python/py-progspace-events.exp: inferior 1
step
FreeProgspaceEvent: <gdb.Progspace object at 0x7fb7e1d19c10>
warning: cannot close "target:/lib64/libm.so.6": Cannot execute this command while the target is running.
Use the "interrupt" command to stop the target
and then try again.
warning: cannot close "target:/lib64/libc.so.6": Cannot execute this command while the target is running.
Use the "interrupt" command to stop the target
and then try again.
warning: cannot close "target:/lib64/ld-linux-x86-64.so.2": Cannot execute this command while the target is running.
Use the "interrupt" command to stop the target
and then try again.
do_parent_stuff () at py-progspace-events.c:41
41 ++global_var;
(gdb) PASS: gdb.python/py-progspace-events.exp: step
The 'FreeProgspaceEvent ...' line is expected, that's my test Python
extension logging the event. What isn't expected are all the blocks
like:
warning: cannot close "target:/lib64/libm.so.6": Cannot execute this command while the target is running.
Use the "interrupt" command to stop the target
and then try again.
It turns out that this has nothing to do with my changes, this is just
a consequence of reading files over the remote protocol. The test
forks a child process which GDB stays attached too. When the child
exits, GDB cleans up by calling prune_inferiors, which in turn can
result in GDB trying to close some files that are open because of the
inferior being deleted.
If the prune_inferiors call occurs when the remote target is
running (and in non-async mode) then GDB will try to send a fileio
packet while the remote target is waiting for a stop reply, and the
remote target will throw an error, see remote_target::putpkt_binary in
remote.c for details.
I'm going to look at fixing this, but, as I said, this is nothing to
do with this change, I just mention it because I ended up needing to
account for these warning messages in one of my tests, and it all
looks a bit weird.
Approved-By: Tom Tromey <tom@tromey.com>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
This backlink is not necessary, we always know the program space from
the context. Pass it down the solib_unloaded observer.
Change-Id: I45a503472dc791f517558b8141901472634e0556
Approved-By: Tom Tromey <tom@tromey.com>
I ran across this site:
https://no-color.org/
... which lobbies for tools to recognize the NO_COLOR environment
variable and disable any terminal styling when it is seen.
This patch implements this for gdb.
Regression tested on x86-64 Fedora 38.
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Reviewed-by: Kevin Buettner <kevinb@redhat.com>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Andrew Burgess <aburgess@redhat.com>
The range check should be checking for the range
ffffffff80000000..7fffffff, not ffffffff70000000.
This patch has been tested for years of AMD Xilinx Yocto
releases as part of the following patch set:
https://github.com/Xilinx/meta-xilinx/tree/master/meta-microblaze/recipes-devtools/binutils/binutils
Signed-off-by: nagaraju <nagaraju.mekala@amd.com>
Signed-off-by: Neal Frager <neal.frager@amd.com>
Signed-off-by: Michael J. Eager <eager@eagercon.com>
Fixed the problem related to the fixup/relocations TLSTPREL.
When the fixup is applied the addend is not added at the correct offset
of the instruction. The offset is hard coded considering its big endian
and it fails for Little endian. This patch allows support for both
big & little-endian compilers.
This patch has been tested for years of AMD Xilinx Yocto
releases as part of the following patch set:
https://github.com/Xilinx/meta-xilinx/tree/master/meta-microblaze/recipes-devtools/binutils/binutils
Signed-off-by: nagaraju <nagaraju.mekala@amd.com>
Signed-off-by: Neal Frager <neal.frager@amd.com>
The PLT entry in executables and shared libraries contains an indirect
branch, like
jmp *foo@GOTPCREL(%rip)
push $index_foo
jmp .PLT0
or
endbr64
jmp *foo@GOTPCREL(%rip)
NOP padding
which is used to branch to the function, foo, defined in another object.
Each R_X86_64_JUMP_SLOT relocation has a corresponding PLT entry.
The dynamic tags have been added to the x86-64 psABI to mark such PLT
entries:
6d824a52a4
Add an x86-64 linker option, -z mark-plt, to mark PLT entries with
#define DT_X86_64_PLT (DT_LOPROC + 0)
#define DT_X86_64_PLTSZ (DT_LOPROC + 1)
#define DT_X86_64_PLTENT (DT_LOPROC + 3)
1. DT_X86_64_PLT: The address of the procedure linkage table.
2. DT_X86_64_PLTSZ: The total size, in bytes, of the procedure linkage
table.
3. DT_X86_64_PLTENT: The size, in bytes, of a procedure linkage table
entry.
and set the r_addend field of the R_X86_64_JUMP_SLOT relocation to the
memory offset of the indirect branch instruction. The dynamic linker
can use these tags to update the PLT section to direct branch.
bfd/
* elf-linker-x86.h (elf_linker_x86_params): Add mark_plt.
* elf64-x86-64.c (elf_x86_64_finish_dynamic_symbol): Set the
r_addend of R_X86_64_JUMP_SLOT to the indirect branch offset
in PLT entry for -z mark-plt.
* elfxx-x86.c (_bfd_x86_elf_size_dynamic_sections): Add
DT_X86_64_PLT, DT_X86_64_PLTSZ and DT_X86_64_PLTENT for
-z mark-plt.
(_bfd_x86_elf_finish_dynamic_sections): Set DT_X86_64_PLT,
DT_X86_64_PLTSZ and DT_X86_64_PLTENT.
(_bfd_x86_elf_get_synthetic_symtab): Ignore addend for
JUMP_SLOT relocation.
(_bfd_x86_elf_link_setup_gnu_properties): Set
plt_indirect_branch_offset.
* elfxx-x86.h (elf_x86_plt_layout): Add plt_indirect_branch_offset.
binutils/
* readelf.c (get_x86_64_dynamic_type): New function.
(get_dynamic_type): Call get_x86_64_dynamic_type.
include/
* elf/x86-64.h (DT_X86_64_PLT): New.
(DT_X86_64_PLTSZ): Likewise.
(DT_X86_64_PLTENT): Likewise.
ld/
* ld.texi: Document -z mark-plt and -z nomark-plt.
* emulparams/elf32_x86_64.sh: Source x86-64-plt.sh.
* emulparams/elf_x86_64.sh: Likewise.
* emulparams/x86-64-plt.sh: New file.
* testsuite/ld-x86-64/mark-plt-1.s: Likewise.
* testsuite/ld-x86-64/mark-plt-1a-x32.d: Likewise.
* testsuite/ld-x86-64/mark-plt-1a.d: Likewise.
* testsuite/ld-x86-64/mark-plt-1b-x32.d: Likewise.
* testsuite/ld-x86-64/mark-plt-1b.d: Likewise.
* testsuite/ld-x86-64/mark-plt-1c-x32.d: Likewise.
* testsuite/ld-x86-64/mark-plt-1c.d: Likewise.
* testsuite/ld-x86-64/mark-plt-1d-x32.d: Likewise.
* testsuite/ld-x86-64/mark-plt-1d.d: Likewise.
* testsuite/ld-x86-64/x86-64.exp: Run -z mark-plt tests.
At one time, circa 2006, there was a bug, which was presumably fixed
without adding a test case:
If you provided some relative path to the shared library, such as
with
export LD_LIBRARY_PATH=.
then gdb would fail to match the shared library name during the
TLS lookup.
I think there may have been a bit more to it than is provided by that
explanation, since the test also takes care to split the debug info
into a separate file.
In any case, this commit is based on one of Red Hat's really old
local patches. I've attempted to update it and remove a fair amount
of cruft, hopefully without losing any critical elements from the
test.
Testing on Fedora 38 (correctly) shows 1 unsupported test for
native-gdbserver and 5 PASSes for the native target as well as
native-extended-gdbserver.
In his review of v1 of this patch, Lancelot SIX observed that
'thread_local' could be used in place of '__thread' in the C source
files. But it only became available via the standard in C11, so I
used additional_flags=-std=c11 for compiling both the shared object
and the main program.
Also, while testing with CC_FOR_TARGET=clang, I found that
'additional_flags=-Wl,-soname=${binsharedbase}' caused clang
to complain that this linker flag was unused when compiling
the source file, so I moved this linker option to 'ldflags='.
My testing for this v2 patch shows the same results as with v1,
but I've done additional testing with CC_FOR_TARGET=clang as
well. The results are the same as when gcc is used.
Co-Authored-by: Jan Kratochvil <jan@jankratochvil.net>
Reviewed-By: Lancelot Six <lancelot.six@amd.com>
I'm starting to work on these files, I thought it would be a good time
to remove unused imports. These were identified by
include-what-you-use. Tested by rebuilding.
Change-Id: I3eaf3fa0ea3506c7ecfbc8ecff5031433b1dadb8
Reviewed-By: John Baldwin <jhb@FreeBSD.org>
Updated show usage for MicroBlaze specific assembler options
to include new entries.
This patch has been tested for years of AMD Xilinx Yocto
releases as part of the following patch set:
https://github.com/Xilinx/meta-xilinx/tree/master/meta-microblaze/recipes-devtools/binutils/binutils
Signed-off-by: nagaraju <nagaraju.mekala@amd.com>
Signed-off-by: Neal Frager <neal.frager@amd.com>
---
V1->V2:
- removed new options which were unnecessary
- added documentation for MicroBlaze specific options
Signed-off-by: Michael J. Eager <eager@eagercon.com>
PR29040 describes a FAIL for test-case gdb.threads/next-fork-other-thread.exp
and target board unix/-m32.
The FAIL happens due to the test executable running into an assert, which is
caused by a forked child segfaulting, like so:
...
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x00000000 in ?? ()
...
I tried to reproduce the segfault with exec next-fork-other-thread-fork, using
TUI layout asm.
I set a breakpoint at fork and ran to the breakpoint, and somewhere during the
following session I ran into a gdb segfault here in
tui_find_disassembly_address:
...
/* Disassemble forward. */
next_addr = tui_disassemble (gdbarch, asm_lines, new_low, max_lines);
last_addr = asm_lines.back ().addr;
...
due to asm_lines being empty after the call to tui_disassemble, while
asm_lines.back () assumes that it's not empty.
I have not been able to reproduce that segfault in that original setting, I'm
not sure of the exact scenario (though looking back it probably involved
"set detach-on-fork off").
What likely happened is that I managed to reproduce PR29040, and TUI (attempted
to) display the disassembly for address 0, which led to the gdb segfault.
When gdb_print_insn encounters an insn it cannot print because it can't read
the memory, it throws a MEMORY_ERROR that is caught by tui_disassemble.
The specific bit that causes the gdb segfault is that if gdb_print_insn throws
a MEMORY_ERROR for the first insn in tui_disassemble, it returns an empty
asm_lines.
FWIW, I did manage to reproduce the gdb segfault as follows:
...
$ gdb -q \
-iex "set pagination off" \
/usr/bin/rustc \
-ex "set breakpoint pending on" \
-ex "b dl_main" \
-ex run \
-ex "up 4" \
-ex "layout asm" \
-ex "print \$pc"
...
<TUI>
...
$1 = (void (*)()) 0x1
(gdb)
...
Now press <up>, and the segfault triggers.
Fix the segfault by handling asm_lines.empty () results of tui_disassemble in
tui_find_disassembly_address.
I've written a unit test that exercises this scenario.
Tested on x86_64-linux.
Reviewed-by: Kevin Buettner <kevinb@redhat.com>
PR tui/30823
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30823
This removes the old gdb_bfd_openr_iovec entirely. I think any new
code should use the type-safe approach.
Reviewed-By: Lancelot Six <lancelot.six@amd.com>
This converts the solib-rocm BFD iovec implementations to the new
type-safe gdb_bfd_openr_iovec. They were already essentially using
this approach, just without the type-safe wrapper.
Thanks to Lancelot Six for testing and fixing this patch.
Co-Authored-By: Lancelot Six <lancelot.six@amd.com>
Acked-by: Lancelot Six <lancelot.six@amd.com>
Reviewed-By: Lancelot Six <lancelot.six@amd.com>
This converts the target fileio BFD iovec implementation to use the
new type-safe gdb_bfd_openr_iovec.
Reviewed-By: Lancelot Six <lancelot.six@amd.com>
This patch adds a new, type-safe variant of gdb_bfd_openr_iovec. In
this approach, the underlying user data is simply an object, the
callbacks are methods, and the "open" function is a function view.
Nothing uses this new code yet.
Reviewed-By: Lancelot Six <lancelot.six@amd.com>
This commit fixes an issue that was discovered while writing the tests
for the previous commit.
I noticed that, when GDB restarts an inferior, the executable_changed
event would trigger twice. The first notification would originate
from:
#0 exec_file_attach (filename=0x4046680 "/tmp/hello.x", from_tty=0) at ../../src/gdb/exec.c:513
#1 0x00000000006f3adb in reopen_exec_file () at ../../src/gdb/corefile.c:122
#2 0x0000000000e6a3f2 in generic_mourn_inferior () at ../../src/gdb/target.c:3682
#3 0x0000000000995121 in inf_child_target::mourn_inferior (this=0x2fe95c0 <the_amd64_linux_nat_target>) at ../../src/gdb/inf-child.c:192
#4 0x0000000000995cff in inf_ptrace_target::mourn_inferior (this=0x2fe95c0 <the_amd64_linux_nat_target>) at ../../src/gdb/inf-ptrace.c:125
#5 0x0000000000a32472 in linux_nat_target::mourn_inferior (this=0x2fe95c0 <the_amd64_linux_nat_target>) at ../../src/gdb/linux-nat.c:3609
#6 0x0000000000e68a40 in target_mourn_inferior (ptid=...) at ../../src/gdb/target.c:2761
#7 0x0000000000a323ec in linux_nat_target::kill (this=0x2fe95c0 <the_amd64_linux_nat_target>) at ../../src/gdb/linux-nat.c:3593
#8 0x0000000000e64d1c in target_kill () at ../../src/gdb/target.c:924
#9 0x00000000009a19bc in kill_if_already_running (from_tty=1) at ../../src/gdb/infcmd.c:328
#10 0x00000000009a1a6f in run_command_1 (args=0x0, from_tty=1, run_how=RUN_STOP_AT_MAIN) at ../../src/gdb/infcmd.c:381
#11 0x00000000009a20a5 in start_command (args=0x0, from_tty=1) at ../../src/gdb/infcmd.c:527
#12 0x000000000068dc5d in do_simple_func (args=0x0, from_tty=1, c=0x35c7200) at ../../src/gdb/cli/cli-decode.c:95
While the second originates from:
#0 exec_file_attach (filename=0x3d7a1d0 "/tmp/hello.x", from_tty=0) at ../../src/gdb/exec.c:513
#1 0x0000000000dfe525 in reread_symbols (from_tty=1) at ../../src/gdb/symfile.c:2517
#2 0x00000000009a1a98 in run_command_1 (args=0x0, from_tty=1, run_how=RUN_STOP_AT_MAIN) at ../../src/gdb/infcmd.c:398
#3 0x00000000009a20a5 in start_command (args=0x0, from_tty=1) at ../../src/gdb/infcmd.c:527
#4 0x000000000068dc5d in do_simple_func (args=0x0, from_tty=1, c=0x35c7200) at ../../src/gdb/cli/cli-decode.c:95
In the first case the call to exec_file_attach first passes through
reopen_exec_file. The reopen_exec_file performs a modification time
check on the executable file, and only calls exec_file_attach if the
executable has changed on disk since it was last loaded.
However, in the second case things work a little differently. In this
case GDB is really trying to reread the debug symbol. As such, we
iterate over the objfiles list, and for each of those we check the
modification time, if the file on disk has changed then we reload the
debug symbols from that file.
However, there is an additional check, if the objfile has the same
name as the executable then we will call exec_file_attach, but we do
so without checking the cached modification time that indicates when
the executable was last reloaded, as a result, we reload the
executable twice.
In this commit I propose that reread_symbols be changed to
unconditionally call reopen_exec_file before performing the objfile
iteration. This will ensure that, if the executable has changed, then
the executable will be reloaded, however, if the executable has
already been recently reloaded, we will not reload it for a second
time.
After handling the executable, GDB can then iterate over the objfiles
list and reload them in the normal way.
With this done I now see the executable reloaded only once when GDB
restarts an inferior, which means I can remove the kfail that I added
to the gdb.python/py-exec-file.exp test in the previous commit.
Approved-By: Tom Tromey <tom@tromey.com>
This commit makes the executable_changed observable available through
the Python API as an event. There's nothing particularly interesting
going on here, it just follows the same pattern as many of the other
Python events we support.
The new event registry is called events.executable_changed, and this
emits an ExecutableChangedEvent object which has two attributes, a
gdb.Progspace called 'progspace', this is the program space in which
the executable changed, and a Boolean called 'reload', which is True
if the same executable changed on disk and has been reloaded, or is
False when a new executable has been loaded.
One interesting thing did come up during testing though, you'll notice
the test contains a setup_kfail call. During testing I observed that
the executable_changed event would trigger twice when GDB restarted an
inferior. However, the ExecutableChangedEvent object is identical for
both calls, so the wrong information is never sent out, we just see
one too many events.
I tracked this down to how the reload_symbols function (symfile.c)
takes care to also reload the executable, however, I've split fixing
this into a separate commit, so see the next commit for details.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
This commit continues the work of the previous few commits.
My goal is to expose the executable_changed observer through the
Python API as an event.
At this point adding executable_changed as an event to the Python API
is trivial, but before I do that I would like to add some additional
arguments to the observable, which currently has no arguments at all.
The new arguments I wish to add are:
1. The program_space in which the executable was changed, and
2. A boolean flag that will indicate if the executable changed to a
whole new path, or if GDB just spotted that the executable changed on
disk (e.g. the user recompiled the executable).
In this commit I change the signature of the observable and then pass
the arguments through at the one place where this observable is
notified.
As there are (currently) no users of this observable nothing else
needs updating. In the next commit I'll add a listener for this
observable in the Python code, and expose this as an event in the
Python API.
Additionally, with this change, it should be possible to update the
insight debugger to make use of this observable rather than using the
deprecated_exec_file_display_hook (as it currently does), which will
then allow this hook to be removed from GDB.
There should be no user visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
This commit continues the work of the previous two commits.
My goal, in the next couple of commits, is to expose the
executable_changed observable in the Python API as an event. However,
before I do that I want to remove the use of the executable_changed
observable from the reread_symbols function in symfile.c as this use
isn't directly associated with a change of the executable file, and so
seems wrong.
In the previous two commits I have removed all users of the
executable_changed observer as I believe those users can, and should,
actually be listening for the new_objfile observable instead, so now
there are no users of the executable_changed observable.
As such, I think removing the use of executable_changed from the
function reread_symbols is perfectly safe, and correct. At this point
the executable has not been changed, so we shouldn't be sending an
executable_changed notification, and, as there is nobody listening to
this observable, we can't break anything by removing this call.
There should be no user visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
