| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Move the implementation over to target_desc_info. Remove the
target_desc_info forward declaration in target-descriptions.h, it's no
longer needed.
Change-Id: Ic95060341685afe0b73af591ca6efe32f5e7e892
|
|
This function is now trivial, we can just copy inferior::tdesc_info
where needed.
Change-Id: I25185e2cd4ba1ef24a822d9e0eebec6e611d54d6
|
|
Remove this function, since it's now a trivial access to
inferior::tdesc_info.
Change-Id: I3e88a8214034f1a4163420b434be11f51eef462c
|
|
I initially made this field a unique pointer, to have automatic memory
management. But I then thought that the field didn't really need to be
allocated separately from struct inferior. So make it a regular
non-pointer field of inferior.
Remove target_desc_info_free, as it's no longer needed.
Change-Id: Ica2b97071226f31c40e86222a2f6922454df1229
|
|
In preparation for the following patch, where struct inferior needs to
"see" struct target_desc_info, move target_desc_info to the header file.
I initially moved the structure to target-descriptions.h, and later made
inferior.h include target-descriptions.h. This worked, but it then
occured to me that target_desc_info is really an inferior property that
involves a target description, so I think it makes sense to have it in
inferior.h.
Change-Id: I3e81d04faafcad431e294357389f3d4c601ee83d
|
|
Since allocate_target_description returns a target_desc_up, use
assignment to initialize the description variable.
Change-Id: Iab3311642c09b95648984f305936f4a4cde09440
|
|
This patch adds the foundation for GDB to be able to debug programs
offloaded to AMD GPUs using the AMD ROCm platform [1]. The latest
public release of the ROCm release at the time of writing is 5.4, so
this is what this patch targets.
The ROCm platform allows host programs to schedule bits of code for
execution on GPUs or similar accelerators. The programs running on GPUs
are typically referred to as `kernels` (not related to operating system
kernels).
Programs offloaded with the AMD ROCm platform can be written in the HIP
language [2], OpenCL and OpenMP, but we're going to focus on HIP here.
The HIP language consists of a C++ Runtime API and kernel language.
Here's an example of a very simple HIP program:
#include "hip/hip_runtime.h"
#include <cassert>
__global__ void
do_an_addition (int a, int b, int *out)
{
*out = a + b;
}
int
main ()
{
int *result_ptr, result;
/* Allocate memory for the device to write the result to. */
hipError_t error = hipMalloc (&result_ptr, sizeof (int));
assert (error == hipSuccess);
/* Run `do_an_addition` on one workgroup containing one work item. */
do_an_addition<<<dim3(1), dim3(1), 0, 0>>> (1, 2, result_ptr);
/* Copy result from device to host. Note that this acts as a synchronization
point, waiting for the kernel dispatch to complete. */
error = hipMemcpyDtoH (&result, result_ptr, sizeof (int));
assert (error == hipSuccess);
printf ("result is %d\n", result);
assert (result == 3);
return 0;
}
This program can be compiled with:
$ hipcc simple.cpp -g -O0 -o simple
... where `hipcc` is the HIP compiler, shipped with ROCm releases. This
generates an ELF binary for the host architecture, containing another
ELF binary with the device code. The ELF for the device can be
inspected with:
$ roc-obj-ls simple
1 host-x86_64-unknown-linux file://simple#offset=8192&size=0
1 hipv4-amdgcn-amd-amdhsa--gfx906 file://simple#offset=8192&size=34216
$ roc-obj-extract 'file://simple#offset=8192&size=34216'
$ file simple-offset8192-size34216.co
simple-offset8192-size34216.co: ELF 64-bit LSB shared object, *unknown arch 0xe0* version 1, dynamically linked, with debug_info, not stripped
^
amcgcn architecture that my `file` doesn't know about ----´
Running the program gives the very unimpressive result:
$ ./simple
result is 3
While running, this host program has copied the device program into the
GPU's memory and spawned an execution thread on it. The goal of this
GDB port is to let the user debug host threads and these GPU threads
simultaneously. Here's a sample session using a GDB with this patch
applied:
$ ./gdb -q -nx --data-directory=data-directory ./simple
Reading symbols from ./simple...
(gdb) break do_an_addition
Function "do_an_addition" not defined.
Make breakpoint pending on future shared library load? (y or [n]) y
Breakpoint 1 (do_an_addition) pending.
(gdb) r
Starting program: /home/smarchi/build/binutils-gdb-amdgpu/gdb/simple
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[New Thread 0x7ffff5db7640 (LWP 1082911)]
[New Thread 0x7ffef53ff640 (LWP 1082913)]
[Thread 0x7ffef53ff640 (LWP 1082913) exited]
[New Thread 0x7ffdecb53640 (LWP 1083185)]
[New Thread 0x7ffff54bf640 (LWP 1083186)]
[Thread 0x7ffdecb53640 (LWP 1083185) exited]
[Switching to AMDGPU Wave 2:2:1:1 (0,0,0)/0]
Thread 6 hit Breakpoint 1, do_an_addition (a=<error reading variable: DWARF-2 expression error: `DW_OP_regx' operations must be used either alone or in conjunction with DW_OP_piece or DW_OP_bit_piece.>,
b=<error reading variable: DWARF-2 expression error: `DW_OP_regx' operations must be used either alone or in conjunction with DW_OP_piece or DW_OP_bit_piece.>,
out=<error reading variable: DWARF-2 expression error: `DW_OP_regx' operations must be used either alone or in conjunction with DW_OP_piece or DW_OP_bit_piece.>) at simple.cpp:24
24 *out = a + b;
(gdb) info inferiors
Num Description Connection Executable
* 1 process 1082907 1 (native) /home/smarchi/build/binutils-gdb-amdgpu/gdb/simple
(gdb) info threads
Id Target Id Frame
1 Thread 0x7ffff5dc9240 (LWP 1082907) "simple" 0x00007ffff5e9410b in ?? () from /opt/rocm-5.4.0/lib/libhsa-runtime64.so.1
2 Thread 0x7ffff5db7640 (LWP 1082911) "simple" __GI___ioctl (fd=3, request=3222817548) at ../sysdeps/unix/sysv/linux/ioctl.c:36
5 Thread 0x7ffff54bf640 (LWP 1083186) "simple" __GI___ioctl (fd=3, request=3222817548) at ../sysdeps/unix/sysv/linux/ioctl.c:36
* 6 AMDGPU Wave 2:2:1:1 (0,0,0)/0 do_an_addition (
a=<error reading variable: DWARF-2 expression error: `DW_OP_regx' operations must be used either alone or in conjunction with DW_OP_piece or DW_OP_bit_piece.>,
b=<error reading variable: DWARF-2 expression error: `DW_OP_regx' operations must be used either alone or in conjunction with DW_OP_piece or DW_OP_bit_piece.>,
out=<error reading variable: DWARF-2 expression error: `DW_OP_regx' operations must be used either alone or in conjunction with DW_OP_piece or DW_OP_bit_piece.>) at simple.cpp:24
(gdb) bt
Python Exception <class 'gdb.error'>: Unhandled dwarf expression opcode 0xe1
#0 do_an_addition (a=<error reading variable: DWARF-2 expression error: `DW_OP_regx' operations must be used either alone or in conjunction with DW_OP_piece or DW_OP_bit_piece.>,
b=<error reading variable: DWARF-2 expression error: `DW_OP_regx' operations must be used either alone or in conjunction with DW_OP_piece or DW_OP_bit_piece.>,
out=<error reading variable: DWARF-2 expression error: `DW_OP_regx' operations must be used either alone or in conjunction with DW_OP_piece or DW_OP_bit_piece.>) at simple.cpp:24
(gdb) continue
Continuing.
result is 3
warning: Temporarily disabling breakpoints for unloaded shared library "file:///home/smarchi/build/binutils-gdb-amdgpu/gdb/simple#offset=8192&size=67208"
[Thread 0x7ffff54bf640 (LWP 1083186) exited]
[Thread 0x7ffff5db7640 (LWP 1082911) exited]
[Inferior 1 (process 1082907) exited normally]
One thing to notice is the host and GPU threads appearing under
the same inferior. This is a design goal for us, as programmers tend to
think of the threads running on the GPU as part of the same program as
the host threads, so showing them in the same inferior in GDB seems
natural. Also, the host and GPU threads share a global memory space,
which fits the inferior model.
Another thing to notice is the error messages when trying to read
variables or printing a backtrace. This is expected for the moment,
since the AMD GPU compiler produces some DWARF that uses some
non-standard extensions:
https://llvm.org/docs/AMDGPUDwarfExtensionsForHeterogeneousDebugging.html
There were already some patches posted by Zoran Zaric earlier to make
GDB support these extensions:
https://inbox.sourceware.org/gdb-patches/20211105113849.118800-1-zoran.zaric@amd.com/
We think it's better to get the basic support for AMD GPU in first,
which will then give a better justification for GDB to support these
extensions.
GPU threads are named `AMDGPU Wave`: a wave is essentially a hardware
thread using the SIMT (single-instruction, multiple-threads) [3]
execution model.
GDB uses the amd-dbgapi library [4], included in the ROCm platform, for
a few things related to AMD GPU threads debugging. Different components
talk to the library, as show on the following diagram:
+---------------------------+ +-------------+ +------------------+
| GDB | amd-dbgapi target | <-> | AMD | | Linux kernel |
| +-------------------+ | Debugger | +--------+ |
| | amdgcn gdbarch | <-> | API | <=> | AMDGPU | |
| +-------------------+ | | | driver | |
| | solib-rocm | <-> | (dbgapi.so) | +--------+---------+
+---------------------------+ +-------------+
- The amd-dbgapi target is a target_ops implementation used to control
execution of GPU threads. While the debugging of host threads works
by using the ptrace / wait Linux kernel interface (as usual), control
of GPU threads is done through a special interface (dubbed `kfd`)
exposed by the `amdgpu` Linux kernel module. GDB doesn't interact
directly with `kfd`, but instead goes through the amd-dbgapi library
(AMD Debugger API on the diagram).
Since it provides execution control, the amd-dbgapi target should
normally be a process_stratum_target, not just a target_ops. More
on that later.
- The amdgcn gdbarch (describing the hardware architecture of the GPU
execution units) offloads some requests to the amd-dbgapi library,
so that knowledge about the various architectures doesn't need to be
duplicated and baked in GDB. This is for example for things like
the list of registers.
- The solib-rocm component is an solib provider that fetches the list of
code objects loaded on the device from the amd-dbgapi library, and
makes GDB read their symbols. This is very similar to other solib
providers that handle shared libraries, except that here the shared
libraries are the pieces of code loaded on the device.
Given that Linux host threads are managed by the linux-nat target, and
the GPU threads are managed by the amd-dbgapi target, having all threads
appear in the same inferior requires the two targets to be in that
inferior's target stack. However, there can only be one
process_stratum_target in a given target stack, since there can be only
one target per slot. To achieve it, we therefore resort the hack^W
solution of placing the amd-dbgapi target in the arch_stratum slot of
the target stack, on top of the linux-nat target. Doing so allows the
amd-dbgapi target to intercept target calls and handle them if they
concern GPU threads, and offload to beneath otherwise. See
amd_dbgapi_target::fetch_registers for a simple example:
void
amd_dbgapi_target::fetch_registers (struct regcache *regcache, int regno)
{
if (!ptid_is_gpu (regcache->ptid ()))
{
beneath ()->fetch_registers (regcache, regno);
return;
}
// handle it
}
ptids of GPU threads are crafted with the following pattern:
(pid, 1, wave id)
Where pid is the inferior's pid and "wave id" is the wave handle handed
to us by the amd-dbgapi library (in practice, a monotonically
incrementing integer). The idea is that on Linux systems, the
combination (pid != 1, lwp == 1) is not possible. lwp == 1 would always
belong to the init process, which would also have pid == 1 (and it's
improbable for the init process to offload work to the GPU and much less
for the user to debug it). We can therefore differentiate GPU and
non-GPU ptids this way. See ptid_is_gpu for more details.
Note that we believe that this scheme could break down in the context of
containers, where the initial process executed in a container has pid 1
(in its own pid namespace). For instance, if you were to execute a ROCm
program in a container, then spawn a GDB in that container and attach to
the process, it will likely not work. This is a known limitation. A
workaround for this is to have a dummy process (like a shell) fork and
execute the program of interest.
The amd-dbgapi target watches native inferiors, and "attaches" to them
using amd_dbgapi_process_attach, which gives it a notifier fd that is
registered in the event loop (see enable_amd_dbgapi). Note that this
isn't the same "attach" as in PTRACE_ATTACH, but being ptrace-attached
is a precondition for amd_dbgapi_process_attach to work. When the
debugged process enables the ROCm runtime, the amd-dbgapi target gets
notified through that fd, and pushes itself on the target stack of the
inferior. The amd-dbgapi target is then able to intercept target_ops
calls. If the debugged process disables the ROCm runtime, the
amd-dbgapi target unpushes itself from the target stack.
This way, the amd-dbgapi target's footprint stays minimal when debugging
a process that doesn't use the AMD ROCm platform, it does not intercept
target calls.
The amd-dbgapi library is found using pkg-config. Since enabling
support for the amdgpu architecture (amdgpu-tdep.c) depends on the
amd-dbgapi library being present, we have the following logic for
the interaction with --target and --enable-targets:
- if the user explicitly asks for amdgcn support with
--target=amdgcn-*-* or --enable-targets=amdgcn-*-*, we probe for
the amd-dbgapi and fail if not found
- if the user uses --enable-targets=all, we probe for amd-dbgapi,
enable amdgcn support if found, disable amdgcn support if not found
- if the user uses --enable-targets=all and --with-amd-dbgapi=yes,
we probe for amd-dbgapi, enable amdgcn if found and fail if not found
- if the user uses --enable-targets=all and --with-amd-dbgapi=no,
we do not probe for amd-dbgapi, disable amdgcn support
- otherwise, amd-dbgapi is not probed for and support for amdgcn is not
enabled
Finally, a simple test is included. It only tests hitting a breakpoint
in device code and resuming execution, pretty much like the example
shown above.
[1] https://docs.amd.com/category/ROCm_v5.4
[2] https://docs.amd.com/bundle/HIP-Programming-Guide-v5.4
[3] https://en.wikipedia.org/wiki/Single_instruction,_multiple_threads
[4] https://docs.amd.com/bundle/ROCDebugger-API-Guide-v5.4
Change-Id: I591edca98b8927b1e49e4b0abe4e304765fed9ee
Co-Authored-By: Zoran Zaric <zoran.zaric@amd.com>
Co-Authored-By: Laurent Morichetti <laurent.morichetti@amd.com>
Co-Authored-By: Tony Tye <Tony.Tye@amd.com>
Co-Authored-By: Lancelot SIX <lancelot.six@amd.com>
Co-Authored-By: Pedro Alves <pedro@palves.net>
|
|
In the ROCm port, we need to access the underlying stream of a
gdb_printing_disassembler, so make it public. The reason we need to
access it is to know whether it supports style escape code. We then
pass that information to a temporary string_file we use while
symbolizing addresses.
Change-Id: Ib95755a4a45b8f6478787993e9f904df60dd8dc1
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
In ROCm-GDB, we install an solib provider for the GPU code objects on
top of the svr4 provider for the host, in order to add solibs
representing the GPU code objects to the solib list containing the host
process' shared libraries. We override the target_so_ops::handle_event
function pointer with our own, in which we call svr4_so_ops.handle_event
(which contains svr4_handle_solib_event) manually. When the host
(un)loads a library, the ROCm part of handle_event is a no-op. When the
GPU (un)loads a code object, we want the host side (svr4) to be a no-op.
The problem is that when handle_event is called because of a GPU event,
svr4_handle_solib_event gets called while not stopped at an svr4
probe. It then assumes this means there's a problem with the probes
interface and disables it through the following sequence of events:
- solib_event_probe_at return nullptr
- svr4_handle_solib_event returns early
- the make_scope_exit callback calls disable_probes_interface
We could fix that by making the ROCm handle_event callback check if an
svr4 probe is that the stop address, and only call
svr4_so_ops.handle_event if so. However, it doesn't feel right to
include some svr4 implementation detail in the ROCm event handler.
Instead, this patch changes svr4_handle_solib_event to not assume it is
an error if called while not at an svr4 probe location, and therefore
not disable the probes interface. That just means moving the
make_scope_exit call below where we lookup the probe by pc.
Change-Id: Ie8ddf5beffa2e92b8ebfdd016454546252519244
Co-Authored-By: Lancelot SIX <lancelot.six@amd.com>
|
|
Add a gdbarch_up unique pointer type, that calls gdbarch_free on
deletion. This is used in the ROCm support patch at the end of this
series.
Change-Id: I4b808892d35d69a590ce83180f41afd91705b2c8
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Add an observable notified in target_detach just before calling the
detach method on the inferior's target stack. This allows observer to
do some work on the inferior while it's still ptrace-attached, in the
case of a native Linux inferior. Specifically, the amd-dbgapi target
will need it in order to call amd_dbgapi_process_detach before the
process gets ptrace-detached.
Change-Id: I28b6065e251012a4c2db8a600fe13ba31671e3c9
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
A following patch will want to install a breakpoint and then keep a
non-owning reference to it. Make install_breakpoint return a non-owning
reference, to make that easy.
Co-Authored-By: Simon Marchi <simon.marchi@efficios.com>
Change-Id: I2e8106a784021ff276ce251e24708cbdccc2d479
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
In the ROCm GDB port, there are some amdgcn architectures known by BFD
that we don't actually support in GDB. We don't want
gdbarch_printable_names to return these architectures.
gdbarch_printable_names is used for a few things:
- completion of the "set architecture" command
- the gdb.architecture_names function in Python
- foreach-arch selftests
Add an optional callback to gdbarch_register that is a predicate
indicating whether the gdbarch supports the given bfd_arch_info. by
default, it is nullptr, meaning that the gdbarch accepts all "mach"s for
that architecture known by BFD.
Change-Id: I712f94351b0b34ed1f42e5cf7fc7ba051315d860
Co-Authored-By: Simon Marchi <simon.marchi@efficios.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
GDB has been converted to a C++ program for many years[1], and the
gdb_indent.sh will not be used any more. Therefore, remove the script as
obvious.
[1] https://sourceware.org/gdb/wiki/cxx-conversion
Approved-By: Simon Marchi <simark@simark.ca>
|
|
I noticed that the gdb.debuginfod/fetch_src_and_symbols.exp script
doesn't work with the native-gdbserver board, I see this error:
ERROR: tcl error sourcing /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp.
ERROR: gdbserver does not support run without extended-remote
while executing
"error "gdbserver does not support $command without extended-remote""
(procedure "gdb_test_multiple" line 51)
invoked from within
This was introduced with this commit:
commit 7dd38e31d67c2548b52bea313ab18e40824c05da
Date: Fri Jan 6 18:45:27 2023 -0500
gdb/linespec.c: Fix missing source file during breakpoint re-set
The problem is that the above commit introduces a direct use of the
"run" command, which doesn't work with 'target remote' targets, as
exercised by the native-gdbserver board.
To avoid this, in this commit I switch to using runto_main. However,
calling runto_main will, by default, delete all the currently set
breakpoints. As the point of the above commit was to check that a
breakpoint set before stating an inferior would be correctly re-set,
we need to avoid this breakpoint deleting behaviour.
To do this I make use of with_override, and override the
delete_breakpoints proc with a dummy proc which does nothing.
By reverting the GDB changes in commit 7dd38e31d67c I have confirmed
that even after my changes in this commit, the test still fails. But
with the fixes in commit 7dd38e31d67c, this test now passed using the
unix, native-gdbserver, and native-extended-gdbserver boards.
|
|
Currently, when GDB loads debug information from a separate debug
file, there are a couple of warnings that could be produced if things
go wrong.
In find_separate_debug_file_by_buildid (build-id.c) GDB can give a
warning if the separate debug file doesn't include any actual debug
information, and in separate_debug_file_exists (symfile.c) we can warn
if the CRC checksum in the separate debug file doesn't match the
checksum in the original executable.
The problem here is that, when looking up debug information, GDB will
try several different approaches, lookup by build-id, lookup by
debug-link, and then a lookup from debuginfod. GDB can potentially
give a warning from an earlier attempt, and then succeed with a later
attempt. In the cases I have run into this is primarily a warning
about some out of date debug information on my machine, but then GDB
finds the correct information using debuginfod. This can be confusing
to a user, they will see warnings from GDB when really everything is
working just fine.
For example:
warning: the debug information found in "/usr/lib/debug//lib64/ld-2.32.so.debug" \
does not match "/lib64/ld-linux-x86-64.so.2" (CRC mismatch).
This diagnostic was printed on Fedora 33 even when the correct
debuginfo was downloaded.
In this patch I propose that we defer any warnings related to looking
up debug information from a separate debug file. If any of the
approaches are successful then GDB will not print any of the warnings.
As far as the user is concerned, everything "just worked". Only if
GDB completely fails to find any suitable debug information will the
warnings be printed.
The crc_mismatch test compiles two executables: crc_mismatch and
crc_mismatch-2 and then strips them of debuginfo creating separate
debug files. The test then replaces crc_mismatch-2.debug with
crc_mismatch.debug to trigger "CRC mismatch" warning. A local
debuginfod server is setup to supply the correct debug file, now when
GDB looks up the debug info no warning is given.
The build-id-no-debug-warning.exp is similar to the previous test. It
triggers the "separate debug info file has no debug info" warning by
replacing the build-id based .debug file with the stripped binary and
then loading it to GDB. It then also sets up local debuginfod server
with the correct debug file to download to make sure no warnings are
emitted.
|
|
See previous patch's commit message for rationale.
Change-Id: I6b8cdc045dffccc1c01ed690ff258af09f6ff076
Approved-By: Tom Tromey <tom@tromey.com>
|
|
I propose to rename cooked_index_vector and cooked_index such that the
"main" object, that is the entry point to the index, is called
cooked_index. The fact that the cooked index is implemented as a vector
of smaller indexes is an implementation detail.
This patch renames cooked_index to cooked_index_shard. The following
patch renames cooked_index_vector to cooked_index.
Change-Id: Id650f97dcb23c48f8409fa0974cd093ca0b75177
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Since commit 7d82b08e9e0a ("gdb/dwarf: dump cooked index contents in
cooked_index_functions::dump"), we see:
maint print objfiles /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/dw2-error/dw2-error^M
^M
Object file /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/dw2-error/dw2-error: Objfile at 0x614000005040, bfd at 0x6120000e08c0, 15 minsyms^M
^M
Cooked index in use:^M
^M
/home/smarchi/src/binutils-gdb/gdb/../gdbsupport/gdb-checked-static-cast.h:58: internal-error: checked_static_cast: Assertion `result != nullptr' failed.^M
A problem internal to GDB has been detected,^M
further debugging may prove unreliable.^M
----- Backtrace -----^M
FAIL: gdb.dwarf2/dw2-error.exp: maint print objfiles /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/dw2-error/dw2-error (GDB internal error)
The problem is that when cooked_index_functions fails to build an index,
per_objfile->index_table is nullptr. Therefore, add a nullptr check,
like other methods of cooked_index_functions already do.
Print the "Cooked index in use" message after the nullptr check, such
that if the cooked index failed to build, that message is not printed.
Change-Id: Id67aef592e76c41b1e3bde9838a4e36cef873253
|
|
Following 7d82b08e9e0a ("gdb/dwarf: dump cooked index contents in
cooked_index_functions::dump"), I see some failures like:
(gdb) mt print objfiles with-mf^M
^M
Object file /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/with-mf/with-mf: Objfile at 0x614000005040, bfd at 0x6120000e08c0, 18 minsyms ^M
^M
Cooked index in use:^M
^M
...
(gdb) FAIL: gdb.base/with-mf.exp: check if index present
This is because the format of the "Cooked index in use" line changed
slightly. Adjust ensure_gdb_index to expect the trailing colon.
Change-Id: If0a87575c02d8a0bc0d4b8ead540c234c62760f8
|
|
I see:
ERROR: wrong # args: should be "xfail message"
while executing
"xfail "no debug info" $gdb_test_name"
("uplevel" body line 3)
invoked from within
"uplevel {
if {!$has_runtime_debug_info} {
xfail "no debug info" $gdb_test_name
} else {
fail $gdb_test_name
}
}"
This is because the xfail takes only one argument, fix that.
Change-Id: I2e304d4fd3aa61067c04b5dac2be2ed34dab3190
|
|
value_next is declared but not defined. It's long obsolete. This
patch removes the stray declaration.
|
|
The i386 builder shows:
../../binutils-gdb/gdb/dwarf2/cooked-index.c: In member function ‘void cooked_index_vector::dump(gdbarch*) const’:
../../binutils-gdb/gdb/dwarf2/cooked-index.c:492:40: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘std::__underlying_type_impl<sect_offset, true>::type’ {aka ‘long long unsigned int’} [-Werror=format=]
492 | gdb_printf (" DIE offset: 0x%lx\n",
| ~~^
| |
| long unsigned int
| %llx
493 | to_underlying (entry->die_offset));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| |
| std::__underlying_type_impl<sect_offset, true>::type {aka long long unsigned int}
The die_offset's underlying type is uint64, so use PRIx64 in the format
string.
Change-Id: Ibdde4c624ed1bb50eced9a514a4e37aec70a1323
|
|
For some reason g++ 12.2.1 on sparc produces spurious warnings for
stringop-overread and restrict in fbsd-tdep.c for a memcpy call.
Use std::copy to avoid the warnings:
In function ‘void* memcpy(void*, const void*, size_t)’,
inlined from ‘gdb::optional<std::vector<unsigned char, gdb::default_init_allocator<unsigned char, std::allocator<unsigned char> > > > fbsd_make_note_desc(target_object, uint32_t)’ at ../../binutils-gdb/gdb/fbsd-tdep.c:666:10:
/usr/include/bits/string_fortified.h:29:33: error: ‘void* __builtin_memcpy(void*, const void*, long unsigned int)’ specified bound 18446744073709551612 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
In function ‘void* memcpy(void*, const void*, size_t)’,
inlined from ‘gdb::optional<std::vector<unsigned char, gdb::default_init_allocator<unsigned char, std::allocator<unsigned char> > > > fbsd_make_note_desc(target_object, uint32_t)’ at ../../binutils-gdb/gdb/fbsd-tdep.c:673:10:
/usr/include/bits/string_fortified.h:29:33: error: ‘void* __builtin_memcpy(void*, const void*, long unsigned int)’ accessing 18446744073709551612 bytes at offsets 0 and 0 overlaps 9223372036854775801 bytes at offset -9223372036854775805 [-Werror=restrict]
gdb/ChangeLog:
* fbsd-tdep.c (fbsd_make_note_desc): Use std::copy instead
of memcpy.
|
|
As I am investigating a crash I see with the cooked index, I thought it
would be useful to have a way to dump the index contents. For those not
too familiar with it (that includes me), it can help get a feel of what
it contains and how it is structured.
The cooked_index_functions::dump function is called as part of the
"maintenance print objfiles" command. I tried to make the output
well structured and indented to help readability, as this prints a lot
of text.
The dump function first dumps all cooked index entries, like this:
[25] ((cooked_index_entry *) 0x621000121220)
name: __ioinit
canonical: __ioinit
DWARF tag: DW_TAG_variable
flags: 0x2 [IS_STATIC]
DIE offset: 0x21a4
parent: ((cooked_index_entry *) 0x6210000f9610) [std]
Then the information about the main symbol:
main: ((cooked_index_entry *) 0x621000123b40) [main]
And finally the address map contents:
[1] ((addrmap *) 0x6210000f7910)
[0x0] ((dwarf2_per_cu_data *) 0)
[0x118a] ((dwarf2_per_cu_data *) 0x60c000007f00)
[0x1cc7] ((dwarf2_per_cu_data *) 0)
[0x1cc8] ((dwarf2_per_cu_data *) 0x60c000007f00)
[0x1cdf] ((dwarf2_per_cu_data *) 0)
[0x1ce0] ((dwarf2_per_cu_data *) 0x60c000007f00)
The display of address maps above could probably be improved, to show it
more as ranges, but I think this is a reasonable start.
Note that this patch depends on Pedro Alves' patch "enum_flags
to_string" [1]. If my patch is to be merged before Pedro's series, I
will cherry-pick this patch from his series and merge it before mine.
[1] https://inbox.sourceware.org/gdb-patches/20221212203101.1034916-8-pedro@palves.net/
Change-Id: Ida13e479fd4c8d21102ddd732241778bc3b6904a
|
|
This commit introduces shared infrastructure that can be used to
implement enum_flags -> to_string functions. With this, if we want to
support converting a given enum_flags specialization to string, we
just need to implement a function that provides the enumerator->string
mapping, like so:
enum some_flag
{
SOME_FLAG1 = 1 << 0,
SOME_FLAG2 = 1 << 1,
SOME_FLAG3 = 1 << 2,
};
DEF_ENUM_FLAGS_TYPE (some_flag, some_flags);
static std::string
to_string (some_flags flags)
{
static constexpr some_flags::string_mapping mapping[] = {
MAP_ENUM_FLAG (SOME_FLAG1),
MAP_ENUM_FLAG (SOME_FLAG2),
MAP_ENUM_FLAG (SOME_FLAG3),
};
return flags.to_string (mapping);
}
.. and then to_string(SOME_FLAG2 | SOME_FLAG3) produces a string like
"0x6 [SOME_FLAG2 SOME_FLAG3]".
If we happen to forget to update the mapping array when we introduce a
new enumerator, then the string representation will pretty-print the
flags it knows about, and then the leftover flags in hex (one single
number). For example, if we had missed mapping SOME_FLAG2 above, we'd
end up with:
to_string(SOME_FLAG2 | SOME_FLAG3) => "0x6 [SOME_FLAG2 0x4]");
Other than in the unit tests included, no actual usage of the
functionality is added in this commit.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Change-Id: I835de43c33d13bc0c95132f42c3f97318b875779
|
|
Simon pointed out that the cooked index template-matching patch
introduced a failure in libstdc++ debug mode. In particular, the new
code violates the assumption of std::lower_bound and std::upper_bound
that the range is sorted with respect to the comparison.
When I first debugged this, I thought the problem was unfixable as-is
and that a second layer of filtering would have to be done. However,
on irc, Simon pointed out that it could perhaps be solved if the
comparison function were assured that one operand always came from the
index, with the other always being the search string.
This patch implements this idea.
First, a new mode is introduced: a sorting mode for
cooked_index_entry::compare. In this mode, strings are compared
case-insensitively, but we're careful to always sort '<' before any
other printable character. This way, two names like "func" and
"func<param>" will be sorted next to each other -- i.e., "func1" will
not be seen between them. This is important when searching.
Second, the compare function is changed to work in a strcmp-like way.
This makes it easier to test and (IMO) understand.
Third, the compare function is modified so that in non-sorting modes,
the index entry is always the first argument. This allows consistency
in compares.
I regression tested this in libstdc++ debug mode on x86-64 Fedora 36.
It fixes the crash that Simon saw.
This is v2. I believe it addresses the review comments, except for
the 'enum class' change, as I mentioned in email on the list.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes the lnp_state_machine constructor to initialize members
directly; and changes lnp_state_machine itself to initialize members
inline when possible.
Reviewed-By: Lancelot Six <lancelot.six@amd.com>
|
|
After the cooked index is created, the addrmaps should be const.
Change-Id: I8234520ab346ced40a8dd6e478ba21fc438c2ba2
|
|
Users of addrmap::find and addrmap::foreach that have a const addrmap
should ideally receive const pointers to objects, to indicate they
should not be modified. However, users that have a non-const addrmap
should still receive a non-const pointer.
To achieve this, without adding more virtual methods, make the existing
find and foreach virtual methods private and prefix them with "do_".
Add small non-const and const wrappers for find and foreach.
Obviously, the const can be cast away, but if using static_cast
instead of C-style casts, then the compiler won't let you cast
the const away. I changed all the callers of addrmap::find and
addrmap::foreach I could find to make them use static_cast.
Change-Id: Ia8e69d022564f80d961413658fe6068edc71a094
|
|
Pedro pointed out that ptype_tagged_param.exp used a kfail, but an
xfail would be more appropriate as the problem appears to be in gcc,
not gdb.
|
|
The workaround for the vCont packet is no longer required due to the
former commit "gdb: Make global feature array a per-remote target array".
The vCont packet is now checked once when the connection is started and
the supported vCont actions are set to the target's remote state
attribute.
|
|
This patch adds per-remote target variables for the configuration of
memory read- and write packet size. It is a further change to commit
"gdb: Make global feature array a per-remote target array" to apply the
fixme notes described in commit 5b6d1e4 "Multi-target support".
The former global variables for that configuration are still available
to allow the command line configuration for all future remote
connections. Similar to the command line configuration of the per-
remote target feature array, the commands
- set remotewritesize (deprecated)
- set remote memory-read-packet-size
- set remote memory-write-packet-size
will configure the current target (if available). If no target is
available, the default configuration for future remote connections is
adapted. The show command will display the current remote target's
packet size configuration. If no remote target is selected, the default
configuration for future connections will be shown.
It is required to adapt the test gdb.base/remote.exp which is failing
for --target_board=native-extended-gdbserver. With that board GDB
connects to gdbserver at gdb start time. Due to this patch two loggings
"The target may not be able to.." are shown if the command 'set remote
memory-write-packet-size fixed' is executed while a target is connected
for the current inferior. To fix this, the clean_restart command is
moved to a later time point of the test. It is sufficient to be
connected to the server when "runto_main" is executed. Now the
connection time is similar to a testrun with
--target_board=native-gdbserver.
To allow the user to distinguish between the packet-size configuration
for future remote connections and for the currently selected target, the
commands' loggings are adapted.
|
|
This patch applies the appropriate FIXME notes described in commit 5b6d1e4
"Multi-target support".
"You'll notice that remote.c includes some FIXME notes. These refer to
the fact that the global arrays that hold data for the remote packets
supported are still globals. For example, if we connect to two
different servers/stubs, then each might support different remote
protocol features. They might even be different architectures, like
e.g., one ARM baremetal stub, and a x86 gdbserver, to debug a
host/controller scenario as a single program. That isn't going to
work correctly today, because of said globals. I'm leaving fixing
that for another pass, since it does not appear to be trivial, and I'd
rather land the base work first. It's already useful to be able to
debug multiple instances of the same server (e.g., a distributed
cluster, where you have full control over the servers installed), so I
think as is it's already reasonable incremental progress."
Using this patch it is possible to configure per-remote targets'
feature packets.
Given the following setup for two gdbservers:
~~~~
gdbserver --multi :1234
gdbserver --disable-packet=vCont --multi :2345
~~~~
Before this patch configuring of range-stepping was not possible for one
of two connected remote targets with different support for the vCont
packet. As one of the targets supports vCont, it should be possible to
configure "set range-stepping". However, the output of GDB looks like:
(gdb) target extended-remote :1234
Remote debugging using :1234
(gdb) add-inferior -no-connection
[New inferior 2]
Added inferior 2
(gdb) inferior 2
[Switching to inferior 2 [<null>] (<noexec>)]
(gdb) target extended-remote :2345
Remote debugging using :2345
(gdb) set range-stepping on
warning: Range stepping is not supported by the current target
(gdb) inferior 1
[Switching to inferior 1 [<null>] (<noexec>)]
(gdb) set range-stepping on
warning: Range stepping is not supported by the current target
~~~~
Two warnings are shown. The warning for inferior 1 should not appear
as it is connected to a target supporting the vCont package.
~~~~
(gdb) target extended-remote :1234
Remote debugging using :1234
(gdb) add-inferior -no-connection
[New inferior 2]
Added inferior 2
(gdb) inferior 2
[Switching to inferior 2 [<null>] (<noexec>)]
(gdb) target extended-remote :2345
Remote debugging using :2345
(gdb) set range-stepping on
warning: Range stepping is not supported by the current target
(gdb) inferior 1
[Switching to inferior 1 [<null>] (<noexec>)]
(gdb) set range-stepping on
(gdb)
~~~~
Now only one warning is shown for inferior 2, which is connected to
a target not supporting vCont.
The per-remote target feature array is realized by a new class
remote_features, which stores the per-remote target array and
provides functions to determine supported features of the target.
A remote_target object now has a new member of that class.
Each time a new remote_target object is initialized, a new per-remote
target array is constructed based on the global remote_protocol_packets
array. The global array is initialized in the function _initialize_remote
and can be configured using the command line. Before this patch the
command line configuration affected current targets and future remote
targets (due to the global feature array used by all remote
targets). This behavior is different and the configuration applies as
follows:
- If a target is connected, the command line configuration affects the
current connection. All other existing remote targets are not
affected.
- If not connected, the command line configuration affects future
connections.
The show command displays the current remote target's configuration. If no
remote target is selected the default configuration for future
connections is shown.
If we have for instance the following setup with inferior 2 being
selected:
~~~~
(gdb) info inferiors
Num Description Connection Executable
1 <null> 1 (extended-remote :1234)
* 2 <null> 2 (extended-remote :2345)
~~~~
Before this patch, if we run 'set remote multiprocess-feature-packet', the
following configuration was set:
The feature array of all remote targets (in this setup the two connected
targets) and all future remote connections are affected.
After this patch, it will be configured as follows:
The feature array of target with port :2345 which is currently selected
will be configured. All other existing remote targets are not affected.
The show command 'show remote multiprocess-feature-packet' will display
the configuration of target with port :2345.
Due to this configuration change, it is required to adapt the test
"gdb/testsuite/gdb.multi/multi-target-info-inferiors.exp" to configure the
multiprocess-feature-packet before the connections are created.
To inform the gdb user about the new behaviour of the 'show remote
PACKET-NAME' commands and the new configuration impact for remote
targets using the 'set remote PACKET-NAME' commands the commands'
outputs are adapted. Due to this change it is required to adapt each
test using the set/show remote 'PACKET-NAME' commands.
|
|
I noticed that iterating over the index yields non-const
cooked_index_entry objects. However, after finalization, they should
not be modified. This patch enforces this by adding const where
needed.
v2 makes the find, all_entries, and wait methods const as well.
|
|
Recent commit 1d98e564c97 ("[gdb/testsuite] Add
gdb.base/unwind-on-each-insn-{amd64,i386}.exp") broke commit eb015bf86b6
("[gdb/testsuite] Avoid using .eh_frame in gdb.base/unwind-on-each-insn.exp"),
in the sense that gdb.base/unwind-on-each-insn.exp no longer uses
-fno-asynchronous-unwind-tables, due to trying to concatenate two lists using:
...
lappend srcfile2_flags $nodebug_flags
...
which should instead be:
...
lappend srcfile2_flags {*}$nodebug_flags
...
Fix this by simplifying gdb.base/unwind-on-each-insn.exp.tcl, completely
leaving the responsibility to set srcfile_flags and srcfile2_flags to each
includer.
Tested on x86_64-linux.
|
|
Simon pointed out that the kfail check in
gdb.ada/ptype_tagged_param.exp is inverted. See:
https://sourceware.org/pipermail/gdb-patches/2023-January/196296.html
This patch fixes the problem.
|
|
Add some additional debug output that I've found really useful while
working on the previous set of patches.
Unless tui debug is turned on, then there should be no user visible
changes with this commit.
|
|
While working on the previous couple of patches I noticed that when I
scroll the src and asm windows vertically, I get two refresh_window
calls.
The two calls can be traced back to
tui_source_window_base::update_source_window_as_is, in here we call
show_source_content, which calls refresh_window, and then
update_exec_info, which also calls refresh_window.
In this commit I propose making the refresh_window call in
update_exec_info optional. In update_source_window_as_is I'll then
call update_exec_info before calling show_source_content, and pass a
flag to update_exec_info to defer the refresh.
There are places where update_exec_info is used without any subsequent
refresh_window call (e.g. when a breakpoint is updated), so
update_exec_info does not to call refresh_window in some cases, which
is why I'm using a flag to control the refresh.
With this changes I'm now only seeing a single refresh_window call for
each vertical scroll.
There should be no user visible changes after this commit.
|
|
While working on the previous patches I noticed that in some cases I
was seeing two calls to tui_source_window_base::refresh_window when
scrolling the window horizontally.
The two calls would trigger in for the tui-disasm-long-lines.exp test
when the pad needed to be refilled. The two called both come from
tui_source_window_base::show_source_content. The first call is nested
within check_and_display_highlight_if_needed, while the second call is
done directly at the end of show_source_content.
The check_and_display_highlight_if_needed is being used to draw the
window box to the window, this is needed here because
show_source_content is what gets called when the window needs
updating, e.g. after a resize. We could potentially do the boxing in
refresh_window, but then we'd be doing it each time we scroll, even
though the box doesn't need changing in this case.
However, we can move the check_and_display_highlight_if_needed to be
the last thing done in show_source_content, this means that we can
rely on the refresh_window call within it to be our single refresh
call.
There should be no user visible changes after this commit.
|
|
This commit addresses an issue that is exposed by the test script
gdb.tui/tui-disasm-long-lines.exp, that is, tui_source_window_base
does not handle very long lines.
The problem can be traced back to the newpad call in
tui_source_window_base::show_source_content, this is where we allocate
a backing pad to hold the window content.
Unfortunately, there appears to be a limit to the size of pad that can
be allocated, and the gdb.tui/tui-disasm-long-lines.exp test goes
beyond this limit. As a consequence the newpad call fails and returns
nullptr.
It just so happens that the reset of the tui_source_window_base code
can handle the pad being nullptr (this happens anyway when the window
is first created, so we already depend on nullptr handling), so all
that happens is the source window displays no content.
... well, sort of ... something weird does happen in the command
window, we seem to see a whole bunch of blank lines. I've not
bothered to track down exactly what's happening there, but it's some
consequence of GDB attempting to write content to a WINDOW* that is
nullptr.
Before explaining my solution, I'll outline how things currently work:
Consider we have the following window content to display:
aaaaaaaaaa
bbbbbbbbbbbbbbbbbbbb
ccccccccccccccc
the longest line here is 20 characters. If our display window is 10
characters wide, then we will create a pad that is 20 characters wide,
and then copy the lines of content into the pad:
.--------------------.
|aaaaaaaaaa |
|bbbbbbbbbbbbbbbbbbbb|
|ccccccccccccccc |
.--------------------.
Now we will copy a 10 character wide view into this pad to the
display, our display will then see:
.----------.
|aaaaaaaaaa|
|bbbbbbbbbb|
|cccccccccc|
.----------.
As the user scrolls left and right we adjust m_horizontal_offset and
use this to select which part of the pad is copied onto the display.
The benefit of this is that we only need to copy the content to the
pad once, which includes processing the ansi escape sequences, and
then the user can scroll left and right as much as they want
relatively cheaply.
The problem then, is that if the longest content line is very long,
then we try to allocate a very large pad, which can fail.
What I propose is that we allow both the pad and the display view to
scroll. Once we allow this, then it becomes possible to allocate a
pad that is smaller than the longest display line. We then copy part
of the content into the pad. As the user scrolls the view left and
right GDB will continue to copy content from the pad just as it does
right now. But, when the user scrolls to the edge of the pad, GDB
will copy a new block of content into the pad, and then update the
view as normal. This all works fine so long as the maximum pad size
is larger than the current window size - which seems a reasonable
restriction, if ncurses can't support a pad of a given size it seems
likely it will not support a display window of that size either.
If we return to our example above, but this time we assume that the
maximum pad size is 15 characters, then initially the pad would be
loaded like this:
.---------------.
|aaaaaaaaaa |
|bbbbbbbbbbbbbbb|
|ccccccccccccccc|
.---------------.
Notice that the last 5 characters from the 'b' line are no longer
included in the pad. There is still enough content though to fill the
10 character wide display, just as we did before.
The pad contents remain unchanged until the user scrolls the display
right to this point:
.----------.
|aaaaa |
|bbbbbbbbbb|
|cccccccccc|
.----------.
Now, when the user scrolls right once more GDB spots that the user has
reached the end of the pad, and the pad contents are reloaded, like
this:
.---------------.
|aaaaa |
|bbbbbbbbbbbbbbb|
|cccccccccc |
.---------------.
The display can now be updated from the pad again just like normal.
With this change in place the gdb.tui/tui-disasm-long-lines.exp test
now correctly loads the assembler code, and we can scroll around as
expected.
Most of the changes are pretty mundane, just updating to match the
above. One interesting change though is the new member function
tui_source_window_base::puts_to_pad_with_skip. This replaces direct
calls to tui_puts when copying content to the pad.
The content strings contain ansi escape sequences. When these strings
are written to the pad these escape sequences are translated into
ncurses attribute setting calls.
Now however, we sometimes only write a partial string to the pad,
skipping some of the leading content. Imagine then that we have a
content line like this:
"\033[31mABCDEFGHIJKLM\033[0m"
Now the escape sequences in this content mean that the actual
content (the 'ABCDEFGHIJKLM') will have a red foreground color.
If we want to copy this to the pad, but skip the first 3 characters,
then what we expect is to have the pad contain 'DEFGHIJKLM', but this
text should still have a red foreground color.
It is this problem that puts_to_pad_with_skip solves. This function
skips some number of printable characters, but processes all the
escape sequences. This means that when we do start printing the
actual content the content will have the expected attributes.
/
|
|
I noticed that tui_source_window_base::m_horizontal_offset was
protected, but could be made private, so lets do that.
This makes more sense in the context of a later commit where I plan to
add another member variable that is similar to m_horizontal_offset.
The new member variable could also be private.
So I had to choose, place the new member variable next to
m_horizontal_offset making it protected, but grouping similar
variables together, or make m_horizontal_offset private, and then add
the new variable as private too.
I chose to make m_horizontal_offset private, which is this commit.
There should be no user visible changes after this commit.
|
|
When an assert triggers in tui mode the output is not great, the
internal backtrace that is generated is printed directly to the file
descriptor for gdb_stderr, and, as a result, does not currently format
itself correctly - the output uses only '\n' at the end of each line,
and so, when the terminal is in raw mode, the cursor does not return
to the start of each line after the '\n'.
This is mostly fixable, we could update bt-utils.c to use '\r\n'
instead of just '\n', and this would fix most of the problems. The
one we can't easily fix is if/when GDB is built to use execinfo
instead of libbacktrace, in this case we use backtrace_symbols_fd to
print the symbols, and this function only uses '\n' as the line
terminator. Fixing this would require switching to backtrace_symbols,
but that API uses malloc, which is something we're trying to
avoid (this code is called when GDB hits an error, so ideally we don't
want to rely on malloc).
However, the execinfo code is only used when libbacktrace is not
available (or the user specifically disables libbacktrace) so maybe we
can ignore that problem...
... but there is another problem. When the backtrace is printed in
raw mode, it is possible that the backtrace fills the screen. With
the terminal in raw mode we don't have the ability to scroll back,
which means we loose some of the backtrace, which isn't ideal.
In this commit I propose that we should disable tui mode whenever we
handle a fatal signal, or when we hit the internal error code
path (e.g. when an assert triggers). With this done then we don't
need to update the bt-utils.c code, and the execinfo version of the
code (using backtrace_symbols_fd) works just fine. We also get the
ability to scroll back to view the error message and all of the
backtrace, assuming the users terminal supports scrolling back.
The only downside I see with this change is if the tui_disable call
itself causes an error for some reason, or, if we handle a single at a
time when it is not safe to call tui_disable, in these cases the extra
tui_disable call might cause GDB to loose the original error.
However, I think (just from personal experience) that the above two
issues are pretty rare and the benefits from this change far out
weighs the possible drawbacks.
|
|
This commit improves (I think) the errors from the tui focus command.
There are a number of errors that can be triggered by the focus
command, they include:
(1) Window name "NAME" is ambiguous
(2) Unrecognized window name "NAME"
(3) Window "NAME" cannot be focused
Error (1) is triggered when the user gives a partial window name, and
the name matches multiple windows in the current layout.
It is worth noting that the ambiguity must be within the current
layout; if the partial name matches one window in the current layout,
and one or more windows not in the current layout, then this is not
ambiguous, and focus will shift to the matching window in the current
layout.
This error was not previous being tested, but in this commit I make
use of the Python API to trigger and test this error.
Error (3) is simple enough, and was already being tested. This is
triggered by something like 'focus status'. The named window needs to
be present in the current layout, and non-focusable in order to
trigger the error.
Error (2) is what I'd like to improve in this commit. This error
triggers if the name the user gives doesn't match any window in the
current layout. Even if GDB does know about the window, but the
window isn't in the current layout, then GDB will say it doesn't
recognize the window name.
In this commit I propose to to split this error into three different
errors. These will be:
(a) Unrecognized window name "NAME"
(b) No windows matching "NAME" in the current layout
(c) Window "NAME" is not in the current layout
Error (a) is the same as before, but will now only trigger if GDB
doesn't know about window NAME at all. If the window is known, but
not in the current layout then one of the other errors will trigger.
Error (b) will trigger if NAME is ambiguous for multiple windows that
are not in the current layout. If NAME identifies a single window in
the current layout then that window will continue to be selected, just
as it currently is. Only in the case where NAME doesn't identify a
window in the current layout do we then check all the other known
windows, if NAME matches multiple of these, then (b) is triggered.
Finally, error (c) is used when NAME uniquely identifies a single
window that is not in the current layout.
The hope with these new errors is that the user will have a better
understanding of what went wrong. Instead of GDB claiming to not know
about a window, the mention of the current layout will hint to the
user that they should first switch layouts.
There are tests included for all the new errors.
|
|
In a following commit I managed to trigger the line feed scrolling
case in tuiterm.exp. This case is currently unhandled, and this
commit fills in this missing functionality.
The implementation is pretty simple, just scroll all the content up
one line at a time until the cursor is back on the screen (a single
line of scroll is all that should be needed).
This change is untested in this commit, but is required by the next
commit.
|
|
Change gdb.base to use clean_restart more consistently.
|
|
Change gdb.python to use clean_restart more consistently.
|
|
Change gdb.cp to use clean_restart more consistently.
|
|
Change gdb.disasm to use clean_restart more consistently.
|
|
Change gdb.perf to use clean_restart more consistently.
|
