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

[Simon Marchi <simark@simark.ca>]

On 10/6/25 11:07 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.

Thanks for this patch, I silently wished for this feature when debugging
problems related to extended-remote runs.  I didn't think about your
logic very hard, but on the surface it seems good.  Some minor comments:

> +/* Get a pointer to the current remote target.  If not connected to a
> +   remote target, return NULL.  The template argument allows users of this
> +   function to ask for 'remote_target' or 'extended_remote_target'.  */
> +
> +template<typename T = remote_target,
> +	 std::enable_if_t<std::is_base_of_v<remote_target, T>, bool> = true>
> +static T *
> +get_current_remote_target ()
> +{
> +  target_ops *proc_target = current_inferior ()->process_target ();
> +  return dynamic_cast<T *> (proc_target);
> +}

Since there are just two options, and it's pretty unlikely that there
will be many more, I think it would be clearer to just spell out the two
functions:

  - get_current_remote_target
  - get_current_extended_remote_target.

get_current_extended_remote_target is also shorter to use than
get_current_remote_target<extended_remote_target>.

> @@ -11313,6 +11356,88 @@ 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.SECOND 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.

std::pair is pretty bad for documentation (and code clarity).  If you
want to switch the std::pair for:

struct remote_exec_file_info
{
  remote_exec_source source;
  std::string value;
};

... I wouldn't be against it.

> +
> +     If INFO.SECOND is ::UNSET_VALUE then the user hasn't 'set remote
> +     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.SECOND 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.second == 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)
> +	  && startswith (exec_file, gdb_sysroot.c_str ()))
> +	{
> +	  /* If the prefix is '/aa/' and EXEC_FILE is '/aa/bb/cc' then we
> +	     only want t remove '/aa' from EXEC_FILE to leave '/bb/cc'.  */
> +	  size_t len = gdb_sysroot.length ();
> +	  while (len > 0 && IS_DIR_SEPARATOR (gdb_sysroot[len - 1]))
> +	    --len;
> +
> +	  return exec_file + len;
> +	}

Edge case:

execfile: /aaaaa/foo.exe
sysroot: /aa

I guess that would be handled incorrectly, given the use of startswith?

Handling paths correctly is hard. Do we have a "is prefix of" file path
util already?

Simon