Remove the 'struct' keyword in occurrences of 'struct thread_info'.
This is a code clean-up.
Tested by rebuilding.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Eli mentioned [1] that given that we use US English spelling in our
documentation, we should use "behavior" instead of "behaviour".
In wikipedia-common-misspellings.txt there's a rule:
...
behavour->behavior, behaviour
...
which leaves this as a choice.
Add an overriding rule to hardcode the choice to common-misspellings.txt:
...
behavour->behavior
...
and add a rule to rewrite behaviour into behavior:
...
behaviour->behavior
...
and re-run spellcheck.sh on gdb*.
Tested on x86_64-linux.
[1] https://sourceware.org/pipermail/gdb-patches/2024-November/213371.html
This function doesn't seem so useful, use `thread_info:🆔:pid`
directly instead.
Change-Id: I7450c4223e5b0bf66788eeb5b070ab6f5287f798
Reviewed-By: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
This function doesn't seem so useful. Use `thread_info::id` directly.
Change-Id: I158cd06a752badd30f68424e329aa42d275e43b7
Reviewed-By: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
I think it just makes things more obscure. Use `thread_info::id`
directly instead.
Change-Id: I141d5fb08ebf45c13cc32c4bba62773249fcb356
Reviewed-By: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Remove the `get_thread_process` function, use `thread_info::process`
instead.
In `server.cc`, use `current_process ()` instead of going through the
current thread.
Change-Id: Ifc61d65852e392d154b854a45d45df584ab3922e
Reviewed-By: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Replace the servers global thread list with a process specific thread
list and a ptid -> thread map similar to 'inferior::ptid_thread_map' on
GDB side. Optimize the 'find_thread' and 'find_thread_ptid' functions
to use std::unordered_map::find for faster lookup of threads without
iterating over all processes and threads, if applicable. This becomes
important when debugging applications with a large thread count, e.g.,
in the context of GPU debugging.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This patch replaces the 'std::list' type of 'all_processes' and
'all_threads' with the more lightweight 'owning_intrusive_list'
type.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Add handling of '.' in gdb/contrib/spellcheck.sh.
While we're at, simplify the sed invocation by using a single s command
instead of 3 s commands.
Also introduce sed_join and grep_join.
Fix the following common misspellings:
...
bandwith -> bandwidth
emmitted -> emitted
immediatly -> immediately
suprize -> surprise
thru -> through
transfered -> transferred
...
Verified with shellcheck.
Add two more separators in spellcheck.sh: colon and comma.
Doing so triggers the "inbetween->between" rule, which gives an incorrect
result. Override this with "inbetween->between, in between, in-between" [1],
in a new file gdb/contrib/common-misspellings.txt.
Fix the following common misspellings:
...
everytime -> every time
sucess -> success
thru -> through
transfered -> transferred
inbetween -> between, in between, in-between
...
Verified with spellcheck.sh. Tested on x86_64-linux.
[1] https://www.grammarly.com/blog/commonly-confused-words/in-between-or-inbetween/
Event tracing allows GDB to show information about interesting asynchronous
events when tracing with Intel PT. Subsequent patches will add support for
displaying each type of event.
Enabling event-tracing unconditionally would result in rather noisy output, as
breakpoints themselves result in interrupt events. Which is why this patch adds
a set/show command to allow the user to enable/disable event-tracing before
starting a recording. The event-tracing setting has no effect on an already
active recording. The default setting is off. As event tracing will use the
auxiliary infrastructure added by ptwrite, the user can still disable printing
events, even when event-tracing was enabled, by using the /a switch for the
record instruction-history/function-call-history commands.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Markus Metzger <markus.t.metzger@intel.com>
Currently, gdbserver hangs after stdin is closed while it tries to
write: "Remote side has terminated connection. GDBserver will reopen
the connection." This hang disappears if --once is also given. Since
the stdin connection won't ever reopen if it's closed, it's safe to
assume --once is desired.
The gdb.server/server-pipe.exp test was also updated to reflect this
change. There is now a second disconnect at the end of the proc,
with a tighter-than-normal timeout to catch if the command hangs as
it used to.
Co-Authored-By: Guinevere Larsen <blarsen@redhat.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29796
Approved-By: Andrew Burgess <aburgess@redhat.com>
This enables gdb and gdbserver to communicate about ptwrite support. If
ptwrite support would be enabled unconditionally, GDBs with older libipt
versions would break.
Approved-By: Markus Metzger <markus.t.metzger@intel.com>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
C++ 11 has a built-in attribute for this, no need to use a compat macro.
Change-Id: I90e4220d26e8f3949d91761f8a13cd9c37da3875
Reviewed-by: Lancelot Six <lancelot.six@amd.com>
Add two overloads of gdb_abspath, one which takes std::string and one
which takes gdb::unique_xmalloc_ptr<char>, then make use of these
overloads throughout GDB and gdbserver.
There should be no user visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
Add a new 'error_message' feature to the qSupported packet. When GDB
supports this feature then gdbserver is able to send
errors in the E.errtext format for the qRcmd and m packets.
Update qRcmd packet and m packets documentation as qRcmd newly
accepts errors in a E.errtext format.
Previously these two packets didn't support E.errtext style errors.
Approved-By: Tom Tromey <tom@tromey.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
I believe that the get_exec_file function is unnecessary, and the code
can be simplified if we remove it.
Consider for instance when you "run" a program on Linux with native
debugging.
1. run_command_1 obtains the executable file from
`current_program_space->exec_filename ()`
2. it passes it to `run_target->create_inferior()`, which is
`inf_ptrace_target::create_inferior()` in this case, which then
passes it to `fork_inferior()`
3. `fork_inferior()` then has a fallback, where if the passed exec file
is nullptr, it gets its from `get_exec_file()`.
4. `get_exec_file()` returns `current_program_space->exec_filename ()`
- just like the things we started with - or errors out if the
current program space doesn't have a specified executable.
If there's no exec filename passed in step 1, there's not going to be
any in step 4, so it seems pointless to call `get_exec_file()`, we could
just error out when `exec_file` is nullptr. But we can't error out
directly in `fork_inferior()`, since the error is GDB-specific, and that
function is shared with GDBserver.
Speaking of GDBserver, all code paths that lead to `fork_inferior()`
provide a non-nullptr exec file.
Therefore, to simplify things:
- Make `fork_inferior()` assume that the passed exec file is not
nullptr, don't call `get_exec_file()`
- Change some targets (darwin-nat, go32-nat, gnu-nat, inf-ptrace,
nto-procfs, procfs) to error out when the exec file passed to their
create_inferior method is nullptr. Some targets are fine with a
nullptr exec file, so we can't check that in `run_command_1()`.
- Add the `no_executable_specified_error()` function, which re-uses the
error message that `get_exec_file()` had.
- Change some targets (go32-nat, nto-procfs) to not call
`get_exec_file()`, since it's pointless for the same reason as in the
example above, if it returns, it's going the be the same value as the
`exec_file` parameter. Just rely on `exec_file`.
- Remove the final use of `get_exec_file()`, in `load_command()`.
- Remove the `get_exec_file()` implementations in GDB and GDBserver and
remove the shared declaration.
Change-Id: I601c16498e455f7baa1f111a179da2f6c913baa3
Approved-By: Tom Tromey <tom@tromey.com>
Calls of `get_exec_file (0)` are equivalent to just getting the exec
filename from the current program space. I'm looking to remove
`get_exec_file`, so replace these uses with
`current_program_space->exec_filename ()`.
Remove the `err` parameter of `get_exec_wrapper` since all the calls
that remain pass 1, meaning to error out if no executable is specified.
Change-Id: I7729ea4c7f03dbb046211cc5aa3858ab3a551965
Approved-By: Tom Tromey <tom@tromey.com>
handle_v_attach calls attach_inferior, which says:
"return -1 if attaching is unsupported, 0 if it succeeded, and call
error() otherwise."
So if attach_inferior return != 0, we have the unsupported case,
meaning we should return the empty packet instead of an error.
In practice, this shouldn't trigger, as vAttach support is supposed to
be reported via qSupported. But it doesn't hurt to be pedantic here.
Change-Id: I99cce6fa678f2370571e6bca0657451300956127
Approved-By: Tom Tromey <tom@tromey.com>
This fixes the same issue as the previous patch, but for "attach"
instead of "run".
If attaching to a process with "attach" (vAttach packet) fails,
GDBserver throws an error that escapes all the way to the top level.
When an error escapes all the way like that, GDBserver interprets it
as a disconnection, and either goes back to waiting for a new GDB
connection, or exits, if --once was specified.
Here's an example:
On the GDB side:
...
(gdb) tar extended-remote :9999
...
Remote debugging using :9999
(gdb) attach 1
Attaching to process 1
Attaching to process 1 failed
(gdb)
On the GDBserver side:
$ gdbserver --once --multi :9999
Listening on port 9999
Remote debugging from host 127.0.0.1, port 37464
gdbserver: Cannot attach to process 1: Operation not permitted (1)
$ # gdbserver exited
This is wrong, as we've connected with extended-remote/--multi.
GDBserver should just report an error to vAttach, and continue
connected to GDB, waiting for other commands.
This commit fixes GDBserver by catching the error locally in
handle_v_attach.
Note we now let pid == 0 pass down to attach_inferior. That is so we
get a useful textual error message to report to GDB.
This fixes a couple KFAILs in gdb.base/attach.exp. Still, I thought
it would be useful to add a new testcase specifically for this
scenario, in case gdb.base/attach.exp is ever split and stops trying
to attach again after a failed attach, with the same GDB session.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=19558
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31554
Change-Id: I25314c7e5f1435eff69cb84d57ecac13d8de3393
Approved-By: Tom Tromey <tom@tromey.com>
If starting the inferior process with "run" (vRun packet) fails,
GDBserver throws an error that escapes all the way to the top level.
When an error escapes all the way like that, GDBserver interprets it
as a disconnection, and either goes back to waiting for a new GDB
connection, or exits, if --once was specified.
E.g., with the testcase program added by this commit, we see:
On GDB side:
...
(gdb) tar extended-remote :999
...
Remote debugging using :9999
(gdb) r
Starting program:
Running ".../gdb.base/run-fail-twice/run-fail-twice.nox" on the remote target failed
(gdb)
On GDBserver side:
$ gdbserver --once --multi :9999
Remote debugging from host 127.0.0.1, port 34344
bash: line 1: .../gdb.base/run-fail-twice/run-fail-twice.nox: Permission denied
bash: line 1: exec: .../gdb.base/run-fail-twice/run-fail-twice.nox: cannot execute: Permission denied
gdbserver: During startup program exited with code 126.
$ # gdbserver exited
This is wrong, as we've connected with extended-remote/--multi.
GDBserver should just report an error to vCont, and continue connected
to GDB, waiting for other commands.
This commit fixes GDBserver by catching the error locally in
handle_v_run.
Change-Id: Ib386f267522603f554b52a885b15229c9639e870
Approved-By: Tom Tromey <tom@tromey.com>
Now that defs.h, server.h and common-defs.h are included via the
`-include` option, it is no longer necessary for source files to include
them. Remove all the inclusions of these files I could find. Update
the generation scripts where relevant.
Change-Id: Ia026cff269c1b7ae7386dd3619bc9bb6a5332837
Approved-By: Pedro Alves <pedro@palves.net>
This commit is the result of the following actions:
- Running gdb/copyright.py to update all of the copyright headers to
include 2024,
- Manually updating a few files the copyright.py script told me to
update, these files had copyright headers embedded within the
file,
- Regenerating gdbsupport/Makefile.in to refresh it's copyright
date,
- Using grep to find other files that still mentioned 2023. If
these files were updated last year from 2022 to 2023 then I've
updated them this year to 2024.
I'm sure I've probably missed some dates. Feel free to fix them up as
you spot them.
Building on the last commit, which added a general --debug=COMPONENT
option to the gdbserver command line, this commit updates the monitor
command to allow for general:
(gdb) monitor set debug COMPONENT off|on
style commands. Just like with the previous commit, the COMPONENT can
be any one of all, threads, remote, event-loop, and correspond to the
same set of global debug flags.
While on the command line it is possible to do:
--debug=remote,event-loop,threads
the components have to be entered one at a time with the monitor
command. I guess there's no reason why we couldn't allow component
grouping within the monitor command, but (to me) what I have here
seemed more in the spirit of GDB's existing 'set debug ...' commands.
If people want it then we can always add component grouping later.
Notice in the above that I use 'off' and 'on' instead of '0' and '1',
which is what the 'monitor set debug' command used to use. The 0/1
can still be used, but I now advertise off/on in all the docs and help
text, again, this feels more inline with GDB's existing boolean
settings.
I have removed the two existing monitor commands:
monitor set remote-debug 0|1
monitor set event-loop-debug 0|1
These are replaced by:
monitor set debug remote off|on
monitor set debug event-loop off|on
respectively.
Approved-By: Tom Tromey <tom@tromey.com>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Currently, gdbserver has the following command line options related to
debugging output:
--debug
--remote-debug
--event-loop-debug
This doesn't scale well. If I want an extra debug component I need to
add another command line flag.
This commit changes --debug to take a list of components.
The currently supported components are: all, threads, remote, and
event-loop. The 'threads' component represents the debug we currently
get from the --debug option. And if --debug is used without a
component list then the threads component is assumed as the default.
Currently the threads component actually includes a lot of output that
is not really threads related. In the future I'd like to split this
up into some new, separate components. But that is not part of this
commit, or even this series.
The special component 'all' does what you'd expect: enables debug
output from all supported components.
The component list is parsed left to write, and you can prefix a
component with '-' to disable that component, so I can write:
target> gdbserver --debug=all,-event-loop
to get debug for all components except the event-loop component.
I've removed the existing --remote-debug and --event-loop-debug
command line options, these are equivalent to --debug=remote and
--debug=event-loop respectively, or --debug=remote,event-loop to
enable both components.
In this commit I've only update the command line options, in the next
commit I'll update the monitor commands to support a similar
interface.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
After this commit:
commit 0b04e52316
Date: Sun Jan 19 14:33:37 2014 -0700
link gdbserver against libiberty
we can cleanup how the help text is generated in monitor_show_help.
This doesn't change the output that the user will see -- it just folds
multiple monitor_output calls into one.
There should be no user visible change after this commit.
On Linux, a thread can only be 16 bytes (including the trailing \0).
A user sent in a test case where this causes a truncated UTF-8
sequence, causing gdbserver to create invalid XML.
I went back and forth about different ways to solve this, and in the
end decided to fix it in gdbserver, with the reason being that it
seems important to generate correct XML for the <thread> response.
I am not totally sure whether the call to setlocale could have
unplanned consequences. This is needed, though, for nl_langinfo to
return the correct result.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30618
Normally, if the last resumed thread on the target exits, the server
sends a no-resumed event to GDB. If however, GDB enables the
GDB_THREAD_OPTION_EXIT option on a thread, and, that thread exits, the
server sends a thread exit event for that thread instead.
In all-stop RSP mode, since events can only be forwarded to GDB one at
a time, and the whole target stops whenever an event is reported, GDB
resumes the target again after getting a THREAD_EXITED event, and then
the server finally reports back a no-resumed event if/when
appropriate.
For non-stop RSP though, events are asynchronous, and if the server
sends a thread-exit event for the last resumed thread, the no-resumed
event is never sent. This patch makes sure that in non-stop mode, the
server queues a no-resumed event after the thread-exit event if it was
the last resumed thread that exited.
Without this, we'd see failures in step-over-thread-exit testcases
added later in the series, like so:
continue
Continuing.
- No unwaited-for children left.
- (gdb) PASS: gdb.threads/step-over-thread-exit.exp: displaced-stepping=off: non-stop=on: target-non-stop=on: schedlock=off: ns_stop_all=1: continue stops when thread exits
+ FAIL: gdb.threads/step-over-thread-exit.exp: displaced-stepping=off: non-stop=on: target-non-stop=on: schedlock=off: ns_stop_all=1: continue stops when thread exits (timeout)
(and other similar ones)
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
Change-Id: I927d78b30f88236dbd5634b051a716f72420e7c7
Currently, GDB does not understand the THREAD_EXITED stop reply in
remote all-stop mode. There's no good reason for this, it just
happened that THREAD_EXITED was only ever reported in non-stop mode so
far. This patch teaches GDB to parse that event in all-stop RSP too.
There is no need to add a qSupported feature for this, because the
server won't send a THREAD_EXITED event unless GDB explicitly asks for
it, with QThreadEvents, or with the GDB_THREAD_OPTION_EXIT
QThreadOptions option added in the next patch.
Change-Id: Ide5d12391adf432779fe4c79526801c4a5630966
This commit extends the logic added by these two commits from a while
ago:
#17b961964f8 (gdbserver: hide fork child threads from GDB),
#2df5ad10200 (gdb, gdbserver: detach fork child when detaching from fork parent)
... to handle thread clone events, which are very similar to (v)fork
events.
For #1, we want to hide clone children as well, so just update the
comments.
For #2, unlike (v)fork children, pending clone children aren't full
processes, they're just threads, so don't detach them in
handle_detach. linux-low.cc will take care of detaching them along
with all other threads of the process, there's nothing special that
needs to be done.
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
Change-Id: I7f5901d07efda576a2522d03e183994e071b8ffc
A previous patch taught GDB about a new TARGET_WAITKIND_THREAD_CLONED
event kind, and made the Linux target report clone events.
A following patch will teach Linux GDBserver to do the same thing.
However, for remote debugging, it wouldn't be ideal for GDBserver to
report every clone event to GDB, when GDB only cares about such events
in some specific situations. Reporting clone events all the time
would be potentially chatty. We don't enable thread create/exit
events all the time for the same reason. Instead we have the
QThreadEvents packet. QThreadEvents is target-wide, though.
This patch makes GDB instead explicitly request that the target
reports clone events or not, on a per-thread basis.
In order to be able to do that with GDBserver, we need a new remote
protocol feature. Since a following patch will want to enable thread
exit events on per-thread basis too, the packet introduced here is
more generic than just for clone events. It lets you enable/disable a
set of options at once, modelled on Linux ptrace's PTRACE_SETOPTIONS.
IOW, this commit introduces a new QThreadOptions packet, that lets you
specify a set of per-thread event options you want to enable. The
packet accepts a list of options/thread-id pairs, similarly to vCont,
processed left to right, with the options field being a number
interpreted as a bit mask of options. The only option defined in this
commit is GDB_THREAD_OPTION_CLONE (0x1), which ask the remote target
to report clone events. Another patch later in the series will
introduce another option.
For example, this packet sets option "1" (clone events) on thread
p1000.2345:
QThreadOptions;1:p1000.2345
and this clears options for all threads of process 1000, and then sets
option "1" (clone events) on thread p1000.2345:
QThreadOptions;0:p1000.-1;1:p1000.2345
This clears options of all threads of all processes:
QThreadOptions;0
The target reports the set of supported options by including
"QThreadOptions=<supported options>" in its qSupported response.
infrun is then tweaked to enable GDB_THREAD_OPTION_CLONE when stepping
over a breakpoint.
Unlike PTRACE_SETOPTIONS, fork/vfork/clone children do NOT inherit
their parent's thread options. This is so that GDB can send e.g.,
"QThreadOptions;0;1:TID" without worrying about threads it doesn't
know about yet.
Documentation for this new remote protocol feature is included in a
documentation patch later in the series.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=19675
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27830
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
Change-Id: Ie41e5093b2573f14cf6ac41b0b5804eba75be37e
The previous patch taught GDB about a new
TARGET_WAITKIND_THREAD_CLONED event kind, and made the Linux target
report clone events.
A following patch will teach Linux GDBserver to do the same thing.
But before we get there, we need to teach the remote protocol about
TARGET_WAITKIND_THREAD_CLONED. That's what this patch does. Clone is
very similar to vfork and fork, and the new stop reply is likewise
handled similarly. The stub reports "T05clone:...".
GDBserver core is taught to handle TARGET_WAITKIND_THREAD_CLONED and
forward it to GDB in this patch, but no backend actually emits it yet.
That will be done in a following patch.
Documentation for this new remote protocol feature is included in a
documentation patch later in the series.
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
Change-Id: If271f20320d864f074d8ac0d531cc1a323da847f
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=19675
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27830
I noticed that in handle_v_run (gdbserver/server.cc) we leak
new_program_name (a string) each time GDB starts an inferior, in the
case where GDB passes a program name to gdbserver.
This bug was introduced with this commit:
commit 7ab2607f97
Date: Wed Apr 13 17:31:02 2022 -0400
gdbsupport: make gdb_abspath return an std::string
When gdbserver receives a program name from GDB, this is first placed
into a malloc'd buffer within handle_v_run, and this buffer is then
used in this call:
program_path.set (new_program_name);
Prior to the above commit this call took ownership of the buffer
passed to it, but now this call uses the buffer to initialise a
std::string, which copies the buffer contents, leaving ownership with
the caller. So now, after this call (in handle_v_run)
new_program_name still owns a buffer.
At no point in handle_v_run do we free new_program_name, as a result
we are leaking the program name each time GDB starts a remote
inferior.
I could solve this by adding a 'free' call into handle_v_run, but I'd
rather automate the memory management.
So, to this end, I have added a new function in gdbserver/server.cc,
decode_v_run_arg. This function takes care of allocating the memory
buffer and decoding the vRun packet into the buffer, but returns a
gdb::unique_xmalloc_ptr<char> (or nullptr on error).
Back in handle_v_run I have converted new_program_name to also be a
gdb::unique_xmalloc_ptr<char>.
Now, after we call program_path.set(), the allocated buffer will be
automatically released when it is no longer needed.
It is worth highlighting that within the new decode_v_run_arg
function, I have wrapped the call to hex2bin in a try/catch block.
The hex2bin function can throw an exception if it encounters an
invalid (non-hex) character. Back in handle_v_run, we have a local
argument new_argv, which is of type std::vector<char *>. Each
'char *' in this vector is a malloc'd buffer. If we allow
hex2bin to throw an exception and don't catch it in either
decode_v_run_arg or handle_v_run then we are going to leak memory from
new_argv.
I chose to catch the exception in decode_v_run_arg, this seemed
cleanest, but I'm not sure it really matters, so long as the exception
is caught before we leave handle_v_run. I am working on a patch that
changes new_argv to automatically manage its memory, but that isn't
ready for posting yet. I think what I have here would be fine if my
follow on patch never arrives.
Additionally, within the handle_v_run loop I have changed an
assignment of nullptr to new_program_name into an assert. Previously,
the assignment could only trigger on the first iteration of the loop,
if we had no new program name to assign. However, new_program_name
always starts as nullptr, so, on the first loop iteration, if we have
nothing to assign to new_program_name, its value must already be
nullptr.
There should be no user visible changes after this commit.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
After the previous commit there is now a redundant string copy in
handle_v_run, this commit cleans that up.
There should be no functional change after this commit.
During review I was pointed to this older series:
https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de/
which also includes this fix as part of a larger set of changes. I'm
giving a Co-Authored-By credit to the author of that original series.
I believe this smaller fix brings some benefits on its own, though the
original series does offer additional improvements. Once this is
merged I'll take a look at rebasing and resubmitting the original series.
Co-Authored-By: Michael Weghorn <m.weghorn@posteo.de>
Approved-By: Tom Tromey <tom@tromey.com>
Similarly to how single quotes were mishandled, which was fixed two
commits ago, this commit fixes handling of newlines in arguments
passed to gdbserver.
We already had a test that covered this, gdb.base/args.exp, which,
when run with the native-extended-gdbserver board contained several
KFAIL covering this situation.
In this commit I remove the unnecessary, attempt to quote incoming
newlines within arguments, and do some minimal cleanup of the related
code. There is additional cleanup that can be done, but I'm leaving
that for the next commit.
Then I've removed the KFAIL from the test case, and performed some
minimal cleanup there too.
After this commit the gdb.base/args.exp is 100% passing with the
native-extended-gdbserver board file.
During review I was pointed to this older series:
https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de/
which also includes this fix as part of a larger set of changes. I'm
giving a Co-Authored-By credit to the author of that original series.
I believe this smaller fix brings some benefits on its own, though the
original series does offer additional improvements. Once this is
merged I'll take a look at rebasing and resubmitting the original series.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27989
Co-Authored-By: Michael Weghorn <m.weghorn@posteo.de>
Approved-By: Tom Tromey <tom@tromey.com>
When I posted the previous patch for review Andreas Schwab pointed out
that passing a trailing empty argument also doesn't work.
The fix for this is in the same area of code as the previous patch,
but is sufficiently different that I felt it deserved a patch of its
own.
I noticed that passing arguments containing single quotes to gdbserver
didn't work correctly:
gdb -ex 'set sysroot' --args /tmp/show-args
Reading symbols from /tmp/show-args...
(gdb) target extended-remote | gdbserver --once --multi - /tmp/show-args
Remote debugging using | gdbserver --once --multi - /tmp/show-args
stdin/stdout redirected
Process /tmp/show-args created; pid = 176054
Remote debugging using stdio
Reading symbols from /lib64/ld-linux-x86-64.so.2...
(No debugging symbols found in /lib64/ld-linux-x86-64.so.2)
0x00007ffff7fd3110 in _start () from /lib64/ld-linux-x86-64.so.2
(gdb) set args abc ""
(gdb) run
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /tmp/show-args \'
stdin/stdout redirected
Process /tmp/show-args created; pid = 176088
2 args are:
/tmp/show-args
abc
Done.
[Inferior 1 (process 176088) exited normally]
(gdb) target native
Done. Use the "run" command to start a process.
(gdb) run
Starting program: /tmp/show-args \'
2 args are:
/tmp/show-args
abc
Done.
[Inferior 1 (process 176095) exited normally]
(gdb) q
The 'shows-args' program used here just prints the arguments passed to
the inferior.
Notice that when starting the inferior using the extended-remote
target there is only a single argument 'abc', while when using the
native target there is a second argument, the blank line, representing
the empty argument.
The problem here is that the vRun packet coming from GDB looks like
this (I've removing the trailing checksum):
$vRun;PROGRAM_NAME;616263;
If we compare this to a packet with only a single argument and no
trailing empty argument:
$vRun;PROGRAM_NAME;616263
Notice the lack of the trailing ';' character here.
The problem is that gdbserver processes this string in a loop. At
each point we maintain a pointer to the character just after a ';',
and then we process everything up to either the next ';' character, or
to the end of the string.
We break out of this loop when the character we start with (in that
loop iteration) is the null-character. This means in the trailing
empty argument case, we abort the loop before doing anything with the
empty argument.
In this commit I've updated the loop, we now break out using a 'break'
statement at the end of the loop if the (sub-)string we just processed
was empty, with this change we now notice the trailing empty
argument.
I've updated the test case to cover this issue.
Approved-By: Tom Tromey <tom@tromey.com>
I noticed that passing arguments containing single quotes to gdbserver
didn't work correctly:
gdb -ex 'set sysroot' --args /tmp/show-args
Reading symbols from /tmp/show-args...
(gdb) target extended-remote | gdbserver --once --multi - /tmp/show-args
Remote debugging using | gdbserver --once --multi - /tmp/show-args
stdin/stdout redirected
Process /tmp/show-args created; pid = 176054
Remote debugging using stdio
Reading symbols from /lib64/ld-linux-x86-64.so.2...
(No debugging symbols found in /lib64/ld-linux-x86-64.so.2)
0x00007ffff7fd3110 in _start () from /lib64/ld-linux-x86-64.so.2
(gdb) set args \'
(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /tmp/show-args \'
stdin/stdout redirected
Process /tmp/show-args created; pid = 176088
2 args are:
/tmp/show-args
\'
Done.
[Inferior 1 (process 176088) exited normally]
(gdb) target native
Done. Use the "run" command to start a process.
(gdb) run
Starting program: /tmp/show-args \'
2 args are:
/tmp/show-args
'
Done.
[Inferior 1 (process 176095) exited normally]
(gdb) q
The 'shows-args' program used here just prints the arguments passed to
the inferior.
Notice that when starting the inferior using the extended-remote
target the second argument is "\'", while when running using native
target the argument is "'". The second of these is correct, the \'
used with the "set args" command is just to show GDB that the single
quote is not opening an argument string.
It turns out that the extra backslash is injected on the gdbserver
side when gdbserver processes the arguments that GDB passes it, the
code that does this was added as part of this much larger commit:
commit 2090129c36
Date: Thu Dec 22 21:11:11 2016 -0500
Share fork_inferior et al with gdbserver
In this commit I propose removing the specific code that adds what I
believe is a stray backslash. I've extended an existing test to cover
this case, and I now see identical behaviour when using an
extended-remote target as with the native target.
This partially fixes PR gdb/27989, though there are still some issues
with newline handling which I'll address in a later commit.
During review I was pointed to this older series:
https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de/
which also includes this fix as part of a larger set of changes. I'm
giving a Co-Authored-By credit to the author of that original series.
I believe this smaller fix brings some benefits on its own, though the
original series does offer additional improvements. Once this is
merged I'll take a look at rebasing and resubmitting the original series.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27989
Co-Authored-By: Michael Weghorn <m.weghorn@posteo.de>
Approved-By: Tom Tromey <tom@tromey.com>
While investigating something else, I noticed some weird code in
agent_run_command (use of memcpy rather than strcpy). Then I noticed
that 'cmd' is used as both an in and out parameter, despite being
const.
Casting away const like this is bad. This patch removes the const and
fixes the memcpy. I also added a static assert to assure myself that
the code in gdbserver is correct -- gdbserver is passing its own
buffer directly to agent_run_command.
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
This patch doesn't change gdbserver behaviour, but after later changes are
made it avoids a null pointer dereference when HWCAP needs to be obtained
for a specific process while current_thread is nullptr.
Fixing linux_read_auxv, linux_get_hwcap and linux_get_hwcap2 to take a PID
parameter seems more correct than setting current_thread in one particular
code path.
Changes are propagated to allow passing the new parameter through the call
chain.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
This commit updates the copyright year displayed by gdb, gdbserver
and gdbreplay's help message from 2022 to 2023, as per our Start
of New Year procedure. The corresponding source files' copyright
header are also updated accordingly.
Gdbserver unconditionally reports support for btrace packets. Do not
report the support, if the underlying target does not say it supports
it. Otherwise GDB would query the server with btrace-related packets
unnecessarily.
In this commit,
commit cf6c1e710e
Date: Mon Jul 11 20:53:48 2022 +0800
gdbserver: remove unused variable
I removed an unused variable in handle_v_run. Pedro then pointed out
that the for loop after it was also unused. After a period of smoke
testing, no exceptions were found.
Tested on x86_64-linux.
When building with clang 15, I got this error:
CXX server.o
server.cc:2985:10: error: variable 'new_argc' set but not used [-Werror,-Wunused-but-set-variable]
int i, new_argc;
^
Remove the unused variable to eliminate the error.
Tested by rebuilding on x86_64-linux with clang 15.
The recent commit 421490af33 ("gdbserver/linux: Access memory even
if threads are running") caused a regression in
gdb.threads/access-mem-running-thread-exit.exp with gdbserver, which I
somehow missed. Like so:
(gdb) print global_var
Cannot access memory at address 0x555555558010
(gdb) FAIL: gdb.threads/access-mem-running-thread-exit.exp: non-stop: access mem (print global_var after writing, inf=2, iter=1)
The problem starts with GDB telling GDBserver to select a thread, via
the Hg packet, which GDBserver complies with, then that thread exits,
and GDB, without knowing the thread is gone, tries to write to memory,
through the context of the previously selected Hg thread.
GDBserver's GDB-facing memory access routines, gdb_read_memory and
gdb_write_memory, call set_desired_thread to make GDBserver re-select
the thread that GDB has selected with the Hg packet. Since the thread
is gone, set_desired_thread returns false, and the memory access
fails.
Now, to access memory, it doesn't really matter which thread is
selected. All we should need is the target process. Even if the
thread that GDB previously selected is gone, and GDB does not yet know
about that exit, it shouldn't matter, GDBserver should still know
which process that thread belonged to.
Fix this by making GDBserver track the current process separately,
like GDB also does. Add a new set_desired_process routine that is
similar to set_desired_thread, but just sets the current process,
leaving the current thread as NULL. Use it in the GDB-facing memory
read and write routines, to avoid failing if the selected thread is
gone, but the process is still around.
Change-Id: I4ff97cb6f42558efbed224b30d5c71f6112d44cd
I'm trying to switch these functions to use std::string instead of char
arrays, as much as possible. Some callers benefit from it (can avoid
doing a copy of the result), while others suffer (have to make one more
copy).
Change-Id: Iced49b8ee2f189744c5072a3b217aab5af17a993
