| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Andry pointed out that the DAP code did not properly handle
gdb.LazyString results from a pretty-printer, yielding:
TypeError: Object of type LazyString is not JSON serializable
This patch fixes the problem, partly with a small patch in varref.py,
but mainly by implementing tp_str for LazyString.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
|
|
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.
|
|
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>
|
|
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>
|
|
|
|
|
|
|
|
__SVE_VQ_BYTES is only available if SVE definitions are available in
the system's headers, and this is not true for all systems.
For this purpose, we define SVE_VQ_BYTES. This patch fixes the
name of the constant being used.
|
|
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>
|
|
|
|
|
|
|
|
|
|
|
|
This commit changes gdb/version.in to 14.0.91.DATE-git.
|
|
This commit changes gdb/version.in to 14.0.91.
|
|
This commit updates the NEWS files for the upcoming GDB 14 release.
|
|
This is done by setting the "development" variable to "false"
in bfd/development.sh.
|
|
Now that the GDB 14 branch has been created,
this commit bumps the version number in gdb/version.in to
14.0.90.DATE-git
For the record, the GDB 14 branch was created
from commit 8f12a1a841cd0c447de7a5a0f134a0efece73588.
|
|
|
|
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.
|
|
|
|
This reverts commit 6bbf249557ba17cfebe01c67370df4da9e6a56f9.
Maciej W. Rozycki <macro@orcam.me.uk>:
Yet it has caused numerous regressions:
microblaze-elf +FAIL: unordered .debug_info references to .debug_ranges
microblaze-elf +FAIL: binutils-all/pr26548
microblaze-elf +FAIL: readelf -Wwi pr26548e (reason: unexpected output)
microblaze-elf +FAIL: readelf --debug-dump=loc locview-1 (reason: unexpected output) Yet it has caused numerous regressions:
microblaze-elf +FAIL: unordered .debug_info references to .debug_ranges
microblaze-elf +FAIL: binutils-all/pr26548
microblaze-elf +FAIL: readelf -Wwi pr26548e (reason: unexpected output)
...
|
|
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),
PR testsuite/30928
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30928
|
|
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>
|
|
When TCL_TRY is FALSE, the wrong check-DEJAGNU is generated.
Place "if TCL_TRY / endif" in the right place.
gprofng/ChangeLog
2023-10-05 Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
PR gprofng/30910
* Makefile.am: Correct "if TCL_TRY / endif".
* Makefile.in: Rebuild.
|
|
|
|
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>
|
|
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:
https://github.com/Xilinx/meta-xilinx/tree/master/meta-microblaze/recipes-devtools/binutils/binutils
Signed-off-by: nagaraju <nagaraju.mekala@amd.com>
Signed-off-by: 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>
|
|
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.
|
|
After this commit:
commit 6a65998a8a94abaaae7ca4ff0ab9c3f25dc2e766
Date: Mon Sep 11 12:42:00 2023 +0100
Convert tdesc's expedite_regs to a string vector
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.
|
|
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.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
After the previous commit there is now a redundant string copy in
handle_v_run, this commit cleans that up.
There should be no functional change after this commit.
During review I was pointed to this older series:
https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de/
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>
|
|
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.
During review I was pointed to this older series:
https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de/
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.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27989
Co-Authored-By: Michael Weghorn <m.weghorn@posteo.de>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
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
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 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.
I've updated the test case to cover this issue.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
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:
commit 2090129c36c7e582943b7d300968d19b46160d84
Date: Thu Dec 22 21:11:11 2016 -0500
Share fork_inferior et al with gdbserver
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.
During review I was pointed to this older series:
https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de/
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.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27989
Co-Authored-By: Michael Weghorn <m.weghorn@posteo.de>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
PR 30940
* elf64-alpha.c (elf64_alpha_check_relocs): Correct error message.
|
|
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>
|
|
|
|
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.
PR go/27238
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27238
[1] https://go-review.googlesource.com/c/gofrontend/+/271726
[2] https://github.com/golang/go/issues/41862#issuecomment-707244103
|
|
Use objfile->pspace instead of current_program_space.
Change-Id: I127a1788e155b321563114452ed5b530f1d1f618
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The free_objfile observable is never called with a nullptr objfile.
Change-Id: I1e990edeb45bc38009ccb129c623911097ab65fe
Approved-By: Tom Tromey <tom@tromey.com>
|
|
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.
[1] https://gitlab.com/gnutools/binutils-gdb/-/commit/a0a031bce0527b1521788b5dad640e7883b3a252
Change-Id: Icb648f72862e056267f30f44dd439bd4ec766f13
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Make the current_program_space references bubble up a bit.
Change-Id: Id047a48cc8d8a45504cdbb5960bafe3e7735d652
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Use objfile->pspace instead of current_program_space in two spots.
Change-Id: Idf94fad486252d1250380f295e71b0fe76dce76c
Approved-By: Tom Tromey <tom@tromey.com>
|
|
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>
|
|
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>
|
|
Make the references to current_program_space bubble up one level.
Change-Id: I82acab5628c30f6535d52aa32ce2c1d0375cbeed
Approved-By: Tom Tromey <tom@tromey.com>
|
|
It was pointed out on the mailing list that a recently added
test (gdb.python/py-progspace-events.exp) was failing when run with
the native-extended-gdbserver board. This test was added with this
commit:
commit 59912fb2d22f8a4bb0862487f12a5cc65b6a013f
Date: Tue Sep 19 11:45:36 2023 +0100
gdb: add Python events for program space addition and removal
It turns out though that the test is failing due to a existing bug
in GDB, the new test just exposes the problem. Additionally, the
failure really doesn't even rely on the new functionality added in the
above commit. I reduced the test to a simple set of steps that
reproduced the failure and tested against GDB 13, and the test passes;
so the bug was introduced since then. In fact, the bug was introduced
with this commit:
commit a2827364e2bf19910fa5a54364a594a5ba3033b8
Date: Fri Sep 8 15:48:16 2023 +0100
gdb: remove final user of the executable_changed observer
This commit changed how the per-inferior auxv data cache is managed,
specifically, when the cache is cleared, and it is this that leads to
the failure.
This bug is interesting because it exposes a number of issues with
GDB, I'll explain all of the problems I see, though ultimately, I only
propose fixing one problem in this commit, which is enough to resolve
the crash we are currently seeing.
The crash that we are seeing manifests like this:
...
[Inferior 2 (process 3970384) exited normally]
+inferior 1
[Switching to inferior 1 [process 3970383] (/tmp/build/gdb/testsuite/outputs/gdb.python/py-progspace-events/py-progspace-events)]
[Switching to thread 1.1 (Thread 3970383.3970383)]
#0 breakpt () at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.python/py-progspace-events.c:28
28 { /* Nothing. */ }
(gdb) step
+step
terminate called after throwing an instance of 'gdb_exception_error'
Fatal signal: Aborted
... etc ...
What's happening is that GDB attempts to refill the auxv cache as a
result of the gdbarch_has_shared_address_space call in
program_space::~program_space, the backtrace looks like this:
#0 0x00007fb4f419a9a5 in raise () from /lib64/libpthread.so.0
#1 0x00000000008b635d in handle_fatal_signal (sig=6) at ../../src/gdb/event-top.c:912
#2 <signal handler called>
#3 0x00007fb4f38e3625 in raise () from /lib64/libc.so.6
#4 0x00007fb4f38cc8d9 in abort () from /lib64/libc.so.6
#5 0x00007fb4f3c70756 in __gnu_cxx::__verbose_terminate_handler() [clone .cold] () from /lib64/libstdc++.so.6
#6 0x00007fb4f3c7c6dc in __cxxabiv1::__terminate(void (*)()) () from /lib64/libstdc++.so.6
#7 0x00007fb4f3c7b6e9 in __cxa_call_terminate () from /lib64/libstdc++.so.6
#8 0x00007fb4f3c7c094 in __gxx_personality_v0 () from /lib64/libstdc++.so.6
#9 0x00007fb4f3a80c63 in _Unwind_RaiseException_Phase2 () from /lib64/libgcc_s.so.1
#10 0x00007fb4f3a8154e in _Unwind_Resume () from /lib64/libgcc_s.so.1
#11 0x0000000000e8832d in target_read_alloc_1<unsigned char> (ops=0x408a3a0, object=TARGET_OBJECT_AUXV, annex=0x0) at ../../src/gdb/target.c:2266
#12 0x0000000000e73dea in target_read_alloc (ops=0x408a3a0, object=TARGET_OBJECT_AUXV, annex=0x0) at ../../src/gdb/target.c:2315
#13 0x000000000058248c in target_read_auxv_raw (ops=0x408a3a0) at ../../src/gdb/auxv.c:379
#14 0x000000000058243d in target_read_auxv () at ../../src/gdb/auxv.c:368
#15 0x000000000058255c in target_auxv_search (match=0x0, valp=0x7ffdee17c598) at ../../src/gdb/auxv.c:415
#16 0x0000000000a464bb in linux_is_uclinux () at ../../src/gdb/linux-tdep.c:433
#17 0x0000000000a464f6 in linux_has_shared_address_space (gdbarch=0x409a2d0) at ../../src/gdb/linux-tdep.c:440
#18 0x0000000000510eae in gdbarch_has_shared_address_space (gdbarch=0x409a2d0) at ../../src/gdb/gdbarch.c:4889
#19 0x0000000000bc7558 in program_space::~program_space (this=0x4544aa0, __in_chrg=<optimized out>) at ../../src/gdb/progspace.c:124
#20 0x00000000009b245d in delete_inferior (inf=0x47b3de0) at ../../src/gdb/inferior.c:290
#21 0x00000000009b2c10 in prune_inferiors () at ../../src/gdb/inferior.c:480
#22 0x00000000009c5e3e in fetch_inferior_event () at ../../src/gdb/infrun.c:4558
#23 0x000000000099b4dc in inferior_event_handler (event_type=INF_REG_EVENT) at ../../src/gdb/inf-loop.c:42
#24 0x0000000000cbc64f in remote_async_serial_handler (scb=0x4090a30, context=0x408a6b0) at ../../src/gdb/remote.c:14859
#25 0x0000000000d83d3a in run_async_handler_and_reschedule (scb=0x4090a30) at ../../src/gdb/ser-base.c:138
#26 0x0000000000d83e1f in fd_event (error=0, context=0x4090a30) at ../../src/gdb/ser-base.c:189
So this is problem #1, if we throw an exception while deleting a
program_space then this is not caught, and is going to crash GDB.
Problem #2 becomes evident when we ask why GDB is throwing an error in
this case; the error is thrown because the remote target, operating in
non-async mode, can't read the auxv data while an inferior is running
and GDB is waiting for a stop reply. The problem here then, is why
does GDB get into a position where it tries to interact with the
remote target in this way, at this time? The problem is caused by the
prune_inferiors call which can be seen in the above backtrace.
In prune_inferiors we check if the inferior is deletable, and if it
is, we delete it. The problem is, I think, we should also check if
the target is currently in a state that would allow us to delete the
inferior. We don't currently have such a check available, we'd need
to add one, but for the remote target, this would return false if the
remote is in async mode and the remote is currently waiting for a stop
reply. With this change in place GDB would defer deleting the
inferior until the remote target has stopped, at which point GDB would
be able to refill the auxv cache successfully.
And then, problem #3 becomes evident when we ask why GDB is needing to
refill the auxv cache now when it didn't need to for GDB 13. This is
where the second commit mentioned above (a2827364e2bf) comes in.
Prior to this commit, the auxv cache was cleared by the
executable_changed observer, while after that commit the auxv cache
was cleared by the new_objfile observer -- but only when the
new_objfile observer is used in the special mode that actually means
that all objfiles have been unloaded (I know, the overloading of the
new_objfile observer is horrible, and unnecessary, but it's not really
important for this bug).
The difference arises because the new_objfile observer is triggered
from clear_symtab_users, which in turn is called from
program_space::~program_space. The new_objfile observer for auxv does
this:
static void
auxv_new_objfile_observer (struct objfile *objfile)
{
if (objfile == nullptr)
invalidate_auxv_cache_inf (current_inferior ());
}
That is, when all the objfiles are unloaded, we clear the auxv cache
for the current inferior.
The problem is, then when we look at the prune_inferiors ->
delete_inferior -> ~program_space path, we see that the current
inferior is not going to be an inferior that exists within the
program_space being deleted; delete_inferior removes the deleted
inferior from the global inferior list, and then only deletes the
program_space if program_space::empty() returns true, which is only
the case if the current inferior isn't within the program_space to
delete, and no other inferior exists within that program_space
either.
What this means is that when the new_objfile observer is called we
can't rely on the current inferior having any relationship with the
program space in which the objfiles were removed. This was an error
in the commit a2827364e2bf, the only thing we can rely on is the
current program space. As a result of this mistake, after commit
a2827364e2bf, GDB was sometimes clearing the auxv cache for a random
inferior. In the native target case this was harmless as we can
always refill the cache when needed, but in the remote target case, if
we need to refill the cache when the remote target is executing, then
we get the crash we observed.
And additionally, if we think about this a little more, we see that
commit a2827364e2bf made another mistake. When all the objfiles are
removed, they are removed from a program_space, a program_space might
contain multiple inferiors, so surely, we should clear the auxv cache
for all of the matching inferiors?
Given these two insights, that the current_inferior is not relevant,
only the current_program_space, and that we should be clearing the
cache for all inferiors in the current_program_space, we can update
auxv_new_objfile_observer to:
if (objfile == nullptr)
{
for (inferior *inf : all_inferiors ())
{
if (inf->pspace == current_program_space)
invalidate_auxv_cache_inf (inf);
}
}
With this change we now correctly clear the auxv cache for the correct
inferiors, and GDB no longer needs to refill the cache at an
inconvenient time, this avoids the crash we were seeing.
And finally, we reach problem #4. Inspired by the observation that
using the current_inferior from within the ~program_space function was
not correct, I added some debug to see if current_inferior() was
called anywhere else (below ~program_space), and the answer is yes,
it's called a often. Mostly the culprit is GDB doing:
current_inferior ()->top_target ()-> ....
But I think all of these calls are most likely doing the wrong thing,
and only work because the top target in all these cases is shared
between all inferiors, e.g. it's the native target, or the remote
target for all inferiors. But if we had a truly multi-connection
setup, then we might start to see odd behaviour.
Problem #1 I'm just ignoring for now, I guess at some point we might
run into this again, and then we'd need to solve this. But in this
case I wasn't sure what a "good" solution would look like. We need
the auxv data in order to implement the linux_is_uclinux() function.
If we can't get the auxv data then what should we do, assume yes, or
assume no? The right answer would probably be to propagate the error
back up the stack, but then we reach ~program_space, and throwing
exceptions from a destructor is problematic, so we'd need to catch and
deal at this point. The linux_is_uclinux() call is made from within
gdbarch_has_shared_address_space(), which is used like:
if (!gdbarch_has_shared_address_space (target_gdbarch ()))
delete this->aspace;
So, we would have to choose; delete the address space or not. If we
delete it on error, then we might delete an address space that is
shared within another program space. If we don't delete the address
space, then we might leak it. Neither choice is great.
A better solution might be to have the address spaces be reference
counted, then we could remove the gdbarch_has_shared_address_space
call completely, and just rely on the reference count to auto-delete
the address space when appropriate.
The solution for problem #2 I already hinted at above, we should have
a new target_can_delete_inferiors() call, which should be called from
prune_inferiors, this would prevent GDB from trying to delete
inferiors when a (remote) target is in a state where we know it can't
delete the inferior. Deleting an inferior often (always?) requires
sending packets to the remote, and if the remote is waiting for a stop
reply then this will never work, so the pruning should be deferred in
this case.
The solution for problem #3 is included in this commit.
And, for problem #4, I'm not sure what the right solution is. Maybe
delete_inferior should ensure the inferior to be deleted is in place
when ~program_space is called? But that seems a little weird, as the
current inferior would, in theory, still be using the current
program_space...
Anyway, after this commit, the gdb.python/py-progspace-events.exp test
now passes when run with the native-extended-remote board.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30935
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Change-Id: I41f0e6e2d7ecc1e5e55ec170f37acd4052f46eaf
|
|
When running from a distribution tarball, bison should not be necessary.
The generated files (QLParser.tab.cc, QLParser.tab.hh) should be distributed.
configure.ac should not abort if bison is missing.
configure.ac should remove temporary files (dummy.c, Simple.class).
bison must be run once to create QLParser.tab.cc and QLParser.tab.hh.
gprofng/ChangeLog
2023-10-03 Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
PR gprofng/30894
* configure.ac: Don't abort if bison is missing. Remove temporary files.
* src/Makefile.am: Distribute QLParser.tab.cc and QLParser.tab.hh.
* Run bison once to create QLParser.tab.cc and QLParser.tab.hh.
* configure: Rebuild.
* src/Makefile.in: Rebuild.
|
