Made sure there is no implicit conversion between signed and unsigned
return value for functions setting the ctf_errno value.
An example of the problem is that in ctf_member_next, the "offset" value
is either 0L or (ctf_id_t)-1L, but it should have been 0L or -1L.
The issue was discovered while building a 64 bit ld binary to be
executed on the Windows platform.
Example object file that demonstrates the issue is attached in the PR.
Tom de Vries [Tue, 17 Oct 2023 09:38:06 +0000 (11:38 +0200)]
[gdb/cli] Keep track of styling failures in source_cache
In source_cache::ensure, keep track of which files failed to be styled, and
don't attempt to style them again in case the file dropped out of the cache.
Tom de Vries [Tue, 17 Oct 2023 09:38:06 +0000 (11:38 +0200)]
[gdb/cli] Factor out try_source_highlight
Function source_cache::ensure contains some code using the GNU
source-highlight library.
The code is a sizable part of the function, and contains conditional
compilation in a slightly convoluted way:
...
if (!already_styled)
#endif /* HAVE_SOURCE_HIGHLIGHT */
{
...
Fix this by factoring out the code into new function try_source_highlight,
such that:
- source_cache::ensure is easier to read, and
- the conditional compilation is at the level of the function body.
Tom de Vries [Tue, 17 Oct 2023 09:38:06 +0000 (11:38 +0200)]
[gdb/cli] Skip string copy in source_cache::ensure
In function source_cache::ensure we have:
...
std::ostringstream output;
...
contents = output.str ();
...
The last line causes an unnecessary string copy.
C++20 allows us to skip it, like this:
...
contents = std::move (output).str ();
...
Before, readelf -d RELASZ is the sum of .rela.dyn size and .rela.plt size.
To consistent with LoongArch lld, RELASZ chang to only the size of .rela.dyn.
Tom Tromey [Wed, 4 Oct 2023 18:58:32 +0000 (12:58 -0600)]
Fix register-setting response from DAP
Andry noticed that given a DAP setExpression request, where the
expression to set is a register, DAP will return the wrong value -- it
will return the old value, not the updated one.
This happens because gdb.Value.assign (which was recently added for
DAP) does not update the value.
In this patch, I chose to have the assign method update the Value
in-place. It's also possible to have it return a new value, but this
didn't seem very useful to me.
Nick Clifton [Mon, 16 Oct 2023 14:48:26 +0000 (15:48 +0100)]
Fix: GNU-ld: ARM: Issues when trying to set target output architecture
PR 28910
* lexsup.c (ld_options): Require that the --architecture option is given exactly two dashes, so that it does not become confused with the -a option.
Tom Tromey [Wed, 27 Sep 2023 17:48:24 +0000 (11:48 -0600)]
Add DAP scope cache
Andry Ogorodnik, a co-worker, noticed that multiple "scopes" requests
with the same frame would yield different variableReference values in
the response.
This patch adds a regression test for this, and adds a scope cache in
scopes.py, ensuring that multiple identical requests will get the same
response.
Tested-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com>
Tom de Vries [Mon, 16 Oct 2023 14:32:28 +0000 (16:32 +0200)]
[gdb/symtab] Work around PR gas/29517
When using glibc debuginfo generated with gas 2.39, we run into PR gas/29517:
...
$ gdb -q -batch a.out -ex start -ex "p (char *)strstr (\"haha\", \"ah\")"
Temporary breakpoint 1 at 0x40051b: file hello.c, line 6.
Temporary breakpoint 1, main () at hello.c:6
6 printf ("hello\n");
Invalid cast.
...
while without glibc debuginfo installed we get the expected result:
...
$n = 0x7ffff7daa1b1 "aha"
...
and likewise with glibc debuginfo generated with gas 2.40.
The strstr ifunc resolves to __strstr_sse2_unaligned. The problem is that gas
generates dwarf that states that the return type is void:
...
<1><3e1e58>: Abbrev Number: 2 (DW_TAG_subprogram)
<3e1e59> DW_AT_name : __strstr_sse2_unaligned
<3e1e5d> DW_AT_external : 1
<3e1e5e> DW_AT_low_pc : 0xbbd2e
<3e1e66> DW_AT_high_pc : 0xbc1c3
...
while the return type should be a DW_TAG_unspecified_type, as is the case
with gas 2.40.
We can still use the workaround of casting to another function type for both
__strstr_sse2_unaligned:
...
(gdb) p ((char * (*) (const char *, const char *))__strstr_sse2_unaligned) \
("haha", "ah")
$n = 0x7ffff7daa211 "aha"
...
and strstr (which requires using *strstr to dereference the ifunc before we
cast):
...
gdb) p ((char * (*) (const char *, const char *))*strstr) ("haha", "ah")
$n = 0x7ffff7daa251 "aha"
...
but that's a bit cumbersome to use.
Work around this in the dwarf reader, such that we have instead:
...
(gdb) p (char *)strstr ("haha", "ah")
$n = 0x7ffff7daa1b1 "aha"
...
This also requires fixing producer_is_gcc to stop returning true for
producer "GNU AS 2.39.0".
Tested on x86_64-linux.
Approved-By: Andrew Burgess <aburgess@redhat.com>
PR symtab/30911
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30911
Luis Machado [Thu, 28 Sep 2023 10:08:29 +0000 (11:08 +0100)]
Only allow closure lookup by address if there are threads displaced-stepping
Since commit 1e5ccb9c5ff4fd8ade4a8694676f99f4abf2d679, we have an assertion in
displaced_step_buffers::copy_insn_closure_by_addr that makes sure a closure
is available whenever we have a match between the provided address argument and
the buffer address.
That is fine, but the report in PR30872 shows this assertion triggering when
it really shouldn't. After some investigation, here's what I found out.
The 32-bit Arm architecture is the only one that calls
gdbarch_displaced_step_copy_insn_closure_by_addr directly, and that's because
32-bit Arm needs to figure out the thumb state of the original instruction
that we displaced-stepped through the displaced-step buffer.
Before the assertion was put in place by commit 1e5ccb9c5ff4fd8ade4a8694676f99f4abf2d679, there was the possibility of
getting nullptr back, which meant we were not doing a displaced-stepping
operation.
Now, with the assertion in place, this is running into issues.
It looks like displaced_step_buffers::copy_insn_closure_by_addr is
being used to return a couple different answers depending on the
state we're in:
1 - If we are actively displaced-stepping, then copy_insn_closure_by_addr
is supposed to return a valid closure for us, so we can determine the
thumb mode.
2 - If we are not actively displaced-stepping, then copy_insn_closure_by_addr
should return nullptr to signal that there isn't any displaced-step buffers
in use, because we don't have a valid closure (but we should always have
this).
Since the displaced-step buffers are always allocated, but not always used,
that means the buffers will always contain data. In particular, the buffer
addr field cannot be used to determine if the buffer is active or not.
For instance, we cannot set the buffer addr field to 0x0, as that can be a
valid PC in some cases.
My understanding is that the current_thread field should be a good candidate
to signal that a particular displaced-step buffer is active or not. If it is
nullptr, we have no threads using that buffer to displaced-step. Otherwise,
it is an active buffer in use by a particular thread.
The following fix modifies the displaced_step_buffers::copy_insn_closure_by_addr
function so we only attempt to return a closure if the buffer has an assigned
current_thread and if the buffer address matches the address argument.
Alternatively, I think we could use a function to answer the question of
whether we're actively displaced-stepping (so we have an active buffer) or
not.
I've also added a testcase that exercises the problem. It should reproduce
reliably on Arm, as that is the only architecture that faces this problem
at the moment.
Regression-tested on Ubuntu 20.04. OK?
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30872 Approved-By: Simon Marchi <simon.marchi@efficios.com>
Andrew Burgess [Wed, 11 Oct 2023 09:30:35 +0000 (10:30 +0100)]
gdb: replace architecture_changed with new_architecture observer
This commit replaces the architecture_changed observer with a
new_architecture observer.
Currently the only user of the architecture_changed observer is the
Python code, which uses this observer to register the Python unwinder
with the architecture.
The problem is that the architecture_changed observer is triggered
from inferior::set_arch(), which only sees the inferior-wide gdbarch
value. For targets that use thread-specific architectures, these
never trigger the architecture_changed observer, and so never have the
Python unwinder registered with them.
When it comes to unwinding GDB makes use of the frame's gdbarch, which
is based on the thread's regcache gdbarch, which is set in
get_thread_regcache to the value returned from
target_thread_architecture, which is not always the inferiors gdbarch
value, it might be a thread-specific gdbarch which has not passed
through inferior::set_arch().
The new_architecture observer will be triggered from
gdbarch_find_by_info, whenever a new gdbarch is created and
initialised. As GDB caches and reuses gdbarch values, we should
expect to see each new architecture trigger the new_architecture
observer just once.
After this commit, targets that make use of thread-specific
architectures should be able to make use of Python unwinders.
As I don't have access to a machine that makes use of thread-specific
architectures right now, I asked Luis to confirm that an AArch64
target that uses SVE/SME can't use the Python unwinders in threads
that are using a thread-specific architectures, and he confirmed that
this is indeed the case, see this discussion:
Tested-By: Lancelot Six <lancelot.six@amd.com> Tested-By: Luis Machado <luis.machado@arm.com> Reviewed-By: Luis Machado <luis.machado@arm.com> Approved-By: Simon Marchi <simon.marchi@efficios.com>
Neal Frager [Fri, 13 Oct 2023 07:28:55 +0000 (08:28 +0100)]
opcodes: microblaze: Add new bit-field instructions
This patches adds new bsefi and bsifi instructions.
BSEFI- The instruction shall extract a bit field from a
register and place it right-adjusted in the destination register.
The other bits in the destination register shall be set to zero.
BSIFI- The instruction shall insert a right-adjusted bit field
from a register at another position in the destination register.
The rest of the bits in the destination register shall be unchanged.
Further documentation of these instructions can be found here:
https://docs.xilinx.com/v/u/en-US/ug984-vivado-microblaze-ref
With version 6 of the patch, no new relocation types are added as
this was unnecessary for adding the bsefi and bsifi instructions.
FIXED: Segfault caused by incorrect termination of microblaze_opcodes.
Signed-off-by: nagaraju <nagaraju.mekala@amd.com> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@amd.com> Signed-off-by: Neal Frager <neal.frager@amd.com> Signed-off-by: Michael J. Eager <eager@eagercon.com>
Clément Chigot [Thu, 5 Oct 2023 09:29:32 +0000 (11:29 +0200)]
ld: allow update of existing QNX stack note
Up to now, the linker would always create a QNX stack note from scratch.
However, object files could already have such note, ending up into
duplicates. QNX loader doesn't handle that.
Update the mechanism to first search through the input files for a .note
section holding a QNX stack note. If none are found, then a new section
is created into the stub file as before. This requires this search to be
done once the file have been opened, moving the whole logic a bit later
in the emulation process.
As part for this update, also allow to request an executable stack
without necessarily having to provide its size as well. In this case, s
etup a default lazy stack of 0x1000.
ld/ChangeLog:
* emultempl/nto.em (nto_create_QNX_note_section): New Function.
(nto_lookup_QNX_note_section): New Function.
(nto_add_note_section): Move the creation of the note section
in the above new functions.
(nto_create_output_section_statements): rename nto_after_open
* testsuite/ld-aarch64/aarch64-nto.exp: add new test.
* testsuite/ld-aarch64/nto-stack-note-3.d: New test.
* testsuite/ld-aarch64/nto-stack-note.s: New test.
Tom Tromey [Sun, 1 Oct 2023 15:16:13 +0000 (09:16 -0600)]
Move -lsocket check to common.m4
A user pointed out that the -lsocket check in gdb should also apply to
gdbserver -- otherwise it can't find the Solaris socketpair. This
patch makes the change. It also removes a couple of redundant
function checks from gdb's configure.ac.
This was tested by the person who reported the bug.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30927 Approved-By: Pedro Alves <pedro@palves.net>
Tom Tromey [Wed, 11 Oct 2023 16:20:04 +0000 (10:20 -0600)]
Fix test suite failure in file-then-restart.exp
Simon pointed out that the new file-then-restart.exp test fails with
the extended-remote target board.
The problem is that the test suite doesn't use gdb_file_cmd -- which
handles things like "set remote exec-file". This patch changes
gdb_file_cmd to make the "kill" command optional, and then switches
the test case to use it.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30933 Approved-By: Simon Marchi <simon.marchi@efficios.com>
Andrew Burgess [Fri, 6 Oct 2023 15:20:41 +0000 (16:20 +0100)]
bfd: add new bfd_cache_size() function
In GDB we have a problem with the BFD cache.
As GDB runs for a potentially extended period of time, if the BFD
cache holds a file descriptor for an open on-disk file, this can, on
some targets (e.g. Win32) prevent the OS writing to the file.
This might, for example, prevent a user from recompiling their
executable as GDB is (via the BFD cache) holding an open reference to
that file.
Another problem, relates to bfd_stat, for BFDs that are using the BFD
cache (i.e. they call cache_bstat to implement bfd_stat). The
cache_bstat function finds the BFD in the cache, opening the file if
needed, and then uses fstat on the open file descriptor.
What this means is that, if the on-disk file changes, but the cache
was holding an open reference to the file, the bfd_stat will return
the 'struct stat' for the old file, not the new file.
Now, for this second problem, we might be tempted to make use of an
actual stat call, instead of calling bfd_stat, however, this isn't
ideal as we have some BFDs that use a custom iovec, and implement the
various functions over GDB's remote protocol. By using bfd_stat we
can have a single call that should work for both local files, and for
remote files.
To solve both of these problems GDB has calls to bfd_cache_close_all
sprinkled around its code base. And in theory this should work fine.
However, I recently ran into a case where we had missed a
bfd_cache_close_all call, and as a result some BFDs were held open.
This caused a bfd_stat call to return an unexpected result (old file
vs new file).
What I'd like is some way within GDB that I can do:
gdb_assert ( /* Nothing is held open in the cache. */ );
As this would allow GDB to quickly identify when we've missed some
bfd_cache_close_all calls.
And so, to support this, I would like to add a new bfd_cache_size
function. This function returns an integer, which is the number of
open files in the cache. I can then start adding:
gdb_assert (bfd_cache_size() == 0);
to GDB in some strategic spots, and start fixing all of the missing
bfd_cache_close_all calls that crop up as a result.
Andrew Burgess [Wed, 11 Oct 2023 13:39:37 +0000 (14:39 +0100)]
bfd/cache: change type used to track cached BFDs from int to unsigned
Within bfd/cache.c change the type for max_open_files and open_files
variables from int to unsigned. As a consequence of this, the return
type for bfd_cache_max_open() is also changed from int to unsigned.
Within bfd_cache_max_open I've left the local 'max' variable as an
int, this should ensure that if the sysconf call fails, and returns
-1, then the computed max value will be less than 10, which means
max_open_files will be set to 10. If 'max' was changed to unsigned
then, should the sysconf call fail, we'd end up with max becoming a
very large positive number ... which is clearly not what we want.
And, while I was auditing how open_files is used, I added an assert
within bfd_cache_delete to ensure that we don't try to reduce
open_files below zero.
There should be no user visible change with this commit.
Jeff Law [Wed, 11 Oct 2023 22:30:05 +0000 (16:30 -0600)]
[RFA] Fix for mcore simulator
I was looking for cases where a GCC patch under evaluation would cause test
results to change. Quite surprisingly the mcore-elf port showed test
differences. After a fair amount of digging my conclusion was the sequences
before/after the patch should have been semantically the same.
Of course if the code is supposed to behave the same, then that points to
problems elsewhere (assembler, linker, simulator). Sure enough the mcore
simulator was mis-handling the sign extension instructions. The simulator
implementation of sextb is via paired shift-by-24 operations. Similarly the
simulator implements sexth via paired shift-by-16 operations.
The temporary holding the value was declared as a "long" thus this approach
worked fine for hosts with a 32 bit wide long and failed miserably for hosts
with a 64 bit wide long.
This patch makes the shift count automatically adjust based on the size of the
temporary. It includes a simple test for sextb and sexth. I have _not_ done a
full audit of the mcore simulator for more 32->64 bit issues.
This also fixes 443 execution tests in the GCC testsuite
Hui Li [Wed, 28 Jun 2023 09:33:05 +0000 (17:33 +0800)]
gdb: LoongArch: Handle special struct in dummy call
When execute the following command on LoongArch:
make check-gdb TESTS="gdb.base/infcall-nested-structs-c++.exp"
there exist some failed testcases:
=== gdb Summary ===
# of expected passes 5533
# of unexpected failures 367
The root cause is related with a struct containing floating-point
members as function argument or return value for a dummy call.
(1) Structure consists of one floating-point member within FRLEN bits
wide, it is passed in an FAR if available.
(2) Structure consists of two floating-point members both within FRLEN
bits wide, it is passed in two FARs if available.
(3) Structure consists of one integer member within GRLEN bits wide and
one floating-point member within FRLEN bits wide, it is passed in a
GAR and an FAR if available.
Note that in the above cases, empty structure or union members are also
ignored even in C++.
Here is a simple test on LoongArch:
loongson@bogon:~$ cat test.c
#include<stdio.h>
struct test {
long a;
double b __attribute__((aligned(16)));
};
struct test val = { 88, 99.99 };
int check_arg_struct (struct test arg)
{
printf("arg.a = %ld\n", arg.a);
printf("arg.b = %f\n", arg.b);
printf("sizeof(val) = %d\n", sizeof(val));
return 1;
}
int main()
{
check_arg_struct (val);
return 0;
}
loongson@bogon:~$ gcc -g test.c -o test
loongson@bogon:~$ ./test
arg.a = 88
arg.b = 99.990000
sizeof(val) = 32
Before:
loongson@bogon:~$ gdb test
...
(gdb) start
...
Temporary breakpoint 1, main () at test.c:19
19 check_arg_struct (val);
(gdb) p check_arg_struct (val)
arg.a = 140737488286128
arg.b = -nan
sizeof(val) = 32
$1 = 1
...
After:
loongson@bogon:~$ gdb test
...
(gdb) start
...
Temporary breakpoint 1, main () at test.c:19
19 check_arg_struct (val);
(gdb) p check_arg_struct (val)
arg.a = 88
arg.b = 99.990000
sizeof(val) = 32
$1 = 1
...
With this patch, there are no failed testcases:
make check-gdb TESTS="gdb.base/infcall-nested-structs-c++.exp"
=== gdb Summary ===
# of expected passes 5900
Signed-off-by: Hui Li <lihui@loongson.cn> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
Simon Marchi [Wed, 4 Oct 2023 02:04:01 +0000 (22:04 -0400)]
gdb: add assertion when marking the remote async flag
As reported in bug 30630 [1], we hit a case where the remote target's
async flag is marked while the target is not configured (yet) to work
async. This should not happen. It is caught thanks to this assert in
remote_target::wait:
/* Start by clearing the flag that asks for our wait method to be called,
we'll mark it again at the end if needed. If the target is not in
async mode then the async token should not be marked. */
if (target_is_async_p ())
rs->clear_async_event_handler ();
else
gdb_assert (!rs->async_event_handler_marked ());
This is helpful, but I think that we could have caught the problem earlier than
that, at the moment we marked the handler. Catching problems earlier
makes them easier to debug.
Simon Marchi [Tue, 10 Oct 2023 14:53:36 +0000 (10:53 -0400)]
gdb: add remote_state::{is_async_p,can_async_p}
A subsequent patch will want to know if the remote is async within a
remote_state method. Add a helper method for that, and for "can async"
as well, for symmetry.
Change-Id: Id0f648ee4896736479fa942f5453eeeb0e5d4352 Approved-By: Andrew Burgess <aburgess@redhat.com>
Simon Marchi [Wed, 4 Oct 2023 02:03:59 +0000 (22:03 -0400)]
gdb: make remote_state's async token private
Make remote_async_inferior_event_token private (rename to
m_async_event_handler_token) and add methods for the various operations
we do on it. This will help by:
- allowing to break on those methods when debugging
- allowing to add assertions in the methods
Change-Id: Ia3b8a2bc48ad4849dbbe83442c3f83920f03334d Approved-By: Andrew Burgess <aburgess@redhat.com>
Simon Marchi [Fri, 29 Sep 2023 18:24:39 +0000 (14:24 -0400)]
gdb: scope down registers_changed call in inferior::set_arch
inferior::set_arch calls registers_changed, which invalidates all
regcaches. It would be enough to invalidate only regcaches of threads
belonging to this inferior. Call registers_changed_ptid instead, with
the proper process target / ptid. If the inferior does not have a
process target, there should be no regcaches for that inferior, so no
need to invalidate anything.
Change-Id: Id8b5500acb7f373b01a534f16d3a7d028dc0d882 Reviewed-By: John Baldwin <jhb@FreeBSD.org> Approved-By: Andrew Burgess <aburgess@redhat.com>
Simon Marchi [Fri, 29 Sep 2023 18:24:38 +0000 (14:24 -0400)]
gdb: remove target_gdbarch
This function is just a wrapper around the current inferior's gdbarch.
I find that having that wrapper just obscures where the arch is coming
from, and that it's often used as "I don't know which arch to use so
I'll use this magical target_gdbarch function that gets me an arch" when
the arch should in fact come from something in the context (a thread,
objfile, symbol, etc). I think that removing it and inlining
`current_inferior ()->arch ()` everywhere will make it a bit clearer
where that arch comes from and will trigger people into reflecting
whether this is the right place to get the arch or not.
Change-Id: I79f14b4e4934c88f91ca3a3155f5fc3ea2fadf6b Reviewed-By: John Baldwin <jhb@FreeBSD.org> Approved-By: Andrew Burgess <aburgess@redhat.com>
Simon Marchi [Fri, 29 Sep 2023 18:24:37 +0000 (14:24 -0400)]
gdb: move set_target_gdbarch to inferior::set_arch
set_target_gdbarch is basically a setter for the current inferior's
arch, that notifies other parts of GDB of the architecture change. Move
the code of set_target_gdbarch to the inferior::set_arch method.
Add gdbarch_initialized_p, so we can keep the assertion.
Change-Id: I276e28eafd4740c94bc5233c81a86c01b4a6ae90 Reviewed-By: John Baldwin <jhb@FreeBSD.org> Approved-By: Andrew Burgess <aburgess@redhat.com>
Simon Marchi [Fri, 29 Sep 2023 18:24:35 +0000 (14:24 -0400)]
gdb: add inferior::{arch, set_arch}
Make the inferior's gdbarch field private, and add getters and setters.
This helped me by allowing putting breakpoints on set_arch to know when
the inferior's arch was set. A subsequent patch in this series also
adds more things in set_arch.
Change-Id: I0005bd1ef4cd6b612af501201cec44e457998eec Reviewed-By: John Baldwin <jhb@FreeBSD.org> Approved-By: Andrew Burgess <aburgess@redhat.com>
Alan Modra [Tue, 10 Oct 2023 07:48:18 +0000 (18:18 +1030)]
asan: buffer overflow in elf32_arm_get_synthetic_symtab
Guard against fuzzed files where .plt size isn't commensurate with
plt relocations.
* elf32-arm.c (elf32_arm_plt0_size): Add data_size param.
Return -1 if data_size is too small.
(elf32_arm_plt_size): Likewise. Delete temp var. Formatting.
(elf32_arm_get_synthetic_symtab): Adjust to suit.
Alan Modra [Tue, 10 Oct 2023 07:48:07 +0000 (18:18 +1030)]
asan: null dereference in read_and_display_attr_value
This fixes multiple places in read_and_display_attr_value dealing with
range and location lists that can segfault when debug_info_p is NULL.
Fuzzed object files can contain arbitrary DW_FORMs.
Alan Modra [Tue, 10 Oct 2023 07:46:38 +0000 (18:16 +1030)]
asan: invalid free in bfd_init_section_compress_status
With specially crafted compressed sections, it's possible to tickle a
problem when decompressing: If the compression headers says the
uncompressed size is zero, this will be seen as an error return from
bfd_compress_section_contents. On errors the caller should free any
malloc'd input buffers, but this isn't really an error and the section
contents have been updated to a bfd_alloc'd buffer which can't be
freed.
* compress.c (bfd_compress_section_contents): Return -1 as error
rather than 0.
(bfd_init_section_compress_status, bfd_compress_section): Adjust.
Jan Vrany [Tue, 10 Oct 2023 10:22:56 +0000 (11:22 +0100)]
gdb/python: implement support for sending custom MI async notifications
This commit adds a new Python function, gdb.notify_mi, that can be used
to emit custom async notification to MI channel. This can be used, among
other things, to implement notifications about events MI does not support,
such as remote connection closed or register change.
Reviewed-By: Eli Zaretskii <eliz@gnu.org> Approved-By: Andrew Burgess <aburgess@redhat.com>
Jan Vrany [Tue, 10 Oct 2023 10:22:56 +0000 (11:22 +0100)]
gdb/python: generalize serialize_mi_result()
This commit generalizes serialize_mi_result() to make usable in
different contexts than printing result of custom MI command.
To do so, the check whether passed Python object is a dictionary has been
moved to the caller - at the very least, different uses require different
error messages. Also it has been renamed to serialize_mi_results() to better
match GDB/MI output syntax (see corresponding section in documentation,
in particular rules 'result-record' and 'async-output'.
Since it is now more generic function, it has been moved to py-mi.c.
This is a preparation for implementing Python support for sending custom
MI async events.
Tom de Vries [Tue, 10 Oct 2023 08:26:40 +0000 (10:26 +0200)]
[readelf] Handle .gdb_index section version 9
Add the abilitity to print a v9 .gdb_index section.
The v9 section contains an extra table, which is printed as follows:
...
Shortcut table:
Language of main: Fortran 95
Name of main: contains_keyword
...
[ For the example, I used the exec of gdb test-case
gdb.fortran/nested-funcs-2-exp when running the test-case with target board
cc-with-gdb-index. ]
[gdb/symtab] Add name_of_main and language_of_main to the DWARF index
This patch adds a new section to the DWARF index containing the name
and the language of the main function symbol, gathered from
`cooked_index::get_main`, if available. Currently, for lack of a better name,
this section is called the "shortcut table". The way this name is both saved and
applied upon an index being loaded in mirrors how it is done in
`cooked_index_functions`, more specifically, the full name of the main function
symbol is saved and `set_objfile_main_name` is used to apply it after it is
loaded.
The main use case for this patch is in improving startup times when dealing with
large binaries. Currently, when an index is used, GDB has to expand symtabs
until it finds out what the language of the main function symbol is. For some
large executables, this may take a considerable amount of time to complete,
slowing down startup. This patch bypasses that operation by having both the name
and language of the main function symbol be provided ahead of time by the index.
In my testing (a binary with about 1.8GB worth of DWARF data) this change brings
startup time down from about 34 seconds to about 1.5 seconds.
When testing the patch with target board cc-with-gdb-index, test-case
gdb.fortran/nested-funcs-2.exp starts failing, but this is due to a
pre-existing issue, filed as PR symtab/30946.
Tested on x86_64-linux, with target board unix and cc-with-gdb-index.
pointed out that within gdb.base/maint.exp, some regexps within a
gdb_test_multiple were failing to match a complete line, while later
regexps within the gdb_test_multiple made use of the '^' anchor, and
so assumed that earlier lines had been completely matched and removed
from expect's buffer.
When testing with READ1 set this assumption was failing.
Fix this by extending the offending patterns with a trailing '\r\n'.
Some older kernels cannot handle the newly generated R_LARCH_32/64_PCREL,
so the assembler generates R_LARCH_ADD32/64+R_LARCH_SUB32/64 by default,
and use the assembler option mthin-add-sub to generate R_LARCH_32/64_PCREL
as much as possible.
The Option of mthin-add-sub does not affect the generation of R_LARCH_32_PCREL
relocation in .eh_frame.
Tom de Vries [Sat, 7 Oct 2023 08:33:29 +0000 (10:33 +0200)]
[gdb/testsuite] Fix gdb.arch/i386-signal.exp on x86_64
On x86_64-linux, with test-case gdb.arch/i386-signal.exp I run into:
...
builtin_spawn -ignore SIGHUP gcc -fno-stack-protector i386-signal.c \
-fdiagnostics-color=never -fno-pie -g -no-pie -lm -o i386-signal^M
/tmp/cc2xydTG.s: Assembler messages:^M
/tmp/cc2xydTG.s:50: Error: operand size mismatch for `push'^M
compiler exited with status 1
output is:
/tmp/cc2xydTG.s: Assembler messages:^M
/tmp/cc2xydTG.s:50: Error: operand size mismatch for `push'^M
gdb compile failed, /tmp/cc2xydTG.s: Assembler messages:
/tmp/cc2xydTG.s:50: Error: operand size mismatch for `push'
UNTESTED: gdb.arch/i386-signal.exp: failed to compile
...
This is with gas 2.41, it compiles without problems with gas 2.40. Some more
strict checking was added in commit 5cc007751cd ("x86: further adjust
extend-to-32bit-address conditions"). This may or may not be a gas regression
( https://sourceware.org/pipermail/binutils/2023-October/129818.html ).
The offending bit is:
...
" push $sigframe\n"
...
which refers to a function:
...
" .globl sigframe\n"
"sigframe:\n"
...
The test-case passes with target board unix/-m32.
Make the test-case work by using pushq instead of push for the
is_amd64_regs_target case.
Tested on x86_64-linux, with target boards:
- unix/-m64 (is_amd64_regs_target == 1), and
- unix/-m32 (is_amd64_regs_target == 0),
Ilya Leoshkevich [Wed, 21 Jun 2023 23:03:04 +0000 (01:03 +0200)]
gdb: support rseq auxvs
Linux kernel commit commit 317c8194e6ae ("rseq: Introduce feature size
and alignment ELF auxiliary vector entries") introduced two new auxvs:
AT_RSEQ_FEATURE_SIZE and AT_RSEQ_ALIGN. Support them in GDB. This
fixes auxv.exp on kernels >= v6.3.
Change-Id: I8966c4d5c73eb7b45de6d410a9b28a6628edad2e
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30540 Approved-By: Tom Tromey <tom@tromey.com>
process-dies-while-detaching.exp: Exit early if GDB misses sync breakpoint
I'm seeing a lot of variability in the failures of
gdb.threads/process-dies-while-detaching.exp on aarch64-linux. On this
platform, a problem yet to be investigated causes GDB to miss the _exit
breakpoint. What happens next is random because after missing that
breakpoint, GDB is out of sync with the inferior. This causes the tests
following that point in the testcase to fail in a random way.
In this scenario it's better to exit the testcase early to avoid random
results in the testsuite.
We are relying on gdb_continue_to_breakpoint to return the result of
gdb_test_multiple. This is already the case because in Tcl the return
value of a function is the return value of the last command it runs. But
change gdb_continue_to_breakpoint to explicitly return this value, to make
it clear this is the intended behaviour.
Tested on aarch64-linux.
Tested-By: Guinevere Larsen <blarsen@redhat.com> Approved-By: Andrew Burgess <aburgess@redhat.com>
Neal Frager [Thu, 5 Oct 2023 12:51:03 +0000 (13:51 +0100)]
opcodes: microblaze: Add new bit-field instructions
This patches adds new bsefi and bsifi instructions.
BSEFI- The instruction shall extract a bit field from a
register and place it right-adjusted in the destination register.
The other bits in the destination register shall be set to zero.
BSIFI- The instruction shall insert a right-adjusted bit field
from a register at another position in the destination register.
The rest of the bits in the destination register shall be unchanged.
Further documentation of these instructions can be found here:
https://docs.xilinx.com/v/u/en-US/ug984-vivado-microblaze-ref
This patch has been tested for years of AMD Xilinx Yocto
releases as part of the following patch set:
Andrew Burgess [Fri, 29 Sep 2023 13:26:01 +0000 (14:26 +0100)]
gdb/NEWS: reorder some entries in the NEWS file
I spotted two entries in the NEWS file that I believe are in the wrong
place, these are:
- An entry about MI v1 being deprecated, this feels like it should
be the first entry under the 'MI changes' heading, and
- An entry for the $_shell convenience function which is currently
under the 'New commands' heading (sort of), when I think this
should be listed in the general news section.
The risc-v, loongarch, and csky gdbserver builds were broken. A use
of target_desc::expedite_regs (for each architecture) was not updated
to take account of the type change.
I've tested that this fixes the risc-v build. I haven't tested the
other architectures, but they should be fine.
Andrew Burgess [Wed, 27 Sep 2023 17:22:52 +0000 (18:22 +0100)]
gdb/testsuite: cleanup in gdb.base/args.exp
The last few commits resolved the KFAILs in gdb.base/args.exp. With
those out of the way we can clean up this test script a little.
In this commit I have:
- Stopped passing 'nowarnings' flag when building the source file.
I see no reason why this source should issue a warning,
- Moved setup of GDBFLAGS into args_test proc, callers that passed a
newline needed a small tweak, and also the matching code needs
updating for newline handling, but I think this is nicer, the
argument lists are now given just once,
- Updated comment on args_test,
- Updated other comments.
There should be no change in what is tested after this commit.
which also includes this fix as part of a larger set of changes. I'm
giving a Co-Authored-By credit to the author of that original series.
I believe this smaller fix brings some benefits on its own, though the
original series does offer additional improvements. Once this is
merged I'll take a look at rebasing and resubmitting the original series.
Co-Authored-By: Michael Weghorn <m.weghorn@posteo.de> Approved-By: Tom Tromey <tom@tromey.com>
Andrew Burgess [Wed, 27 Sep 2023 16:54:15 +0000 (17:54 +0100)]
gdbserver: handle newlines in inferior arguments
Similarly to how single quotes were mishandled, which was fixed two
commits ago, this commit fixes handling of newlines in arguments
passed to gdbserver.
We already had a test that covered this, gdb.base/args.exp, which,
when run with the native-extended-gdbserver board contained several
KFAIL covering this situation.
In this commit I remove the unnecessary, attempt to quote incoming
newlines within arguments, and do some minimal cleanup of the related
code. There is additional cleanup that can be done, but I'm leaving
that for the next commit.
Then I've removed the KFAIL from the test case, and performed some
minimal cleanup there too.
After this commit the gdb.base/args.exp is 100% passing with the
native-extended-gdbserver board file.
which also includes this fix as part of a larger set of changes. I'm
giving a Co-Authored-By credit to the author of that original series.
I believe this smaller fix brings some benefits on its own, though the
original series does offer additional improvements. Once this is
merged I'll take a look at rebasing and resubmitting the original series.
Andrew Burgess [Wed, 27 Sep 2023 16:18:01 +0000 (17:18 +0100)]
gdbserver: fix handling of trailing empty argument
When I posted the previous patch for review Andreas Schwab pointed out
that passing a trailing empty argument also doesn't work.
The fix for this is in the same area of code as the previous patch,
but is sufficiently different that I felt it deserved a patch of its
own.
I noticed that passing arguments containing single quotes to gdbserver
didn't work correctly:
gdb -ex 'set sysroot' --args /tmp/show-args
Reading symbols from /tmp/show-args...
(gdb) target extended-remote | gdbserver --once --multi - /tmp/show-args
Remote debugging using | gdbserver --once --multi - /tmp/show-args
stdin/stdout redirected
Process /tmp/show-args created; pid = 176054
Remote debugging using stdio
Reading symbols from /lib64/ld-linux-x86-64.so.2...
(No debugging symbols found in /lib64/ld-linux-x86-64.so.2)
0x00007ffff7fd3110 in _start () from /lib64/ld-linux-x86-64.so.2
(gdb) set args abc ""
(gdb) run
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /tmp/show-args \'
stdin/stdout redirected
Process /tmp/show-args created; pid = 176088
2 args are:
/tmp/show-args
abc
Done.
[Inferior 1 (process 176088) exited normally]
(gdb) target native
Done. Use the "run" command to start a process.
(gdb) run
Starting program: /tmp/show-args \'
2 args are:
/tmp/show-args
abc
The 'shows-args' program used here just prints the arguments passed to
the inferior.
Notice that when starting the inferior using the extended-remote
target there is only a single argument 'abc', while when using the
native target there is a second argument, the blank line, representing
the empty argument.
The problem here is that the vRun packet coming from GDB looks like
this (I've removing the trailing checksum):
$vRun;PROGRAM_NAME;616263;
If we compare this to a packet with only a single argument and no
trailing empty argument:
$vRun;PROGRAM_NAME;616263
Notice the lack of the trailing ';' character here.
The problem is that gdbserver processes this string in a loop. At
each point we maintain a pointer to the character just after a ';',
and then we process everything up to either the next ';' character, or
to the end of the string.
We break out of this loop when the character we start with (in that
loop iteration) is the null-character. This means in the trailing
empty argument case, we abort the loop before doing anything with the
empty argument.
In this commit I've updated the loop, we now break out using a 'break'
statement at the end of the loop if the (sub-)string we just processed
was empty, with this change we now notice the trailing empty
argument.
Andrew Burgess [Tue, 26 Sep 2023 16:32:24 +0000 (17:32 +0100)]
gdbserver: fix handling of single quote arguments
I noticed that passing arguments containing single quotes to gdbserver
didn't work correctly:
gdb -ex 'set sysroot' --args /tmp/show-args
Reading symbols from /tmp/show-args...
(gdb) target extended-remote | gdbserver --once --multi - /tmp/show-args
Remote debugging using | gdbserver --once --multi - /tmp/show-args
stdin/stdout redirected
Process /tmp/show-args created; pid = 176054
Remote debugging using stdio
Reading symbols from /lib64/ld-linux-x86-64.so.2...
(No debugging symbols found in /lib64/ld-linux-x86-64.so.2)
0x00007ffff7fd3110 in _start () from /lib64/ld-linux-x86-64.so.2
(gdb) set args \'
(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /tmp/show-args \'
stdin/stdout redirected
Process /tmp/show-args created; pid = 176088
2 args are:
/tmp/show-args
\'
Done.
[Inferior 1 (process 176088) exited normally]
(gdb) target native
Done. Use the "run" command to start a process.
(gdb) run
Starting program: /tmp/show-args \'
2 args are:
/tmp/show-args
'
Done.
[Inferior 1 (process 176095) exited normally]
(gdb) q
The 'shows-args' program used here just prints the arguments passed to
the inferior.
Notice that when starting the inferior using the extended-remote
target the second argument is "\'", while when running using native
target the argument is "'". The second of these is correct, the \'
used with the "set args" command is just to show GDB that the single
quote is not opening an argument string.
It turns out that the extra backslash is injected on the gdbserver
side when gdbserver processes the arguments that GDB passes it, the
code that does this was added as part of this much larger commit:
In this commit I propose removing the specific code that adds what I
believe is a stray backslash. I've extended an existing test to cover
this case, and I now see identical behaviour when using an
extended-remote target as with the native target.
This partially fixes PR gdb/27989, though there are still some issues
with newline handling which I'll address in a later commit.
which also includes this fix as part of a larger set of changes. I'm
giving a Co-Authored-By credit to the author of that original series.
I believe this smaller fix brings some benefits on its own, though the
original series does offer additional improvements. Once this is
merged I'll take a look at rebasing and resubmitting the original series.
If you want to install GDB in a custom prefix, have it look for debug info
in that prefix but also in the distro's default location (typically,
/usr/lib/debug) and run the GDB testsuite before doing "make install", you
have a bit of a problem:
Configuring GDB with '--prefix=$PREFIX' sets the GDB 'debug-file-directory'
parameter to $PREFIX/lib/debug. Unfortunately this precludes GDB from
looking for distro-installed debug info in /usr/lib/debug. For regular GDB
use you could set debug-file-directory to $PREFIX:/usr/lib/debug in
$PREFIX/etc/gdbinit so that GDB will look in both places, but if you want
to run the testsuite then that doesn't help because in that case GDB runs
with the '-nx' option.
There's the configure option '--with-separate-debug-dir' to set the default
value for 'debug-file-directory', but it accepts only one directory and not
a list. I considered modifying it to accept a list, but it's not obvious
how to do that because its value is also used by BFD, as well as processed
for "relocatability".
I thought it was simpler to add a new option to specify a list of
additional directories that will be appended to the debug-file-directory
setting.
Reviewed-By: Eli Zaretskii <eliz@gnu.org> Approved-By: Tom Tromey <tom@tromey.com>
Tom de Vries [Thu, 5 Oct 2023 21:22:11 +0000 (23:22 +0200)]
[gdb/go] Handle v3 go_0 mangled prefix
With gcc-10 we have:
...
(gdb) break package2.Foo^M
Breakpoint 2 at 0x402563: file package2.go, line 5.^M
(gdb) PASS: gdb.go/package.exp: setting breakpoint 1
...
but with gcc-11:
...
gdb) break package2.Foo^M
Function "package2.Foo" not defined.^M
Make breakpoint pending on future shared library load? (y or [n]) n^M
(gdb) FAIL: gdb.go/package.exp: gdb_breakpoint: set breakpoint at package2.Foo
...
In the gcc-10 case, though the exec contains dwarf, it's not used to set the
breakpoint (which is an independent problem, filed as PR go/30941), instead
the minimal symbol information is used.
The minimal symbol information changed between gcc-10 and gcc-11:
...
$ nm a.out.10 | grep Foo 000000000040370d T go.package2.Foo 0000000000404e50 R go.package2.Foo..f
$ nm a.out.11 | grep Foo 0000000000403857 T go_0package2.Foo 0000000000405030 R go_0package2.Foo..f
...
A new v3 mangling scheme was used. The mangling schemes define a separator
character and mangling character:
- for v2, dot is used both as separator character and mangling character, and
- for v3, dot is used as separator character and underscore as mangling
character.
For more details, see [1] and [2].
In v3, "_0" demangles to ".". [ See gcc commit a01dda3c23b ("compiler, libgo:
change mangling scheme"), function Special_char_code::Special_char_code. ]
Handle the new go_0 prefix in unpack_mangled_go_symbol, which fixes the
test-case.
Note that this doesn't fix this regression:
...
$ gccgo-10 package2.go -c -g0
$ gccgo-10 package1.go package2.o -g0
$ gdb -q -batch a.out -ex "break go.package2.Foo"
Breakpoint 1 at 0x40370d
$ gccgo-11 package2.go -c -g0
$ gccgo-11 package1.go package2.o -g0
$ gdb -q -batch a.out -ex "break go.package2.Foo"
Function "go.package2.Foo" not defined.
...
With gcc-10, we set a breakpoint on the mangled minimal symbol. That
one has simply changed for gcc-11, so it's equivalent to using:
...
$ gdb -q -batch a.out -ex "break go_0package2.Foo"
Breakpoint 1 at 0x403857
...
which does work.
Tested on x86_64-linux:
- openSUSE Leap 15.4, using gccgo-7,
- openSUSE Tumbleweed, using gccgo-13.
Simon Marchi [Wed, 4 Oct 2023 02:20:21 +0000 (22:20 -0400)]
gdb: add all_objfiles_removed observer
The new_objfile observer is currently used to indicate both when a new
objfile is added to program space (when passed non-nullptr) and when all
objfiles of a program space were just removed (when passed nullptr).
I think this is confusing (and Andrew apparently thinks so too [1]).
Add a new "all_objfiles_removed" observer to remove the second role from
"new_objfile".
Some existing users of new_objfile do nothing if the passed objfile is
nullptr. For them, we can simply drop the nullptr check. For others,
add a new all_objfiles_removed callback, and refactor things a bit to
keep the existing behavior as much as possible.
Some callbacks relied on current_program_space, and following
the refactoring now use either objfile->pspace or the pspace passed to
all_objfiles_removed. I think this should be relatively safe, and in
general a step in the right direction.
On the notify side, I found only one call site to change from
new_objfile to all_objfiles_removed, in clear_symtab_users. It is not
entirely clear to me that this is entirely correct. clear_symtab_users
appears to be called in spots that don't remove all objfiles
(functions finish_new_objfile, remove_symbol_file_command, reread_symbols,
do_module_cleanups). But I think that this patch at least makes the
current code clearer.
Simon Marchi [Wed, 4 Oct 2023 02:20:18 +0000 (22:20 -0400)]
gdb: add program_space parameter to emit_clear_objfiles_event
Add program_space space parameters to emit_clear_objfiles_event and
create_clear_objfiles_event_object, making the reference to
current_program_space bubble up a bit.
Change-Id: I5fde2071712781e5d45971fa0ab34d85d3a49a71 Approved-By: Tom Tromey <tom@tromey.com>
Simon Marchi [Wed, 4 Oct 2023 02:20:17 +0000 (22:20 -0400)]
gdb: add program_space parameters to some functions in symtab.c
Add some program_space parameters to functions related to getting and
setting the main name, making the references to current_program_space
bubble up a bit. find_main_name calls ada_main_name, which implicitly
relies on the current program space, so I didn't add a parameter to that
function.
Change-Id: I9996955e8ae56832bbd461964d978e700e6feaf4 Approved-By: Tom Tromey <tom@tromey.com>