ada_bitwise_operation differs from the "usual" bitwise operations only
in that it calls value_cast at the end. However, because gdb is
generally fairly lax about integer types, and because (perhaps oddly)
C-style binary promotion is done here anyway, it seems to me that this
code isn't needed.
ptype is a bit funny, in that it accepts both expressions and type
names. It also evaluates the resulting expression using
EVAL_AVOID_SIDE_EFFECTS -- which both seems sensible (as a user would
you expect ptype to possibly cause inferior execution?), but is also a
historical artifact of how expressions are implemented (there's no
EVAL_FOR_TYPE).
In Ada, calling a function with an array will sometimes result in a
"thick pointer" array descriptor being made. This is essentially a
structure holding a pointer and bounds information.
Currently, in such a callee, printing the type of the array will yield
funny results:
(gdb) print str.all
$1 = "Hello World"
(gdb) ptype str
type = array (<>) of character
(gdb) ptype str.all
type = array (1 .. 0) of character
That "1 .. 0" is the result of an EVAL_AVOID_SIDE_EFFECTS branch
trying to do "something" with an array descriptor, without doing too
much.
I tried briefly to make this code really dereference the array
descriptor and get the correct runtime type. However, that proved to
be tricky; it certainly can't be done for all access types, because
that will cause dynamic type resolution and end up printing just the
runtime type -- which with variants may be pretty far from what the
user may expect.
Instead, this patch arranges to just leave such types alone in this
situation. I don't think this should have an extra effects, because
things like array subscripting still work on thick pointers.
This patch also touches arrayptr.exp, because in that case the access
type is a "thin pointer", and this ensures that the output does not
change in that scenario.
quirk_rust_enum makes string copies to store in an unordered_map.
However, the original strings have an appropriate lifetime, and so no
copying is needed. With C++17, we can rely on string_view having a
std::hash specialization, so this patch changes this code to use
string_view rather than string.
This patch arranges to set __file__ when source'ing a Python script.
This fixes a problem that was introduced by the "source" rewrite, and
then pointed out by Lancelot Six.
Reviewed-by: Lancelot Six <lancelot.six@amd.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
When certain DAP tests are run in a certain order, dejagnu will throw
an exception during shutdown. After adding many logging statements, I
tracked this down to kill_wait_spawned_process not clearing the
'fileid' board_info entry, causing dejagnu to try to wait for the
process a second time -- and fail.
Tom de Vries then pointed out a second instance of this, which I
tracked down to the same problem occurring when spawning gdbserver.
This version of the patch fixes this by adding a new proc that can be
used to clean up board_info after waiting for a process to exit. I'm
not sure why this problem hasn't affected the test suite in the past.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31435
Approved-By: Andrew Burgess <aburgess@redhat.com>
bdfio.c is defining bfd_get_current_time which is returning a time_t.
This type is defined in time.h and thus, must be included in bfd main
header to avoid undefined type when include bfd.h.
Note that most of the time, <time.h> is pulled by <sys/stat.h> already
included in bfd.h. That's why it went unnoticed.
Fedora GDB has carried around a patch for a while which tested
attaching to an i386 process which is stopped within the vDSO library
region. Apparently, at some point in the distant past there was an
issue finding symbol information for this region in this situation.
I'm struggling to track down the precise details of what the original
bug was, however, acquiring symbol information for the vDSO region is
different than for "normal" shared libraries -- the vDSO information
is synthesised within GDB during the attach / inferior creation
process -- so it's not unreasonable to imagine that there could be a
bug specifically in this area of GDB which wouldn't impact "normal"
shared libraries.
I looked for references to vDSO in our testsuite and couldn't find
any tests that looked like they did the same sort of thing, so I'd
like to propose adding this test to our testsuite.
It's a pretty simple test, and doesn't take long to run, so the cost
of adding this is not huge.
Approved-By: Tom Tromey <tom@tromey.com>
Except for bfml{a,s} their 1st and 3rd operands need to match - pass
the TIED macro argument accordingly. While doing that also slightly
re-arrange table entries, such that all predicated insns are close
together.
At the same time change the existing test source to actually use non-
matching operands for the respective bfml{a,s} forms.
Their index is in bits 19, 20, and 22. Bit 11 in particular is already
set in the base opcode. Note also how disassembler output didn't match
assembler input in the respective testcase.
On debian 12, aarch64-linux I run into:
...
(gdb) list .^M
No symbol table is loaded. Use the "file" command.^M
(gdb) FAIL: gdb.base/list-nodebug.exp: first 'list .'
...
The test-case expects some debug info, but none for main. Instead, there's no
debug info at all.
Fix this by adding another source file to the test-case, and compiling it with
debug info.
Tested on aarch64-linux.
Approved-By: Andrew Burgess <aburgess@redhat.com>
PR testsuite/31290
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31290
BFD recently changed bfd_mmap to use size_t, not bfd_size_type. This
patch updates gdb_bfd_section_data to follow. Without this patch, if
the two types ever differ, gdb would fail to build.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Irrespective of the encoding being EVEX, the usable SIMD register range
continues to be limited to %xmm0-%xmm15. Enforce this in gas (but
continue to generate code, as in principle we know how to encode
things) and recognize/flag the case in the disassembler.
Oddly enough wrong forms were actually used in the testsuite (register-
only forms are then really meaningless to test here, and are hence
dropped instead of adjusted).
Convert the POP2 test that needs touching anyway (due to a bad ModR/M
byte having been chosen) to .insn.
I tried building gdbserver on postmarketos (which is based on alpine linux,
which uses musl libc), and ran into:
...
gdbserver/linux-low.cc: In lambda function:
gdbserver/linux-low.cc:1907:41: error: \
'W_EXITCODE' was not declared in this scope
1907 | mark_lwp_dead (leader_lp, W_EXITCODE (0, 0), true);
| ^~~~~~~~~~
...
The macro W_EXITCODE is not defined in gdbsupport/gdb_wait.h.
OTOH, WSETEXIT is defined there, but unused:
...
/* These are not defined in POSIX, but are used by our programs. */
#ifndef WSETEXIT
# ifdef W_EXITCODE
#define WSETEXIT(w,status) ((w) = W_EXITCODE(status,0))
# else
#define WSETEXIT(w,status) ((w) = (0 | ((status) << 8)))
# endif
#endif
...
Fix this by dropping the WSETEXIT definition, and instead defining W_EXITCODE.
Tested on x86_64-linux, in combination with an "#undef W_EXITCODE" to make
sure the definition is exercised.
Approved-By: Tom Tromey <tom@tromey.com>
PR build/31483
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31483
Make gdbserver's build system locate libiconv when building for Linux.
Commit 07b3255c3b ("Filter invalid encodings from Linux thread names")
make libiconv madantory for building gdbserver on Linux.
While trying to cross-compile gdb for xtensa-fsf-linux-uclibc (with a
toolchain generated with crosstool-ng), I got:
/home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:48:10: fatal error: iconv.h: No such file or directory
48 | #include <iconv.h>
| ^~~~~~~~~
I downloaded GNU libiconv, built it for that host, and installed it in
an arbitrary directory. I had to modify the gdbserver build system to
locate libiconv and use it, the result is this patch.
I eventually found that crosstool-ng has a config option to make uclibc
provide an implementation of iconv, which is of course much easier. But
given that this patch is now written, I think it would be worth merging
it, it could help some people who do not have iconv built-in their libc
in the future (and may not have the luxury of rebuilding their libc like
I do).
Using AM_ICONV in configure.ac adds these options for configure (the
same we have for gdb):
--with-libiconv-prefix[=DIR] search for libiconv in DIR/include and DIR/lib
--without-libiconv-prefix don't search for libiconv in includedir and libdir
--with-libiconv-type=TYPE type of library to search for (auto/static/shared)
It sets the `LIBICONV` variable with whatever is needed to link with
libiconv, and adds the necessary `-I` flag to `CPPFLAGS`.
To avoid unnecessarily linking against libiconv on hosts that don't need
it, set `MAYBE_LIBICONV` with the contents of `LIBICONV` only if the
host is Linux, and use `MAYBE_LIBICONV` in `Makefile.in`.
Since libiconv is a hard requirement for Linux hosts, error out if it is
not found.
The bits in acinclude.m4 are similar to what we have in
gdb/acinclude.m4.
Update the top-level build system to support building against an in-tree
libiconv (I did not test this part though). Something tells me that the
all-gdbserver dependency on all-libiconv is unnecessary, since there is
already a dependency of configure-gdbserver on all-libiconv (and
all-gdbserver surely depends on configure-gdbserver). I just copied
what's done for GDB though.
ChangeLog:
* Makefile.def: Add configure-gdbserver and all-gdbserver
dependencies on all-libiconv.
* Makefile.in: Re-generate.
Change-Id: I90f8ef88dd4917df5a68b45550d93622fc9cfed4
Approved-By: Tom Tromey <tom@tromey.com>
When the GCC compiler plugin responds with GCC_C_FE_VERSION_2, gdb can
use the new 'finish_record_with_alignment' method. This lets gdb pass
alignment information to the compiler, which in turn fixes the test
case included in this patch.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31397
This removes the embedded 'if' from GDB_PY_HANDLE_EXCEPTION and
GDB_PY_SET_HANDLE_EXCEPTION. I believe this 'if' was necessary with
the old gdb try/catch macros, but it no longer is: these should only
ever be called from a 'catch' block, where it's already known that an
exception was thrown.
Simon pointed out, though, that in a few spots, these were in facts
called outside of 'catch' blocks. This patch cleans up these spots.
I also found one spot where a redundant 'return nullptr' could be
removed.
On aarch64-linux, with test-case gdb.base/watchpoint-unaligned.exp I run into:
...
(gdb) watch data.u.size8twice[1]^M
Hardware watchpoint 241: data.u.size8twice[1]^M
(gdb) PASS: gdb.base/watchpoint-unaligned.exp: watch data.u.size8twice[1]
continue^M
Continuing.^M
FAIL: gdb.base/watchpoint-unaligned.exp: continue (timeout)
FAIL: gdb.base/watchpoint-unaligned.exp: size8twice write
...
This happens as follows.
We start the exec and set an 8-byte hardware watchpoint on
data.u.size8twice[1] at address 0x440048:
...
(gdb) p sizeof (data.u.size8twice[1])
$1 = 8
(gdb) p &data.u.size8twice[1]
$2 = (uint64_t *) 0x440048 <data+16>
...
We continue execution, and a 16-byte write at address 0x440040 triggers the
hardware watchpoint:
...
4101c8: a9000801 stp x1, x2, [x0]
...
When checking whether a watchpoint has triggered in
aarch64_stopped_data_address, we check against address 0x440040 (passed in
parameter addr_trap). This behaviour is documented:
...
/* ADDR_TRAP reports the first address of the memory range
accessed by the CPU, regardless of what was the memory
range watched. ... */
...
and consequently the matching logic compares against an addr_watch_aligned:
...
&& addr_trap >= addr_watch_aligned
&& addr_trap < addr_watch + len)
...
However, the comparison fails:
...
(gdb) p /x addr_watch_aligned
$3 = 0x440048
(gdb) p addr_trap >= addr_watch_aligned
$4 = false
...
Consequently, aarch64_stopped_data_address returns false, and
stopped_by_watchpoint returns false, and watchpoints_triggered returns 0,
which make infrun think it's looking at a delayed hardware
breakpoint/watchpoint trap:
...
[infrun] handle_signal_stop: stop_pc=0x4101c8
[infrun] handle_signal_stop: delayed hardware breakpoint/watchpoint trap, ignoring
...
Infrun then ignores the trap and continues, but runs into the same situation
again and again, causing a hang which then causes the test timeout.
Fix this by allowing a match 8 bytes below addr_watch_aligned. This
introduces the possibility for false positives, so we only do this for regular
"value changed" watchpoints.
An earlier version of this patch worked by aligning addr_watch_aligned to 16
instead of 8:
...
- const CORE_ADDR addr_watch_aligned = align_down (state->dr_addr_wp[i], 8);
+ const CORE_ADDR addr_watch_aligned = align_down (state->dr_addr_wp[i], 16);
...
but while that fixed the test-case, it didn't fix the problem completely, so
extend the test-case to check more scenarios.
Tested on aarch64-linux.
Tested-By: Luis Machado <luis.machado@arm.com>
Approved-By: Luis Machado <luis.machado@arm.com>
PR tdep/29423
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29423
Change the size type in the BFD mmap interface from bfd_size_type to
size_t to be consistent with the size type of the host mmap interface.
* bfdio.c (bfd_iovec): Change the bmmap size type to size_t.
(bfd_mmap): Likewise.
(memory_bmmap): Likewise.
* cache.c (cache_bmmap): Change the bmmap size type to size_t.
* opncls.c (opncls_bmmap): Change the bmmap size type to size_t.
* bfd-in2.h: Regenerated.
* libbfd.h: Likewise.
Use MAP_FAILED, instead of ((void *) -1), for mmap failure and use
((void *) -1) only if MAP_FAILED is undefined.
* bfdio.c (bfd_mmap): Replace (void *) -1 with MAP_FAILED for
mmap failure.
* bfdwin.c: Don't include <sys/mman.h>.
(MAP_FILE): Removed.
(bfd_get_file_window): Replace (void *) -1 with MAP_FAILED for
mmap failure.
* cache.c: Don't include <sys/mman.h>.
(cache_bmmap): Replace (void *) -1 with MAP_FAILED for mmap
failure.
* opncls.c (opncls_bmmap): Likewise.
* sysdep.h: Include <sys/mman.h> if HAVE_MMAP is define.
(MAP_FILE): New. Defined as 0 if undefined.
(MAP_FAILED): New. Defined as ((void *) -1) if undefined.
Use -march=help for gas to print all supported extensions and versions.
Here is part of the output of `as -march=help`:
All available -march extensions for RISC-V:
e 1.9
i 2.1, 2.0
m 2.0
a 2.1, 2.0
f 2.2, 2.0
d 2.2, 2.0
q 2.2, 2.0
c 2.0
v 1.0
h 1.0
zicbom 1.0
zicbop 1.0
...
This patch assumes that the supported extensions with the same versions
are listed together. For example:
static struct riscv_supported_ext riscv_supported_std_ext[] =
{
...
{"i", ISA_SPEC_CLASS_20191213, 2, 1, 0 },
{"i", ISA_SPEC_CLASS_20190608, 2, 1, 0 },
{"i", ISA_SPEC_CLASS_2P2, 2, 0, 0 },
...
};
For the "i" extension, 2.1.0 with different spec class are listed together.
This patch records the previous printed extension and version. If the
current extension and version are the same as the previous one, skip
printing.
bfd/
* elfxx-riscv.c (riscv_print_extensions): New function. Print
available extensions and versions.
* elfxx-riscv.h (riscv_print_extensions): New declaration.
gas/
* gas/config/tc-riscv.c (md_parse_option): Parse 'help' keyword in
-march option to print available extensions and versions.
* testsuite/gas/riscv/march-help.l: New testcase for -march=help.
* testsuite/gas/riscv/riscv.exp: Updated.
On aarch64-linux, with test-case gdb.base/watch-bitfields.exp I run into:
...
(gdb) continue^M
Continuing.^M
^M
Hardware watchpoint 2: -location q.a^M
^M
Old value = 1^M
New value = 0^M
main () at watch-bitfields.c:42^M
42 q.h--;^M
(gdb) FAIL: $exp: -location watch against bitfields: q.e: 0->5: continue
...
In a minimal form, if we step past line 37 which sets q.e, and we have a
watchpoint set on q.e, it triggers:
...
$ gdb -q -batch watch-bitfields -ex "b 37" -ex run -ex "watch q.e" -ex step
Breakpoint 1 at 0x410204: file watch-bitfields.c, line 37.
Breakpoint 1, main () at watch-bitfields.c:37
37 q.e = 5;
Hardware watchpoint 2: q.e
Hardware watchpoint 2: q.e
Old value = 0
New value = 5
main () at /home/vries/gdb/src/gdb/testsuite/gdb.base/watch-bitfields.c:38
38 q.f = 6;
...
However, if we set in addition a watchpoint on q.a, the watchpoint on q.e
doesn't trigger.
How does this happen?
Bitfield q.a is just bit 0 of byte 0, and bitfield q.e is bit 4..7 of byte 1
and bit 1 of byte 2. So, watch q.a should watch byte 0, and watch q.e should
watch bytes 1 and 2.
Using "maint set show-debug-regs on" (and some more detailed debug prints) we
get:
...
WP2: addr=0x440028 (orig=0x440029), ctrl=0x000000d5, ref.count=1
ctrl: enabled=1, offset=1, len=2
WP3: addr=0x440028 (orig=0x440028), ctrl=0x00000035, ref.count=1
ctrl: enabled=1, offset=0, len=1
...
which matches that.
When executing line 37, a hardware watchpoint trap triggers and we hit
aarch64_stopped_data_address with addr_trap == 0x440028:
...
(gdb) p /x addr_trap
$1 = 0x440028
....
and since the loop in aarch64_stopped_data_address walks backward, we check
WP3 first, which matches, and consequently target_stopped_by_watchpoint
returns true in watchpoints_triggered.
Likewise for target_stopped_data_address, which also returns addr == 0x440028.
Watchpoints_triggered matches watchpoint q.a to that address, and sets
watch_triggered_yes.
However, subsequently the value of q.a is checked, and it's the same value as
before (becase the insn in line 37 didn't change q.a), so the watchpoint
hardware trap is not reported to the user.
The problem originates from that fact that aarch64_stopped_data_address picked
WP3 instead of WP2.
There's something we can do about this. In the example above, both
target_stopped_by_watchpoint and target_stopped_data_address returned true.
Instead we can return true in target_stopped_by_watchpoint but false in
target_stopped_data_address. This lets watchpoints_triggered known that a
watchpoint was triggered, but we don't know where, and both watchpoints
get set to watch_triggered_unknown.
Subsequently, the values of both q.a and q.e are checked, and since q.e is not
the same value as before, the watchpoint hardware trap is reported to the user.
Note that this works well for regular (write) watchpoints (watch command), but
not for read watchpoints (rwatch command), because for those no value is
checked. Likewise for access watchpoints (awatch command).
So, fix this by:
- passing a nullptr in aarch64_fbsd_nat_target::stopped_by_watchpoint and
aarch64_linux_nat_target::stopped_by_watchpoint to make clear we're not
interested in the stop address,
- introducing a two-phase approach in aarch64_stopped_data_address, where:
- phase one handles access and read watchpoints, as before, and
- phase two handles write watchpoints, where multiple matches cause:
- return true if addr_p == null, and
- return false if addr_p != null.
Tested on aarch64-linux.
Approved-By: Luis Machado <luis.machado@arm.com>
PR tdep/31214
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31214
This syncs dg-extract-results.sh with GCC.
It contains two commits: r14-4333-g346f5991569fae and r14-9393-g64273a7e6bd8ba.
contrib/ChangeLog:
* dg-extract-results.sh: Sync with GCC.
Approved-By: Tom Tromey <tom@tromey.com>
This syncs dg-extract-results.py with GCC.
It contains only one commit: r14-7145-g8f67953d0198fe.
contrib/ChangeLog:
* dg-extract-results.py: Sync with GCC.
Approved-By: Tom Tromey <tom@tromey.com>
This patch deprecates the MPX commands "show/set mpx bound".
Intel listed Intel(R) Memory Protection Extensions (MPX) as removed
in 2019. Following gcc v9.1, the linux kernel v5.6 and glibc v2.35,
deprecate MPX in GDB.
Currently, gas will exit immediately and report an error when
it sees illegal operands, and will not process the remaining
instructions. Replace as_fatal with as_bad to check for all
illegal operands.
Add test cases for illegal operands of some instructions.
* After adding the old LE relax, all old LE relocations will have
an R_LARCH_RELAX relocation. Fix the gas test case failure caused
by the implementation of the old LE relax.
* loongarch64-elf does not support pie and -z norelro options,
removed in test files.
I see some changes in the generated files when running update-gnulib.sh.
The changes appeared with commit 35b38b0182 ("Finalized intl-update
patches (trois)"). This is most likely due to how the autotools were
ran in that commit, possibly with some different -I arguments.
Change-Id: Idaad8084b0e91e22d066f573775e21d0c7a039cb
Note that this includes Nick's fix from edf64cd235 which
landed upstream a bit differently as r13-1566-g9ed57796235abc in GCC.
This pulls in libbacktrace as of r14-9404-gc775a030af9cad in GCC trunk.
Note that I have dropped a top-level Darwin change from r14-4825-g6a6d3817afa02b
which would've required an autoreconf, as it should be handled separately.
Approved-By: Tom Tromey <tom@tromey.com>
The other day on irc, we were discussing the "m_line" hack in
tui-out.c, and I mentioned that it would be nice to replace this with
a new ui_out_flag.
Later, I looked at ui_out_flag and found:
ui_source_list = (1 << 0),
... and sure enough, this is tested already.
This patch removes tui-out.[ch] and changes the TUI to use an ordinary
cli-out object without this flag set.
As far as I can tell, this doesn't affect behavior at all -- the TUI
tests all pass, and interactively I tried switching stack frames,
"list", etc, and it all seems to work.
New in v2: fixed the problem pointed out by Keith, and added a test
case for that scenario.
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
aclocal picks up the relevant include paths from AC_CONFIG_MACRO_DIRS in
configure.ac, so there's no need to pass `-I ../config` here.
Passing `-I ../config` is actually annoying, because it makes the output
different between when the update is triggered by the maintainer mode
and when aclocal or autoreconf is ran with no special flags. The
difference in the output is due to the order of include paths being
different.
Change-Id: I2c963876516570842f20b4a6a470867e7a941006
Approved-By: Tom Tromey <tom@tromey.com>
commit f18fc7e5 ("gdb, types: Resolve pointer types dynamically")
caused a regression on a test case in the AdaCore internal test suite.
The issue here is that gdb would try to resolve the type of a dynamic
pointer that happened to be NULL. In this case, the "Location address
is not set." error would end up being thrown from the DWARF expression
evaluator.
I think it makes more sense to special-case NULL pointers and not try
to resolve their target type, as that type can't really be accessed
anyway.
This patch implements this idea, and also adds the missing Ada test
case.
A Python file in my previous commit (5eb2254a1d) was formatted with
an older version of black, which gives slightly different results.
Reformat with a newer version of black. This should make our
post-commit testing happy again.
No functional changes in this commit.
Just because a path is an error path doesn't mean the program terminates
there if you don't ask it to. And we don't want to -- but that means
we need to initialize the variables that are missed if an error happens to
*something*. Type ID 0 (unimplemented) will do: it'll induce further
ECTF_BADID errors, but that's no bad thing.
libctf/ChangeLog:
* testsuite/libctf-writable/libctf-errors.c: Initialize variables.
I get some changes when running `autoreconf -vf` in the gdb directory,
fix that.
I did a bisect, it appears to have been introduced in this commit, not
sure why we haven't spotted that before.
commit 862776f26a
Author: Arsen Arsenovi? <arsen@aarsen.me>
AuthorDate: Wed Nov 15 12:53:04 2023 +0000
Commit: Nick Clifton <nickc@redhat.com>
CommitDate: Wed Nov 15 12:53:04 2023 +0000
Change-Id: I798d2fbff40c39dbc899832c64e72b2859b536b9
When we improved error messages in
cd393cec3a gdb, btrace: improve error messages
we cleared the original errno. When the error reason can not be explained
in a more detailed error message, and we fall back to the default error
message, it now gives Success as error.
Restore the original errno to fix that.
This started with a Red Hat bug report which can be seen here:
https://bugzilla.redhat.com/show_bug.cgi?id=1850710
The problem reported here was using GDB on GNU/Linux for S390, the
user stepped into JIT generated code. As they enter the JIT code GDB
would report 'PC not saved', and this same message would be reported
after each step/stepi.
Additionally, the user had 'set disassemble-next-line on', and once
they entered the JIT code this output was not displayed, nor were any
'display' directives displayed.
The user is not making use of the JIT plugin API to provide debug
information. But that's OK, they aren't expecting any source level
debug here, they are happy to use 'stepi', but the missing 'display'
directives are a problem, as is the constant 'PC not saved' (error)
message.
What is happening here is that as GDB is failing to find any debug
information for the JIT generated code, it is falling back on to the
S390 prologue unwinder to try and unwind frame #0. Unfortunately,
without being able to identify the function boundaries, the S390
prologue scanner can't help much, in fact, it doesn't even suggest an
arbitrary previous $pc value (some targets that use a link-register
will, by default, assume the link-register contains the previous $pc),
instead the S390 will just say, "sorry, I have no previous $pc value".
The result of this is that when GDB tries to find frame #1 we end
throwing an error from frame_unwind_pc (the 'PC not saved' error).
This error is not caught anywhere except at the top-level interpreter
loop, and so we end up skipping all the 'display' directive handling.
While thinking about this, I wondered, could I trigger the same error
using the Python Unwinder API? What happens if a Python unwinder
claims a frame, but then fails to provide a previous $pc value?
Turns out that exactly the same thing happens, which is great, as that
means we now have a way to reproduce this bug on any target. And so
the test included with this patch does just this. I have a Python
unwinder that claims a frame, but doesn't provide any previous
register values.
I then do two tests, first I stop in the claimed frame (i.e. frame #0
is the frame that can't be unwound), I perform a few steps, and check
the backtrace. And second, I stop in a child of the problem
frame (i.e. frame #1 is the frame that can't be unwound), and from
here I check the backtrace.
While all this is going on I have a 'display' directive in place, and
each time GDB stops I check that the display directive triggers.
Additionally, when checking the backtrace, I am checking that the
backtrace finishes with the message 'Backtrace stopped: frame did not
save the PC'.
As for the fix I chose to add a call to frame_unwind_pc directly to
get_prev_frame_always_1. Calling frame_unwind_pc will cache the
unwound $pc value, so this doesn't add much additional work as
immediately after the new frame_unwind_pc call, we call
get_prev_frame_maybe_check_cycle, which actually generates the
previous frame, which will always (I think) require a call to
frame_unwind_pc anyway.
The reason for adding the frame_unwind_pc call into
get_prev_frame_always_1, is that if the frame_unwind_pc call fails we
want to set the frames 'stop_reason', and get_prev_frame_always_1
seems to be the place where this is done, so I wanted to keep the new
stop_reason setting code next to all the existing stop_reason setting
code.
Additionally, once we enter get_prev_frame_maybe_check_cycle we
actually create the previous frame, then, if it turns out that the
previous frame can't be created we need to remove the frame .. this
seemed more complex than just making the check in
get_prev_frame_always_1.
With this fix in place the original S390 bug is fixed, and also the
test added in this commit, that uses the Python API, is also fixed.
Reviewed-By: Kevin Buettner <kevinb@redhat.com>
The test gdb.threads/threadcrash.exp demanded GDB to fully unwind and
print the names of all functions. However, some of the functions are
from the libc library, and so the test implicitly demanded libc symbols
to be available, and would fail otherwise, as was raised in PR
gdb/31293.
This commit changes it so we only explicitly check for functions that
are not provided by threadcrash.c if they are indeed available.
Tested on arm-linux and x86_64-linux.
Approved-By: Tom de Vries <tdevries@suse.de>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31293
