Re: [PATCH 2/2] gdb: use current executable for 'remote exec-file' in some cases

[Simon Marchi <simark@simark.ca>]

On 10/11/25 9:34 AM, Andrew Burgess wrote:
> This commit allows GDB to make use of the file set with the 'file'
> command when starting a new inferior on an extended-remote target.
> There are however some restrictions.
> 
> If the user has used 'set remote exec-file', then this setting is
> always used in preference to the file set with the 'file' command.
> 
> Similarly, if the qExecAndArgs packet has succeeded, and GDB knows
> that the remote target has an executable set, then this will be used
> in preference to the file set with the 'file' command; this preserves
> GDB's existing behaviour.  In effect, when GDB connects to the remote
> target, the remote sets the 'remote exec-file' and this prevents GDB
> from using the 'file' filename.
> 
> And, GDB can only use the file set with the 'file' command if it
> believes that both GDB and the remote target will both be able to
> access this file.  This means that one of these is true:
> 
>   + the the remote_target::filesystem_is_local function returns
>     true (see the implementation of that function for details of when
>     this can happen).  This means GDB and the remote target can see
>     the same file system, GDB can just use the current executable's
>     filename as is, or
> 
>   + the user has set the 'file' to something with a 'target:' prefix,
>     e.g. 'file target:/path/to/exec'.  In this last case, GDB will use
>     the exec filename without the 'target:' prefix, this filename is,
>     by definition, something the remote target can see, or
> 
>   + the sysroot has been updated by the user and no longer contains a
>     'target:' prefix.  In this case, if the 'file' filename is within
>     the sysroot, then it is assumed the remote will also be able to
>     see a file with the same filename.  For example, if the sysroot is
>     '/aa/', and the current executable is '/aa/bb/cc', then GDB will
>     tell the remote to run '/bb/cc'.  One common case here is when the
>     sysroot is set to the empty string, which is usually done when GDB
>     and the remote target can see the same filesystem, in this case
>     GDB will use the current executable's filename unmodified.
> 
> If one of these conditions is met, then GDB will use the current
> executable's filename (with possible modifications as mentioned
> above), when starting a new extended-remote inferior, in all other
> cases, GDB will use the file name  set with 'set remote exec-file'.
> 
> This change could be useful any time a user is running a remote target
> on the same machine as GDB, but I am specifically thinking of the case
> where GDB is using a tool other than gdbserver, e.g. valgrind, as this
> saves one additional step that a user must remember.  The current
> steps to start valgrind with GDB, as given on the valgrind
> website (https://valgrind.org/docs/manual/manual-core-adv.html) are:
> 
>   $ gdb prog
>   (gdb) set remote exec-file prog
>   (gdb) set sysroot /
>   (gdb) target extended-remote | vgdb --multi --vargs -q
>   (gdb) start
> 
> With this GDB work, and once support for the qExecAndArgs packet is
> added to valgrind, then the 'set remote exec-file' line can be dropped
> from those instructions.
> 
> This commit also extends the 'show remote exec-file' command so that
> GDB will display the automatic value that it plans to use.  Here's an
> example of the new output:
> 
>   $ gdb -q /tmp/hello
>   Reading symbols from /tmp/hello...
>   (gdb) set sysroot
>   (gdb) target extended-remote | ./gdbserver/gdbserver --multi --once -
>   Remote debugging using | ./gdbserver/gdbserver --multi --once -
>   Remote debugging using stdio
>   (gdb) show remote exec-file
>   The remote exec-file is unset, using automatic value "/tmp/hello".
> 
> The last line shows the new output.
> 
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>

I am investigating a regression when running gdb.gdb/selftest.exp with
the native-extended-gdbserver board, caused by my commit d6340aa42e25
("gdb/testsuite: use libtool to launch selftests").  A side-effect of my
change is that the test no longer does a "set remote exec-file", which
is currently required in order to run a new process with an
extended-remote board.  I started writing a patch to add a manual "set
remote exec-file", but then remembered about this patch.  I gave it a
try, and indeed it fixes those regressions.  I would therefore be very
happy to see this patch merged (I had forgotten about it before just now
to be honest).

I'm fine with what you have see below for small comments.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

> @@ -43,9 +43,11 @@
>  #include "target-descriptions.h"
>  #include "gdb_bfd.h"
>  #include "gdbsupport/filestuff.h"
> +#include "gdbsupport/pathstuff.h"
>  #include "gdbsupport/rsp-low.h"
>  #include "disasm.h"
>  #include "location.h"
> +#include "filenames.h"
>  
>  #include "gdbsupport/gdb_sys_time.h"
>  
> @@ -1555,8 +1557,49 @@ class extended_remote_target final : public remote_target
>  
>    void post_attach (int) override;
>    bool supports_disable_randomization () override;
> +
> +  /* Return the file name for the executable that GDB should ask the remote
> +     target to use when starting an inferior.
> +
> +     EXEC_FILE is the string passed from GDB core, which is the file name
> +     of the current executable, which can be NULL.
> +
> +     If the user has done 'set remote exec-file', or the remote told GDB
> +     which executable to use via the 'qExecAndArgs' packet, then this is
> +     the value returned by this function, EXEC_FILE is ignored.
> +
> +     But if there is no remote exec-file set, then we might be able to use
> +     EXEC_FILE, or a variation of EXEC_FILE, as the 'remote exec-file'
> +     setting.  This will depend on the value of EXEC_FILE and/or the
> +     current gdb_sysroot setting.  In this case the file name derived from
> +     EXEC_FILE is returned.
> +
> +     If neither approach comes up with a suitable file name to run then
> +     the empty string is returned.  */
> +
> +  std::string get_exec_file_for_create_inferior (const char *exec_file);
>  };
>  
> +/* Get a pointer to the current remote target.  If not connected to a
> +   remote target, return NULL.  */
> +
> +static remote_target *
> +get_current_remote_target ()
> +{
> +  target_ops *proc_target = current_inferior ()->process_target ();
> +  return dynamic_cast<remote_target *> (proc_target);
> +}
> +
> +/* Get a pointer to the current remote target.  If not connected to a
> +   remote target, return NULL.  */

Here, I'd say:

/* Get a pointer to the current extended-remote target.  If not connected to an
   extended-remote target, return NULL.  */

> @@ -11319,6 +11369,84 @@ directory: %s"),
>      }
>  }
>  
> +/* See class declaration above.  */
> +
> +std::string
> +extended_remote_target::get_exec_file_for_create_inferior
> +  (const char *exec_file)
> +{
> +  const remote_exec_file_info &info
> +    = get_remote_exec_file_info (current_program_space);
> +
> +  /* Historically, when the remote target started a new inferior GDB would
> +     ignore the filename from GDB core (EXEC_FILE) and would use whatever
> +     value the user had set in 'remote exec-file'.  Now we try to be
> +     smarter.
> +
> +     Obviously, if 'remote exec-file' has been set, then this should be
> +     considered definitive.  But if 'remote exec-file' has not been set,
> +     then, in some cases, we might be able to use EXEC_FILE, or a
> +     derivative of EXEC_FILE.
> +
> +     It can also happen that EXEC_FILE is NULL.  This is mostly a bit of an
> +     edge case where GDB has attached to a running process, and couldn't
> +     figure out the filename for the executable.  If the user then does
> +     'run' we could end up with EXEC_FILE being NULL.  If this happens then
> +     the only option is to use the 'remote exec-file' setting.
> +
> +     If INFO.SOURCE is ::VALUE_FROM_REMOTE or ::VALUE_FROM_GDB then there
> +     is a value in 'remote exec-file', we should not do anything with
> +     EXEC_FILE and just retain the 'remote exec-file' value.
> +
> +     If INFO.SOURCE is ::UNSET_VALUE then the user hasn't 'set remote

The apostrophe is at the wrong place... it seems.  Not sure if it's
intentional.

> +     exec-file' value, and the remote target has specifically told us (via
> +     the qExecAndArgs packet) that it has no default executable set.  In
> +     this case, if GDB and the remote can see the same filesystem, we can
> +     potentially use EXEC_FILE.
> +
> +     If INFO.SOURCE is ::DEFAULT_VALUE then the user hasn't set a 'remote
> +     exec-file' value, but the remote target was unable to tell us (maybe
> +     the qExecAndArgs packet isn't supported) if it has a default
> +     executable set.  We might be tempted to treat this like the
> +     ::UNSET_VALUE case, however, this could potentially break backward
> +     compatibility in the case where the remote does have a default
> +     executable set.  To maintain compatibility, we send over the 'remote
> +     exec-file' setting, whatever it might be.  */
> +  if (exec_file != nullptr
> +      && info.source == remote_exec_source::UNSET_VALUE)
> +    {
> +      /* If the user has set the core exec file to a file on the target
> +	 then we can just strip the target prefix and use that as the
> +	 remote exec file name.  */
> +      if (is_target_filename (exec_file))
> +	return exec_file + strlen (TARGET_SYSROOT_PREFIX);
> +
> +      /* If the target filesystem is local then the remote can see
> +	 everything GDB can see.  In this case the remote should be able to
> +	 access EXEC_FILE.  */
> +      if (target_filesystem_is_local ())
> +	return exec_file;
> +
> +      /* If the sysroot is not a target path, then GDB can see a copy of
> +	 the remote target's filesystem, or if sysroot is empty, then the
> +	 remote and GDB could be sharing a filesystem.
> +
> +	 In either case, by removing the sysroot from the front of
> +	 EXEC_FILE, we can build a filename that the remote can see.  */
> +      if (!is_target_filename (gdb_sysroot))
> +	{
> +	  const char *in_sysroot_path = child_path (gdb_sysroot.c_str (),
> +						    exec_file);
> +	  if (in_sysroot_path != nullptr)
> +	    return path_join ("/", in_sysroot_path);
> +	}
> +    }
> +
> +  /* The user has set the remote exec-file, or GDB doesn't think the remote
> +     target and GDB can see the same filesystem.  */
> +  return info.filename;
> +}

Is it possible to organize the code in an early return style, where you
check the cases in order of precedence first?  I think that would be a
bit clearer, and it would probably match the order you explain things in
your comment.  I haven't actually tried it, if it doesn't work out,
you can ignore this.

> diff --git a/gdb/testsuite/gdb.server/ext-run.exp b/gdb/testsuite/gdb.server/ext-run.exp
> index f4ff546c393..18205e11ec8 100644
> --- a/gdb/testsuite/gdb.server/ext-run.exp
> +++ b/gdb/testsuite/gdb.server/ext-run.exp
> @@ -30,43 +30,147 @@ if {[build_executable $testfile.exp $testfile $srcfile debug] == -1} {
>  # allow_xml_test must be called while gdb is not running.
>  set do_xml_test [allow_xml_test]
>  
> -save_vars { GDBFLAGS } {
> -    # If GDB and GDBserver are both running locally, set the sysroot to avoid
> -    # reading files via the remote protocol.
> -    if { ![is_remote host] && ![is_remote target] } {
> -	set GDBFLAGS "$GDBFLAGS -ex \"set sysroot\""
> +# This is used as an override function.
> +proc do_nothing {} { return 0 }
> +
> +# Start an exetended-remote gdbserver, connect to it, and then use

"exetended"

> +# 'run' to start an inferior.
> +#
> +# If CLEAR_SYSROOT is true then the 'set sysroot' command is issued,
> +# clearing the sysroot, otherwise the sysroot is left unchanged.
> +#
> +# If SET_REMOTE_EXEC is true then the 'set remote-exec ...' command is

I guess you mean 'set remote exec-file', since 'set remote-exec' doesn't
exist.

> +# issued to point GDB at the executable on the target (after copying
> +# the executable over).  Otherwise, we rely on GDB and gdbserver being
> +# able to see the same filesystem, remote exec-file is not set, and
> +# GDB will just use the path to the executable.
> +proc do_test { clear_sysroot set_remote_exec fetch_exec_and_args } {
> +
> +    # If we don't clear the sysroot, then the sysroot will remain as
> +    # 'target:'.  In this case, if we don't 'set remote exec-file'
> +    # then GDB will not be able to start a remote inferior.
> +    if { !$clear_sysroot && !$set_remote_exec } {
> +	return
>      }
>  
>      clean_restart $::testfile
> -}
>  
> -# Make sure we're disconnected, in case we're testing with an
> -# extended-remote board, therefore already connected.
> -gdb_test "disconnect" ".*"
> +    # Disable, or enable, use of the qExecAndArgs packet.
> +    gdb_test "set remote fetch-exec-and-args-packet ${fetch_exec_and_args}" \
> +	".*"

You can just omit the ".*".

>  
> -set target_exec [gdbserver_download_current_prog]
> -gdbserver_start_extended
> +    # Make sure we're disconnected, in case we're testing with an
> +    # extended-remote board, therefore already connected.
> +    gdb_test "disconnect" ".*"

Likewise.

Simon