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
This commit changes how gdbserver stores the inferior arguments from
being a vector of separate arguments into a single string with all of
the arguments combined together.
Making this change might feel a little strange; intuitively it feels
like we would be better off storing the arguments as a vector, but
this change is part of a larger series of work that aims to improve
GDB's inferior argument handling. The full series was posted here:
https://inbox.sourceware.org/gdb-patches/cover.1730731085.git.aburgess@redhat.com
But asking people to review a 14 patch series in unreasonable, so I'm
instead posting the patches in smaller batches. This patch can stand
alone, and I do think this change makes sense on its own:
First, GDB already stores the inferior arguments as a single string,
so doing this moves gdbserver into line with GDB. The common code
into which gdbserver calls requires the arguments to be a single
string, so currently each target's create_inferior implementation
merged the arguments anyway, so all this commit really does is move
the merging up the call stack, and store the merged result rather than
storing the separate parts.
However, the biggest reason for why this commit is needed, is an issue
with passing arguments from GDB to gdbserver when starting a new
inferior.
Consider:
(gdb) set args $VAR
(gdb) run
...
When using a native target the inferior will see the value of $VAR
expanded by the shell GDB uses to start the inferior. However, if
using an extended-remote target the inferior will see literally $VAR,
the unexpanded name of the variable, the reason for this is that,
although GDB sends '$VAR' to gdbserver, when gdbserver receives this,
it converts this to '\$VAR', which prevents the variable from being
expanded by the shell.
The reason for this is that construct_inferior_arguments escapes all
special shell characters within its arguments, and it is
construct_inferior_arguments that is used to combine the separate
arguments into a single string.
In the future I will change construct_inferior_arguments so that
it can apply different escaping strategies. When this happens we will
want to escape arguments coming from the gdbserver command line
differently than arguments coming from GDB (via a vRun packet), which
means we need to call construct_inferior_arguments earlier, at the
point where we know if the arguments came from the gdbserver command
line, or from the vRun packet.
This argument escaping issue is discussed in PR gdb/28392.
This commit doesn't fix any issues, nor does it change
construct_inferior_arguments to actually do different escaping, that
will all come later. This is purely a restructuring.
There should be no user visible changes after this commit.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28392
Tested-By: Guinevere Larsen <guinevere@redhat.com>
Approved-By: Simon Marchi <simon.marchi@efficios.com>
We have gdb::handle_eintr, which allows us to rewrite:
...
ssize_t ret;
do
{
errno = 0;
ret = ::write (pipe[1], "+", 1);
}
while (ret == -1 && errno == EINTR);
...
into:
...
ssize_t ret = gdb::handle_eintr (-1, ::write, pipe[1], "+", 1);
...
However, the call to write got a bit mangled, requiring effort to decode it
back to its original form.
Instead, add a new function gdb::write that allows us to write:
...
ssize_t ret = gdb::write (pipe[1], "+", 1);
...
Likewise for waitpid, read and close.
Tested on x86_64-linux.
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>
Same idea as the previous patch, but for `remove_thread`.
Change-Id: I7e227655be5fcf29a3256e8389eb32051f27882d
Reviewed-By: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Since thread_info objects are now basically children of process_info
objects, I think that makes sense.
Change-Id: I7f0a67b921b468e512851cb2f36015d1003412d7
Reviewed-By: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Remove this overload, prefer to use `process_info::for_each_thread`. In
many instances, the `process_info` is already available, so this saves a
map lookup. In other instances, add the `process_info` lookup at the
call site.
In `linux-arm-low.cc` and `win32-i386-low.cc`, use `current_process ()`
instead of `current_thread->id.pid ()`. I presume that if
`current_process ()` and `current_thread` don't match, it's a bug
orthogonal to this change.
Change-Id: I751ed497cb1f313cf937b35125151bee9316fc51
Reviewed-By: Tankut Baris Aktemur <tankut.baris.aktemur@intel.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.
This changes gdb to use the C++17 [[fallthrough]] attribute rather
than special comments.
This was mostly done by script, but I neglected a few spellings and so
also fixed it up by hand.
I suspect this fixes the bug mentioned below, by switching to a
standard approach that, presumably, clang supports.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23159
Approved-By: John Baldwin <jhb@FreeBSD.org>
Approved-By: Luis Machado <luis.machado@arm.com>
Approved-By: Pedro Alves <pedro@palves.net>
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 is the result of running the gdb/copyright.py script,
which automated the update of the copyright year range for all
source files managed by the GDB project to be updated to include
year 2023.
This commit brings all the changes made by running gdb/copyright.py
as per GDB's Start of New Year Procedure.
For the avoidance of doubt, all changes in this commits were
performed by the script.
Replace the direct assignments to current_thread with
switch_to_thread. Use scoped_restore_current_thread when appropriate.
There is one instance remaining in linux-low.cc's wait_for_sigstop.
This will be handled in a separate patch.
Regression-tested on X86-64 Linux using the native-gdbserver and
native-extended-gdbserver board files.
I stumbled on a bug caused by the fact that a code path read
target_waitstatus::value::sig (expecting it to contain a gdb_signal
value) while target_waitstatus::kind was TARGET_WAITKIND_FORKED. This
meant that the active union field was in fact
target_waitstatus::value::related_pid, and contained a ptid. The read
signal value was therefore garbage, and that caused GDB to crash soon
after. Or, since that GDB was built with ubsan, this nice error
message:
/home/simark/src/binutils-gdb/gdb/linux-nat.c:1271:12: runtime error: load of value 2686365, which is not a valid value for type 'gdb_signal'
Despite being a large-ish change, I think it would be nice to make
target_waitstatus safe against that kind of bug. As already done
elsewhere (e.g. dynamic_prop), validate that the type of value read from
the union matches what is supposed to be the active field.
- Make the kind and value of target_waitstatus private.
- Make the kind initialized to TARGET_WAITKIND_IGNORE on
target_waitstatus construction. This is what most users appear to do
explicitly.
- Add setters, one for each kind. Each setter takes as a parameter the
data associated to that kind, if any. This makes it impossible to
forget to attach the associated data.
- Add getters, one for each associated data type. Each getter
validates that the data type fetched by the user matches the wait
status kind.
- Change "integer" to "exit_status", "related_pid" to "child_ptid",
just because that's more precise terminology.
- Fix all users.
That last point is semi-mechanical. There are a lot of obvious changes,
but some less obvious ones. For example, it's not possible to set the
kind at some point and the associated data later, as some users did.
But in any case, the intent of the code should not change in this patch.
This was tested on x86-64 Linux (unix, native-gdbserver and
native-extended-gdbserver boards). It was built-tested on x86-64
FreeBSD, NetBSD, MinGW and macOS. The rest of the changes to native
files was done as a best effort. If I forgot any place to update in
these files, it should be easy to fix (unless the change happens to
reveal an actual bug).
Change-Id: I0ae967df1ff6e28de78abbe3ac9b4b2ff4ad03b7
This commits the result of running gdb/copyright.py as per our Start
of New Year procedure...
gdb/ChangeLog
Update copyright year range in copyright header of all GDB files.
This eliminates the need to specify the return type when using
handle_eintr. We let the compiler deduce it for us.
Also, use lowercase for function parameter names. Uppercase should
only be used on template parameters.
gdb/ChangeLog:
* nat/linux-waitpid.c: Include "gdbsupport/eintr.h".
(my_waitpid): Use gdb::handle_eintr.
gdbserver/ChangeLog:
* netbsd-low.cc (netbsd_waitpid, netbsd_process_target::kill)
(netbsd_qxfer_libraries_svr4): Use gdb::handle_eintr without
explicit type.
gdbsupport/ChangeLog:
* eintr.h (handle_eintr): Replace Ret template parameter with
ErrorValType. Use it as type of the failure value. Deduce the
function's return type using decltype. Use lowercase for function
parameter names.
Currently it does not add any value.
The netbsd_tdesc local variable is no longer needed. Remove it.
The tdesc value is set by the low target now.
gdbserver/ChangeLog:
* netbsd-low.cc (netbsd_tdesc): Remove.
(netbsd_add_process): Likewise.
(netbsd_process_target::create_inferior): Update.
Switch from target->read_memory to netbsd_nat::read_memory and
cleanup the code.
No functional change.
gdbserver/ChangeLog:
* netbsd-low.cc (get_dynamic, get_r_debug, read_one_ptr)
(netbsd_qxfer_libraries_svr4): Remove "target" argument and update.
(netbsd_process_target::qxfer_libraries_svr4): Update.
Instead of sharing the native-only code with all BSDs with slightly
different semantics of the kernels, share the NetBSD-only behavior beteen
the NetBSD native and gdbserver setup.
NetBSD does not differentiate the address space I and D in the
operations (contrary to OpenBSD). NetBSD handles EACCES that integrates
with NetBSD specific PaX MPROTECT error handling.
Add a verbose message in the native client that an operation could be
cancelled due to PaX MPROTECT setup.
gdb/ChangeLog:
* nat/netbsd-nat.c (write_memory, read_memory): Add.
* nat/netbsd-nat.h (write_memory, read_memory): Likewise.
* nbsd-nat.c (nbsd_nat_target::xfer_partial): Update.
gdbserver/ChangeLog:
* netbsd-low.cc (netbsd_process_target::read_memory)
(netbsd_process_target::write_memory): Update.
Replace the pre-C++ construct of netbsd_target_ops, netbsd_regset_info
and netbsd_tdesc with C++ inheritance approach found in the Linux
gdbserver code. Add netbsd_amd64_target, that inherits from the
netbsd_process_target class and add proper singleton object for
the_netbsd_target, initialized from netbsd_amd64_target.
Call low_arch_setup () on post process creation, which sets machine
specific properties of the traced process.
Remove global singleton the_netbsd_target object from the generic
gdbserver code.
This refactoring introduces no functional change from the end-user
point of view.
gdbserver/ChangeLog:
* netbsd-amd64-low.cc (netbsd_x86_64_arch_setup): Remove.
(netbsd_target_regsets): Now const.
(the_low_target): Remove.
(class netbsd_amd64_target, the_netbsd_amd64_target)
(the_netbsd_target): Add.
* netbsd-low.cc (netbsd_process_target::post_create_inferior): Call
low_arch_setup ().
(netbsd_process_target::fetch_registers)
(netbsd_process_target::store_registers, initialize_low): Update.
(the_netbsd_target): Remove.
* netbsd-low.h (netbsd_target_regsets, netbsd_target_ops)
(the_low_target, netbsd_tdesc): Remove.
(netbsd_process_target::get_regs_info)
(netbsd_process_target::low_arch_setup): Add.
