Tom de Vries [Sat, 23 Nov 2024 11:20:34 +0000 (12:20 +0100)]
[gdb/contrib] Add two rules in common-misspellings.txt
Eli mentioned [1] that given that we use US English spelling in our
documentation, we should use "behavior" instead of "behaviour".
In wikipedia-common-misspellings.txt there's a rule:
...
behavour->behavior, behaviour
...
which leaves this as a choice.
Add an overriding rule to hardcode the choice to common-misspellings.txt:
...
behavour->behavior
...
and add a rule to rewrite behaviour into behavior:
...
behaviour->behavior
...
and re-run spellcheck.sh on gdb*.
Guinevere Larsen [Thu, 14 Nov 2024 12:31:00 +0000 (09:31 -0300)]
gdb/record: introduce recoding support for vpor
This commit adds recording support for the AVX instruction vpor, and the
AVX2 extension. Since the encoding of vpor and vpxor are the same, and
their semantics are basically the same, modulo the mathematical
operation, they are handled by the same switch case block.
This also updates the vpxor function, to test vpor and vpxor, and
updates the name to vpor_xor_test to better reflect what it does.
Guinevere Larsen [Tue, 12 Nov 2024 20:45:05 +0000 (17:45 -0300)]
gdb/record: Add support for recording vpmovmskb
This commit adds support for recording the AVX instruction vpmovmskb,
and tests to the relevant file. The test didn't really support checking
general purpose registers, so this commit also adds a proc to
gdb.reverse/i386-avx-reverse.exp, which can be used to test them
Guinevere Larsen [Tue, 12 Nov 2024 19:37:32 +0000 (16:37 -0300)]
gdb/record: Add support for all vpcmpeq instructions
This commit adds support to recording instructions of the form
VPCMPEQ[B|W|D]. They are all encoded in the same way and only
differentiated by the opcode, so they are all processed together. This
commit also updates the test to (quite exhaustively) test the new
instruction.
This commit adds support for recording the instruction vpxor,
introduced in the AVX extension, and extended in AVX2 to use 256 bit
registers. The test gdb.reverse/i386-avx-reverse.exp has been extended
to test this instruction as well.
Guinevere Larsen [Thu, 20 Jun 2024 17:52:35 +0000 (14:52 -0300)]
gdb: Introduce RAII signal handler setter
This patch is motivated by the wait function for the record-full target,
that would install a custom signal handler for SIGINT, but could throw
an exception and never reset the SIGINT handler. This is clearly a bad
idea, so this patch introduces the class scoped_signal_handler in a new
.h file. The file is added to gdbsupport, even though only gdb code is
using it, because it feels like an addition that would be useful for
more than just directly gdb.
The implementation of the RAII class is based on the implementation
on gdb/utils.c. That is, it uses preprocessor ifdefs to probe for
sigaction support, and uses it if possible, defaulting to a raw call to
signal only if sigaction isn't supported. sigaction is preferred based
on the "portability" section of the manual page for the signal function.
There are 3 places where this class can just be dropped in,
gdb/record-full.c, gdb/utils.c and gdb/extension.c. This third place
already had a specialized RAII signal handler setter, but it is
substituted for the new general purpose one.
Hannes Domani [Fri, 22 Nov 2024 19:10:22 +0000 (20:10 +0100)]
Use appropriate context flags for Wow64 processes
When I implemented debugging of Wow64 processes, I missed that there are
extra ContextFlags defines for them.
It's a bit surprising that the wrong ones actually worked, except that
CONTEXT_EXTENDED_REGISTERS is not available for x86_64, and they are
needed for i686, since that's where the xmm registers are stored.
So this replaces the ContextFlags values with their WOW64_* equivalents.
On gdbserver this also duplicates the fallback logic if the
GetThreadContext call failed with CONTEXT_EXTENDED_REGISTERS.
Fixes these fails:
FAIL: gdb.arch/i386-sse.exp: check float contents of %xmm0
FAIL: gdb.arch/i386-sse.exp: check int8 contents of %xmm0
FAIL: gdb.arch/i386-sse.exp: check float contents of %xmm1
FAIL: gdb.arch/i386-sse.exp: check int8 contents of %xmm1
FAIL: gdb.arch/i386-sse.exp: check float contents of %xmm2
FAIL: gdb.arch/i386-sse.exp: check int8 contents of %xmm2
FAIL: gdb.arch/i386-sse.exp: check float contents of %xmm3
FAIL: gdb.arch/i386-sse.exp: check int8 contents of %xmm3
FAIL: gdb.arch/i386-sse.exp: check float contents of %xmm4
FAIL: gdb.arch/i386-sse.exp: check int8 contents of %xmm4
FAIL: gdb.arch/i386-sse.exp: check float contents of %xmm5
FAIL: gdb.arch/i386-sse.exp: check int8 contents of %xmm5
FAIL: gdb.arch/i386-sse.exp: check float contents of %xmm6
FAIL: gdb.arch/i386-sse.exp: check int8 contents of %xmm6
FAIL: gdb.arch/i386-sse.exp: check float contents of %xmm7
FAIL: gdb.arch/i386-sse.exp: check int8 contents of %xmm7
FAIL: gdb.arch/i386-sse.exp: check contents of data[0]
FAIL: gdb.arch/i386-sse.exp: check contents of data[1]
FAIL: gdb.arch/i386-sse.exp: check contents of data[2]
FAIL: gdb.arch/i386-sse.exp: check contents of data[3]
FAIL: gdb.arch/i386-sse.exp: check contents of data[4]
FAIL: gdb.arch/i386-sse.exp: check contents of data[5]
FAIL: gdb.arch/i386-sse.exp: check contents of data[6]
FAIL: gdb.arch/i386-sse.exp: check contents of data[7]
Tom de Vries [Fri, 22 Nov 2024 18:34:24 +0000 (19:34 +0100)]
[gdb/python] Handle failure to initialize without exiting
I tried out making python initialization fail by passing an incorrect
PYTHONHOME, and got:
...
$ PYTHONHOME=foo ./gdb.sh -q
Python path configuration:
PYTHONHOME = 'foo'
...
Python initialization failed: \
failed to get the Python codec of the filesystem encoding
Python not initialized
$
...
The relevant part of the code is:
...
static void
gdbpy_initialize (const struct extension_language_defn *extlang)
{
if (!do_start_initialization () && py_isinitialized && PyErr_Occurred ())
gdbpy_print_stack ();
gdbpy_enter enter_py;
...
What happens is:
- gdbpy_enter::gdbpy_enter () is called, where we run into:
'if (!gdb_python_initialized) error (_("Python not initialized"));'
- the error propagates to gdb's toplevel
- gdb print the error and exits.
It seems unnecesssary that we exit gdb. We could continue the
session without python support.
Fix this by:
- bailing out of gdbpy_initialize if !do_start_initialization
- bailing out of finalize_python if !gdb_python_initialized
This gets us instead:
...
$ PYTHONHOME=foo gdb -q
Python path configuration:
PYTHONHOME = 'foo'
...
Python initialization failed: \
failed to get the Python codec of the filesystem encoding
(gdb) python print (1)
Python not initialized
(gdb)
...
Tom de Vries [Fri, 22 Nov 2024 18:34:24 +0000 (19:34 +0100)]
[gdb/python] Fix abort on Py_Initialize
I tried out making python initialization fail by passing an incorrect
PYTHONHOME with python 3.6, and got:
...
$ PYTHONHOME=foo gdb -q
Fatal Python error: Py_Initialize: Unable to get the locale encoding
ModuleNotFoundError: No module named 'encodings'
Current thread 0x0000ffff89269c80 (most recent call first):
This is as per spec: when Py_Initialize () fails, a fatal error is raised
using Py_FatalError.
This can be worked around using:
...
$ PYTHONHOME=foo gdb -q -eiex "set python ignore-environment on"
(gdb)
...
but it would be better if gdb didn't abort.
I found an article [1] describing two solutions:
- try out Py_Initialize in a separate process, and
- catch the abort using a signal handler.
This patch implements the latter solution.
Obviously we cannot call into python anymore after the abort, so we avoid
calling Py_IsInitialized (), and instead use a new variable py_isinitialized.
This gets us instead:
...
$ PYTHONHOME=foo gdb -q
Fatal Python error: Py_Initialize: Unable to get the locale encoding
ModuleNotFoundError: No module named 'encodings'
Current thread 0x0000fffecfd49c80 (most recent call first):
Python not initialized
$
...
Tested on aarch64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
PR python/32379
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32379
Tom de Vries [Fri, 22 Nov 2024 18:34:24 +0000 (19:34 +0100)]
[gdb/python] Handle !Py_IsInitialized () in gdbpy_initialize
I tried out making python initialization fail by passing an incorrect
PYTHONHOME, and got:
...
$ PYTHONHOME=foo gdb -q
Python path configuration:
PYTHONHOME = 'foo'
...
Python Exception <class 'ModuleNotFoundError'>: No module named 'encodings'
Python not initialized
$
...
The relevant part of the code is:
...
static void
gdbpy_initialize (const struct extension_language_defn *extlang)
{
if (!do_start_initialization () && PyErr_Occurred ())
gdbpy_print_stack ();
gdbpy_enter enter_py;
...
What happens is that:
- do_start_initialization returns false because Py_InitializeFromConfig fails,
leaving us in the !Py_IsInitialized () state
- PyErr_Occurred () returns true
- gdbpy_print_stack is called, which prints
"Python Exception <class 'ModuleNotFoundError'>: No module named 'encodings"
The problem is that in the Py_IsInitialized () == false state, very few
functions can be called, and PyErr_Occurred is not one of them [1], and
likewise functions in gdbpy_print_stack.
Fix this by:
- guarding the PyErr_Occurred / gdbpy_print_stack part with Py_IsInitialized ().
- handling the !Py_IsInitialized () case by printing the failure PyStatus
in do_start_initialization
This gets us instead:
...
$ PYTHONHOME=foo ./gdb.sh -q
Python path configuration:
PYTHONHOME = 'foo'
...
Python initialization failed: failed to get the Python codec of the filesystem encoding
Python not initialized
$
...
Tested on aarch64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
[1] https://docs.python.org/3/c-api/init.html#before-python-initialization
Tom de Vries [Fri, 22 Nov 2024 16:44:29 +0000 (17:44 +0100)]
[gdbsupport] Add gdb::{waitpid,read,write,close}
We have gdb::handle_eintr, which allows us to rewrite:
...
ssize_t ret;
do
{
errno = 0;
ret = ::write (pipe[1], "+", 1);
}
while (ret == -1 && errno == EINTR);
...
into:
...
ssize_t ret = gdb::handle_eintr (-1, ::write, pipe[1], "+", 1);
...
However, the call to write got a bit mangled, requiring effort to decode it
back to its original form.
Instead, add a new function gdb::write that allows us to write:
...
ssize_t ret = gdb::write (pipe[1], "+", 1);
...
Andrew Burgess [Mon, 18 Nov 2024 10:58:26 +0000 (10:58 +0000)]
gdb/disasm: fix demangling when disassembling the current function
When disassembling function symbols in C++ code, if GDB is asked to
disassemble a function by name then the function name in the header
line can be demangled by turning on `set print asm-demangle on`, e.g.:
(gdb) disassemble foo_type::some_function
Dump of assembler code for function _ZN8foo_type13some_functionE7my_type:
0x0000000000401142 <+0>: push %rbp
... etc ...
End of assembler dump.
(gdb) set print asm-demangle on
(gdb) disassemble foo_type::some_function
Dump of assembler code for function foo_type::some_function(my_type):
0x0000000000401142 <+0>: push %rbp
... etc ... │
End of assembler dump. │
However, if GDB is disassembling the current function, then this
demangling doesn't work, e.g.:
(gdb) break foo_type::some_function
Breakpoint 1 at 0x401152: file mangle.cc, line 16.
(gdb) run
Starting program: /tmp/mangle
Breakpoint 1, foo_type::some_function (this=0x7fffffffa597, obj=...) at mangle.cc:16
16 obj.update ();
(gdb) disassemble
Dump of assembler code for function _ZN8foo_type13some_functionE7my_type:
0x0000000000401142 <+0>: push %rbp
... etc ...
End of assembler dump.
(gdb) set print asm-demangle on
(gdb) disassemble
Dump of assembler code for function _ZN8foo_type13some_functionE7my_type:
0x0000000000401142 <+0>: push %rbp
... etc ...
End of assembler dump.
This commit fixes this issue, and extends gdb.cp/disasm-func-name.exp,
which was already testing the first case (disassemble by name) to also
cover disassembling the current function.
Tom de Vries [Fri, 22 Nov 2024 16:34:50 +0000 (17:34 +0100)]
[gdb/python] Ensure locale is restored in do_start_initialization
I noticed in do_start_initialization:
...
std::string oldloc = setlocale (LC_ALL, NULL);
setlocale (LC_ALL, "");
...
if (count == (size_t) -1)
{
fprintf (stderr, "Could not convert python path to string\n");
return false;
}
setlocale (LC_ALL, oldloc.c_str ());
...
that the old locale is not restored if the "return false" is triggered.
Sam James [Mon, 18 Nov 2024 05:53:54 +0000 (05:53 +0000)]
libiberty: sync with gcc
This imports the following commits from GCC as of r15-5375-gbeec291225be9b: 94bea5dd6c9a libiberity: ANSIfy test-demangle.c aa84020b2edb libiberty: Fix comment typos c1b2100e736c libiberty: Restore build with CP_DEMANGLE_DEBUG defined bb8dd0980b39 libiberty: Fix up > 64K section handling in simple_object_elf_copy_lto_debug_section [PR116614]
Simplify amd64_windows_store_arg_in_reg by:
- replacing memset with value initialization,
- making valbuf a gdb::array_view, allowing us to:
- replace memcpy with std::copy, and
- use valbuf.size () instead of arg->type->size (), and
- dropping the std::min in std::min (type->length (), (ULONGEST) 8), since
we're already asserting that type->length () <= 8.
Suggested-By: Tom Tromey <tom@tromey.com>
Tested by rebuilding on x86_64-linux.
Tom de Vries [Fri, 22 Nov 2024 11:54:57 +0000 (12:54 +0100)]
[gdb/build] Workaround tsan select bug
When building gdb with -O0 and -fsanitize-thread, I run into a large number of
timeouts caused by gdb hanging, for instance:
...
(gdb) continue^M
Continuing.^M
[Inferior 1 (process 378) exited normally]^M
FAIL: gdb.multi/stop-all-on-exit.exp: continue until exit (timeout)
...
What happens is the following:
- two inferiors are added, stopped at main
- inferior 1 is setup to exit after 1 second
- inferior 2 is setup to exit after 10 seconds
- the continue command is issued
- because of set schedule-multiple on, both inferiors continue
- the first inferior exits
- gdb sends a SIGSTOP to the second inferior
- the second inferior receives the SIGSTOP, and raises a SIGCHILD
- gdb calls select, and blocks
- the signal arrives, and interrupts select
- ThreadSanitizers signal handler is called, which marks the signal pending
internally
- select returns -1 with errno == EINTR
- gdb calls select again, and blocks
- gdb hangs, waiting for gdb's sigchild_handler to be called
This is a bug [1] in ThreadSanitizer. When select is called with
timeout == nullptr, it is blocking but ThreadSanitizer doesn't consider it so,
and consequently doesn't see the need to call sigchild_handler.
Work around this by:
- instead of using the blocking select variant, forcing a small timeout and
- upon timeout calling a function that ThreadSanitizer does consider
blocking: usleep, forcing sigchild_handler to be called.
Tom de Vries [Fri, 22 Nov 2024 11:54:57 +0000 (12:54 +0100)]
[gdb] Add gdb_select variant for looping
In interruptible_select we run gdb_select in a loop:
...
do
{
res = gdb_select (n, readfds, writefds, exceptfds, timeout);
}
while (res == -1 && errno == EINTR);
...
but man select tells us that:
- if using select() within a loop, the sets (readfds, writefds and
exceptfds) must be reinitialized before each call, and
- timeout should be considered to be undefined after select() returns.
Add a gdb_select variant:
...
static int
gdb_select (int n,
const fd_set *req_readfds, fd_set *ret_readfds,
const fd_set *req_writefds, fd_set *ret_writefds,
const fd_set *req_exceptfds, fd_set *ret_exceptfds,
const struct timeval *req_timeout, struct timeval *ret_timeout)
...
that keeps requested and returned values separate, ensuring that the requested
values stay constant.
Tested on x86_64-linux.
Reviewed-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com>
Martin Storsjö [Fri, 6 Sep 2024 13:18:02 +0000 (16:18 +0300)]
ld/PE: Handle MS style import libraries for files named *.exe too
When handling MS style import libraries (also called short import
libraries, or ILF), we need to detect the kind of library.
So far, this has been done by looking at the member file names
in the import library - in an MS style import library, all the
member files for a specific library have the same member file
name - the name of the runtime module to link against. Usually
this is a DLL - thus we do a case insensitive comparison and
check if the suffix is .dll.
However, an .exe can also export symbols which can be linked
against in the same way. In particular, if linking against
WDK (Windows Driver Kit) import libraries, e.g. wdmsec.lib, the
import libraries can provide imports for ntoskrnl.exe.
Instead of specifically checking for *.dll (and *.exe, etc),
invert the condition and skip archive members named *.o and *.obj.
For any remaining archive members, that do contain .idata
sections, apply the renaming. (The renaming is also mostly
harmless if applied where it isn't needed; if archive members
already have unique file names, their relative ordering should
remain intact except for very contrieved cases.)
Nelson Chu [Wed, 20 Nov 2024 08:30:39 +0000 (16:30 +0800)]
RISC-V: Support SiFive extensions: xsfvqmaccdod, xsfvqmaccqoq and xsfvfnrclipxfqf
Those SiFive extensions have been published on the web for a while, and we plan
to implement intrinsics in GCC for those instructions soon.
NOTE: The original patch was written by Nelson when he was still working at
SiFive, and Kito rebased it to the trunk. Therefore, I kept the author as Nelson
with his SiFive email.
Tom Tromey [Wed, 13 Nov 2024 15:49:53 +0000 (08:49 -0700)]
Don't put JIT_READER_DIR into help text
The 80-column-help-string self-test can fail if gdb's install
directory is too long, because the help for "jit-reader-load" includes
JIT_READER_DIR.
This help text is actually somewhat misleading, though.
JIT_READER_DIR is not actually used directly -- instead the relocated
variant is used.
This patch adds a new "show jit-reader-directory" command and changes
the help text to refer to this instead. I considered adding a "set"
command as well, but since absolute paths are acceptable here, and
since this is a very niche command anyway, I figured there was no need
to bother.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32357 Reviewed-By: Kévin Le Gouguec <legouguec@adacore.com> Reviewed-By: Eli Zaretskii <eliz@gnu.org> Approved-By: Andrew Burgess <aburgess@redhat.com>
Andrew Burgess [Mon, 18 Nov 2024 16:19:43 +0000 (16:19 +0000)]
gdb/build-id: protect against weirdly short build-ids
While looking at build_id_to_bfd_suffix (in gdb/build-id.c) I realised
that GDB would likely not do what we wanted if a build-id was ever a
single byte.
Right now, build-ids generated by the GNU linker are 32 bytes, but
there's nothing that forces this to be the case, it's pretty easy to
create a fake, single byte, build-id. Given that the build-id is an
external input (read from the objfile), GDB should protect itself
against these edge cases.
The problem with build_id_to_bfd_suffix is that this function creates
the path used to lookup either the debug information, or an
executable, based on its build-id. So a 3-byte build-id 0xaabbcc will
look in the path: `$DEBUG_FILE_DIRECTORY/.build-id/aa/bbcc.debug`.
However, a single byte build-id 0xaa, will look in the file:
`$DEBUG_FILE_DIRECTORY/.build-id/aa/.debug` which doesn't seem right.
Worse, when looking for an objfile given a build-id GDB will look for
a file called `$DEBUG_FILE_DIRECTORY/.build-id/aa/` with a trailing
'/' character.
I propose that, in build_id_to_bfd_suffix we just return early if the
build-id is 1 byte (or less) with a return value that indicates no
separate file was found.
For testing I made use of the DWARF assembler. I needed to update the
build-id creation proc, the existing code assumes that the build-id is
a multiple of 4 bytes, so I added some additional padding to ensure
that the generated note was a multiple of 4 bytes, even if the
build-id was not.
I added a test with a 1 byte build-id, and also for the case where the
build-id has zero length. The zero length case already does what
you'd expect (no debug is loaded) as the bfd library rejects the
build-id when loading it from the objfile, but adding this additional
test is pretty cheap.
Rohr, Stephan [Thu, 19 Sep 2024 10:55:19 +0000 (12:55 +0200)]
testsuite: skip confirmation in 'gdb_reinitialize_dir'
Some shells automatically confirm the 'dir' command:
(gdb) dir
Reinitialize source path to empty? (y or n)
[answered Y; input not from terminal]
Source directories searched: $cdir;$cwd
(gdb) y
dir <...>/gdb/testsuite/gdb.base
Undefined command: "y". Try "help".
For example, this reprdocues in a MinGW32 environment with
'TERM=dumb'. Skip sending 'y' if the command is already confirmed.
Tom Tromey [Mon, 4 Nov 2024 14:31:20 +0000 (07:31 -0700)]
Improve choice sorting in ada-lang.c
ada-lang.c has a "sort_choices" function that claims to sort the
symbol choices, but which does not really implement sorting. This
patch changes this code to really sort the result vector, sorting
first by filename, then line number, and finally by the symbol name.
The filename sorting is done first by comparing basenames. It turns
out that gnatmake and gprbuild invoke the compiler a bit differently,
so depending on which one you use, the results of a naive sort might
be different (due to the use of absolute or relative paths).
Andre Vieira [Wed, 20 Nov 2024 17:15:28 +0000 (17:15 +0000)]
arm: Support pac_key_* register operand for MRS/MSR in Armv8.1-M Mainline
Add support for pac_key_[pu]_[0-3](_ns)? register operands for the MRS and MSR
instructions when assembling for Armv8.1-M Mainline, as well as adding the
corresponding support for disassembling instructions that use it.
Andrew Burgess [Tue, 19 Nov 2024 10:37:49 +0000 (10:37 +0000)]
gdb/python: fix reference leak in gdb.BreakpointLocation.thread_groups
While reviewing another patch which uses PyList_Append I took a look
at our other uses of PyList_Append in GDB. I spotted something odd
about the use in bplocpy_get_thread_groups.
We do:
gdbpy_ref<> num = gdb_py_object_from_ulongest (inf->num);
At which point `num` will own a reference to the `int` object. But
when we add the object to the result list we do:
if (PyList_Append (list.get (), num.release ()) != 0)
return nullptr;
By calling `release` we pass ownership of the reference to
PyList_Append, however, PyList_Append acquires its own reference, it
doesn't take ownership of an existing reference.
The consequence of this is that we leak the reference held in `num`.
This mostly isn't a problem though. For small (< 257) integers Python
keeps a single instance of each and just hands out new references. By
leaking the references, these small integers will not be cleaned up as
the Python interpreter shuts down, but that is only done when GDB
exits, so hardly a disaster. As we're dealing with GDB's internal
inferior number here, unless the user has 257+ inferiors, we'll not
actually be leaking memory.
Still, lets do things right. Switch to using `num.get ()`. Now when
`num` goes out of scope it will decrement the reference count as
needed.
Jiawei [Tue, 29 Oct 2024 12:44:23 +0000 (20:44 +0800)]
RISC-V: Add Zcmt instructions and csr.
This patch supports Zcmt[1] instruction 'cm.jt' and 'cm.jalt'.
Add new CSR jvt for tablejump using. Since 'cm.jt' and 'cm.jalt'
have the same instructiong encoding, use 'match_cm_jt' and 'match_cm_jalt'
check the 'zcmt_index' field to distinguish them.
Co-Authored by: Charlie Keaney <charlie.keaney@embecosm.com>
Co-Authored by: Mary Bennett <mary.bennett@embecosm.com>
Co-Authored by: Nandni Jamnadas <nandni.jamnadas@embecosm.com>
Co-Authored by: Sinan Lin <sinan.lin@linux.alibaba.com>
Co-Authored by: Simon Cook <simon.cook@embecosm.com>
Co-Authored by: Shihua Liao <shihua@iscas.ac.cn>
Co-Authored by: Yulong Shi <yulong@iscas.ac.cn>
bfd/ChangeLog:
* elfxx-riscv.c (riscv_multi_subset_supports): New extension.
(riscv_multi_subset_supports_ext): Ditto.
gas/ChangeLog:
* config/tc-riscv.c (enum riscv_csr_class): New CSR.
(riscv_csr_address): Ditto.
(validate_riscv_insn): New operand.
(riscv_ip): Ditto.
* testsuite/gas/riscv/csr-version-1p10.d: New CSR.
* testsuite/gas/riscv/csr-version-1p10.l: Ditto.
* testsuite/gas/riscv/csr-version-1p11.d: Ditto.
* testsuite/gas/riscv/csr-version-1p11.l: Ditto.
* testsuite/gas/riscv/csr-version-1p12.d: Ditto.
* testsuite/gas/riscv/csr-version-1p12.l: Ditto.
* testsuite/gas/riscv/csr.s: Ditto.
* testsuite/gas/riscv/march-help.l: New extension.
* testsuite/gas/riscv/zcmt-fail.d: New test.
* testsuite/gas/riscv/zcmt-fail.l: New test.
* testsuite/gas/riscv/zcmt-fail.s: New test.
* testsuite/gas/riscv/zcmt.d: New test.
* testsuite/gas/riscv/zcmt.s: New test.
include/ChangeLog:
* opcode/riscv-opc.h (MATCH_CM_JT): New opcode.
(MASK_CM_JT): New mask.
(MATCH_CM_JALT): New opcode.
(MASK_CM_JALT): New mask.
(CSR_JVT): New CSR.
(DECLARE_INSN): New declaration.
(DECLARE_CSR): Ditto.
* opcode/riscv.h (EXTRACT_ZCMT_INDEX): New marco.
(ENCODE_ZCMT_INDEX): Ditto.
(enum riscv_insn_class): New class.
opcodes/ChangeLog:
* riscv-dis.c (print_insn_args): New operand.
* riscv-opc.c (match_cm_jt): New function.
(match_cm_jalt): Ditto.
Tom de Vries [Tue, 19 Nov 2024 11:32:40 +0000 (12:32 +0100)]
[gdb/contrib] Fix shellcheck warnings in spellcheck.sh
Fix shellcheck warnings in spellcheck.sh, found using shellcheck v0.10.0.
Ran shellcheck v0.10.0 (on a system with shellcheck version 0.8.0) using this
command from an RFC patch [1]:
...
$ ./gdb/contrib/pre-commit-shellcheck.sh ./gdb/contrib/spellcheck.sh
...
Nelson Chu [Tue, 8 Oct 2024 04:35:43 +0000 (12:35 +0800)]
RISC-V: Don't report warnings when linking different privileged spec objects.
Since only the abandoned privileged spec v1.9.1 will have conflict csrs, to
keep the compatible we still report warnings when linking privileged spec
v1.9.1 objects with others. But don't report warnings for other compatible
cases because it is actually a bit noisy and useless...
bfd/
* elfnn-riscv.c (riscv_merge_attributes): Only report warnings when
linking the abandoned privileged spec v1.9.1 object with others.
ld/
* testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-01.d: Removed.
* testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-02.d: Removed.
* testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-03.d: Removed.
* testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-04.d: Removed.
* testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-05.d: Removed.
* testsuite/ld-riscv-elf/attr-merge-priv-spec-failed-06.d: Removed.
* testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated.
Lulu Cai [Tue, 5 Nov 2024 08:21:28 +0000 (16:21 +0800)]
LoongArch: Do not relax pcalau12i+ld.d when there is overflow
There is no overflow check for the relaxation of pcalau12i+ld.d =>
pcalau12i+addi.d. For instruction sequences that can be relaxed,
they are directly relaxed to pcalau12i+addi.d. However, when the
relative distance between the symbol and the pc exceeds the 32-bit
range, the symbol value cannot be obtained correctly.
Adds an overflow check for the relaxation of pcalau12i+ld.d.
If it is found that the relaxation will overflow, it will not
be relaxed.
LAM: Enable tagged pointer support for watchpoints.
The Intel (R) linear address masking (LAM) feature modifies the checking
applied to 64-bit linear addresses. With this so-called "modified
canonicality check" the processor masks the metadata bits in a pointer
before using it as a linear address. LAM supports two different modes that
differ regarding which pointer bits are masked and can be used for
metadata: LAM 48 resulting in a LAM width of 15 and LAM 57 resulting in a
LAM width of 6.
This patch adjusts watchpoint addresses based on the currently enabled
LAM mode using the untag mask provided in the /proc/<pid>/status file.
As LAM can be enabled at runtime or as the configuration may change
when entering an enclave, GDB checks enablement state each time a watchpoint
is updated.
In contrast to the patch implemented for ARM's Top Byte Ignore "Clear
non-significant bits of address on memory access", it is not necessary to
adjust addresses before they are passed to the target layer cache, as
for LAM tagged pointers are supported by the system call to read memory.
Additionally, LAM applies only to addresses used for data accesses.
Thus, it is sufficient to mask addresses used for watchpoints.
The following examples are based on a LAM57 enabled program.
Before this patch tagged pointers were not supported for watchpoints:
~~~
(gdb) print pi_tagged
$2 = (int *) 0x10007ffffffffe004
(gdb) watch *pi_tagged
Hardware watchpoint 2: *pi_tagged
(gdb) c
Continuing.
Couldn't write debug register: Invalid argument.
~~~~
Once LAM 48 or LAM 57 is enabled for the current program, GDB can now
specify watchpoints for tagged addresses with LAM width 15 or 6,
respectively.
Approved-By: Felix Willgerodt <felix.willgerodt@intel.com>
The gdbarch function gdbarch_remove_non_address_bits adjusts addresses to
enable debugging of programs with tagged pointers on Linux, for instance for
ARM's feature top byte ignore (TBI).
Once the function is implemented for an architecture, it adjusts addresses for
memory access, breakpoints and watchpoints.
Linear address masking (LAM) is Intel's (R) implementation of tagged
pointer support. It requires certain adaptions to GDB's tagged pointer
support due to the following:
- LAM supports address tagging for data accesses only. Thus, specifying
breakpoints on tagged addresses is not a valid use case.
- In contrast to the implementation for ARM's TBI, the Linux kernel supports
tagged pointers for memory access.
This patch makes GDB's tagged pointer support configurable such that it is
possible to enable the address adjustment for a specific feature only (e.g
memory access, breakpoints or watchpoints). This way, one can make sure
that addresses are only adjusted when necessary. In case of LAM, this
avoids unnecessary parsing of the /proc/<pid>/status file to get the
untag mask.
Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
(AArch64) Tested-By: Luis Machado <luis.machado@arm.com> Approved-By: Luis Machado <luis.machado@arm.com>
Jan Beulich [Mon, 18 Nov 2024 10:46:28 +0000 (11:46 +0100)]
x86: rename SPACE_{,E}VEX_MAP<N>
Map7 already has dual purpose for USER-MSR (and is to gain more for
MSR-IMM), while Map5 is about to gain VEX uses for AMX extensions. Drop
the not really meaningful infixes and (in the opcode table) prefixes,
retaining merely EVexMap4 for encoding EVex128 at the same time.
Tom de Vries [Mon, 18 Nov 2024 10:42:44 +0000 (11:42 +0100)]
[gdb/contrib] Fix spellcheck.sh for bash < 5.1
Since commit 5cb0406bb64 ("[gdb/contrib] Handle capitalized words in
spellcheck.sh"), spellcheck.sh uses '${pat@u}' which is available starting
bash 5.1, and consequently the script breaks with bash 4.4.
Fix this by checking for the bash version, and using an alternative
implementation for bash < 5.1.
Benjamin Drung [Mon, 18 Nov 2024 10:38:25 +0000 (11:38 +0100)]
ld: Support percent-encoded JSON in --package-metadata
Specifying the compiler flag `-Wl,--package-metadata=<JSON>` will not
work in case the JSON contains a comma, because compiler drivers eat
commas. Example:
```
$ echo "void main() { }" > test.c
$ gcc '-Wl,--package-metadata={"type":"deb","os":"ubuntu"}' test.c
/usr/bin/ld: cannot find "os":"ubuntu"}: No such file or directory
collect2: error: ld returned 1 exit status
```
The quotation marks in the JSON value do not work well with shell nor
make. Specifying the `--package-metadata` linker flag in a `LDFLAGS`
environment variable might loose its quotation marks when it hits the
final compiler call.
So support percent-encoded and %[string] encoded JSON data in the
`--package-metadata` linker flag. Percent-encoding is used because it is
a standard, simple to implement, and does take too many additional
characters. %[string] encoding is supported for having a more readable
encoding.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32003
Bug-Ubutru: https://bugs.launchpad.net/bugs/2071468 Signed-off-by: Benjamin Drung <benjamin.drung@canonical.com>
Jan Beulich [Mon, 18 Nov 2024 10:37:31 +0000 (11:37 +0100)]
gas: move had_errors() invocation in finishing of subsegs
Invoking this repeatedly in an inner loop is not only inefficient, but
may lead to inconsistencies in e.g. the listings that the original
comment author cared about. (Accept potential inconsistencies across
distinct sections though, to cover all invocations of the function.)
Jan Beulich [Mon, 18 Nov 2024 10:36:57 +0000 (11:36 +0100)]
ELF: SHF_STRINGS isn't really tied to SHF_MERGE
It's not overly useful without it, but the spec doesn't name any
dependency between the two. People may want to use it for purely
informational purposes, for example. Adjust, in particular, entity size
processing to be engaged if either flag is set, as mandated by the spec.
Jan Beulich [Mon, 18 Nov 2024 10:36:30 +0000 (11:36 +0100)]
ELF: SHF_MERGE vs SHT_NOBITS
bfd/merge.c puts in quite some effort to track mergable sections. That's
all wasted for sections which don't have contents, as for them
_bfd_write_merged_section() will never be called.
With the combination not having any useful effect, also warn about this
in gas.
C23 removes support for unprototyped functions. Fix function pointer types
accordingly.
This does not fix all instances, there's a few left as I commented on in
PR32374 (e.g. setitimer which I have a local workaround for but it involves
a glibc implementation detail; the Linaro precommit CI tester pointed that
out too, so dropped that).
Sam James [Sat, 16 Nov 2024 07:07:02 +0000 (07:07 +0000)]
ld: fix -std=gnu23 compatibility wrt _Bool
GCC trunk now defaults to -std=gnu23. We return false in a few places
which can't work when true/false are a proper type (_Bool). Return NULL
where appropriate instead of false. All callers handle this appropriately.
Sam James [Sat, 16 Nov 2024 05:12:51 +0000 (05:12 +0000)]
binutils: fix -std=gnu23 compatibility wrt _Bool
GCC trunk now defaults to -std=gnu23. We return false in a few places
which can't work when true/false are a proper type (_Bool). Return NULL
where appropriate instead of false. All callers handle this appropriately.
Sam James [Sat, 16 Nov 2024 05:01:58 +0000 (05:01 +0000)]
bfd: fix -std=gnu23 compatibility wrt _Bool
GCC trunk now defaults to -std=gnu23. We return false in a few places
which can't work when true/false are a proper type (_Bool). Return NULL
where appropriate instead of false. All callers handle this appropriately.
H.J. Lu [Sun, 17 Nov 2024 00:49:00 +0000 (08:49 +0800)]
x86-64: Always check IBT PLT before BND PLT
Since BND PLT has been deprecated and the same IBT PLT is used for both
x86-64 and x32, always check IBT PLT before BND PLT when synthesizing
PLT symtab.
* elf64-x86-64.c (elf_x86_64_get_synthetic_symtab): Always check
elf_x86_64_lazy_ibt_plt and elf_x86_64_non_lazy_ibt_plt first.
Ijaz, Abdul B [Tue, 23 Jul 2024 15:59:29 +0000 (17:59 +0200)]
gdb: Update linkage name lookup function to allow mst_file_data/bss types.
From the commit 667ed4b14ddaa9af196481f1757c0e517e80b6ed onward, instead
of normal name GDB looks for the "jit_descriptor" linkage name in the JIT
code initialization. Without this change, the function
"lookup_minimal_symbol_linkage", only matches the non-static data. So in
case jit_debugger is static type then setting up breakpoint in the JIT code
fails. Issue is seen for the intel compilers, where jit_debug_descriptor has
static type i.e. "mst_file_data". Hence lookup_minimal_symbol_linkage returns
nullptr for it. So, in this case breakpoint does not hit in the JIT code.
To resolve this, the commit introduces a new boolean argument to the
lookup_minimal_symbol_linkage function. This argument allows the function to
also match mst_file_data and mst_file_bss types when set to true. The
function is called with this new argument set to true only from JIT code
initialization handling, ensuring that the current behavior remains unchanged
for other cases. Because handling of static types of data symbols for all cases
result in regression for "gdb.base/print-file-var.exp" test.
Example of minsym for the JIT code emitted by the intel compilers where
lookup_minimal_symbol_linkage fails without this change because jit_debugger
type is "mst_file_data".
Tom de Vries [Fri, 15 Nov 2024 21:48:37 +0000 (22:48 +0100)]
[gdb/symtab] Fix segfault with dwp file
Consider the following test-case:
...
$ cat test.c
int main (void) { return 0; }
$ clang -g -gsplit-dwarf test.c -o test
$ llvm-dwp -e test -o test.dwp
...
This runs into a segmentation fault:
...
$ gdb -q -batch test
Fatal signal: Segmentation fault
...
The segmentation fault happens because in read_dwo_str_index this line sets p
to nullptr:
...
const gdb_byte *p = reader->dwo_file->sections.str_offsets.buffer;
...
while the following code expects it to point to some data.
The section we're trying to read is:
...
(gdb) p reader->dwo_file->sections.str_offsets
$4 = {s = {section = 0xffffcc00a9d0, containing_section = 0xffffcc00a9d0},
buffer = 0x0, size = 28, virtual_offset = 0, readin = false, is_virtual = true}
...
At first glance, the section is not readin, but actually it is.
This is a virtual section, meaning part of a containing section:
...
(gdb) p *reader->dwo_file->sections.str_offsets.s.containing_section
$8 = {s = {section = 0xffffcc00cde8, containing_section = 0xffffcc00cde8},
buffer = 0xffffcc009650 "\030", size = 28, virtual_offset = 0, readin = true,
is_virtual = false}
...
which is readin.
Fix this in create_dwp_v2_or_v5_section by initializing the buffer of the
virtual section using the buffer of the containing section:
...
result.buffer = section->buffer + offset;
...
Unfortunately it's difficult to write a test-case for this. We'll have to
teach the dwarf assembler to generate dwp files.
Tested on aarch64-linux.
This is a partial fix for PR symtab/31497.
Approved-By: Tom Tromey <tom@tromey.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31497
Tom Tromey [Fri, 15 Nov 2024 15:02:25 +0000 (08:02 -0700)]
Improvements to gdb.LazyString documentation
I noticed the gdb.LazyString documentation did not mention how to
create one. Then, while adding this, I found a couple other ways that
this documentation could be clarified.
For gcc 7.x and older, without the -gstatement-frontiers work, the
compiler uses DW_AT_entry_pc differently, which leads to a poorer
debug experience.
Here is the interesting source line from inline-entry.c:
if ((global && bar (1)) || bar (2))
And here's some of the relevant disassembly output:
Dump of assembler code for function main:
0x401020 <+0>: mov 0x3006(%rip),%eax (1)
0x401026 <+6>: test %eax,%eax (2)
0x401028 <+8>: mov 0x2ffe(%rip),%eax (3)
0x40102e <+14>: je 0x401038 <main+24> (4)
0x401030 <+16>: sub $0x1,%eax (5)
0x401033 <+19>: jne 0x40103d <main+29> (6)
Lines (1), (2), and (4) represent the check of 'global'. However,
line (3) is actually the first instruction for 'bar' which has been
inlined. Lines (5) and (6) are also part of the first inlined 'bar'
function.
If the check of 'global' returns false then the first call to 'bar'
should never happen, this is accomplished by the branch at (4) being
taken.
For gcc 8+, gcc generates a DW_AT_entry_pc with the value 0x401030,
this is where GDB places a breakpoint for 'bar', and this address is
after the branch at line (4), and so, if the call to 'bar' never
happens, the breakpoint is never hit.
For gcc 7 and older, gcc generates a DW_AT_entry_pc with the value
0x401028, which is the first address associated with the inline 'bar'
function. Unfortunately, this address is also before the check of
'global' has completed, this means that GDB hits the 'bar' breakpoint
before the inferior has decided if 'bar' should actually be called or
not.
I don't think there's really much GDB can do in the older gcc
versions, we are placing the breakpoint at the entry point, and this
is within bar. Given that this test does really depend on the newer
gcc behaviour, I think the only sensible solution is to skip this test
when an older version of gcc is being used.
I've incorporated the check for -gstatement-frontiers support that
Bernd suggested and now the test will be skipped for older versions of
GCC.
Andrew Burgess [Sun, 10 Nov 2024 19:58:25 +0000 (19:58 +0000)]
gdb/python: missing PyObject_IsTrue error check in bppy_init
As with the previous two commits, this commit fixes a location where
we called PyObject_IsTrue without including an error check, this time
in bppy_init.
The 'qualified' argument is supposed to be a bool, the docs say:
The optional QUALIFIED argument is a boolean that allows
interpreting the function passed in 'spec' as a fully-qualified
name. It is equivalent to 'break''s '-qualified' flag (*note
Linespec Locations:: and *note Explicit Locations::).
It's not totally clear that the only valid values are True or False,
but I'm choosing to interpret the docs that way, and so I've added a
PyBool_Type check during argument parsing. Now, if a non-bool is
passed the user will get a TypeError during argument parsing. I've
added a test to cover this case.
This is a potentially breaking change to the Python API, but hopefully
this will not impact too many people. I've added a NEWS entry to
highlight this change.
Reviewed-By: Eli Zaretskii <eliz@gnu.org> Approved-By: Tom Tromey <tom@tromey.com>
Andrew Burgess [Sun, 10 Nov 2024 15:50:26 +0000 (15:50 +0000)]
gdb/python: missing PyObject_IsTrue error check in micmdpy_set_installed
Like the previous commit, I discovered that in micmdpy_set_installed
we were calling PyObject_IsTrue, but not checking for a possible error
value being returned.
The micmdpy_set_installed function implements the
gdb.MICommand.installed attribute, and the documentation indicates that
this attribute should only be assigned a bool:
This attribute is read-write, setting this attribute to 'False'
will uninstall the command, removing it from the set of available
commands. Setting this attribute to 'True' will install the
command for use.
So I propose that instead of using PyObject_IsTrue we use
PyBool_Check, and if the new value fails this check we raise an
error. We can then compare the new value to Py_True directly instead
of calling PyObject_IsTrue.
This is a potentially breaking change to the Python API, but hopefully
this will not impact too many people, and the fix is pretty
easy (switch to using a bool). I've added a NEWS entry to draw
attention to this change.
Andrew Burgess [Sun, 10 Nov 2024 14:33:23 +0000 (14:33 +0000)]
gdb/python: missing PyObject_IsTrue error check in py-arch.c
Building on the previous two commits, I was auditing our uses of
PyObject_IsTrue looking for places where we were missing an error
check.
The gdb.Architecture.integer_type() function takes a 'signed' argument
which should be a 'bool', and the docs do say:
If SIGNED is not specified, it defaults to 'True'. If SIGNED is
'False', the returned type will be unsigned.
Currently we use PyObject_IsTrue, but we are missing an error check.
To fix this I've tightened the code to enforce the bool requirement at
the point that the arguments are parsed. With that done I can remove
the call to PyObject_IsTrue and instead compare to Py_True directly,
the object in question will always be a PyBool_Type.
However, we were testing that passing a non-bool argument for 'signed'
is treated as Py_False, this was added with this commit:
gdb/python: don't use the 'p' format for parsing args
which is when the PyObject_IsTrue call was added. Given that the docs
do seem pretty clear that only True or False are suitable argument
values, my proposal is that we just remove these tests and instead
test that any non-bool argument value for 'signed' gives a TypeError.
This is a breaking change to the Python API, however, my hope is that
this is such a edge case that it will not cause too many problem.
I've added a NEWS entry to highlight this change though.
Reviewed-By: Eli Zaretskii <eliz@gnu.org> Approved-By: Tom Tromey <tom@tromey.com>
Andrew Burgess [Sun, 10 Nov 2024 14:34:58 +0000 (14:34 +0000)]
gdb/python: remove some additional PyObject_IsTrue calls
After the previous commit I audited all our uses of PyObject_IsTrue
looking for places where we were missing an error check. I did find
some that are missing error checks in places where we really should
have error checks, and I'll fix those in later commits.
This commit however, focuses on those locations where PyObject_IsTrue
is called, there is no error check, and the error check isn't really
necessary because we already know that the object we are dealing with
is of type PyBool_Type.
Inline with the previous commit, in these cases I have removed the
PyObject_IsTrue call, and replaced it with a comparison against
Py_True. In one location where it is not obvious that the object we
have is PyBool_Type I've added an assert, but in the other cases the
comparison to Py_True immediately follows a PyBool_Check call, so an
assert would be redundant.
I've added a test for the gdb.Value.format_string styling argument
being passed a non-bool value as this wasn't previously being tested,
though this new test will pass before and after this commit.
There should be no functional change after this commit.
Tom pointed out that using PyObject_IsTrue as I was doing, though
technically fine, at least appears to be missing an error check, and
that it would be better to compare to Py_True directly. I made that
change in the patch Tom was commenting on, but I'd actually copied
that code from elsewhere in python/python.c, so this commit updates
the original code inline with Tom's review feedback.
There should be no functional change after this commit.
Tom de Vries [Wed, 13 Nov 2024 21:41:35 +0000 (22:41 +0100)]
[gdb/tdep] Handle syscall clock_gettime64 for arm-linux
When running test-case gdb.reverse/time-reverse.exp on arm-linux, I run into:
...
(gdb) continue^M
Continuing.^M
Process record and replay target doesn't support syscall number 403^M
Process record does not support instruction 0xdf00 at address 0xf7ebf774.^M
Process record: failed to record execution log.^M
^M
Program stopped.^M
0xf7ebf774 in ?? () from /lib/arm-linux-gnueabihf/libc.so.6^M
(gdb) FAIL: $exp: mode=c: continue to breakpoint: marker2
...
Syscall number 403 stands for clock_gettime64 on arm-linux.
Fix this by handling 403 in arm_canonicalize_syscall, and handling
gdb_sys_clock_gettime64 elsewhere.
Since i386_canonicalize_syscall is the identity function, enum value
gdb_sys_clock_gettime64 gets a value to match i386, which also happens to be
403.
Tom de Vries [Wed, 13 Nov 2024 21:38:19 +0000 (22:38 +0100)]
[gdb/contrib] Handle capitalized words in spellcheck.sh
The dictionary contains a few entries with capital letters:
...
$ grep -E '[A-Z]' .git/wikipedia-common-misspellings.txt | wc -l
143
...
but they don't look too interesting in the gdb context (for instance,
Habsbourg->Habsburg), so filter them out.
That leaves us with entries looking only like "foobat->foobar", so add
handling of capitalized words, such that we also rewrite "Foobat" to "Foobar".
Tested on aarch64-linux. Verified with shellcheck.
Tom de Vries [Wed, 13 Nov 2024 20:06:58 +0000 (21:06 +0100)]
[gdb/contrib] Handle double quotes in spellcheck.sh
Add handling of double quotes in gdb/contrib/spellcheck.sh, and fix the
following typos:
...
inheritence -> inheritance
psuedo -> pseudo
succeded -> succeeded
...
Tom de Vries [Wed, 13 Nov 2024 20:03:42 +0000 (21:03 +0100)]
[precommit] Add some documentation in .pre-commit-config.yaml
Add some documention to .pre-commit-config.yaml that explains:
- what the file is,
- how it can be used, and
- how to skip specific hooks in case of trouble.
Tom de Vries [Wed, 13 Nov 2024 18:44:21 +0000 (19:44 +0100)]
[gdb/tdep] Fix recording of T1 push
When running test-case gdb.reverse/recursion.exp on arm-linux with target
board unix/-mthumb, I run into:
...
(gdb) PASS: gdb.reverse/recursion.exp: Skipping recursion from inside
reverse-next^M
bar (x=4195569) at /home/linux/gdb/src/gdb/testsuite/gdb.reverse/recursion.c:34^M
34 int r = foo (x);^M
(gdb) FAIL: gdb.reverse/recursion.exp: print frame when stepping out
...
The problem is the recording of the T1 push instruction [1,2], specifically:
... 000004d8 <foo>:
4d8: b580 push {r7, lr}
...
The current code fails to add a memory record for the memory written with the
value of the lr register.
Tom de Vries [Wed, 13 Nov 2024 18:37:04 +0000 (19:37 +0100)]
[gdb/tdep] Handle sycall statx for arm-linux
When running test-case gdb.reverse/fstatat-reverse.exp on arm-linux, I run
into:
...
(gdb) continue^M
Continuing.^M
Process record and replay target doesn't support syscall number 397^M
Process record does not support instruction 0xdf00 at address 0xf7ebf774.^M
Process record: failed to record execution log.^M
^M
Program stopped.^M
0xf7ebf774 in ?? () from /lib/arm-linux-gnueabihf/libc.so.6^M
(gdb) FAIL: gdb.reverse/fstatat-reverse.exp: continue to breakpoint: marker2
...
Syscall number 397 stands for statx on arm-linux.
Fix this by handling 397 in arm_canonicalize_syscall.
Tested on arm-linux.
Reviewed-By: Guinevere Larsen <guinevere@redhat.com> Approved-By: Luis Machado <luis.machado@arm.com>
And I have written the stand alone test for this issue. The original
patch included this paragraph to explain this change (I've fixed one
typo in this text replacing 'program' with 'function'):
... it may happen that the infrun machinery steps from one inline
range to another inline range of the same inline function. That can
look like jumping back and forth from the calling function to the
inline function, while really the inline function just jumps from a
hot to a cold section of the code, i.e. error handling.
The important thing that happens here is that both the outer function
and the inline function must both have multiple ranges. When the
inferior is within the inline function and moves from one range to
another it is critical that the address we stop at is the start of a
range in both the outer function and the inline function.
The diagram below represents how the functions are split and aligned:
(A) (B)
bar: |------------| |---|
foo: |------------------| |--------|
The inferior is stepping through 'bar' and eventually reaches
point (A) at which point control passes to point (B).
Currently, when the inferior stops, GDB notices that both 'foo' and
'bar' start at address (B), and so GDB uses the inline frame mechanism
to skip 'bar' and tells the user that the inferior is in 'foo'.
However, as we were in 'bar' before the step then it makes sense that
we should be in 'bar' after the step, and this is what the patch does.
There are two tests using the DWARF assembler, the first checks the
above situation and ensures that GDB reports 'bar' after the step.
The second test is similar, but after the step we enter a new range
where a different inline function starts, something like this:
(A) (B)
bar: |------------|
baz: |---|
foo: |------------------| |--------|
In this case as we step at (A) and land at (B) we leave 'bar' and
expect to stop in 'foo', GDB shouldn't automatically enter 'baz' as
that is a completely different inline function. And this is, indeed,
what we see.
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Andrew Burgess [Thu, 10 Oct 2024 10:37:34 +0000 (11:37 +0100)]
gdb: fix handling of DW_AT_entry_pc of inlined subroutines
The entry PC for a DIE, e.g. an inline function, might not be the base
address of the DIE. Currently though, in block::entry_pc(), GDB
always returns the base address (low-pc or the first address of the
first range) as the entry PC.
This commit extends the block class to carry the entry PC as a
separate member variable. Then the DWARF reader is extended to read
and set the entry PC for the block. Now in block::entry_pc(), if the
entry PC has been set, this is the value returned.
If the entry-pc has not been set to a specific value then the old
behaviour of block::entry_pc() remains, GDB will use the block's base
address. Not every DIE will set the entry-pc, but GDB still needs to
have an entry-pc for every block, so the existing logic supplies the
entry-pc for any block where the entry-pc was not set.
The DWARF-5 spec for reading the entry PC is a super-set of the spec
as found in DWARF-4. For example, if there is no DW_AT_entry_pc then
DWARF-4 says to use DW_AT_low_pc while DWARF-5 says to use the base
address, which is DW_AT_low_pc or the first address in the first range
specified by DW_AT_ranges if there is no DW_AT_low_pc.
I have taken the approach of just implementing the DWARF-5 spec for
everyone. There doesn't seem to be any benefit to deliberately
ignoring a ranges based entry PC value for DWARF-4. If some naughty
compiler has emitted that, then lets use it.
Similarly, DWARF-4 says that DW_AT_entry_pc is an address. DWARF-5
allows an address or a constant, where the constant is an offset from
the base address. I allow both approaches for all DWARF versions.
There doesn't seem to be any downsides to this approach.
I ran into an issue when testing this patch where GCC would have the
DW_AT_entry_pc point to an empty range. When GDB parses the ranges
any empty ranges are ignored. As a consequence, the entry-pc appears
to be outside the address range of a block.
The empty range problem is certainly something that we can, and should
address, but that is not the focus of this patch, so for now I'm
ignoring that problem. What I have done is added a check: if the
DW_AT_entry_pc is outside the range of a block then the entry-pc is
ignored, GDB will then fall-back to its default algorithm for
computing the entry-pc.
If/when in the future we address the empty range problem, these
DW_AT_entry_pc attributes will suddenly become valid and GDB will
start using them. Until then, GDB continues to operate as it always
has.
An early version of this patch stored the entry-pc within the block
like this:
std::optional<CORE_ADDR> m_entry_pc;
However, a concern was raised that this, on a 64-bit host, effectively
increases the size of block by 16-bytes (8-bytes for the CORE_ADDR,
and 8-bytes for the std::optional's bool plus padding).
If we remove the std::optional part and just use a CORE_ADDR then we
need to have a "special" address to indicate if m_entry_pc is in use
or not. I don't really like using special addresses; different
targets can access different address ranges, even zero is a valid
address on some targets.
However, Bernd Edlinger suggested storing the entry-pc as an offset,
and I think that will resolve my concerns. So, we store the entry-pc
as a signed offset from the block's base address (the first address of
the first range, or the start() address value if there are now
ranges). Remember, ranges can be out of order, in which case the
first address of the first range might be greater than the entry-pc.
When GDB needs to read the entry-pc we can add the offset onto the
blocks base address to recalculate it.
With this done, on a 64-bit host, block only needs to increase by
8-bytes.
The inline-entry.exp test was originally contributed by Bernd here:
though I have made some edits, making more use of lib/gdb.exp
functions, making the gdb_test output patterns a little tighter, and
updating the test to run with Clang. I also moved the test to
gdb.opt/ as that seemed like a better home for it.