Commit Graph

449 Commits

Author SHA1 Message Date
Andrew Burgess
7816b81e9b gdb/gdbserver: share I386_LINUX_XSAVE_XCR0_OFFSET definition
Share the definition of I386_LINUX_XSAVE_XCR0_OFFSET between GDB and
gdbserver.

This commit is part of a series that aims to share more of the x86
target description creation code between GDB and gdbserver.  The
I386_LINUX_XSAVE_XCR0_OFFSET #define is used as part of the target
description creation, and I noticed that this constant is defined
separately for GDB and gdbserver.

This commit moves the definition into gdb/nat/x86-linux.h, which
allows the #define to be shared.

There should be no user visible changes after this commit.

Approved-By: John Baldwin <jhb@FreeBSD.org>
2024-03-25 17:14:19 +00:00
Andrew Burgess
0a7bb97ad2 gdbserver/x86: move no-xml code earlier in x86_linux_read_description
This commit is part of a series that aims to share more of the x86
target description reading/generation code between GDB and gdbserver.

There are a huge number of similarities between the code in
gdbserver's x86_linux_read_description function and GDB's
x86_linux_nat_target::read_description function, and it is this
similarity that I plan, in a later commit, to share between GDB and
gdbserver.

However, one thing that is different in x86_linux_read_description is
the code inside the '!use_xml' block.  This is the code that handles
the case where gdbserver is not allowed to send an XML target
description back to GDB.  In this case gdbserver uses some predefined,
fixed, target descriptions.

First, it's worth noting that I suspect this code is not tested any
more.  I couldn't find anything in the testsuite that tries to disable
XML target description support.  And the idea of having a single
"fixed" target description really doesn't work well when we think
about all the various x86 extensions that exist.  Part of me would
like to rip out the no-xml support in gdbserver (at least for x86),
and if a GDB connects that doesn't support XML target descriptions,
gdbserver can just give an error and drop the connection.  GDB has
supported XML target descriptions for 16 years now, I think it would
be reasonable for our shipped gdbserver to drop support for the old
way of doing things.

Anyway.... this commit doesn't do that.

What I did notice was that, over time, the '!use_xml' block appears to
have "drifted" within the x86_linux_read_description function; it's
now not the first check we do.  Instead we make some ptrace calls and
return a target description generated based on the result of these
ptrace calls.  Surely it only makes sense to generate variable target
descriptions if we can send these back to GDB?

So in this commit I propose to move the '!use_xml' block earlier in
the x86_linux_read_description function.

The benefit of this is that this leaves the later half of
x86_linux_read_description much more similar to the GDB function
x86_linux_nat_target::read_description and sets us up for potentially
sharing code between GDB and gdbserver in a later commit.

Approved-By: John Baldwin <jhb@FreeBSD.org>
2024-03-25 17:14:18 +00:00
Andrew Burgess
5920765d75 gdbserver: convert have_ptrace_getregset to a tribool
Convert the have_ptrace_getregset global within gdbserver to a
tribool.  This brings the flag into alignment with the corresponding
flag in GDB.

The gdbserver have_ptrace_getregset variable is already used as a
tribool, it just doesn't have the tribool type.

In a future commit I plan to share more code between GDB and
gdbserver, and having this variable be the same type in both code
bases will make the sharing much easier.

There should be no user visible changes after this commit.

Approved-By: John Baldwin <jhb@FreeBSD.org>
2024-03-25 17:14:18 +00:00
H.J. Lu
4bb20a6244 gdbserver: Clear X86_XSTATE_MPX bits in xcr0 on x32
Since MPX isn't available for x32, we should clear X86_XSTATE_MPX bits
on x32.

	PR server/31511
	* linux-x86-low.cc (x86_linux_read_description): Clear
	X86_XSTATE_MPX bits in xcr0 on x32.
Reviewed-by: Felix Willgerodt <felix.willgerodt@intel.com>
2024-03-21 12:44:40 -07:00
Simon Marchi
da48217f31 gdbserver/linux: probe for libiconv in configure
Make gdbserver's build system locate libiconv when building for Linux.

Commit 07b3255c3b ("Filter invalid encodings from Linux thread names")
make libiconv madantory for building gdbserver on Linux.

While trying to cross-compile gdb for xtensa-fsf-linux-uclibc (with a
toolchain generated with crosstool-ng), I got:

    /home/smarchi/src/binutils-gdb/gdbserver/linux-low.cc:48:10: fatal error: iconv.h: No such file or directory
       48 | #include <iconv.h>
          |          ^~~~~~~~~

I downloaded GNU libiconv, built it for that host, and installed it in
an arbitrary directory.  I had to modify the gdbserver build system to
locate libiconv and use it, the result is this patch.

I eventually found that crosstool-ng has a config option to make uclibc
provide an implementation of iconv, which is of course much easier.  But
given that this patch is now written, I think it would be worth merging
it, it could help some people who do not have iconv built-in their libc
in the future (and may not have the luxury of rebuilding their libc like
I do).

Using AM_ICONV in configure.ac adds these options for configure (the
same we have for gdb):

    --with-libiconv-prefix[=DIR]  search for libiconv in DIR/include and DIR/lib
    --without-libiconv-prefix     don't search for libiconv in includedir and libdir
    --with-libiconv-type=TYPE     type of library to search for (auto/static/shared)

It sets the `LIBICONV` variable with whatever is needed to link with
libiconv, and adds the necessary `-I` flag to `CPPFLAGS`.

To avoid unnecessarily linking against libiconv on hosts that don't need
it, set `MAYBE_LIBICONV` with the contents of `LIBICONV` only if the
host is Linux, and use `MAYBE_LIBICONV` in `Makefile.in`.

Since libiconv is a hard requirement for Linux hosts, error out if it is
not found.

The bits in acinclude.m4 are similar to what we have in
gdb/acinclude.m4.

Update the top-level build system to support building against an in-tree
libiconv (I did not test this part though).  Something tells me that the
all-gdbserver dependency on all-libiconv is unnecessary, since there is
already a dependency of configure-gdbserver on all-libiconv (and
all-gdbserver surely depends on configure-gdbserver).  I just copied
what's done for GDB though.

ChangeLog:

	* Makefile.def: Add configure-gdbserver and all-gdbserver
	dependencies on all-libiconv.
	* Makefile.in: Re-generate.

Change-Id: I90f8ef88dd4917df5a68b45550d93622fc9cfed4
Approved-By: Tom Tromey <tom@tromey.com>
2024-03-14 13:40:08 -04:00
Thiago Jung Bauermann
bbb12eb9c8 gdb/arm: Remove tpidruro register from non-FreeBSD target descriptions
Commit 92d48a1e4e ("Add an arm-tls feature which includes the tpidruro
register from CP15.") introduced the org.gnu.gdb.arm.tls feature, which
adds the tpidruro register, and unconditionally enabled it in
aarch32_create_target_description.

In Linux, the tpidruro register isn't available via ptrace in the 32-bit
kernel but it is available for an aarch32 program running under an arm64
kernel via the ptrace compat interface.  This isn't currently implemented
however, which causes GDB on arm-linux with 64-bit kernel to list the
register but show it as unavailable, as reported by Tom de Vries:

  $ gdb -q -batch a.out -ex start -ex 'p $tpidruro'
  Temporary breakpoint 1 at 0x512

  Temporary breakpoint 1, 0xaaaaa512 in main ()
  $1 = <unavailable>

Simon Marchi then clarified:

> The only time we should be seeing some "unavailable" registers or memory
> is in the context of tracepoints, for things that are not collected.
> Seeing an unavailable register here is a sign that something is not
> right.

Therefore, disable the TLS feature in aarch32 target descriptions for Linux
and NetBSD targets (the latter also doesn't seem to support accessing
tpidruro either, based on a quick look at arm-netbsd-nat.c).

This patch fixes the following tests:

Running gdb.base/inline-frame-cycle-unwind.exp ...
FAIL: gdb.base/inline-frame-cycle-unwind.exp: cycle at level 3: backtrace when the unwind is broken at frame 3
FAIL: gdb.base/inline-frame-cycle-unwind.exp: cycle at level 5: backtrace when the unwind is broken at frame 5
FAIL: gdb.base/inline-frame-cycle-unwind.exp: cycle at level 1: backtrace when the unwind is broken at frame 1

Tested with Ubuntu 22.04.3 on armv8l-linux-gnueabihf in native,
native-gdbserver and native-extended-gdbserver targets with no regressions.

PR tdep/31418
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31418

Approved-By: John Baldwin <jhb@FreeBSD.org>
2024-02-29 12:27:27 -03:00
Tiezhu Yang
4a4fd10d17 gdb: Modify the output of "info breakpoints" and "delete breakpoints"
The output of "info breakpoints" includes breakpoint, watchpoint,
tracepoint, and catchpoint if they are created, so it should show
all the four types are deleted in the output of "info breakpoints"
to report empty list after "delete breakpoints".

It should also change the output of "delete breakpoints" to make it
clear that watchpoints, tracepoints, and catchpoints are also being
deleted. This is suggested by Guinevere Larsen, thank you.

$ make check-gdb TESTS="gdb.base/access-mem-running.exp"
$ gdb/gdb gdb/testsuite/outputs/gdb.base/access-mem-running/access-mem-running
[...]
(gdb) break main
Breakpoint 1 at 0x12000073c: file /home/loongson/gdb.git/gdb/testsuite/gdb.base/access-mem-running.c, line 32.
(gdb) watch global_counter
Hardware watchpoint 2: global_counter
(gdb) trace maybe_stop_here
Tracepoint 3 at 0x12000071c: file /home/loongson/gdb.git/gdb/testsuite/gdb.base/access-mem-running.c, line 27.
(gdb) catch fork
Catchpoint 4 (fork)
(gdb) info breakpoints
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   0x000000012000073c in main at /home/loongson/gdb.git/gdb/testsuite/gdb.base/access-mem-running.c:32
2       hw watchpoint  keep y                      global_counter
3       tracepoint     keep y   0x000000012000071c in maybe_stop_here at /home/loongson/gdb.git/gdb/testsuite/gdb.base/access-mem-running.c:27
	not installed on target
4       catchpoint     keep y                      fork

Without this patch:

(gdb) delete breakpoints
Delete all breakpoints? (y or n) y
(gdb) info breakpoints
No breakpoints or watchpoints.
(gdb) info breakpoints 3
No breakpoint or watchpoint matching '3'.

With this patch:

(gdb) delete breakpoints
Delete all breakpoints, watchpoints, tracepoints, and catchpoints? (y or n) y
(gdb) info breakpoints
No breakpoints, watchpoints, tracepoints, or catchpoints.
(gdb) info breakpoints 3
No breakpoint, watchpoint, tracepoint, or catchpoint matching '3'.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
Approved-by: Kevin Buettner <kevinb@redhat.com>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
2024-02-26 19:19:58 +08:00
Simon Marchi
aca8a74923 gdb: remove some GCC version checks
Since we now require C++17, and therefore gcc >= 9, it's no longer
useful to have gcc version checks for older gcc version.

Change-Id: I3a87baa03c475f2b847b422b16b69c5ff7215b54
Reviewed-by: Kevin Buettner <kevinb@redhat.com>
Approved-By: Pedro Alves <pedro@palves.net>
2024-02-21 13:30:17 -05:00
Pedro Alves
38065394e9 Fix crash in aarch64-linux gdbserver
Since commit 393a6b5947 ("Thread options & clone events (Linux
GDBserver)"), aarch64-linux gdbserver crashes when the inferior
vforks.  This happens in aarch64_get_debug_reg_state:

  struct process_info *proc = find_process_pid (pid);

  return &proc->priv->arch_private->debug_reg_state;

Here, find_process_pid returns nullptr -- the new inferior hasn't yet
been created in linux_process_target::handle_extended_wait.

This patch fixes the problem by having
linux_process_target::handle_extended_wait create the child process
earlier, before the child LWP is created.  This is what the function
did before it was reorganized by the commit referred above.

Change-Id: Ib8b3a2e6048c3ad2b91a92ea4430da507db03c50
Co-Authored-By: Tom Tromey <tromey@adacore.com>
2024-02-09 11:58:40 +00:00
Feiyang Chen
e4d74c01e7 gdb: LoongArch: Add LBT extension support
Loongson Binary Translation (LBT) is used to accelerate binary
translation, which contains 4 scratch registers (scr0 to scr3),
x86/ARM eflags (eflags) and x87 fpu stack pointer (ftop). This
patch support gdb to fetch/store these registers.

Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn> #  Framework
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>   #  Detail Optimizes
Signed-off-by: Hui Li <lihui@loongson.cn>             #  Error Fixes
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
2024-02-06 18:40:19 +08:00
Hui Li
1e9569f383 gdb: LoongArch: Add vector extensions support
Add LoongArch's vector extensions support, which including
128bit LSX (i.e., Loongson SIMD eXtension) and 256bit LASX
(i.e., Loongson Advanced SIMD eXtension). This patch support
gdb to fetch/store vector registers.

Signed-off-by: Hui Li <lihui@loongson.cn>
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
2024-02-06 18:40:02 +08:00
Tom Tromey
a197d5f7eb Really fix Windows gdbserver segment registers
My earlier attempt to mask the segment registers in gdbserver for
Windows introduced some bugs.  That patch is here:

https://sourceware.org/pipermail/gdb-patches/2023-December/205306.html

The problem turned out to be that these fields in the Windows
'CONTEXT' type are just 16 bits, so while masking the values on read
is fine, writing the truncated values back then causes zeros to be
written to some subsequent field.

This patch cleans this up by arranging never to write too much data to
a field.

I also noticed that two register numbers here were never updated for
the 64-bit port.  This patch fixes this as well, and renames the "FCS"
register to follow gdb.

Finally, this patch applies the same treatment to windows-nat.c.

Reviewed-by: John Baldwin <jhb@FreeBSD.org>
2024-01-30 10:18:38 -07:00
Andrew Burgess
1d506c26d9 Update copyright year range in header of all files managed by GDB
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.
2024-01-12 15:49:57 +00:00
Mike Frysinger
761ed61e7b gdbsupport: tighten up libiberty code a bit with dnl
No functional change here, just touch up generated output slightly.

Approved-By: Tom Tromey <tom@tromey.com>
2024-01-10 19:53:14 -05:00
Mike Frysinger
908ff469f5 gdb: libiberty: switch to AC_CHECK_DECLS_ONCE
Only check these decls once in case other m4 macros also look for them.

Approved-By: Tom Tromey <tom@tromey.com>
2024-01-10 19:53:05 -05:00
Mike Frysinger
3f258212a8 gdb: move libiberty.m4 to gdbsupport
This is used by gdb, gdbsupport, and gdbserver.  We want to use it
in the sim tree too.  Move it to gdbsupport which is meant as the
common sharing space for these projects.

Approved-By: Tom Tromey <tom@tromey.com>
2024-01-10 19:52:52 -05:00
Tom Tromey
862180a2d1 Fix two bugs in gdbserver thread name handling
Simon pointed out that my earlier patch to gdbserver's thread name
code:

    commit 07b3255c3b
    Author: Tom Tromey <tom@tromey.com>
    Date:   Thu Jul 13 17:28:48 2023 -0600

	Filter invalid encodings from Linux thread names

... introduced a regression.  This bug was that the iconv output was
not \0-terminated.

Looking at it, I found another bug as well -- replace_non_ascii would
not \0-terminate, and also would return the wrong pointer

This patch fixes both of them.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31153
2024-01-09 07:07:41 -07:00
Simon Marchi
51e6b8cfd6 gdb: change regcache interface to use array_view
Change most of regcache (and base classes) to use array_view when
possible, instead of raw pointers.  By propagating the use of array_view
further, it enables having some runtime checks to make sure the what we
read from or write to regcaches has the expected length (such as the one
in the `copy(array_view, array_view)` function.  It also integrates well
when connecting with other APIs already using gdb::array_view.

Add some overloads of the methods using raw pointers to avoid having to
change all call sites at once (which is both a lot of work and risky).

I tried to do this change in small increments, but since many of these
functions use each other, it ended up simpler to do it in one shot than
having a lot of intermediary / transient changes.

This change extends into gdbserver as well, because there is some part
of the regcache interface that is shared.

Changing the reg_buffer_common interface to use array_view caused some
build failures in nat/aarch64-scalable-linux-ptrace.c.  That file
currently "takes advantage" of the fact that
reg_buffer_common::{raw_supply,raw_collect} operates on `void *`, which
IMO is dangerous.  It uses raw_supply/raw_collect directly on
uint64_t's, which I guess is fine because it is expected that native
code will have the same endianness as the debugged process.  To
accomodate that, add some overloads of raw_collect and raw_supply that
work on uint64_t.

This file also uses raw_collect and raw_supply on `char` pointers.
Change it to use `gdb_byte` pointers instead.  Add overloads of
raw_collect and raw_supply that work on `gdb_byte *` and make an
array_view on the fly using the register's size.  Those call sites could
be converted to use array_view with not much work, in which case these
overloads could be removed, but I didn't want to do it in this patch, to
avoid starting to dig in arch-specific code.

During development, I inadvertently changed reg_buffer::raw_compare's
behavior to not accept an offset equal to the register size.  This
behavior (effectively comparing 0 bytes, returning true) change was
caught by the AArch64 SME core tests.  Add a selftest to make sure that
this raw_compare behavior is preserved in the future.

Change-Id: I9005f04114543ddff738949e12d85a31855304c2
Reviewed-By: John Baldwin <jhb@FreeBSD.org>
2023-12-14 16:04:49 +00:00
Simon Marchi
e4e20d4511 gdb: use reg_buffer_common throughout gdbsupport/common-regcache.h
Right now, gdbsupport/common-regcache.h contains two abstractons for a
regcache.  An opaque type `regcache` (gdb and gdbserver both have their
own regcache that is the concrete version of this) and an abstract base
class `reg_buffer_common`, that is the base of regcaches on both sides.
These abstractions allow code to be written for both gdb and gdbserver,
for instance in the gdb/arch sub-directory.

However, having two
different abstractions is impractical.  If some common code has a regcache,
and wants to use an operation defined on reg_buffer_common, it can't.
It would be better to have just one.  Change all instances of `regcache
*` in gdbsupport/common-regcache.h to be `reg_buffer_common *`, then fix
fallouts.

Implementations in gdb and gdbserver now need to down-cast (using
gdb::checked_static_cast) from reg_buffer_common to their concrete
regcache type.  Some of them could be avoided by changing free functions
(like regcache_register_size) to be virtual methods on
reg_buffer_common.  I tried it, it seems to work, but I did not include
it in this series to avoid adding unnecessary changes.

Change-Id: Ia5503adb6b5509a0f4604bd2a68b4642cc5283fd
Reviewed-by: John Baldwin <jhb@FreeBSD.org>
2023-12-14 16:04:49 +00:00
Stefano Moioli
fa659800b0 gdbserver/win32: fix crash on detach
this patch fixes a crash in gdbserver whenever a process is detached.
the crash is caused by `detach` calling `remove_process` before `win32_clear_inferiors`

error message:

Detaching from process 184
../../gdbserver/inferiors.cc:160: A problem internal to GDBserver has been detec
ted.
remove_process: Assertion `find_thread_process (process) == NULL' failed.

This application has requested the Runtime to terminate it in an unusual way.
Please contact the application's support team for more information.

Approved-By: Tom Tromey <tom@tromey.com>
2023-12-12 13:19:30 -07:00
Andrew Burgess
8fd5a6058f gdbserver: allow for general 'monitor set debug COMPONENT VALUE' use
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>
2023-12-08 17:52:00 +00:00
Andrew Burgess
3bf9e166ca gdbserver: allow the --debug command line option to take a value
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>
2023-12-08 17:52:00 +00:00
Tom Tromey
74bbf9359a Update fall-through comment in gdbserver
I noticed that gdbserver/win32-low.cc has a fall-through comment that
should have been converted to use the annotation instead.

This patch makes the change.
2023-12-04 08:23:34 -07:00
Tom Tromey
c6f7f9c80c Bail out of "attach" if a thread cannot be traced
On Linux, threads are treated much like separate processes by the
kernel.  In particular, it's possible to ptrace just a single thread.
If gdb tries to attach to a multi-threaded inferior, where a non-main
thread is already being traced (e.g., by strace), then gdb will get
into an infinite loop attempting to attach.

This patch fixes this problem by having the attach fail if ptrace
fails to attach to any thread of the inferior.
2023-12-01 10:36:00 -07:00
Tom Tromey
69f6730df3 Remove gdb_static_assert
C++17 makes the second parameter to static_assert optional, so we can
remove gdb_static_assert now.
2023-11-29 14:29:44 -07:00
Tom Tromey
d57f38ec74 Switch to -Wimplicit-fallthrough=5
This changes the various gdb-related directories to use
-Wimplicit-fallthrough=5, meaning that only the fallthrough attribute
can be used in switches -- special 'fallthrough' comments will no
longer be usable.

Approved-By: Pedro Alves <pedro@palves.net>
2023-11-29 14:29:43 -07:00
Tom Tromey
d182e39881 Use C++17 [[fallthrough]] attribute
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>
2023-11-29 14:29:43 -07:00
Tom Tromey
602971b386 Introduce throw_winerror_with_name
This introduces throw_winerror_with_name, a Windows analog of
perror_with_name, and changes various places in gdb to call it.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30770
2023-11-27 12:55:14 -07:00
Andrew Burgess
27365c5189 gdbserver: cleanup monitor_show_help
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.
2023-11-22 15:20:31 +00:00
Lancelot Six
6b09f1342c gdb: Replace gdb::optional with std::optional
Since GDB now requires C++17, we don't need the internally maintained
gdb::optional implementation.  This patch does the following replacing:
  - gdb::optional -> std::optional
  - gdb::in_place -> std::in_place
  - #include "gdbsupport/gdb_optional.h" -> #include <optional>

This change has mostly been done automatically.  One exception is
gdbsupport/thread-pool.* which did not use the gdb:: prefix as it
already lives in the gdb namespace.

Change-Id: I19a92fa03e89637bab136c72e34fd351524f65e9
Approved-By: Tom Tromey <tom@tromey.com>
Approved-By: Pedro Alves <pedro@palves.net>
2023-11-21 11:52:35 +00:00
Sam James
9e11e7460d Finalized intl-update patches (deux)
Doing this on behalf of Arsen as obvious.

* gdb: Regenerate.
* gdbserver: Regenerate.
* gprofng: Regenerate.
2023-11-15 13:47:03 +00:00
Tom Tromey
07b3255c3b Filter invalid encodings from Linux thread names
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
2023-11-14 09:02:46 -07:00
Pedro Alves
e8a625d126 Report thread exit event for leader if reporting thread exit events
If GDB sets the GDB_THREAD_OPTION_EXIT option on a thread, then if the
thread disappears from the thread list, GDB expects to shortly see a
thread exit event for it.  See e.g., here, in
remote_target::update_thread_list():

    /* Do not remove the thread if we've requested to be
       notified of its exit.  For example, the thread may be
       displaced stepping, infrun will need to handle the
       exit event, and displaced stepping info is recorded
       in the thread object.  If we deleted the thread now,
       we'd lose that info.  */
    if ((tp->thread_options () & GDB_THREAD_OPTION_EXIT) != 0)
      continue;

There's one scenario that is deleting a thread from the
remote/gdbserver thread list without ever reporting a corresponding
thread exit event though -- check_zombie_leaders.  This can lead to
GDB getting confused.  For example, with a following patch that
enables GDB_THREAD_OPTION_EXIT whenever schedlock is enabled, we'd see
this regression:

 $ make check RUNTESTFLAGS="--target_board=native-extended-gdbserver" TESTS="gdb.threads/no-unwaited-for-left.exp"
 ...
 Running src/gdb/testsuite/gdb.threads/no-unwaited-for-left.exp ...
 FAIL: gdb.threads/no-unwaited-for-left.exp: continue stops when the main thread exits (timeout)
 ... some more cascading FAILs ...

gdb.log shows:

 (gdb) continue
 Continuing.
 FAIL: gdb.threads/no-unwaited-for-left.exp: continue stops when the main thread exits (timeout)

A passing run would have resulted in:

 (gdb) continue
 Continuing.
 No unwaited-for children left.
 (gdb) PASS: gdb.threads/no-unwaited-for-left.exp: continue stops when the main thread exits

note how the leader thread is not listed in the remote-reported XML
thread list below:

 (gdb) set debug remote 1
 (gdb) set debug infrun 1
 (gdb) info threads
   Id   Target Id                                Frame
 * 1    Thread 1163850.1163850 "no-unwaited-for" main () at /home/pedro/rocm/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.threads/no-unwaited-for-left.c:65
   3    Thread 1163850.1164130 "no-unwaited-for" [remote] Sending packet: $Hgp11c24a.11c362#39
 (gdb) c
 Continuing.
 [infrun] clear_proceed_status_thread: 1163850.1163850.0
 ...
     [infrun] resume_1: step=1, signal=GDB_SIGNAL_0, trap_expected=1, current thread [1163850.1163850.0] at 0x55555555534f
     [remote] Sending packet: $QPassSignals:#f3
     [remote] Packet received: OK
     [remote] Sending packet: $QThreadOptions;3:p11c24a.11c24a#f3
     [remote] Packet received: OK
 ...
     [infrun] target_set_thread_options: [options for Thread 1163850.1163850 are now 0x3]
 ...
   [infrun] do_target_resume: resume_ptid=1163850.1163850.0, step=0, sig=GDB_SIGNAL_0
   [remote] Sending packet: $vCont;c:p11c24a.11c24a#98
   [infrun] prepare_to_wait: prepare_to_wait
   [infrun] reset: reason=handling event
   [infrun] maybe_set_commit_resumed_all_targets: enabling commit-resumed for target extended-remote
   [infrun] maybe_call_commit_resumed_all_targets: calling commit_resumed for target extended-remote
 [infrun] fetch_inferior_event: exit
 [infrun] fetch_inferior_event: enter
   [infrun] scoped_disable_commit_resumed: reason=handling event
   [infrun] random_pending_event_thread: None found.
   [remote] wait: enter
     [remote] Packet received: N
   [remote] wait: exit
   [infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
   [infrun] print_target_wait_results:   -1.0.0 [process -1],
   [infrun] print_target_wait_results:   status->kind = NO_RESUMED
   [infrun] handle_inferior_event: status->kind = NO_RESUMED
   [remote] Sending packet: $Hgp0.0#ad
   [remote] Packet received: OK
   [remote] Sending packet: $qXfer:threads:read::0,1000#92
   [remote] Packet received: l<threads>\n<thread id="p11c24a.11c362" core="0" name="no-unwaited-for" handle="0097d8f7ff7f0000"/>\n</threads>\n
   [infrun] handle_no_resumed: TARGET_WAITKIND_NO_RESUMED (ignoring: found resumed)
 ...

... however, infrun decided there was a resumed thread still, so
ignored the TARGET_WAITKIND_NO_RESUMED event.  Debugging GDB, we see
that the "found resumed" thread that GDB finds, is the leader thread.
Even though that thread is not on the remote-reported thread list, it
is still on the GDB thread list, due to the special case in remote.c
mentioned above.

This commit addresses the issue by fixing GDBserver to report a thread
exit event for the zombie leader too, i.e., making GDBserver respect
the "if thread has GDB_THREAD_OPTION_EXIT set, report a thread exit"
invariant.  To do that, it takes a bit more code than one would
imagine off hand, due to the fact that we currently always report LWP
exit pending events as TARGET_WAITKIND_EXITED, and then decide whether
to convert it to TARGET_WAITKIND_THREAD_EXITED just before reporting
the event to GDBserver core.  For the zombie leader scenario
described, we need to record early on that we want to report a
THREAD_EXITED event, and then make sure that decision isn't lost along
the way to reporting the event to GDBserver core.

Reviewed-By: Andrew Burgess <aburgess@redhat.com>
Change-Id: I1e68fccdbc9534434dee07163d3fd19744c8403b
2023-11-13 14:16:11 +00:00
Pedro Alves
ef980d654b gdbserver: Queue no-resumed event after thread exit
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
2023-11-13 14:16:11 +00:00
Pedro Alves
4898949800 Implement GDB_THREAD_OPTION_EXIT support for Linux GDBserver
This implements support for the new GDB_THREAD_OPTION_EXIT thread
option for Linux GDBserver.

Reviewed-By: Andrew Burgess <aburgess@redhat.com>
Change-Id: I96b719fdf7fee94709e98bb3a90751d8134f3a38
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27338
2023-11-13 14:16:10 +00:00
Pedro Alves
ad320fbf91 gdbserver/linux-low.cc: Ignore event_ptid if TARGET_WAITKIND_IGNORE
gdbserver's linux_process_target::wait loops if:

 - called in sync mode, and,
 - wait_1 returns TARGET_WAITKIND_IGNORE, _and_,
 - wait_1 also returns null_ptid.

The null_ptid check fails however when this path is taken:

   ptid_t
   linux_process_target::filter_exit_event (lwp_info *event_child,
					    target_waitstatus *ourstatus)
   {
   ...
     if (!is_leader (thread))
       {
	 if (report_exit_events_for (thread))
	   ourstatus->set_thread_exited (0);
	 else
	   ourstatus->set_ignore ();            <<<<<<<

	 delete_lwp (event_child);
       }
     return ptid;
   }

This makes linux_process_target::wait return TARGET_WAITKIND_IGNORE in
sync mode, which is unexpected by the core and fails an assertion.

This commit fixes it by just making linux_process_target::wait loop if
it got a TARGET_WAITKIND_IGNORE, irrespective of event_ptid.

Change-Id: I39776908a6c75cbd68aa04139ffcf7be334868cf
2023-11-13 14:16:10 +00:00
Pedro Alves
00b0dc819d all-stop/synchronous RSP support thread-exit events
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
2023-11-13 14:16:10 +00:00
Pedro Alves
faf44a3105 gdbserver: Hide and don't detach pending clone children
This commit extends the logic added by these two commits from a while
ago:

 #1  7b961964f8  (gdbserver: hide fork child threads from GDB),
 #2  df5ad10200  (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
2023-11-13 14:16:10 +00:00
Pedro Alves
393a6b5947 Thread options & clone events (Linux GDBserver)
This patch teaches the Linux GDBserver backend to report clone events
to GDB, when GDB has requested them with the GDB_THREAD_OPTION_CLONE
thread option, via the new QThreadOptions packet.

This shuffles code in linux_process_target::handle_extended_wait
around to a more logical order when we now have to handle and
potentially report all of fork/vfork/clone.

Raname lwp_info::fork_relative -> lwp_info::relative as the field is
no longer only about (v)fork.

With this, gdb.threads/stepi-over-clone.exp now cleanly passes against
GDBserver, so remove the native-target-only requirement from that
testcase.

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: I3a19bc98801ec31e5c6fdbe1ebe17df855142bb2
2023-11-13 14:16:10 +00:00
Pedro Alves
65c459abeb Thread options & clone events (core + remote)
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
2023-11-13 14:16:09 +00:00
Pedro Alves
53de5394f7 Support clone events in the remote protocol
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
2023-11-13 14:16:09 +00:00
Tom Tromey
cf0d07fd07 Remove EXTERN_C and related defines
common-defs.h has a few defines that I suspect were used during the
transition to C++.  These aren't needed any more, so remove them.

Tested by rebuilding.

Approved-By: Simon Marchi <simon.marchi@efficios.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
2023-11-06 20:31:12 -07:00
Lancelot Six
f74dc26792 gdb/gdbsupport/gdbserver: Require c++17
This patch proposes to require a C++17 compiler to build gdb /
gdbsupport / gdbserver.  Before this patch, GDB required a C++11
compiler.

The general policy regarding bumping C++ language requirement in GDB (as
stated in [1]) is:

    Our general policy is to wait until the oldest compiler that
    supports C++NN is at least 3 years old.

    Rationale: We want to ensure reasonably widespread compiler
    availability, to lower barrier of entry to GDB contributions, and to
    make it easy for users to easily build new GDB on currently
    supported stable distributions themselves. 3 years should be
    sufficient for latest stable releases of distributions to include a
    compiler for the standard, and/or for new compilers to appear as
    easily installable optional packages. Requiring everyone to build a
    compiler first before building GDB, which would happen if we
    required a too-new compiler, would cause too much inconvenience.

    See the policy proposal and discussion
    [here](https://sourceware.org/ml/gdb-patches/2016-10/msg00616.html).

The first GCC release which with full C++17 support is GCC-9[2],
released in 2019[3], which is over 4 years ago.  Clang has had C++17
support since Clang-5[4] released in 2018[5].

A discussions with many distros showed that a C++17-able compiler is
always available, meaning that this no hard requirement preventing us to
require it going forward.

[1] https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#When_is_GDB_going_to_start_requiring_C.2B-.2B-NN_.3F
[2] https://gcc.gnu.org/projects/cxx-status.html#cxx17
[3] https://gcc.gnu.org/gcc-9/
[4] https://clang.llvm.org/cxx_status.html
[5] https://releases.llvm.org/

Change-Id: Id596f5db17ea346e8a978668825787b3a9a443fd
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
Approved-By: Pedro Alves <pedro@palves.net>
2023-10-28 19:25:34 +00:00
Lancelot Six
eb4de4047d gdb/ax_cxx_compile_stdcxx.m4: upgrade
This patch upgrades gdb/ax_cxx_compile_stdcxx.m4 to follow changes
available in [1] and regenerates the configure script.

[1] https://www.gnu.org/software/autoconf-archive/ax_cxx_compile_stdcxx.html

Change-Id: I5b16adc65c9e48a13ad65202d58ab7a9d487214e
Approved-By: Tom Tromey <tom@tromey.com>
Approved-By: Pedro Alves <pedro@palves.net>
2023-10-28 19:21:32 +00:00
Andrew Burgess
fd492bf1e2 gdb: handle main thread exiting during detach
Overview
========

Consider the following situation, GDB is in non-stop mode, the main
thread is running while a second thread is stopped.  The user has the
second thread selected as the current thread and asks GDB to detach.
At the exact moment of detach the main thread exits.

This situation currently causes crashes, assertion failures, and
unexpected errors to be reported from GDB for both native and remote
targets.

This commit addresses this situation for native and remote targets.
There are a number of different fixes, but all are required in order
to get this functionality working correct for native and remote
targets.

Native Linux Target
===================

For the native Linux target, detaching is handled in the function
linux_nat_target::detach.  In here we call stop_wait_callback for each
thread, and it is this callback that will spot that the main thread
has exited.

GDB then detaches from everything except the main thread by calling
detach_callback.

After this the first problem is this assert:

  /* Only the initial process should be left right now.  */
  gdb_assert (num_lwps (pid) == 1);

The num_lwps call will return 0 as the main thread has exited and all
of the other threads have now been detached.  I fix this by changing
the assert to allow for 0 or 1 lwps at this point.  As the 0 case can
only happen in non-stop mode, the assert becomes:

  gdb_assert (num_lwps (pid) == 1
	      || (target_is_non_stop_p () && num_lwps (pid) == 0));

The next problem is that we do:

  main_lwp = find_lwp_pid (ptid_t (pid));

and then proceed assuming that main_lwp is not nullptr.  In the case
that the main thread has exited though, main_lwp will be nullptr.

However, we only need main_lwp so that GDB can detach from the
thread.  If the main thread has exited, and GDB has already detached
from every other thread, then GDB has finished detaching, GDB can skip
the calls that try to detach from the main thread, and then tell the
user that the detach was a success.

For Remote Targets
==================

On remote targets there are two problems.

First is that when the exit occurs during the early phase of the
detach, we see the stop notification arrive while GDB is removing the
breakpoints ahead of the detach.  The 'set debug remote on' trace
looks like this:

  [remote] Sending packet: $z0,7f1648fe0241,1#35
  [remote]   Notification received: Stop:W0;process:2a0ac8
  # At this point an unpatched gdbserver segfaults, and the connection
  # is broken.  A patched gdbserver continues as below...
  [remote] Packet received: E01
  [remote] Sending packet: $z0,7f1648ff00a8,1#68
  [remote] Packet received: E01
  [remote] Sending packet: $z0,7f1648ff132f,1#6b
  [remote] Packet received: E01
  [remote] Sending packet: $D;2a0ac8#3e
  [remote] Packet received: E01

I was originally running into Segmentation Faults, from within
gdbserver/mem-break.cc, in the function find_gdb_breakpoint.  This
function calls current_process() and then dereferences the result to
find the breakpoint list.

However, in our case, the current process has already exited, and so
the current_process() call returns nullptr.  At the point of failure,
the gdbserver backtrace looks like this:

  #0  0x00000000004190e4 in find_gdb_breakpoint (z_type=48 '0', addr=4198762, kind=1) at ../../src/gdbserver/mem-break.cc:982
  #1  0x000000000041930d in delete_gdb_breakpoint (z_type=48 '0', addr=4198762, kind=1) at ../../src/gdbserver/mem-break.cc:1093
  #2  0x000000000042d8db in process_serial_event () at ../../src/gdbserver/server.cc:4372
  #3  0x000000000042dcab in handle_serial_event (err=0, client_data=0x0) at ../../src/gdbserver/server.cc:4498
  ...

The problem is that, as a result non-stop being on, the process
exiting is only reported back to GDB after the request to remove a
breakpoint has been sent.  Clearly gdbserver can't actually remove
this breakpoint -- the process has already exited -- so I think the
best solution is for gdbserver just to report an error, which is what
I've done.

The second problem I ran into was on the gdb side, as the process has
already exited, but GDB has not yet acknowledged the exit event, the
detach -- the 'D' packet in the above trace -- fails.  This was being
reported to the user with a 'Can't detach process' error.  As the test
actually calls detach from Python code, this error was then becoming a
Python exception.

Though clearly the detach has returned an error, and so, maybe, having
GDB throw an error would be fine, I think in this case, there's a good
argument that the remote error can be ignored -- if GDB tries to
detach and gets back an error, and if there's a pending exit event for
the pid we tried to detach, then just ignore the error and pretend the
detach worked fine.

We could possibly check for a pending exit event before sending the
detach packet, however, I believe that it might be possible (in
non-stop mode) for the stop notification to arrive after the detach is
sent, but before gdbserver has started processing the detach.  In this
case we would still need to check for pending stop events after seeing
the detach fail, so I figure there's no point having two checks -- we
just send the detach request, and if it fails, check to see if the
process has already exited.

Testing
=======

In order to test this issue I needed to ensure that the exit event
arrives at the same time as the detach call.  The window of
opportunity for getting the exit to arrive is so small I've never
managed to trigger this in real use -- I originally spotted this issue
while working on another patch, which did manage to trigger this
issue.

However, if we trigger both the exit and the detach from a single
Python function then we never return to GDB's event loop, as such GDB
never processes the exit event, and so the first time GDB gets a
chance to see the exit is during the detach call.  And so that is the
approach I've taken for testing this patch.

Tested-By: Kevin Buettner <kevinb@redhat.com>
Approved-By: Kevin Buettner <kevinb@redhat.com>
2023-10-26 18:11:54 +01:00
Andrew Burgess
fe7c8e26fc gdbserver: don't leak program name in handle_v_run
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>
2023-10-25 16:51:19 +01:00
Tom Tromey
5772d79823 Move -lsocket check to common.m4
A user pointed out that the -lsocket check in gdb should also apply to
gdbserver -- otherwise it can't find the Solaris socketpair.  This
patch makes the change.  It also removes a couple of redundant
function checks from gdb's configure.ac.

This was tested by the person who reported the bug.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30927
Approved-By: Pedro Alves <pedro@palves.net>
2023-10-12 18:23:13 -06:00
Andrew Burgess
dd6a9ec16e gdbserver: fix gdbserver builds after expedite_regs changes
After this commit:

  commit 6a65998a8a
  Date:   Mon Sep 11 12:42:00 2023 +0100

      Convert tdesc's expedite_regs to a string vector

The risc-v, loongarch, and csky gdbserver builds were broken.  A use
of target_desc::expedite_regs (for each architecture)  was not updated
to take account of the type change.

I've tested that this fixes the risc-v build.  I haven't tested the
other architectures, but they should be fine.
2023-10-06 13:16:16 +01:00
Andrew Burgess
f9089d2f7b gdbserver: cleanup in handle_v_run
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>
2023-10-06 13:02:36 +01:00
Andrew Burgess
67e6945b7e gdbserver: handle newlines in inferior arguments
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>
2023-10-06 13:02:36 +01:00