| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Remove the use of xmalloc (and the arbitrary allocation size) in
format_pieces. This turned out a bit more involved than expected, but
not too bad.
format_pieces::m_storage is a buffer with multiple concatenated
null-terminated strings, referenced by format_piece::string. Change
this to an std::string, while keeping its purpose (use the std::string
as a buffer with embedded null characters).
However, because the std::string's internal buffer can be reallocated as
it grows, and I do not want to hardcode a big reserved size like we have
now, it's not possible to store the direct pointer to the string in
format_piece::string. Those pointers would become stale as the buffer
gets reallocated. Therefore, change format_piece to hold an index into
the storage instead. Add format_pieces::piece_str for the callers to be
able to access the piece's string. This requires changing the few
callers, but in a trivial way.
The selftest also needs to be updated. I want to keep the test cases
as-is, where the expected pieces contain the expected string, and not
hard-code an expected index. To achieve this, add the
expected_format_piece structure. Note that the previous
format_piece::operator== didn't compare the n_int_args fields, while the
test provides expected values for that field. I guess that was a
mistake. The new code checks it, and the test still passes.
Change-Id: I80630ff60e01c8caaa800ae22f69a9a7660bc9e9
Reviewed-By: Keith Seitz <keiths@redhat.com>
|
|
Found by the codespell pre-commit hook.
Change-Id: Iafadd9485ce334c069dc8dbdab88ac3fb5fba674
|
|
GDB holds the inferior arguments as a single string. Currently when
GDB needs to pass the inferior arguments to a remote target as part of
a vRun packet, this is done by splitting the single argument string
into its component arguments by calling gdb::remote_args::split, which
uses the gdb_argv class to split the arguments for us.
The same gdb_argv class is used when the user has asked GDB/gdbserver
to start the inferior without first invoking a shell; the gdb_argv
class is used to split the argument string into it component
arguments, and each is passed as a separate argument to the execve
call which spawns the inferior.
There is however, a problem with using gdb_argv to split the arguments
before passing them to a remote target. To understand this problem we
must first understand how gdb_argv is used when invoking an inferior
without a shell.
And to understand how gdb_argv is used to start an inferior without a
shell, I feel we need to first look at an example of starting an
inferior with a shell.
Consider these two cases:
(a) (gdb) set args \$VAR
(b) (gdb) set args $VAR
When starting with a shell, in case (a) the user expects the inferior
to receive a literal '$VAR' string as an argument, while in case (b)
the user expects to see the shell expanded value of the variable $VAR.
If the user does 'set startup-with-shell off', then in (a) GDB will
strip the '\' while splitting the arguments, and the inferior will be
passed a literal '$VAR'. In (b) there is no '\' to strip, so also in
this case the inferior will receive a literal '$VAR', remember
startup-with-shell is off, so there is no shell that can ever expand
$VAR.
Notice, that when startup-with-shell is off, we end up with a many to
one mapping, both (a) and (b) result in the literal string $VAR being
passed to the inferior. I think this is the correct behaviour in this
case.
However, as we use gdb_argv to split the remote arguments we have the
same many to one mapping within the vRun packet. But the vRun packet
will be used when startup-with-shell is both on and off. What this
means is that when gdbserver receives a vRun packet containing '$VAR'
it doesn't know if GDB actually had '$VAR', or if GDB had '\$VAR'.
And this is a huge problem.
We can address this by making the argument splitting for remote
targets smarter, and I do have patches that try to do this in this
series:
https://inbox.sourceware.org/gdb-patches/cover.1730731085.git.aburgess@redhat.com
That series was pretty long, and wasn't getting reviewed, so I'm
pulling the individual patches out and posting them separately.
This patch doesn't try to improve remote argument splitting. I think
that splitting and then joining the arguments is a mistake which can
only introduce problems. The patch in the above series which tries to
make the splitting and joining "smarter" handles unquoted, single
quoted, and double quoted strings. But that doesn't really address
parameter substitution, command substitution, or arithmetic expansion.
And even if we did try to address these cases, what rules exactly
would we implement? Probably POSIX shell rules, but what if the
remote target doesn't have a POSIX shell? The only reason we're
talking about which shell rules to follow is because the splitting and
joining logic needs to mirror those rules. If we stop splitting and
joining then we no longer need to care about the target's shell.
Clearly, for backward compatibility we need to maintain some degree of
argument splitting and joining as we currently have; and that's why I
have a later patch (see the series above) that tries to improve that
splitting and joining a little. But I think, what we should really
do, is add a new feature flag (as used by the qSupported packet) and,
if GDB and the remote target agree, we should pass the inferior
arguments as a single string.
This solves all our problems. In the startup with shell case, we no
longer need to worry about splitting at all. The arguments are passed
unmodified to the remote target, that can then pass the arguments to
the shell directly.
In the 'startup-with-shell off' case it is now up to the remote target
to split the arguments, though in gdbserver we already did this, so
nothing really changes in this case. And if the remote target doesn't
have a POSIX shell, well GDB just doesn't need to worry about it!
Something similar to this was originally suggested in this series:
https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de/
though this series didn't try to maintain backward compatibility,
which I think is an issue that my patch solves. Additionally, this
series only passed the arguments as a single string in some cases,
I've simplified this so that, when GDB and the remote agree, the
arguments are always passed as a single string. I think this is a
little cleaner.
I've also added documentation and some tests with this commit,
including ensuring that we test both the new single string approach,
and the fallback split/join approach.
I've credited the author of the referenced series as co-author as they
did come to a similar conclusion, though I think my implementation is
different enough that I'm happy to list myself as primary author.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28392
Co-Authored-By: Michael Weghorn <m.weghorn@posteo.de>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Tested-By: Guinevere Larsen <guinevere@redhat.com>
Approved-by: Kevin Buettner <kevinb@redhat.com>
|
|
This introduces a new '--no-escape-args' option for gdb and gdbserver.
I (Andrew Burgess) have based this patch from work done in this
series:
https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de/
I have changed things slightly from the original series. I think this
work is close enough that I've left the original author (Michael) in
place and added myself as co-author. Any bugs introduced by my
modifications to the original patch should be considered mine. I've
also added documentation and tests which were missing from the
originally proposed patch.
When the startup-with-shell option is enabled, arguments passed
directly as 'gdb --args <args>' or 'gdbserver <args>', are by default
escaped so that they are passed to the inferior as passed on the
command line, no globbing or variable substitution happens within the
shell GDB uses to start the inferior.
For gdbserver, this is the case since commit:
commit bea571ebd78ee29cb94adf648fbcda1e109e1be6
Date: Mon May 25 11:39:43 2020 -0400
Use construct_inferior_arguments which handles special chars
Only arguments set via 'set args <args>', 'run <args>', or through the
Python API are not escaped in standard upstream GDB right now.
For the 'gdb --args' case, directly setting unescaped args on gdb
invocation is possible e.g. by using the "--eval-command='set args
<args>'", while this possibility does not exist for gdbserver.
This commit adds a new '--no-escape-args' command line option for GDB
and gdbserver. This option is used with GDB as a replacement for the
current '--args' option, and for gdbserver this new option is a flag
which changes how gdbserver handles inferior arguments on the command
line. When '--no-escape-args' is used inferior arguments passed on
the command line will not have escaping added by GDB or gdbserver.
For gdbserver, using this new option allows having the behaviour from
before commit bea571ebd78ee29cb94adf648fbcda1e109e1be6, while keeping
the default behaviour unified between GDB and GDBserver.
For GDB the --no-escape-args option can be used as a replacement for
--args, like this:
shell> gdb --no-escape-args my-program arg1 arg2 arg3
While for gdbserver, the --no-escape-args option is a flag, which can
be used like:
shell> gdbserver --no-escape-args --once localhost:54321 \
my-program arg1 arg2 arg3
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28392
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Tested-By: Guinevere Larsen <guinevere@redhat.com>
|
|
PR ada/33217 points out that gdb incorrectly calls the <ctype.h>
functions. In particular, gdb feels free to pass a 'char' like:
char *str = ...;
... isdigit (*str)
This is incorrect as isdigit only accepts EOF and values that can be
represented as 'unsigned char' -- that is, a cast is needed here to
avoid undefined behavior when 'char' is signed and a character in the
string might be sign-extended. (As an aside, I think this API seems
obviously bad, but unfortunately this is what the standard says, and
some systems check this.)
Rather than adding casts everywhere, this changes all the code in gdb
that uses any <ctype.h> API to instead call the corresponding c-ctype
function.
Now, c-ctype has some limitations compared to <ctype.h>. It works as
if the C locale is in effect, so in theory some non-ASCII characters
may be misclassified. This would only affect a subset of character
sets, though, and in most places I think ASCII is sufficient -- for
example the many places in gdb that check for whitespace.
Furthermore, in practice most users are using UTF-8-based locales,
where these functions aren't really informative for non-ASCII
characters anyway; see the existing workarounds in gdb/c-support.h.
Note that safe-ctype.h cannot be used because it causes conflicts with
readline.h. And, we canot poison the <ctype.h> identifiers as this
provokes errors from some libstdc++ headers.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33217
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes gdb and related programs to use the gnulib c-ctype code
rather than safe-ctype.h. The gdb-safe-ctype.h header is removed.
This changes common-defs.h to include the c-ctype header, making it
available everywhere in gdb.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33217
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Add the org.gnu.gdb.aarch64.gcs feature with the GCSPR register, and the
org.gnu.gdb.aarch64.gcs.linux feature with "registers" to represent the
Linux kernel ptrace and prctl knobs that enable and lock specific GCS
functionality.
This code supports GCS only in Linux userspace applications, so the
GCSPR that is exposed is the one at EL0.
Also, support for calling inferior functions is enabled by adding an
implementation for the shadow_stack_push gdbarch method.
If for some reason a target description contains the
org.gnu.gdb.aarch64.gcs feature but not the
org.gnu.gdb.aarch64.gcs.linux feature then GCS support is disabled and
GDB continues the debugging session. Features that need GCS
support (for example, calling inferior functions) will not work and the
inferior will get a segmentation fault signal instead. There's a
testcase for this scenario but it only checks the native debugging case,
even though in practice this problem would only occur in remote
debugging with a broken stub or gdbserver. I tested manually with a
gdbserver hacked to send a broken target description and it worked as
described.
Testcases gdb.arch/aarch64-gcs.exp, gdb.arch/aarch64-gcs-core.exp and
gdb.arch/aarch64-gcs-wrong-tdesc.exp are included to cover the added
functionality.
Reviewed-By: Christina Schimpe <christina.schimpe@intel.com>
Approved-By: Luis Machado <luis.machado@arm.com>
|
|
This patch adds the user mode register PL3_SSP which is part of the
Intel(R) Control-Flow Enforcement Technology (CET) feature for support
of shadow stack.
For now, only native and remote debugging support for shadow stack
userspace on amd64 linux are covered by this patch including 64 bit and
x32 support. 32 bit support is not covered due to missing Linux kernel
support.
This patch requires fixing the test gdb.base/inline-frame-cycle-unwind
which is failing in case the shadow stack pointer is unavailable.
Such a state is possible if shadow stack is disabled for the current thread
but supported by HW.
This test uses the Python unwinder inline-frame-cycle-unwind.py which fakes
the cyclic stack cycle by reading the pending frame's registers and adding
them to the unwinder:
~~~
for reg in pending_frame.architecture().registers("general"):
val = pending_frame.read_register(reg)
unwinder.add_saved_register(reg, val)
return unwinder
~~~
However, in case the python unwinder is used we add a register (pl3_ssp) that is
unavailable. This leads to a NOT_AVAILABLE_ERROR caught in
gdb/frame-unwind.c:frame_unwind_try_unwinder and it is continued with standard
unwinders. This destroys the faked cyclic behavior and the stack is
further unwinded after frame 5.
In the working scenario an error should be triggered:
~~~
bt
0 inline_func () at /tmp/gdb.base/inline-frame-cycle-unwind.c:49^M
1 normal_func () at /tmp/gdb.base/inline-frame-cycle-unwind.c:32^M
2 0x000055555555516e in inline_func () at /tmp/gdb.base/inline-frame-cycle-unwind.c:45^M
3 normal_func () at /tmp/gdb.base/inline-frame-cycle-unwind.c:32^M
4 0x000055555555516e in inline_func () at /tmp/gdb.base/inline-frame-cycle-unwind.c:45^M
5 normal_func () at /tmp/gdb.base/inline-frame-cycle-unwind.c:32^M
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
(gdb) PASS: gdb.base/inline-frame-cycle-unwind.exp: cycle at level 5: backtrace when the unwind is broken at frame 5
~~~
To fix the Python unwinder, we simply skip the unavailable registers.
Also it makes the test gdb.dap/scopes.exp fail. The shadow stack feature is
disabled by default, so the pl3_ssp register which is added with my CET
shadow stack series will be shown as unavailable and we see a TCL error:
~~
>>> {"seq": 12, "type": "request", "command": "variables", "arguments": {"variablesReference": 2, "count": 85}}
Content-Length: 129^M
^M
{"request_seq": 12, "type": "response", "command": "variables", "success": false, "message": "value is not available", "seq": 25}FAIL: gdb.dap/scopes.exp: fetch all registers success
ERROR: tcl error sourcing /tmp/gdb/testsuite/gdb.dap/scopes.exp.
ERROR: tcl error code TCL LOOKUP DICT body
ERROR: key "body" not known in dictionary
while executing
"dict get $val body variables"
(file "/tmp/gdb/testsuite/gdb.dap/scopes.exp" line 152)
invoked from within
"source /tmp/gdb/testsuite/gdb.dap/scopes.exp"
("uplevel" body line 1)
invoked from within
"uplevel #0 source /tmp/gdb/testsuite/gdb.dap/scopes.exp"
invoked from within
"catch "uplevel #0 source $test_file_name" msg"
UNRESOLVED: gdb.dap/scopes.exp: testcase '/tmp/gdb/testsuite/gdb.dap/scopes.exp' aborted due to Tcl error
~~
I am fixing this by enabling the test for CET shadow stack, in case we
detect that the HW supports it:
~~~
# If x86 shadow stack is supported we need to configure GLIBC_TUNABLES
# such that the feature is enabled and the register pl3_ssp is
# available. Otherwise the reqeust to fetch all registers will fail
# with "message": "value is not available".
if { [allow_ssp_tests] } {
append_environment GLIBC_TUNABLES "glibc.cpu.hwcaps" "SHSTK"
}
~~~
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Luis Machado <luis.machado@arm.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
The XSAVE function set is organized in state components, which are a set of
registers or parts of registers. So-called XSAVE-supported features are
organized using state-component bitmaps, each bit corresponding to a
single state component.
The Intel Software Developer's Manual uses the term xstate_bv for a
state-component bitmap, which is defined as XCR0 | IA32_XSS. The control
register XCR0 only contains a state-component bitmap that specifies user state
components, while IA32_XSS contains a state-component bitmap that specifies
supervisor state components.
Until now, XCR0 is used as input for target description creation in GDB.
However, a following patch will add userspace support for the CET shadow
stack feature by Intel. The CET state is configured in IA32_XSS and consists
of 2 state components:
- State component 11 used for the 2 MSRs controlling user-mode
functionality for CET (CET_U state)
- State component 12 used for the 3 MSRs containing shadow-stack pointers
for privilege levels 0-2 (CET_S state).
Reading the CET shadow stack pointer register on linux requires a separate
ptrace call using NT_X86_SHSTK. To pass the CET shadow stack enablement
state we would like to pass the xstate_bv value instead of xcr0 for target
description creation. To prepare for that, we rename the xcr0 mask
values for target description creation to xstate_bv. However, this
patch doesn't add any functional changes in GDB.
Future states specified in IA32_XSS such as CET will create a combined
xstate_bv_mask including xcr0 register value and its corresponding bit in
the state component bitmap. This combined mask will then be used to create
the target descriptions.
Reviewed-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Approved-By: Luis Machado <luis.machado@arm.com>
|
|
On x86 the PTRACE_GETREGSET request is currently only used for the xstate regset.
The size of the xstate regset is initialized to 0 such that it can be reset to
the appropriate size once we know it is supported for the current target
in x86_linux_read_description.
However, this configuration would not just affect the xstate regset but any regset
with PTRACE_GETREGSET request that is added in the future. The new regset would be
misconfigured with the xstate regset size. To avoid this we add an assert for
unsupported regsets and check explicitly for the note type of the register set.
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Approved-By: Luis Machado <luis.machado@arm.com>
|
|
Some register sets can be activated and deactivated by the OS during the runtime of
a process. One example register is the Intel CET shadow stack pointer. This patch
adds a new type of register set to handle such cases. We shouldn't deactivate these
regsets and should not show a warning if the register set is not active but supported
by the kernel. However, it is safe to deactivate them, if they are unsupported by the
kernel. To differentiate those scenarios we can use the errno returned by the ptrace
call.
Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Approved-By: Luis Machado <luis.machado@arm.com>
|
|
Stop using AC_HEADER_STDC since it is no longer supported in autoconf
2.72+. We require a C++ compiler that supports c++17, it's probably
safe to assume that the C compiler fully supports C89.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
The copyright notices printed by these programs still use year 2024.
Update to 2025.
It seems like a trivial patch to me, but I am posting it for review, in
case there's something wrong in the new-year process that caused them to
be missed, in which case we should address that too.
Change-Id: I7d9541bb154b8000e66cfead4e4228e33c49f18b
Approved-by: Kevin Buettner <kevinb@redhat.com>
|
|
This patch gives gdbserver the ability to omit the 's' reply to
'vCont?'. This tells gdb that hardware single-step is definitely not
supported, causing it to fall back to using software single-step.
This is useful for testing the earlier change to
maybe_software_singlestep.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
Update gdbserver to use getopt_long for argument processing. This
turned out to be easier than I expected.
Interesting parts of this patch are:
I pass '+' as the OPTSTRING to getopt_long, this prevents getopt_long
from reordering the contents of ARGV. This is needed so that things
like this will work:
gdbserver :54321 program --arg1 --arg2
Without the '+', getopt_long will reorder ARGV, moving '--arg1' and
'--arg2' forward and handling them as arguments to gdbserver.
Because of this (not reordering) and to maintain backward
compatibility, we retain a special case to deal with '--attach' which
can appear after the PORT, like this:
gdbserver :54321 --attach PID
I did consider adding a warning to this special case, informing the
user that placing --attach after the PORT was deprecated, but in the
end I didn't want to really change the behaviour as part of this
commit, so I've left that as an optional change for the future.
The getopt_long function supports two value passing forms, there is
'--option=value', and also '--option value'. Traditionally, gdbserver
only supports the first of these. To maintain this behaviour, after
the call to getopt_long, I spot if '--option value' was used, and if
so, modify the state so that it seems that no value was passed, and
that 'value' is the next ARGV entry to be parsed. We could, possibly,
remove this code in the future, but that would be a functional change,
which is not something I want in this commit.
Handling of "-" for stdio connection has now moved out of the argument
processing loop as '-' isn't considered a valid option by getopt_long,
this is an improvement as all PORT handling is now in one place.
I've tried as much as possible to leave user visible functionality
unchanged after this commit, and as far as I can tell from testing,
nothing has changed.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
When using the command:
$ gdbserver --disable-packet
gdbserver lists all the packets that can be disabled, and then exits.
I think that this output is a helpful error message that is printed
when the user has forgotten to entry a packet name. We get similar
output if we run the command:
$ gdbserver --disable-packet=foo
where gdbserver tells us that 'foo' is invalid, and then lists the
packets that we can use.
The difference is that, in the first case, gdbserver exits with a code
of 0, and in the second, gdbserver exits with a code of 1.
I think both these cases should exit with a code of 1.
With the exception of '--help' and '--version', where we are asking
gdbserver to print some message then exit (which are, and should exit
with a code of 0), in all other cases where we do an early exit, I
think this is an indication that the user has done something
wrong (entered and invalid argument, or missed an argument value), and
gdbserver should exit with a non-zero exit code to indicate this.
This commit updates the exit code in the above case from 0 to 1.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Within gdbserver/server.cc, in captured_main, convert some locals to
bool. Move the declaration of some locals into the body of the
function.
There should be no user visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The armhf buildbot fails to build GDB, with:
../../binutils-gdb/gdbserver/server.cc: In function ‘void handle_general_set(char*)’:
../../binutils-gdb/gdbserver/server.cc:1021:23: error: loop variable ‘<structured bindings>’ creates a copy from type ‘const std::pair<thread_info*, enum_flags<gdb_thread_option> >’ [-Werror=range-loop-construct]
1021 | for (const auto [thread, options] : set_options)
| ^~~~~~~~~~~~~~~~~
../../binutils-gdb/gdbserver/server.cc:1021:23: note: use reference type to prevent copying
1021 | for (const auto [thread, options] : set_options)
| ^~~~~~~~~~~~~~~~~
| &
I did not use a reference on purpose, because the pair is very small. I
don't see the problem when building on amd64, I presume it is because
the pair is considered too big to copy on a 32-bit architecture, but not
on a 64-bit architecture.
In any case, fix it by adding a reference.
Change-Id: I8e95235d6e53f032361950cf6e0c7d46b082f951
|
|
I wanted to change one of these, so I searched for more similar
instances, while at it. I think this looks a bit tidier, over unpacking
the pairs by hand.
Change-Id: Ife4b678f7a6aed01803434197c564d2ab93532a7
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This commit works around a problem introduced by commit:
commit e58beedf2c8a1e0c78e0f57aeab3934de9416bfc
Date: Tue Jan 23 16:00:59 2024 +0000
gdb: attach to a process when the executable has been deleted
The above commit extended GDB for Linux, so that, of the executable
for a process had been deleted, GDB would instead try to use
/proc/PID/exe as the executable.
This worked by updating linux_proc_pid_to_exec_file to introduce the
/proc/PID/exe fallback. However, the result of
linux_proc_pid_to_exec_file is then passed to exec_file_find to
actually find the executable, and exec_file_find, will take into
account the sysroot. In addition, if GDB is attaching to a process in
a different MNT and/or PID namespace then the executable lookup is
done within that namespace.
This all means two things:
1. Just because linux_proc_pid_to_exec_file cannot see the
executable doesn't mean that GDB is actually going to fail to
find the executable, and
2. returning /proc/PID/exe isn't useful if we know GDB is then going
to look for this within a sysroot, or within some other
namespace (where PIDs might be different).
There was an initial attempt to fix this issue here:
https://inbox.sourceware.org/gdb-patches/20250511141517.2455092-4-kilger@sec.in.tum.de/
This proposal addresses the issue in PR gdb/32955, which is all about
the namespace side of the problem. The fix in this original proposal
is to check the MNT namespace inside linux_proc_pid_to_exec_file, and
for the namespace problem this is fine. But we should also consider
the sysroot problem.
And for the sysroot problem, the fix cannot fully live inside
linux_proc_pid_to_exec_file, as linux_proc_pid_to_exec_file is shared
between GDB and gdbserver, and gdbserver has no sysroot.
And so, I propose a slightly bigger change.
Now, linux_proc_pid_to_exec_file takes a flag which indicates if
GDB (or gdbserver) will look for the inferior executable in the
local file system, where local means the same file system as GDB (or
gdbserver) is running in.
This local file system check is true if:
1. The MNT namespace of the inferior is the same as for GDB, and
2. for GDB only, the sysroot must either be empty, or 'target:'.
If the local file system check is false then GDB (or gdbserver) is
going to look elsewhere for the inferior executable, and so, falling
back to /proc/PID/exe should not be done, as GDB will end up looking
for this file in the sysroot, or within the alternative MNT
namespace (which in also likely to be a different PID namespace).
Now this is all a bit of a shame really. It would be nice if
linux_proc_pid_to_exec_file could return /proc/PID/exe in such a way
that exec_file_find would know that the file should NOT be looked for
in the sysroot, or in the alternative namespace. But fixing that
problem would be a much bigger change, so for now lets just disable
the /proc/PID/exe fallback for cases where it might not work.
For testing, the sysroot case is now tested.
I don't believe we have any alternative namespace testing. It would
certainly be interesting to add some, but I'm not proposing any with
this patch, so the code for checking the MNT namespace has been tested
manually by me, but isn't covered by a new test I'm adding here.
Author of the original fix is listed as co-author here. Credit for
identifying the original problem, and proposing a solution belongs to
them.
Co-Authored-By: Fabian Kilger <kilger@sec.in.tum.de>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32955
|
|
Tom de Vries reported a build failure on x86_64-w64-mingw32 after
commit:
commit bd389c9515d240f55b117075b43184efdea41287
Date: Wed Jun 11 22:52:16 2025 +0200
gdb: implement linux namespace support for fileio_lstat and vFile::lstat
The build failure looks like this:
../../src/gdbserver/hostio.cc: In function 'void handle_lstat(char*, int*)':
../../src/gdbserver/hostio.cc:544:63: error: cannot convert '_stat64*' to 'stat*'
544 | ret = the_target->multifs_lstat (hostio_fs_pid, filename, &st);
| ^~~
| |
| _stat64*
In file included from ./../../src/gdbserver/server.h:58,
from <command-line>:
./../../src/gdbserver/target.h:448:74: note: initializing argument 3 of 'virtual int process_stratum_target::multifs_lstat(int, const char*, stat*)'
448 | virtual int multifs_lstat (int pid, const char *filename, struct stat *sb);
| ~~~~~~~~~~~~~^~
The problem is that in sys/stat.h for mingw, 'stat' is #defined to
_stat64, but target.h doesn't include sys/stat.h, and so doesn't see
this #define.
However, target.h does, by luck, manages to see the actual definition
of 'struct stat', which isn't in sys/stat.h itself, but is in some
other header that just happens to be pulled in by chance.
As a result of all this, the declaration of
process_stratum_target::multifs_lstat in target.h uses 'struct stat'
for its argument type, while the call in hostio.cc, uses 'struct
_stat64' as its argument type, which causes the build error seen
above.
The fix is to include sys/stat.h in target.h so that the declaration's
argument type will change to 'struct _stat64' (via the #define).
|
|
Currently gdbserver uses the require_int() function to parse the
requested offset (in vFile::pread packet and the like). This function
allows integers up to 0x7fffffff (to fit in 32-bit int), however the
offset (for the pread system call) has an off_t type which can be
larger than 32-bit.
This patch allows require_int() function to parse offset up to the
maximum value implied by the off_t type.
Approved-By: Pedro Alves <pedro@palves.net>
Change-Id: I3691bcc1ab1838c0db7f8b82d297d276a5419c8c
|
|
The new algorithm to look for a build-id-based debug file
(introduced by commit 22836ca88591ac7efacf06d5b6db191763fd8aba)
makes use of fileio_lstat. As lstat was not supported by
linux-namespace.c, all lstat calls would be performed on the host
and not inside the namespace. Fixed by adding namespace lstat
support.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32956
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
This commit continues the work of the previous two commits.
In the following commits I added the target_fileio_stat function, and
the target_ops::fileio_stat member function:
* 08a115cc1c4 gdb: add target_fileio_stat, but no implementations yet
* 3055e3d2f13 gdb: add GDB side target_ops::fileio_stat implementation
* 6d45af96ea5 gdbserver: add gdbserver support for vFile::stat packet
* 22836ca8859 gdb: check for multiple matching build-id files
Unfortunately I messed up, despite being called 'stat' these function
actually performed an 'lstat'. The 'lstat' is the correct (required)
implementation, it's the naming that is wrong.
Additionally, to support remote targets, these commit added the
vFile::stat packet, which again, performed an 'lstat'.
In the previous two commits I changed the GDB code to replace 'stat'
with 'lstat' in the fileio function names. I then added a new
vFile:lstat packet which GDB now uses instead of vFile:stat.
And that just leaves the vFile:stat packet which is, right now,
performing an 'lstat'.
Now, clearly when I wrote this code I fully intended for this packet
to perform an lstat, it's the lstat that I needed. But now, I think,
we should "fix" vFile:stat to actually perform a 'stat'.
This is risky. This is a change in remote protocol behaviour.
Reasons why this might be OK:
- vFile:stat was only added in GDB 16, so it's not been "in the
wild" for too long yet. If we're quick, we might be able to "fix"
this before anyone realises I messed up.
- The documentation for vFile:stat is pretty vague. It certainly
doesn't explicitly say "this does an lstat". Most implementers
would (I think), given the name, start by assuming this should be
a 'stat' (given the name). Only if they ran the full GDB
testsuite, or examined GDB's implementation, would they know to
use lstat.
Reasons why this might not be OK:
- Some other debug client could be connecting to gdbserver, sending
vFile:stat and expecting to get lstat behaviour. This would break
after this patch.
- Some other remote server might have implemented vFile:stat
support, and either figured out, or copied, the lstat behaviour
from gdbserver. This remote server would technically be wrong
after this commit, but as GDB no longer uses vFile:stat, then this
will only become a problem if/when GDB or some other client starts
to use vFile:stat in the future.
Given the vague documentation for vFile:stat, and that it was only
added in GDB 16, I think we should fix it now to perform a 'stat', and
that is what this commit does.
The change in behaviour is documented in the NEWS file. I've improved
the vFile:stat documentation in the manual to better explain what is
expected from this packet, and I've extended the existing test to
cover vFile:stat.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
In the following commits I added the target_fileio_stat function, and
the target_ops::fileio_stat member function:
* 08a115cc1c4 gdb: add target_fileio_stat, but no implementations yet
* 3055e3d2f13 gdb: add GDB side target_ops::fileio_stat implementation
* 6d45af96ea5 gdbserver: add gdbserver support for vFile::stat packet
* 22836ca8859 gdb: check for multiple matching build-id files
Unfortunately I messed up, despite being called 'stat' these function
actually performed an 'lstat'. The 'lstat' is the correct (required)
implementation, it's the naming that is wrong.
In the previous commit I fixed the naming within GDB, renaming 'stat'
to 'lstat' throughout.
However, in order to support target_fileio_stat (as was) on remote
targets, the above patches added the vFile:stat packet, which actually
performed an 'lstat' call. This is really quite unfortunate, and I'd
like to do as much as I can to try and clean up this mess. But I'm
mindful that changing packets is not really the done thing.
So, this commit doesn't change anything.
Instead, this commit adds vFile:lstat as a new packet.
Currently, this packet is handled identically as vFile:stat, the
packet performs an 'lstat' call.
I then update GDB to send the new vFile:lstat instead of vFile:stat
for the remote_target::fileio_lstat implementation.
After this commit GDB will never send the vFile:stat packet.
However, I have retained the 'set remote hostio-stat-packet' control
flag, just in case someone was trying to set this somewhere.
Then there's one test in the testsuite which used to disable the
vFile:stat packet, that test is updated to now disable vFile:lstat.
There's a new test that does a more direct test of vFile:lstat. This
new test can be extended to also test vFile:stat, but that is left for
the next commit.
And so, after this commit, GDB sends the new vFile:lstat packet in
order to implement target_ops::fileio_lstat. The new packet is more
clearly documented than vFile:stat is. But critically, this change
doesn't risk breaking any other clients or servers that implement
GDB's remote protocol.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Gdbsupport functions phex and phex_nz have a parameter sizeof_l:
...
extern const char *phex (ULONGEST l, int sizeof_l);
extern const char *phex_nz (ULONGEST l, int sizeof_l);
...
and a lot of calls use:
...
phex (l, sizeof (l))
...
Make this easier by reimplementing the functions as a template, allowing us to
simply write:
...
phex (l)
...
Simplify existing code using:
...
$ find gdb* -type f \
| xargs sed -i 's/phex (\([^,]*\), sizeof (\1))/phex (\1)/'
$ find gdb* -type f \
| xargs sed -i 's/phex_nz (\([^,]*\), sizeof (\1))/phex_nz (\1)/'
...
and manually review:
...
$ find gdb* -type f | xargs grep "phex (.*, sizeof.*)"
$ find gdb* -type f | xargs grep "phex_nz (.*, sizeof.*)"
...
Tested on x86_64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This is a refactoring commit. When passing inferior arguments to
gdbserver we have two actions that need to be performed, splitting and
joining.
On the GDB side, we take the inferior arguments, a single string, and
split the string into a list of individual arguments. These are then
sent to gdbserver over the remote protocol.
On the gdbserver side we receive the list of individual arguments and
join these back together into a single inferior argument string.
In the next commit I plan to add some unit testing for this remote
argument passing process. Ideally, for unit testing, we need the code
being tested to be located in some easily callable function, rather
than being inline at the site of use.
So in this commit I propose to move the splitting and joining logic
out into a separate file, we can then use this within GDB and
gdbserver when passing arguments between GDB and gdbserver, but we can
also call the same functions for some unit testing.
In this commit I'm not adding the unit tests, they will be added next,
so for now there should be no user visible changes after this commit.
Tested-By: Guinevere Larsen <guinevere@redhat.com>
|
|
This updates the copyright headers to include 2025. I did this by
running gdb/copyright.py and then manually modifying a few files as
noted by the script.
Approved-By: Eli Zaretskii <eliz@gnu.org>
|
|
Since commit 84da4a1ea0ae ("gdbserver: refactor the definition and uses of
supply_regblock") there is no case where supply_regblock is passed a
nullptr for the BUF argument, and there is even a gdb_assert to make
sure of it.
Therefore remove that part of the documentation comment.
|
|
Commit 51e6b8cfd649013ae16a3d00f1451b2531ba6bc9 fixed a
regression for SVE/SME registers on gdb's side by using a <= comparison for
regcache's raw_compare assertion check. We seem to have failed to do the same
for gdbserver's raw_compare counterpart.
With the code as it is, I'm seeing a lot of crashes for gdbserver on a machine
with SVE enabled. For instance, with the following invocation:
make check-gdb RUNTESTFLAGS="--target_board=native-gdbserver" TESTS=gdb.base/break.exp
Running /work/builds/binutils-gdb/gdb/testsuite/../../../../repos/binutils-gdb/gdb/testsuite/gdb.base/break.exp ...
FAIL: gdb.base/break.exp: test_break: run until function breakpoint
FAIL: gdb.base/break.exp: test_break: run until breakpoint set at a line number (the program is no longer running)
FAIL: gdb.base/break.exp: test_break: run until file:function(6) breakpoint (the program is no longer running)
FAIL: gdb.base/break.exp: test_break: run until file:function(5) breakpoint (the program is no longer running)
FAIL: gdb.base/break.exp: test_break: run until file:function(4) breakpoint (the program is no longer running)
FAIL: gdb.base/break.exp: test_break: run until file:function(3) breakpoint (the program is no longer running)
FAIL: gdb.base/break.exp: test_break: run until file:function(2) breakpoint (the program is no longer running)
FAIL: gdb.base/break.exp: test_break: run until file:function(1) breakpoint (the program is no longer running)
FAIL: gdb.base/break.exp: test_break: run until quoted breakpoint (the program is no longer running)
FAIL: gdb.base/break.exp: test_break: run until file:linenum breakpoint (the program is no longer running)
FAIL: gdb.base/break.exp: test_break: breakpoint offset +1
FAIL: gdb.base/break.exp: test_break: step onto breakpoint (the program is no longer running)
FAIL: gdb.base/break.exp: test_break: setting breakpoint at }
FAIL: gdb.base/break.exp: test_break: continue to breakpoint at } (the program is no longer running)
FAIL: gdb.base/break.exp: test_no_break_on_catchpoint: runto: run to main
FAIL: gdb.base/break.exp: test_break_nonexistent_line: runto: run to main
FAIL: gdb.base/break.exp: test_break_default: runto: run to main
FAIL: gdb.base/break.exp: test_break_silent_and_more: runto: run to main
FAIL: gdb.base/break.exp: test_break_line_convenience_var: runto: run to main
FAIL: gdb.base/break.exp: test_break_user_call: runto: run to main
FAIL: gdb.base/break.exp: test_finish_arguments: runto: run to main
FAIL: gdb.base/break.exp: test_next_with_recursion: kill program
FAIL: gdb.base/break.exp: test_next_with_recursion: run to factorial(6)
FAIL: gdb.base/break.exp: test_next_with_recursion: continue to factorial(5) (the program is no longer running)
FAIL: gdb.base/break.exp: test_next_with_recursion: backtrace from factorial(5)
FAIL: gdb.base/break.exp: test_next_with_recursion: next to recursive call (the program is no longer running)
FAIL: gdb.base/break.exp: test_next_with_recursion: next over recursive call (the program is no longer running)
FAIL: gdb.base/break.exp: test_next_with_recursion: backtrace from factorial(5.1)
FAIL: gdb.base/break.exp: test_next_with_recursion: continue until exit at recursive next test (the program is no longer running)
FAIL: gdb.base/break.exp: test_break_optimized_prologue: run until function breakpoint, optimized file
FAIL: gdb.base/break.exp: test_break_optimized_prologue: run until breakpoint set at small function, optimized file (the program is no longer running)
FAIL: gdb.base/break.exp: test_rbreak_shlib: rbreak junk
Adjusting the regcache raw_compare assertion check to use <= fixes
the problem on aarch64-linux on a SVE-capable system.
This patch also adds a simple selftest to gdbserver that validates this
particular case by simulating a raw_compare operation.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32775
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Signed-off-by: David Holsgrove <david.holsgrove@petalogix.com>
Signed-off-by: Nathan Rossi <nathan.rossi@petalogix.com>
Signed-off-by: Mahesh Bodapati <mbodapat@xilinx.com>
Signed-off-by: Gopi Kumar Bulusu <gopi@sankhya.com>
Signed-off-by: Michael Eager <eager@eagercon.com>
|
|
Add a pre-commit codespell hook for directories gdbsupport and gdbserver,
which are codespell-clean:
...
$ pre-commit run codespell --all-files
codespell................................................................Passed
...
A non-trivial question is where the codespell configuration goes.
Currently we have codespell sections in gdbsupport/setup.cfg and
gdbserver/setup.cfg, but codespell doesn't automatically use those because the
pre-commit hook runs codespell at the root of the repository.
A solution would be to replace those 2 setup.cfg files with a setup.cfg in the
root of the repository. Not ideal because generally we try to avoid adding
files related to subdirectories at the root.
Another solution would be to add two codespell hooks, one using
--config gdbsupport/setup.cfg and one using --config gdbserver/setup.cfg, and
add a third one once we start supporting gdb. Not ideal because it creates
duplication, but certainly possible.
I went with the following solution: a setup.cfg file in gdb/contrib (alongside
codespell-ignore-words.txt) which is used for both gdbserver and gdbsupport.
So, what can this new setup do for us? Let's demonstrate by simulating a typo:
...
$ echo "/* aways */" >> gdbsupport/agent.cc
...
We can check unstaged changes before committing:
...
$ pre-commit run codespell --all-files
codespell................................................................Failed
- hook id: codespell
- exit code: 65
gdbsupport/agent.cc:282: aways ==> always, away
...
Likewise, staged changes (no need for the --all-files):
...
$ git add gdbsupport/agent.cc
$ pre-commit run codespell
codespell................................................................Failed
- hook id: codespell
- exit code: 65
gdbsupport/agent.cc:282: aways ==> always, away
...
Or we can try to commit, and run into the codespell failure:
...
$ git commit -a
black................................................(no files to check)Skipped
flake8...............................................(no files to check)Skipped
isort................................................(no files to check)Skipped
codespell................................................................Failed
- hook id: codespell
- exit code: 65
gdbsupport/agent.cc:282: aways ==> always, away
check-include-guards.................................(no files to check)Skipped
...
which makes the commit fail.
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Fix a typo:
...
$ codespell --config gdbserver/setup.cfg gdbserver/tracepoint.cc
gdbserver/tracepoint.cc:951: mistakingly ==> mistakenly
...
|
|
The function remove_thread() was changed to a method in 2500e7d7d (gdbserver:
make remove_thread a method of process_info).
Signed-off-by: Wataru Ashihara <wsh@iij.ad.jp>
Change-Id: I4b2d8a6f84b5329b8d450b268fa9453fe424914e
|
|
The function construct_inferior_arguments (gdbsupport/common-inferior.cc)
currently escapes all special shell characters. After this commit
there will be two "levels" of quoting:
1. The current "full" quoting, where all posix shell special
characters are quoted, and
2. a new "reduced" quoting, where only the characters that GDB sees
as special (quotes and whitespace) are quoted.
After this, almost all construct_inferior_arguments calls will use the
"full" quoting, which is the current quoting. The "reduced" quoting
will be used in this commit to restore the behaviour that was lost in
the previous commit (more details below).
In the future, the reduced quoting will be useful for some additional
inferior argument that I have planned. I already posted my full
inferior argument work here:
https://inbox.sourceware.org/gdb-patches/cover.1730731085.git.aburgess@redhat.com
But that series is pretty long, and wasn't getting reviewed, so I'm
posted the series in parts now.
Before the previous commit, GDB behaved like this:
$ gdb -eiex 'set startup-with-shell off' --args /tmp/exec '$FOO'
(gdb) show args
Argument list to give program being debugged when it is started is "$FOO".
Notice that with 'startup-with-shell' off, the argument was left as
just '$FOO'. But after the previous commit, this changed to:
$ gdb -eiex 'set startup-with-shell off' --args /tmp/exec '$FOO'
(gdb) show args
Argument list to give program being debugged when it is started is "\$FOO".
Now the '$' is escaped with a backslash. This commit restores the
original behaviour, as this is (currently) the only way to unquoted
shell special characters into arguments from the GDB command line.
The series that I listed above includes a new command line option for
GDB which provides a better approach for controlling the quoting of
special shell characters, but that work requires these patches to be
merged first.
I've split out the core of construct_inferior_arguments into the new
function escape_characters, which takes a set of characters to escape.
Then the two functions escape_shell_characters and
escape_gdb_characters call escape_characters with the appropriate
character sets.
Finally, construct_inferior_arguments, now takes a boolean which
indicates if we should perform full shell escaping, or just perform
the reduced escaping.
I've updated all uses of construct_inferior_arguments to pass a
suitable value to indicate what escaping to perform (mostly just
'true', but one case in main.c is different), also I've updated
inferior::set_args to take the same boolean flag, and pass it through
to construct_inferior_arguments.
Tested-By: Guinevere Larsen <guinevere@redhat.com>
|
|
Add a few -Wunused-* diagnostic flags that look useful. Some are known
to gcc, some to clang, some to both. Fix the fallouts.
-Wunused-const-variable=1 is understood by gcc, but not clang.
-Wunused-const-variable would be undertsood by both, but for gcc at
least it would flag the unused const variables in headers. This doesn't
make sense to me, because as soon as one source file includes a header
but doesn't use a const variable defined in that header, it's an error.
With `=1`, gcc only warns about unused const variable in the main source
file. It's not a big deal that clang doesn't understand it though: any
instance of that problem will be flagged by any gcc build.
Change-Id: Ie20d99524b3054693f1ac5b53115bb46c89a5156
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Put them one per line and sort alphabetically.
Change-Id: Idb6947d444dc6e556a75645b04f97a915bba7a59
Approved-By: Tom Tromey <tom@tromey.com>
|
|
In gdbserver/xtensa-xtregs.cc, there's a table:
...
const xtensa_regtable_t xtensa_regmap_table[] = {
/* gnum,gofs,cpofs,ofs,siz,cp, dbnum, name */
{ 44, 176, 0, 0, 4, -1, 0x020c, "scompare1" },
{ 0 }
};
...
on which codespell triggers:
...
$ codespell --config ./gdbserver/setup.cfg gdbserver
gdbserver/xtensa-xtregs.cc:34: siz ==> size, six
...
Fix this by laying out the table in vertical fashion, and using the full field
names instead of the abbreviations ("size" instead of "siz", "offset" instead
of "ofs", etc).
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Add a codespell section in new config file gdbserver/setup.cfg, similar to the
one in gdbsupport/setup.cfg.
There's just one item left:
...
$ codespell --config ./gdbserver/setup.cfg gdbserver
gdbserver/xtensa-xtregs.cc:34: siz ==> size, six
...
|
|
Fix typos in gdbserver:
...
gdbreplay.cc:444: substract ==> subtract
notif.cc:35: Enque ==> Enqueue
notif.cc:42: enque ==> enqueue
i387-fp.cc:233: simplifed ==> simplified
i387-fp.cc:508: simplifed ==> simplified
linux-arc-low.cc:221: shoudn't ==> shouldn't
linux-sparc-low.cc:112: ans ==> and
linux-ppc-low.cc:1134: Followings ==> Following
linux-ppc-low.cc:1160: Followings ==> Following
linux-ppc-low.cc:1193: Followings ==> Following
linux-ppc-low.cc:1226: Followings ==> Following
configure.ac:141: defintions ==> definitions
...
Regenerate configure from configure.ac using autoconf.
|
|
I noticed a
// namespace selftests
comment, which doesn't follow our comment formatting convention. I did
a find & replace to fix all the offenders.
Change-Id: Idf8fe9833caf1c3d99e15330db000e4bab4ec66c
|
|
GDB prints the target id of a thread in various places such as the
output of the "info threads" command in the "Target Id" column or when
switching to a thread. A target can define what to print for a given
ptid by overriding the `pid_to_str` method.
The remote target is a gateway behind which one of many various
targets could be running. The remote target converts a given ptid to
a string in a uniform way, without consulting the low target at the
server-side.
In this patch we introduce a new attribute in the XML that is sent in
response to the "qXfer:threads:read" RSP packet, so that a low target
at the server side, if it wishes, can specify what to print as the
target id of a thread.
Note that the existing "name" attribute or the "extra" text provided
in the XML are not sufficient for the server-side low target to
achieve the goal. Those attributes, when present, are simply appended
to the target id by GDB.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Reviewed-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Replace the few uses of `std::unordered_map` in gdbserver with
`gdb::unordered_map`.
The only one of these that is likely to ever see a lot of elements is
probably `process_info::m_ptid_thread_map`. It was added precisely to
improve performance when there are a lot of threads, so I guess using
`gdb::unordered_map` here won't hurt. I changed the others too, since
it's easy.
Change-Id: Ibc4ede5245551fdd7717cb349a012d05726f4363
Reviewed-By: Stephan Rohr <stephan.rohr@intel.com>
|
|
When the changes on the remote protocol are made,
we want to test all the corner cases to prevent
regressions. Currently it can be tricky to simulate
some corner case conditions that would expose possible
regressions. When I want to add or change the remote
protocol packet, I need to hack gdbserver to send a
corrupted packet or an error to make sure GDB is able
to handle such a case.
This test makes it easy to send a corruped packet or
an error message to GDB using the gdbreplay tool and
check GDB deals with it as we expect it to.
This test starts a communication with gdbsever setting
the remotelog file. Then, it modifies the remotelog with
update_log proc, injects an error message instead of
the expected replay to the vMustReplyEmpty packet in order
to test GDB reacts to the error response properly. After
the remotelog modification, this test restarts GDB and starts
communication with gdbreply instead of the gdbserver using
the remotelog.
Add a lib/gdbreplay-support.exp. update_log proc matches lines
from GDB to gdbserver in a remotelogfile. Once a match is found then
the custom line is used to build a replacement line to send from
gdbserver to GDB.
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
The register_status field of regcache is declared as `unsigned char *`.
This is incorrect, because `enum register_status` from
gdbsupport/common-regcache.h is based on signed char and
REG_UNAVAILABLE is defined as -1. Fix the declared type.
Now that we are modifying the declaration, also use a unique_ptr
and make the field private.
The get/set methods already use the correct type, but we update cast
operations in two places.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
The supply_regblock function takes a pointer to a buffer as an
argument and implements two different behavior based on the pointer
being null. There are two cases where we pass nullptr, all in
tracepoint.cc, where we are essentially doing a reset on the regcache.
In fast_tracepoint_ctx::regcache, register_status array does not
even exist. Hence, that use simply boils down to zeroing of register
data. Do this at the time of creating the buffer and remove the call
to supply_regblock.
In fetch_traceframe_registers, inline the use with a call to `reset`.
Hence, there are no more cases left, where a nullptr would be passed
to supply_regblock. Assert that the buffer argument is non-null and
simplify the implementation.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Define a `reset` method for a regcache and use it for code
simplification. This patch allows further simplification in the next
patch.
The reset method fills the register data with zeroes. For the use in
get_thread_regcache, this is added behavior, making the patch not a
pure refactoring, and may look like extra overhead. However, it is
better to avoid having arbitrary values left in the data buffer.
Hence, it is considered a behavioral improvement.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
When a regcache is initialized, the values of registers are not
fetched yet. Thus, initialize the register statuses to REG_UNKNOWN
instead of REG_UNAVAILABLE, because the latter rather means "we
attempted to fetch but could not obtain the value".
The definitions of the reg status enums (from
gdbsupport/common-regcache.h) as a reminder:
/* The register value is not in the cache, and we don't know yet
whether it's available in the target (or traceframe). */
REG_UNKNOWN = 0,
/* The register value is valid and cached. */
REG_VALID = 1,
/* The register value is unavailable. E.g., we're inspecting a
traceframe, and this register wasn't collected. Note that this
"unavailable" is different from saying the register does not
exist in the target's architecture --- in that case, the target
should have given us a target description that does not include
the register in the first place. */
REG_UNAVAILABLE = -1
Similarly, when the regcache is invalidated, change all the statuses
back to REG_UNKNOWN.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Store the regcache pointer in thread_info as a unique_ptr. This
allows us delete the thread_info destructor.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Convert the `free_register_cache` function into a destructor of the
regcache struct. In one place, we completely remove the call to free
the regcache object by stack-allocating the object.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
