Files
binutils-gdb/gdbserver/server.h
Andrew Burgess 8e28eef6cd gdb/gdbserver: pass inferior arguments as a single string
GDB holds the inferior arguments as a single string.  Currently when
GDB needs to pass the inferior arguments to a remote target as part of
a vRun packet, this is done by splitting the single argument string
into its component arguments by calling gdb::remote_args::split, which
uses the gdb_argv class to split the arguments for us.

The same gdb_argv class is used when the user has asked GDB/gdbserver
to start the inferior without first invoking a shell; the gdb_argv
class is used to split the argument string into it component
arguments, and each is passed as a separate argument to the execve
call which spawns the inferior.

There is however, a problem with using gdb_argv to split the arguments
before passing them to a remote target.  To understand this problem we
must first understand how gdb_argv is used when invoking an inferior
without a shell.

And to understand how gdb_argv is used to start an inferior without a
shell, I feel we need to first look at an example of starting an
inferior with a shell.

Consider these two cases:

  (a)  (gdb) set args \$VAR
  (b)  (gdb) set args $VAR

When starting with a shell, in case (a) the user expects the inferior
to receive a literal '$VAR' string as an argument, while in case (b)
the user expects to see the shell expanded value of the variable $VAR.

If the user does 'set startup-with-shell off', then in (a) GDB will
strip the '\' while splitting the arguments, and the inferior will be
passed a literal '$VAR'.  In (b) there is no '\' to strip, so also in
this case the inferior will receive a literal '$VAR', remember
startup-with-shell is off, so there is no shell that can ever expand
$VAR.

Notice, that when startup-with-shell is off, we end up with a many to
one mapping, both (a) and (b) result in the literal string $VAR being
passed to the inferior.  I think this is the correct behaviour in this
case.

However, as we use gdb_argv to split the remote arguments we have the
same many to one mapping within the vRun packet.  But the vRun packet
will be used when startup-with-shell is both on and off.  What this
means is that when gdbserver receives a vRun packet containing '$VAR'
it doesn't know if GDB actually had '$VAR', or if GDB had '\$VAR'.
And this is a huge problem.

We can address this by making the argument splitting for remote
targets smarter, and I do have patches that try to do this in this
series:

  https://inbox.sourceware.org/gdb-patches/cover.1730731085.git.aburgess@redhat.com

That series was pretty long, and wasn't getting reviewed, so I'm
pulling the individual patches out and posting them separately.

This patch doesn't try to improve remote argument splitting.  I think
that splitting and then joining the arguments is a mistake which can
only introduce problems.  The patch in the above series which tries to
make the splitting and joining "smarter" handles unquoted, single
quoted, and double quoted strings.  But that doesn't really address
parameter substitution, command substitution, or arithmetic expansion.
And even if we did try to address these cases, what rules exactly
would we implement?  Probably POSIX shell rules, but what if the
remote target doesn't have a POSIX shell?  The only reason we're
talking about which shell rules to follow is because the splitting and
joining logic needs to mirror those rules.  If we stop splitting and
joining then we no longer need to care about the target's shell.

Clearly, for backward compatibility we need to maintain some degree of
argument splitting and joining as we currently have; and that's why I
have a later patch (see the series above) that tries to improve that
splitting and joining a little.  But I think, what we should really
do, is add a new feature flag (as used by the qSupported packet) and,
if GDB and the remote target agree, we should pass the inferior
arguments as a single string.

This solves all our problems.  In the startup with shell case, we no
longer need to worry about splitting at all.  The arguments are passed
unmodified to the remote target, that can then pass the arguments to
the shell directly.

In the 'startup-with-shell off' case it is now up to the remote target
to split the arguments, though in gdbserver we already did this, so
nothing really changes in this case.  And if the remote target doesn't
have a POSIX shell, well GDB just doesn't need to worry about it!

Something similar to this was originally suggested in this series:

  https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de/

though this series didn't try to maintain backward compatibility,
which I think is an issue that my patch solves.  Additionally, this
series only passed the arguments as a single string in some cases,
I've simplified this so that, when GDB and the remote agree, the
arguments are always passed as a single string.  I think this is a
little cleaner.

I've also added documentation and some tests with this commit,
including ensuring that we test both the new single string approach,
and the fallback split/join approach.

I've credited the author of the referenced series as co-author as they
did come to a similar conclusion, though I think my implementation is
different enough that I'm happy to list myself as primary author.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28392

Co-Authored-By: Michael Weghorn <m.weghorn@posteo.de>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Tested-By: Guinevere Larsen <guinevere@redhat.com>
Approved-by: Kevin Buettner <kevinb@redhat.com>
2025-09-12 11:06:00 +01:00

213 lines
6.7 KiB
C

/* Common definitions for remote server for GDB.
Copyright (C) 1993-2025 Free Software Foundation, Inc.
This file is part of GDB.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 3 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
#ifndef GDBSERVER_SERVER_H
#define GDBSERVER_SERVER_H
#include "gdbsupport/common-defs.h"
#undef PACKAGE
#undef PACKAGE_NAME
#undef PACKAGE_VERSION
#undef PACKAGE_STRING
#undef PACKAGE_TARNAME
#include <config.h>
static_assert (sizeof (CORE_ADDR) >= sizeof (void *));
#include "gdbsupport/version.h"
#if !HAVE_DECL_PERROR
#ifndef perror
extern void perror (const char *);
#endif
#endif
#if !HAVE_DECL_VASPRINTF
extern int vasprintf(char **strp, const char *fmt, va_list ap);
#endif
#if !HAVE_DECL_VSNPRINTF
int vsnprintf(char *str, size_t size, const char *format, va_list ap);
#endif
#ifdef IN_PROCESS_AGENT
# define PROG "ipa"
#else
# define PROG "gdbserver"
#endif
#include "gdbsupport/xml-utils.h"
#include "regcache.h"
#include "gdbsupport/gdb_signals.h"
#include "target.h"
#include "mem-break.h"
#include "gdbsupport/environ.h"
/* Target-specific functions */
void initialize_low ();
/* Public variables in server.c */
extern bool server_waiting;
extern bool disable_packet_vCont;
extern bool disable_packet_vCont_step;
extern bool disable_packet_Tthread;
extern bool disable_packet_qC;
extern bool disable_packet_qfThreadInfo;
extern bool disable_packet_T;
extern bool run_once;
extern bool non_stop;
#include "gdbsupport/event-loop.h"
/* Functions from server.c. */
extern void handle_v_requests (char *own_buf, int packet_len,
int *new_packet_len);
extern void handle_serial_event (int err, gdb_client_data client_data);
extern void handle_target_event (int err, gdb_client_data client_data);
/* Get rid of the currently pending stop replies that match PTID. */
extern void discard_queued_stop_replies (ptid_t ptid);
/* Returns true if there's a pending stop reply that matches PTID in
the vStopped notifications queue. */
extern int in_queued_stop_replies (ptid_t ptid);
#include "remote-utils.h"
#include "utils.h"
#include "debug.h"
/* Maximum number of bytes to read/write at once. The value here
is chosen to fill up a packet (the headers account for the 32). */
#define MAXBUFBYTES(N) (((N)-32)/2)
/* Buffer sizes for transferring memory, registers, etc. Set to a constant
value to accommodate multiple register formats. This value must be at least
as large as the largest register set supported by gdbserver. */
#define PBUFSIZ 131104
/* Definition for an unknown syscall, used basically in error-cases. */
#define UNKNOWN_SYSCALL (-1)
/* Definition for any syscall, used for unfiltered syscall reporting. */
#define ANY_SYSCALL (-2)
/* After fork_inferior has been called, we need to adjust a few
signals and call startup_inferior to start the inferior and consume
its first events. This is done here. PID is the pid of the new
inferior and PROGRAM is its name. */
extern void post_fork_inferior (int pid, const char *program);
/* Get the gdb_environ being used in the current session. */
extern gdb_environ *get_environ ();
extern unsigned long signal_pid;
/* Description of the client remote protocol state for the currently
connected client. */
struct client_state
{
client_state ():
own_buf ((char *) xmalloc (PBUFSIZ + 1))
{}
/* The thread set with an `Hc' packet. `Hc' is deprecated in favor of
`vCont'. Note the multi-process extensions made `vCont' a
requirement, so `Hc pPID.TID' is pretty much undefined. So
CONT_THREAD can be null_ptid for no `Hc' thread, minus_one_ptid for
resuming all threads of the process (again, `Hc' isn't used for
multi-process), or a specific thread ptid_t. */
ptid_t cont_thread;
/* The thread set with an `Hg' packet. */
ptid_t general_thread;
int multi_process = 0;
int report_fork_events = 0;
int report_vfork_events = 0;
int report_exec_events = 0;
int report_thread_events = 0;
/* True if the "swbreak+" feature is active. In that case, GDB wants
us to report whether a trap is explained by a software breakpoint
and for the server to handle PC adjustment if necessary on this
target. Only enabled if the target supports it. */
int swbreak_feature = 0;
/* True if the "hwbreak+" feature is active. In that case, GDB wants
us to report whether a trap is explained by a hardware breakpoint.
Only enabled if the target supports it. */
int hwbreak_feature = 0;
/* True if the "vContSupported" feature is active. In that case, GDB
wants us to report whether single step is supported in the reply to
"vCont?" packet. */
int vCont_supported = 0;
/* Whether we should attempt to disable the operating system's address
space randomization feature before starting an inferior. */
int disable_randomization = 1;
int pass_signals[GDB_SIGNAL_LAST];
int program_signals[GDB_SIGNAL_LAST];
int program_signals_p = 0;
/* Last status reported to GDB. */
struct target_waitstatus last_status;
ptid_t last_ptid;
char *own_buf;
/* If true, then GDB has requested noack mode. */
int noack_mode = 0;
/* If true, then we tell GDB to use noack mode by default. */
int transport_is_reliable = 0;
/* The traceframe to be used as the source of data to send back to
GDB. A value of -1 means to get data from the live program. */
int current_traceframe = -1;
/* If true, memory tagging features are supported. */
bool memory_tagging_feature = false;
/* If true then E.errtext style errors are supported everywhere,
including for the qRcmd and m packet. When false E.errtext errors
are not supported with qRcmd and m packets, but are still supported
everywhere else. This is for backward compatibility reasons. */
bool error_message_supported = false;
/* If true then we've agreed that the debugger will send all inferior
arguments as a single string. When false the debugger will attempt
to split the inferior arguments before sending them. */
bool single_inferior_argument = false;
};
client_state &get_client_state ();
#include "gdbthread.h"
#include "inferiors.h"
#endif /* GDBSERVER_SERVER_H */