I spotted this call missing an argument.
For context, init_target_desc gained this osabi parameter in this
commit:
Author: Andrew Burgess <aburgess@redhat.com>
AuthorDate: Fri Oct 4 19:30:04 2024 +0100
Commit: Andrew Burgess <aburgess@redhat.com>
CommitDate: Tue Nov 12 12:51:36 2024 +0000
gdbserver: pass osabi to GDB in more target descriptions
This bug was present in GDB 16. I wonder if anybody uses this today.
Change-Id: Id5483be3efa0ca9d238d59af8abae94e8bdbd57c
Approved-By: Tom Tromey <tom@tromey.com>
The tix6x gdbserver port was modified to use target descriptions in
commit 506fe5f499 ("Change tic6x target descriptions"). The old
regformats .dat files were kept as a way to make sure the new target
descriptions matched the old register decsriptions. I think by now it's
not necessary to keep the .dat files.
I don't have a way to build-test this though.
Change-Id: Ia90b5ae6381234c6e95555201d3e65ed9be880ea
Approved-By: Tom Tromey <tom@tromey.com>
We had a customer bug report which was eventually tracked down to
gdbserver not fully sending a target description to gdb. (This
presented as a timeout on the gdb side.)
The customer was using the WINAPI code, which does this:
# define write(fd, buf, len) send (fd, (char *) buf, len, 0)
In this setup, I think it's possible to have a partial write.
However, gdbserver does not account for this possibility, despite the
fact that write_prim documents this.
This patch attempts to fix the problem by always writing the full
buffer in write_prim. In this case the customer fixed their bug in a
different way, so we haven't actually tested this in the wild.
v2: Return bool from write_prim.
Reviewed-by: Kévin Le Gouguec <legouguec@adacore.com>
I noticed my IDE (VSCode) starting to automatically trim trailing
whitespaces on save, despite the setting for it being disabled. I
realized that this is because the .editorconfig file now has
trim_trailing_whitespace = true
for many file types. If we have this EditorConfig setting forcing
editors to trim trailing whitespaces, I think it would make sense to
clean up trailing whitespaces from our files. Otherwise, people will
always get spurious whitespace changes when editing these files.
I did a mass cleanup using this command:
$ find gdb gdbserver gdbsupport -type f \( \
-name "*.c" -o \
-name "*.h" -o \
-name "*.cc" -o \
-name "*.texi" -o \
-name "*.exp" -o \
-name "*.tcl" -o \
-name "*.py" -o \
-name "*.s" -o \
-name "*.S" -o \
-name "*.asm" -o \
-name "*.awk" -o \
-name "*.ac" -o \
-name "Makefile*" -o \
-name "*.sh" -o \
-name "*.adb" -o \
-name "*.ads" -o \
-name "*.d" -o \
-name "*.go" -o \
-name "*.F90" -o \
-name "*.f90" \
\) -exec sed -ri 's/[ \t]+$//' {} +
I then did an autotools regen, because we don't actually want to change
the Makefile and Makefile.in files that are generated.
Change-Id: I6f91b83e3b8c4dc7d5d51a2ebf60706120efe691
This commit adds a new remote protocol packet qExecAndArgs, and
updates GDB to use it.
When gdbserver is started a user can provide an executable and
arguments, these are used (by the remote target) to start an initial
inferior, this is the inferior to which GDB first connects.
When GDB is connected in extended-remote mode, if the user does a
'run' without specifying a new 'remote exec-file' then the executable
given on the gdbserver command line is reused to start the new
inferior.
Interestingly, the arguments given on the gdbserver command line are
only used when starting the first inferior, subsequent inferiors will
be passed an empty argument string by GDB. This might catch out a
user, causing the rerun to behave differently than the first run.
In this commit I will add a new qExecAndArgs packet, which I think
will improve the experience in this area.
The new qExecAndArgs packet is sent from GDB, and gdbserver replies
with a packet that includes the executable filename and the arguments
string that were used for starting the initial inferior.
On the GDB side this information can be used to update GDB's state,
the 'show remote exec-file' will reflect how gdbserver was started,
and 'show args' will reflect the arguments used for starting the
inferior.
As a result of updating the args, if the user restarts the inferior,
then this same argument string will be passed back to the remote
target, and used for the new inferior. Thus, rerunning the inferior
will behave just like the initial inferior, which I think is a good
improvement.
Finally, GDB will warn if the user has 'set remote exec-file' and
then connects to a gdbserver that was started with some alternative
filename, like this:
(gdb) set remote exec-file /tmp/foo
(gdb) target remote | gdbserver --once - /tmp/bar
... snip ...
warning: updating 'remote exec-file' to '/tmp/bar' to match remote target
... snip ...
I made the choice to have GDB update the remote exec-file setting to
match the remote, as, after the 'target remote', we are connected to
an inferior that is running /tmp/bar (in this case), so trying to hang
onto the non-matching user supplied setting doesn't seem helpful.
There is one case where I can see this choice being a problem, if a
user does:
(gdb) set remote exec-file /tmp/foo
(gdb) target extended-remote | gdbserver --multi --once - /tmp/bar
... snip ...
warning: updating 'remote exec-file' to '/tmp/bar' to match remote target
... snip ...
(gdb) run
In this case, prior to this patch, they would 'run' /tmp/foo, while
after this patch, they will run /tmp/bar. I think it is unfortunate
that I'm breaking this use case, but, I'm not _that_ sorry -- just
start gdbserver with the correct executable, or even no executable,
and the problem goes away.
This last point is important, in extended-remote mode, it is possible
to start gdbserver without specifying an executable, like this:
$ gdbserver --multi --once :54321
In this case gdbserver doesn't start an initial inferior. When GDB
connects the qExecAndArgs reply from gdbserver indicates that no
information (executable or arguments) were set, and any existing
information is retained, as in this session:
(gdb) set sysroot
(gdb) set remote exec-file /tmp/foo
(gdb) set args a b c
(gdb) target extended-remote | ./gdbserver/gdbserver --multi --once -
Remote debugging using | ./gdbserver/gdbserver --multi --once -
Remote debugging using stdio
(gdb) show remote exec-file
The remote exec-file is "/tmp/foo".
(gdb) show args
Argument list to give program being debugged when it is started is "a b c".
(gdb)
This is the second time proposing this new packet. The first attempt
can be found here:
https://inbox.sourceware.org/gdb-patches/80d8b37d757033976b1a8ddd370c294c7aae8f8c.1692200989.git.aburgess@redhat.com
The review feedback on this patch was that the inferior arguments
should be passed back as a vector of individual strings. This makes
sense, at the time that feedback was given, GDB would pass arguments
to gdbserver as a vector of individual arguments, so it would seem
sensible that gdbserver should adopt the same approach for passing
arguments back to GDB.
However, since then I have been working on how GDB passes the inferior
arguments to gdbserver, fixing a lot of broken corner cases, which
culminated in this patch:
commit 8e28eef6cd
Date: Thu Nov 23 18:46:54 2023 +0000
gdb/gdbserver: pass inferior arguments as a single string
Though we do retain the vector of individual arguments behaviour for
backward compatibility with old remote targets, the preferred approach
now is for GDB to pass arguments to gdbserver as a single string.
This removes the need for GDB/gdbserver to try and figure out what is
the correct escaping to apply to the arguments, and fixes some
argument passing corner cases.
And so, now, I think it makes sense that gdbserver should also pass
the arguments back to GDB as a single string. I've updated the
documentation a little to (I hope) explain how gdbserver should escape
things before passing them back to GDB (TLDR: no additional escaping
should be added just for sending to GDB. The argument string should
be sent to GDB as if it were being sent to the 'set args' GDB
command).
The main test for this new functionality is
gdb.server/fetch-exec-and-args.exp, but I've also added a test
gdb.replay/fetch-exec-and-args.exp, which allows me to test a corner
case that isn't currently exercised by gdbserver, this is the case for
sending pack inferior arguments, but no executable.
The qExecAndArgs reply format is 'S;exec;args;' where 'exec' and
'args' are hex encoded strings. If 'args' is empty then this is
perfectly valid, this just means there were no command line
arguments. But what if 'exec' is empty? I needed to decide what to
do in this case. The easiest choice is to treat empty 'exec' as the
executable is not set. But currently, due to how gdbserver works, it
is not possible to hit this case, so I used the gdbreplay testing
framework to exercise this instead. There were a few supporting
changes needed to write this test though.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
While testing another patch I'm working on I discovered that passing
an empty program name to gdbserver would trigger an assertion, like
this:
$ gdbserver --multi :54321 ""
../../gdb/gdbserver/../gdb/nat/fork-inferior.c:240: A problem internal to GDBserver has been detected.
fork_inferior: Assertion `exec_file != nullptr' failed.
User input, no matter how weird, shouldn't be triggering an assertion,
so lets fix that.
In extended mode, it is valid to start gdbserver without an executable
name, like this:
$ gdbserver --multi :54321
Here gdbserver doesn't start an inferior, and it is up to GDB to
connect, and tell gdbserver what to run, and to then start it running.
I did wonder if the empty string case should handled like the no
executable name case, but then you get into the situation where the
user can specify command line arguments without an inferior, like:
$ gdbserver --multi :54321 "" a b c
And while there's nothing really wrong with this, and I'm sure someone
could come up with a use case for it. I'd like to propose that for
now at least, we take the simple approach of not allowing an empty
executable name, instead we should give an error, like this:
$ gdbserver --multi :54321 ""
No program to debug
Exiting
We can always relax this requirement in the future, and allow the
empty executable with or without inferior arguments, if we decide
there's a compelling reason for it. It would be simple enough to add
this in the future, but once we add support for it, it's much harder
to remove the feature in the future, so lets start simple.
The non-extended remote case works much the same. It too triggers the
assertion currently, and after this patch exits with the same error.
Of course, the non-extended remote case never supported not having an
inferior, if you did:
$ gdbserver :54321
You'd be shown the usage text and gdbserver would exit.
Approved-By: Tom Tromey <tom@tromey.com>
It affects following files:
- gdb/riscv-linux-tdep.c: a function to get syscall number.
- gdbserver/linux-riscv-low.cc: syscall trapinfo function to enable
catching syscalls on remote targets.
- gdb/syscalls/riscv-linux.xml.in: a file with syscalls, generated from
linux kernel sources using gdb/syscalls/update-linux-from-src.sh script.
- gdb/syscalls/riscv-linux.xml: a file with syscalls, patched with group
names gdb/syscalls/apply-defaults.xsl using xsltproc tool.
- gdb/syscalls/update-linux.sh: set startyear to 2025 on riscv.
- gdb/syscalls/update-linux-from-src.sh: riscv syscall table must be
generated from kernel headers.
- gdb/NEWS: catch-syscall feature is now available on riscv.
- gdb/data-directory/Makefile.in: adding file with syscalls to Makefile.
Approved-By: Tom Tromey <tom@tromey.com>
By passing ':' within the optstring to getopt_long, the getopt_long
call will now return ':' for missing value errors and '?' for unknown
argument errors, rather than returning '?' for all error types.
We can now print a different error message for missing argument
values. For example:
$ gdbserver --debug-file :54321 /tmp/hello
Missing argument value for: --debug-file
Compared to:
$ gdbserver --unknown :54321 ~/tmp/hello.x
Unknown argument: --unknown
Current HEAD gdbserver treats every error as the 'Unknown argument'
error.
While I was messing with the code that prints these error messages,
I've wrapped then with _(...) to allow for internationalisation.
Approved-By: Tom Tromey <tom@tromey.com>
Now that we use getopt_long for argument processing in gdbserver, it
is relatively easy to support GNU style arguments, that is, arguments
passed without an '=' between the argument and the value.
As support for GNU style arguments is the default from getopt_long,
the first part of this commit is to remove the code which deliberately
disables the GNU argument support.
With that done, we now need to consider optional arguments. In this
case, getopt_long doesn't automatically grab the next word from ARGV
to be the argument value, so I've added some code to do this.
I've also tried to make this code a little smart. As the first
argument passed to gdbserver that doesn't have a '--' at the start is
the PORT number, the new code block I've added tries to spot if the
argument value might be the port number. If it is, then we don't
allow the port number to become the argument value, and instead, we
pretend the argument value is missing. This seems to give better
error messages.
There are going to be UI changes in how gdbserver handles incorrect
arguments after this commit. However, the behaviour for valid
command lines should be unchanged.
Approved-By: Tom Tromey <tom@tromey.com>
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>
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 bea571ebd7
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 bea571ebd7, 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 e58beedf2c
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 bd389c9515
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 22836ca885)
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:
* 08a115cc1c gdb: add target_fileio_stat, but no implementations yet
* 3055e3d2f1 gdb: add GDB side target_ops::fileio_stat implementation
* 6d45af96ea gdbserver: add gdbserver support for vFile::stat packet
* 22836ca885 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:
* 08a115cc1c gdb: add target_fileio_stat, but no implementations yet
* 3055e3d2f1 gdb: add GDB side target_ops::fileio_stat implementation
* 6d45af96ea gdbserver: add gdbserver support for vFile::stat packet
* 22836ca885 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 84da4a1ea0 ("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 51e6b8cfd6 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>
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>
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>
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
...
