| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
The type-allocation patches introduced a small regression that was
picked up by the AdaCore internal test suite. Previously, the name of
a range type was preserved by resolve_dynamic_range, but now it is
not. This patch changes this code to preserve the name.
Reviewed-By: Bruno Larsen <blarsen@redhat.com>
|
|
The evaluate command supports a "context" parameter which tells the
adapter the context in which an evaluation occurs. One of the
supported values is "repl", which we took to mean evaluation of a gdb
command. That is what this patch implements.
Note that some gdb commands probably will not work correctly with the
rest of the protocol. For example if the user types "continue",
confusion may result.
This patch requires the earlier patch to fix up scopes in DAP.
|
|
Since commit 6d263fe46e0 ("Avoid bad breakpoints with --gc-sections"), there
was a silent regression on openSUSE Leap 15.4 for test-case
gdb.cp/m-static.exp, from:
...
(gdb) info variable everywhere^M
All variables matching regular expression "everywhere":^M
^M
File /home/vries/tmp.local-remote-host-native/m-static.h:^M
8: const int gnu_obj_4::everywhere;^M
(gdb)
...
to:
...
(gdb) info variable everywhere^M
All variables matching regular expression "everywhere":^M
^M
File /data/vries/gdb/src/gdb/testsuite/gdb.cp/m-static.h:^M
8: const int gnu_obj_4::everywhere;^M
^M
File /data/vries/gdb/src/gdb/testsuite/gdb.cp/m-static1.cc:^M
8: const int gnu_obj_4::everywhere;^M
(gdb)
...
Another regression was found due to that commit, and it was fixed in commit
99d679e7b30 ("[gdb/symtab] Fix "file index out of range" complaint") by
limiting the scope of the fix in the original commit.
Fix this regression by yet further limiting the scope of that fix, making sure
that this bit in dwarf_decode_lines is executed again for m-static1.cc:
...
/* Make sure a symtab is created for every file, even files
which contain only variables (i.e. no code with associated
line numbers). */
...
Tested on x86_64-linux.
PR symtab/30265
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30265
|
|
Internal AdaCore DAP testing on Windows has had occasional failures
that show:
assert threading.current_thread() is _dap_thread
I think this is a race in DAP startup: the _dap_thread global is only
set on return from start_thread, but it seems possible that the thread
itself could already run and encounter a @in_dap_thread decorator.
This patch fixes the problem by setting the global before running any
of the code in the new thread. This also lets us remove a FIXME.
|
|
There were reports of gdb throwing internal errors when calling
inferior_thread ()/get_current_regcache () on a system with
Pointer Authentication enabled.
In such cases, gdb produces the following backtrace:
../../../repos/binutils-gdb/gdb/thread.c:86: internal-error: inferior_thread: Assertion `current_thread_ != nullptr' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
----- Backtrace -----
0xaaaae04a571f gdb_internal_backtrace_1
../../../repos/binutils-gdb/gdb/bt-utils.c:122
0xaaaae04a57f3 _Z22gdb_internal_backtracev
../../../repos/binutils-gdb/gdb/bt-utils.c:168
0xaaaae0b52ccf internal_vproblem
../../../repos/binutils-gdb/gdb/utils.c:401
0xaaaae0b5310b _Z15internal_verrorPKciS0_St9__va_list
../../../repos/binutils-gdb/gdb/utils.c:481
0xaaaae0e24b8f _Z18internal_error_locPKciS0_z
../../../repos/binutils-gdb/gdbsupport/errors.cc:58
0xaaaae0a88983 _Z15inferior_threadv
../../../repos/binutils-gdb/gdb/thread.c:86
0xaaaae0956c87 _Z20get_current_regcachev
../../../repos/binutils-gdb/gdb/regcache.c:428
0xaaaae035223f aarch64_remove_non_address_bits
../../../repos/binutils-gdb/gdb/aarch64-tdep.c:3572
0xaaaae03e8abb _Z31gdbarch_remove_non_address_bitsP7gdbarchm
../../../repos/binutils-gdb/gdb/gdbarch.c:3109
0xaaaae0a692d7 memory_xfer_partial
../../../repos/binutils-gdb/gdb/target.c:1620
0xaaaae0a695e3 _Z19target_xfer_partialP10target_ops13target_objectPKcPhPKhmmPm
../../../repos/binutils-gdb/gdb/target.c:1684
0xaaaae0a69e9f target_read_partial
../../../repos/binutils-gdb/gdb/target.c:1937
0xaaaae0a69fdf _Z11target_readP10target_ops13target_objectPKcPhml
../../../repos/binutils-gdb/gdb/target.c:1977
0xaaaae0a69937 _Z18target_read_memorymPhl
../../../repos/binutils-gdb/gdb/target.c:1773
0xaaaae08be523 ps_xfer_memory
../../../repos/binutils-gdb/gdb/proc-service.c:90
0xaaaae08be6db ps_pdread
../../../repos/binutils-gdb/gdb/proc-service.c:124
0x40001ed7c3b3 _td_fetch_value
/build/glibc-RIFKjK/glibc-2.31/nptl_db/fetch-value.c:115
0x40001ed791ef td_ta_map_lwp2thr
/build/glibc-RIFKjK/glibc-2.31/nptl_db/td_ta_map_lwp2thr.c:194
0xaaaae07f4473 thread_from_lwp
../../../repos/binutils-gdb/gdb/linux-thread-db.c:413
0xaaaae07f6d6f _ZN16thread_db_target4waitE6ptid_tP17target_waitstatus10enum_flagsI16target_wait_flagE
../../../repos/binutils-gdb/gdb/linux-thread-db.c:1420
0xaaaae0a6b33b _Z11target_wait6ptid_tP17target_waitstatus10enum_flagsI16target_wait_flagE
../../../repos/binutils-gdb/gdb/target.c:2586
0xaaaae0789cf7 do_target_wait_1
../../../repos/binutils-gdb/gdb/infrun.c:3825
0xaaaae0789e6f operator()
../../../repos/binutils-gdb/gdb/infrun.c:3884
0xaaaae078a167 do_target_wait
../../../repos/binutils-gdb/gdb/infrun.c:3903
0xaaaae078b0af _Z20fetch_inferior_eventv
../../../repos/binutils-gdb/gdb/infrun.c:4314
0xaaaae076652f _Z22inferior_event_handler19inferior_event_type
../../../repos/binutils-gdb/gdb/inf-loop.c:41
0xaaaae07dc68b handle_target_event
../../../repos/binutils-gdb/gdb/linux-nat.c:4206
0xaaaae0e25fbb handle_file_event
../../../repos/binutils-gdb/gdbsupport/event-loop.cc:573
0xaaaae0e264f3 gdb_wait_for_event
../../../repos/binutils-gdb/gdbsupport/event-loop.cc:694
0xaaaae0e24f9b _Z16gdb_do_one_eventi
../../../repos/binutils-gdb/gdbsupport/event-loop.cc:217
0xaaaae080f033 start_event_loop
../../../repos/binutils-gdb/gdb/main.c:411
0xaaaae080f1b7 captured_command_loop
../../../repos/binutils-gdb/gdb/main.c:475
0xaaaae0810b97 captured_main
../../../repos/binutils-gdb/gdb/main.c:1318
0xaaaae0810c1b _Z8gdb_mainP18captured_main_args
../../../repos/binutils-gdb/gdb/main.c:1337
0xaaaae0338453 main
../../../repos/binutils-gdb/gdb/gdb.c:32
---------------------
../../../repos/binutils-gdb/gdb/thread.c:86: internal-error: inferior_thread: Assertion `current_thread_ != nullptr' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n)
We also see failures across the testsuite if the tests get executed on a target
that has native support for the pointer authentication feature. But
gdb.base/break.exp and gdb.base/access-mem-running.exp are two examples of
tests that run into errors and internal errors.
This issue started after commit d88cb738e6a7a7179dfaff8af78d69250c852af1, which
enabled more broad use of pointer authentication masks to remove non-address
bits of pointers, but wasn't immediately detected because systems with native
support for pointer authentication are not that common yet.
The above crash happens because gdb is in the middle of handling an event,
and do_target_wait_1 calls switch_to_inferior_no_thread, nullifying the
current thread. This means a call to inferior_thread () will assert, and
attempting to call get_current_regcache () will also call inferior_thread (),
resulting in an assertion as well.
target_has_registers was one function that seemed useful for detecting these
types of situation where we don't have a register cache. The problem with that
is the inconsistent state of inferior_ptid, which is used by
target_has_registers.
Despite the call to switch_to_no_thread in switch_to_inferior_no_thread from
do_target_wait_1 in the backtrace above clearing inferior_ptid, the call to
ps_xfer_memory sets inferior_ptid momentarily before reading memory:
static ps_err_e
ps_xfer_memory (const struct ps_prochandle *ph, psaddr_t addr,
gdb_byte *buf, size_t len, int write)
{
scoped_restore_current_inferior restore_inferior;
set_current_inferior (ph->thread->inf);
scoped_restore_current_program_space restore_current_progspace;
set_current_program_space (ph->thread->inf->pspace);
scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
inferior_ptid = ph->thread->ptid;
CORE_ADDR core_addr = ps_addr_to_core_addr (addr);
int ret;
if (write)
ret = target_write_memory (core_addr, buf, len);
else
ret = target_read_memory (core_addr, buf, len);
return (ret == 0 ? PS_OK : PS_ERR);
}
Maybe this shouldn't happen, or maybe it is just an unfortunate state to be
in. But this prevents the use of target_has_registers to guard against the
lack of registers, since, although current_thread_ is still nullptr,
inferior_ptid is valid and is not null_ptid.
There is another crash scenario after we kill a previously active inferior, in
which case the gdbarch will still say we support pointer authentication but we
will also have no current thread (inferior_thread () will assert etc).
If the target has support for pointer authentication, gdb needs to use
a couple (or 4, for bare-metal) mask registers to mask off some bits of
pointers, and for that it needs to access the registers.
At some points, like the one from the backtrace above, there is no active
thread/current regcache because gdb is in the middle of doing event handling
and switching between threads.
Simon suggested the use of inferior_ptid to fetch the register cache, as
opposed to relying on the current register cache. Though we need to make sure
inferior_ptid is valid (not null_ptid), I think this works nicely.
With inferior_ptid, we can do safety checks along the way, making sure we have
a thread to fetch a register cache from and checking if the thread is actually
stopped or running.
The following patch implements this idea with safety checks to make sure we
don't run into assertions or errors. If any of the checks fail, we fallback to
using a default mask to remove non-address bits of a pointer.
I discussed with Pedro the possibility of caching the mask register values
(which are per-process and can change mid-execution), but there isn't a good
spot to cache those values. Besides, the mask registers can change constantly
for bare-metal debugging when switching between exception levels.
In some cases, it is just not possible to get access to these mask registers,
like the case where threads are running. In those cases, using a default mask
to remove the non-address bits should be enough.
This can happen when we let threads run in the background and then we attempt
to access a memory address (now that gdb is capable of reading memory even
with threads running). Thus gdb will attempt to remove non-address bits
of that memory access, will attempt to access registers, running into errors.
Regression-tested on aarch64-linux Ubuntu 20.04.
|
|
In proc mi_expect_stop there's a proc argument reason that's handled like so:
...
set r "reason=\"$reason\","
...
That's fine for say:
...
set reason "foo"
...
for which this evaluates to:
...
set r "reason=\"foo\","
...
But there are more complex uses, for instance:
...
set reason "breakpoint-hit\",disp=\"keep\",bkptno=\"$decimal"
...
which evaluates to:
...
set r "\"breakpoint-hit\",disp=\"keep\",bkptno=\"$decimal\""
...
Note how in this reason argument, the first two '\"' seems to form a pair
surrounding ',disp=', which is not the case, which is confusing.
Fix this by only adding the quotes in mi_expect_stop if the string doesn't
already contain quotes, such that we have the more readable:
...
set reason "\"breakpoint-hit\",disp=\"keep\",bkptno=\"$decimal\""
...
Tested on x86_64-linux.
|
|
In commit 722c4596034 ("[gdb/testsuite] Fix gdb.cp/*.exp for remote host"), I
needed to change ".*/" into "(.*/)?" in:
...
gdb_test "info variable everywhere" \
"File .*/m-static\[.\]h.*const int gnu_obj_4::everywhere;"
...
However, due to the fact that I got this output:
...
(gdb) info variable everywhere^M
All variables matching regular expression "everywhere":^M
^M
File /data/vries/gdb/src/gdb/testsuite/gdb.cp/m-static.h:^M
8: const int gnu_obj_4::everywhere;^M
^M
File /data/vries/gdb/src/gdb/testsuite/gdb.cp/m-static1.cc:^M
8: const int gnu_obj_4::everywhere;^M
...
I decided to make the matching somewhat stricter, to make sure that the two
matched lines were subsequent.
The commit turned out to be more strict than intended, and caused a regression
on Ubuntu 20.04, where the output was instead:
...
(gdb) info variable everywhere^M
All variables matching regular expression "everywhere":^M
^M
File /data/vries/gdb/src/gdb/testsuite/gdb.cp/m-static.h:^M
8: const int gnu_obj_4::everywhere;^M
...
At that point I realized I'm looking at a bug (filed as PR symtab/30265),
which manifests on openSUSE Leap 15.4 for native and readnow, and on Ubuntu
20.04 for readnow, but not for native.
Before my commit, the test-case passed whether the bug manifested or not.
After my commit, the test-case only passed when the bug manifested.
Fix the test-case regression by reverting to the situation before the commit:
pass whether the bug manifests or not. We could add an xfail for the PR, but
I'm expecting a fix soon, so that doesn't look worth the effort.
Tested on x86_64-linux, both on openSUSE Leap 15.4 and Ubuntu 20.04, both with
native and readnow.
Reported-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This input sequence is accepted by DAP:
...
{"seq": 4, "type": "request", "command": "configurationDone"}Content-Length: 84
...
This input sequence has the same effect:
...
{"seq": 4, "type": "request", "command": "configurationDone"}ignorethis
Content-Length: 84
...
but the 'ignorethis' part is silently ignored.
Log the ignored bit, such that we have:
...
READ: <<<{"seq": 4, "type": "request", "command": "configurationDone"}>>>
WROTE: <<<{"request_seq": 4, "type": "response", "command": "configurationDone"
, "success": true}>>>
+++ run
IGNORED: <<<b'ignorethis'>>>
...
|
|
I noticed a minor grammar problem in the 'GDB/MI Commands In Python'
node of the manual. I'm checking in this patch to correct it.
|
|
Simon reported that doing:
...
$ while make check-parallel TESTS='gdb.opencl/*.exp' -j 100; do true; done
...
could run into:
...
ERROR: remote_download to target of \
/data/vries/gdb/src/gdb/testsuite/lib/opencl_kernel.cl to opencl_kernel.cl: \
cp: cannot create regular file 'opencl_kernel.cl': File exists
...
Fix this by using gdb_remote_download (instead of plain remote_download) in
allow_opencl_test, which takes care of:
- downloading to a location which is safe for parallel testing, by
using standard_output_file, and
- cleaning up the downloaded file, meaning we can remove the corresponding
"remote_file target delete ${clprogram}" lines in allow_opencl_test.
Tested on x86_64-linux.
Reported-by: Simon Marchi <simon.marchi@efficios.com>
|
|
This commit is initial support for SystemTap for RISC-V Linux. The
following two tests exercise SystemTap functionality, and are showing
many failures, which are all fixed by this commit:
gdb.cp/exceptprint.exp
gdb.base/stap-probe.exp
One thing I wasn't sure about is if the SystemTap support should be
Linux specific, or architecture specific. For aarch64, arm, ia64, and
ppc, the SystemTap support seems to libe in the ARCH-linux-tdep.c
file, while for amd64, i386, and s390 the implementation lives in
ARCH-tdep.c. I have no idea which of these is the better choice -- or
maybe both choices are correct in the right circumstances, and I'm
just not aware of how to choose between them.
Anyway, for this patch I selected riscv-tdep.c (though clearly, moving
the changes to riscv-linux-tdep.c is trivial if anyone thinks that's a
more appropriate location).
The stap-probe.exp file tests immediate, register, and register
indirect operands, all of which appear to be working fine with this
commit. The generic expression support doesn't appear to be
architecture specific, so I'd expect that to work fine too.
|
|
The comment on the gdbarch_displaced_step_fixup gdbarch method
indicates that this method is optional and that GDB will perform some
default if this method is not supplied. As such we define a predicate
gdbarch_displaced_step_fixup_p.
It may have been true at one point that the fixup method was optional,
but it is no longer true. If this method is not defined and GDB tries
to complete a displaced step, then GDB is going to crash.
Additionally the gdbarch_displaced_step_fixup_p predicate is not used
anywhere in GDB.
In this commit I have removed the gdbarch_displaced_step_fixup_p
predicate, and I have updated the validation check for the
gdbarch_displaced_step_fixup method; if the
gdbarch_displaced_step_copy_insn method is defined then the fixup
method must also be defined.
I believe I've manually checked all the current places where
gdbarch_displaced_step_copy_insn is defined and they all also define
the fixup method, so this change should cause no problems for anyone.
There should be no user visible changes after this commit.
Approved-By: Pedro Alves <pedro@palves.net>
|
|
I found an upcast from template_symbol to symbol. This was necessary
long ago, but since symbols use inheritance now, it is not. This
patch removes it. Tested by rebuilding.
|
|
change
Commit 904d9b02a185 ("gdb: make "maintenance info line-table" show
relocated addresses again") changed the format of that command, but
failed to adjust some test cases that relied on it. This patch fixes
it.
The failures fixed are:
FAIL: gdb.base/maint.exp: maint info line-table w/o a file name
FAIL: gdb.dwarf2/dw2-out-of-range-end-of-seq.exp: END with address 1 eliminated
FAIL: gdb.dwarf2/dw2-ranges-base.exp: count END markers in line table
Change-Id: I946580d5e100f1beeac99a9e90d7819c6bb4ac6c
|
|
Fix test-case gdb.cp/cp-relocate.exp for remote host using
gdb_remote_download.
Tested on x86_64-linux.
|
|
When running test-cases gdb.cp/annota{2,3}.exp with target board
native-extended-gdbserver, we run into a few FAILs, due to the test-cases
trying to match inferior output together with gdb output.
Fix this by ignoring the inferior output in this case.
Tested on x86_64-linux.
|
|
Fix a few test-cases in gdb.cp/*.exp for remote host using new proc
include_file.
Tested on x86_64-linux.
|
|
Commit 1acc9dca423f ("Change linetables to be objfile-independent")
changed "maintenance info line-table" to print unrelocated addresses
instead of relocated. This breaks a few tests on systems where that
matters. The ones I see are:
Running /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.base/consecutive.exp ...
FAIL: gdb.base/consecutive.exp: stopped at bp, 2nd instr (missing hex prefix)
Running /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.base/async.exp ...
FAIL: gdb.base/async.exp: stepi&
FAIL: gdb.base/async.exp: nexti&
FAIL: gdb.base/async.exp: finish&
These tests run "maintenance info line-table" to record the address of
some lines, and then use these addresses in expected patterns. It
therefore expects these addresses to match the runtime addresses,
therefore the relocated addresses.
Add back the relocated addresses, next to the unrelocated addresses,
like so:
INDEX LINE REL-ADDRESS UNREL-ADDRESS IS-STMT PROLOGUE-END
0 6 0x0000555555555119 0x0000000000001119 Y
1 7 0x000055555555511d 0x000000000000111d Y
2 8 0x0000555555555123 0x0000000000001123 Y
3 END 0x0000555555555125 0x0000000000001125 Y
The unrelocated addresses can always be useful trying to map this
information with a DWARF info dump.
Adjust the is_stmt_addresses proc in the testsuite to match the new
output.
Change-Id: I59558f167e13e63421c9e0f2cad192e7c95c10cf
|
|
The recent commit:
commit 2a8339b71f37f2d02f5b2194929c9d702ef27223
Author: Carl Love <cel@us.ibm.com>
Date: Thu Mar 9 16:10:18 2023 -0500
PowerPC: fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp
PPC64 multiple entry points, a normal entry point and an alternate entry
point. The alternate entry point is to setup the Table of Contents (TOC)
register before continuing at the normal entry point. When the TOC is
already valid, the normal entry point is used, this is typically the case.
The alternate entry point is typically referred to as the global entry
point (GEP) in IBM. The normal entry point is typically referred to as
the local entry point (LEP).
.....
Is causing regression failures on on PowerPC platforms. The regression
failures are in tests:
gdb.reverse/finish-precsave.exp
gdb.btrace/tailcall.exp
gdb.mi/mi-reverse.exp
gdb.btrace/step.exp
gdb.reverse/until-precsave.exp
gdb.reverse/finish-reverse.exp
gdb.btrace/tailcall-only.exp
The issue is in gdb/infcmd.c, function finish_command. The value of the
two new variables ALT_ENTRY_POINT and ENTRY_POINT are being initializezed
to SAL.PC. However, SAL has just been declared. The value of SAL.PC is
zero at this point. The intialization of ALT_ENTRY_POINT and ENTRY_POINT
needs to be after the initialization of SAL.
This patch moves the initialization of ALT_ENTRY_POINT and ENTRY_POINT
variables to fix the regression failures.
The patch has been tested on Power10 and on X86.
|
|
Make sure the result of each remote_exec in gdb/testsuite/boards/*.exp is
checked.
Tested on x86_64-linux.
|
|
In a recent commit I forgot to add a double quote before chmod here:
...
remote_exec build $rsh_cmd chmod go-rx ."
...
Fix it by adding the missing double quote.
|
|
Looking at the implementation of ${board}_file in remote-stdio-gdbserver.exp,
I don't see a relevant difference with the implementation of standard_file
in dejagnu.
Simplify the board by removing ${board}_file.
Tested on x86_64-linux, by running gdb.testsuite/board-sanity.exp.
|
|
Some boards in gdb/testsuite/boards use the hardcoded ipv4 address "127.0.0.1".
Use instead "localhost".
Tested on x86_64-linux.
|
|
Fix test-case gdb.xml/tdesc-regs.exp for remote host by using appropriate
filenames.
Tested on x86_64-linux.
|
|
Fix test-case gdb.xml/tdesc-reload.exp for remote host by using appropriate
filenames.
Tested on x86_64-linux.
|
|
In commit ff581559f9d ("[gdb/testsuite] Add gdb.testsuite/board-sanity.exp") I
removed handling of HOST_DIR in local-remote-host-native.exp to fix FAILs
in test-case gdb.testsuite/board-sanity.exp.
Reintroduce handling of HOST_DIR using remotedir, now that using remotedir for
a host board no longer make compilation fail due to commit 80d6c79866f
("[gdb/testsuite] Handle remotedir in remote_upload").
This fixes an XFAIL in gdb.testsuite/board-sanity.exp, introduced in commit
3741934fdb0 ("[gdb/testsuite] Set remotedir by default in some boards").
Tested on x86_64-linux.
|
|
Since the tdesc_powerpc_vsx32, tdesc_powerpc_vsx64, tdesc_powerpc_altivec32 and tdesc_powerpc_altivec64
definitions are moved to ppc-tdep.h we no longer need to import these .c files.
|
|
I noticed a weird comment in one of the .y files, and then ended up
removing some unnecessary #includes from these files.
Tested by rebuilding.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
The mi_version function is unused, and I think it's better overall if
it is never used. This patch removes it. Tested by rebuilding.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
The type allocation changes introduced a failure in python-helper.exp
that I did not notice. The bug is that, with these patches,
arch-allocated integer types have a TYPE_SPECIFIC_INT object attached.
This patch updates the test to allow this.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30253
|
|
Dejagnu's remotedir implementation has support in remote_exec and
remote_download, but not remote_upload.
Consider the following scenario:
- downloading an executable to target,
- running it,
- uploading a file produced by the executable
while assuming remote target user remote-target with homedir
/home/remote-target and remotedir set to /home/remote-target/tmp.
Concretely, it looks like this:
...
# binfile == "$outputs/gdb.abc/a.out"
set target_binfile [remote_download target $binfile]
# target_binfile == "/home/remote-target/tmp/a.out"
remote_exec target $target_binfile
# Running $target_binfile produced /home/remote-target/tmp/result.txt.
set result [remote_upload target /home/remote-target/tmp/result.txt \
$outputs/gdb.abc/result.txt]
# result == $outputs/gdb.abc/result.txt.
...
Add a remote_upload implementation that also handles remotedir in lib/gdb.exp,
overriding dejagnu's remote_upload, such that we can simplify the
remote_upload call to:
...
set result [remote_upload target result.txt $outputs/gdb.abc/result.txt]
...
Tested on x86_64-linux.
PR testsuite/30250
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30250
|
|
In some cases GDB will fail when attempting to complete a command that
involves a rust symbol, the failure can manifest as a crash.
The problem is caused by the completion_match_for_lcd object being
left containing invalid data during calls to cp_symbol_name_matches_1.
The first question to address is why we are calling a C++ support
function when handling a rust symbol. That's due to GDB's auto
language detection for msymbols, in some cases GDB can't tell if a
symbol is a rust symbol, or a C++ symbol.
The test application contains symbols for functions which are
statically linked in from various rust support libraries. There's no
DWARF for these symbols, so all GDB has is the msymbols built from the
ELF symbol table.
Here's the problematic symbol that leads to our crash:
mangled: _ZN4core3str21_$LT$impl$u20$str$GT$5parse17h5111d2d6a50d22bdE
demangled: core::str::<impl str>::parse
As an msymbol this is initially created with language auto, then GDB
eventually calls symbol_find_demangled_name, which loops over all
languages calling language_defn::sniff_from_mangled_name, the first
language that can demangle the symbol gets assigned as the language
for that symbol.
Unfortunately, there's overlap in the mangled symbol names,
some (legacy) rust symbols can be demangled as both rust and C++, see
cplus_demangle in libiberty/cplus-dem.c where this is mentioned.
And so, because we check the C++ language before we check for rust,
then the msymbol is (incorrectly) given the C++ language.
Now it's true that is some cases we might be able to figure out that a
demangled symbol is not actually a valid C++ symbol, for example, in
our case, the construct '::<impl str>::' is not, I believe, valid in a
C++ symbol, we could look for ':<' and '>:' and refuse to accept this
as a C++ symbol.
However, I'm not sure it is always possible to tell that a demangled
symbol is rust or C++, so, I think, we have to accept that some times
we will get this language detection wrong.
If we accept that we can't fix the symbol language detection 100% of
the time, then we should make sure that GDB doesn't crash when it gets
the language wrong, that is what this commit addresses.
In our test case the user tries to complete a symbol name like this:
(gdb) complete break pars
This results in GDB trying to find all symbols that match 'pars',
eventually we consider our problematic symbol, and we end up with a
call stack that looks like this:
#0 0x0000000000f3c6bd in strncmp_iw_with_mode
#1 0x0000000000706d8d in cp_symbol_name_matches_1
#2 0x0000000000706fa4 in cp_symbol_name_matches
#3 0x0000000000df3c45 in compare_symbol_name
#4 0x0000000000df3c91 in completion_list_add_name
#5 0x0000000000df3f1d in completion_list_add_msymbol
#6 0x0000000000df4c94 in default_collect_symbol_completion_matches_break_on
#7 0x0000000000658c08 in language_defn::collect_symbol_completion_matches
#8 0x0000000000df54c9 in collect_symbol_completion_matches
#9 0x00000000009d98fb in linespec_complete_function
#10 0x00000000009d99f0 in complete_linespec_component
#11 0x00000000009da200 in linespec_complete
#12 0x00000000006e4132 in complete_address_and_linespec_locations
#13 0x00000000006e4ac3 in location_completer
In cp_symbol_name_matches_1 we enter a loop, this loop repeatedly
tries to match the demangled problematic symbol name against the user
supplied text ('pars'). Each time around the loop another component
of the symbol name is stripped off, thus, we check 'pars' against
these options:
core::str::<impl str>::parse
str::<impl str>::parse
<impl str>::parse
parse
As soon as we get a match the cp_symbol_name_matches_1 exits its loop
and returns. In our case, when we're looking for 'pars', the match
occurs on the last iteration of the loop, when we are comparing to
'parse'.
Now the problem here is that cp_symbol_name_matches_1 uses the
strncmp_iw_with_mode, and inside strncmp_iw_with_mode we allow for
skipping over template parameters. This allows GDB to match the
symbol name 'foo<int>(int,int)' if the user supplies 'foo(int,'.
Inside strncmp_iw_with_mode GDB will record any template arguments
that it has skipped over inside the completion_match_for_lcd object
that is passed in as an argument.
And so, when GDB tries to match against '<impl str>::parse', the first
thing it sees is '<impl str>', GDB assumes this is a template argument
and records this as a skipped region within the
completion_match_for_lcd object. After '<impl str>' GDB sees a ':'
character, which doesn't match with the 'pars' the user supplied, so
strncmp_iw_with_mode returns a value indicating a non-match. GDB then
removes the '<impl str>' component from the symbol name and tries
again, this time comparing to 'parse', which does match.
Having found a match, then in cp_symbol_name_matches_1 we record the
match string, and the full symbol name within the
completion_match_result object, and return.
The problem here is that the skipped region, the '<impl str>' that we
recorded in the penultimate loop iteration was never discarded, its
still there in our returned result.
If we look at what the pointers held in the completion_match_result
that cp_symbol_name_matches_1 returns, this is what we see:
core::str::<impl str>::parse
| \________/ |
| | '--- completion match string
| '---skip range
'--- full symbol name
When GDB calls completion_match_for_lcd::finish, GDB tries to create a
string using the completion match string (parse), but excluding the
skip range, as the stored skip range is before the start of the
completion match string, then GDB tries to do some weird string
creation, which will cause GDB to crash.
The reason we don't often see this problem in C++ is that for C++
symbols there is always some non-template text before the template
argument. This non-template text means GDB is likely to either match
the symbol, or reject the symbol without storing a skip range.
However, notice, I did say, we don't often see this problem. Once I
understood the issue, I was able to reproduce the crash using a pure
C++ example:
template<typename S>
struct foo
{
template<typename T>
foo (int p1, T a)
{
s = 0;
}
S s;
};
int
main ()
{
foo<int> obj (2.3, 0);
return 0;
}
Then in GDB:
(gdb) complete break foo(int
The problem here is that the C++ symbol for the constructor looks like
this:
foo<int>::foo<double>(int, double)
When GDB enters cp_symbol_name_matches_1 the symbols it examines are:
foo<int>::foo<double>(int, double)
foo<double>(int, double)
The first iteration of the loop will match the 'foo', then add the
'<int>' template argument will be added as a skip range. When GDB
find the ':' after the '<int>' the first iteration of the loop fails
to match, GDB removes the 'foo<int>::' component, and starts the
second iteration of the loop.
Again, GDB matches the 'foo', and now adds '<double>' as a skip
region. After that the '(int' successfully matches, and so the second
iteration of the loop succeeds, but, once again we left the '<int>' in
place as a skip region, even though this occurs before the start of
our match string, and this will cause GDB to crash.
This problem was reported to the mailing list, and a solution
discussed in this thread:
https://sourceware.org/pipermail/gdb-patches/2023-January/195166.html
The solution proposed here is similar to one proposed by the original
bug reported, but implemented in a different location within GDB.
Instead of placing the fix in strncmp_iw_with_mode, I place the fix in
cp_symbol_name_matches_1. I believe this is a better location as it
is this function that implements the loop, and it is this loop, which
repeatedly calls strncmp_iw_with_mode, that should be resetting the
result object state (I believe).
What I have done is add an assert to strncmp_iw_with_mode that the
incoming result object is empty.
I've also added some other asserts in related code, in
completion_match_for_lcd::mark_ignored_range, I make some basic
assertions about the incoming range pointers, and in
completion_match_for_lcd::finish I also make some assertions about how
the skip ranges relate to the match pointer.
There's two new tests. The original rust example that was used in the
initial bug report, and a C++ test. The rust example depends on which
symbols are pulled in from the rust libraries, so it is possible that,
at some future date, the problematic symbol will disappear from this
test program. The C++ test should be more reliable, as this only
depends on symbols from within the C++ source code.
Since I originally posted this patch to the mailing list, the
following patch has been merged:
commit 6e7eef72164c00d6a5a7b0bce9fa01f5481f33cb
Date: Sun Mar 19 09:13:10 2023 -0600
Use rust_demangle to fix a crash
This solves the problem of a rust symbol ending up in the C++ specific
code by changing the order languages are sorted. However, this new
commit doesn't address the issue in the C++ code which was fixed with
this commit.
Given that the C++ issue is real, and has a reproducer, I'm still
going to merge this fix. I've left the discussion of rust in this
commit message as I originally wrote it, but it should be read within
the context of GDB prior to commit 6e7eef72164c00d6a5a7.
Co-Authored-By: Zheng Zhan <zzlossdev@163.com>
|
|
PR rust/30211 points out a crash caused by a particular completion.
This turns out to happen because a Rust minsym winds up in a
C++-specific path in strncmp_iw_with_mode, which ultimately causes the
completer to pass invalid arguments to string::append.
This patch fixes the bug by reordering the language constants so that
Rust comes before C++, and then using rust_demangle. This ensures
that minsyms are correctly marked as "Rust", avoiding this code and
thus the crash.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=20367
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30211
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
|
|
I noticed that ui_out::do_progress_end is public, just to support one
use in debuginfod-support.c. This patch makes it private, updates
progress_info to call it from its destructor, and finally changes
debuginfod-support.c to follow.
|
|
I noticed that breakpoint::print_recreate_thread was printing the
global thread-id. This function is used to implement the 'save
breakpoints' command, and should be writing out suitable CLI commands
for recreating the current breakpoints. The CLI does not use global
thread-ids, but instead uses the inferior specific thread-ids,
e.g. "2.1".
After some discussion on the mailing list it was suggested that the
most consistent solution would be for the saved breakpoints file to
always contain the inferior-qualified thread-id, so the file would
include "thread 1.1" instead of just "thread 1", even when there is
only a single inferior.
So, this commit adds print_full_thread_id, which is just like the
existing print_thread_id, only it always prints the inferior-qualified
thread-id.
I then update the existing print_thread_id to make use of this new
function, and finally, I update breakpoint::print_recreate_thread to
also use this new function.
There's a multi-inferior test that confirms the saved breakpoints file
correctly includes the fully-qualified thread-id, and I've also
updated the single inferior test gdb.base/save-bp.exp to have it
validate that the saved breakpoints file includes the
inferior-qualified thread-id, even for this single inferior case.
|
|
psymbol_functions::fill_psymbol_map was removed, but I forgot to
remove the declaration. This patch removes it. Tested by rebuilding.
|
|
This removes objfile_type, in favor of always using the per-arch
builtins.
Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This adds some types to struct builtin_type, ensuring it contains all
the types currently used by objfile_type.
Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This renames objfile_type to be an overload of builtin_type, in
preparation for their unification.
Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
|
|
There are a few spots that check whether a type is objfile-owned, and
then choose either the objfile- or arch-specific builtin type. I
don't think there is a need to do this any more (if there ever was),
because it is ok for an objfile-allocated type to refer to an
arch-allocated type.
Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes the set type creation function to accept a type
allocator, and updates all the callers. Note that symbol readers
should generally allocate on the relevant objfile, regardless of the
underlying type of the set, which is what this patch implements.
Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes the array type creation functions to accept a type
allocator, and updates all the callers. Note that symbol readers
should generally allocate on the relevant objfile, regardless of the
placement of the index type of the array, which is what this patch
implements.
Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes the range type creation functions to accept a type
allocator, and updates all the callers. Note that symbol readers
should generally allocate on the relevant objfile, regardless of the
underlying type of the range, which is what this patch implements.
Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This unifies arch_pointer_type and init_pointer_type by using a type
allocator.
Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This unifies arch_decfloat_type and init_decfloat_type by using a type
allocator.
Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This unifies arch_float_type and init_float_type by using a type
allocator.
Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This unifies arch_boolean_type and init_boolean_type by using a type
allocator.
Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This unifies arch_character_type and init_character_type by using a
type allocator.
Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This unifies arch_integer_type and init_integer_type by using a type
allocator.
Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This removes init_type, replacing all uses with the new type
allocator.
Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
|
