Re: [PATCHv3 6/6] gdb: native target invalid architecture detection

[Pedro Alves <pedro@palves.net>]

On 2022-06-13 17:15, Andrew Burgess via Gdb-patches wrote:

> This leaves just one question, what about native targets that support
> multiple architectures?
> 

I have another question.  I'm confused on how if e.g., on x86-64, you
load a RISC-V binary into GDB, and the try to run it, we don't end up
with architecture of the target description instead.  If the target
reports a target description, and the resulting architecture is not
compatible with the executable, then I think it would make sense to
use the architecture of the target?

If I start a 64-bit program on x86-64, like:

 $ gdb ~/gdb/tests/main64
 GNU gdb (GDB) 13.0.50.20220624-git
 ...
 (gdb) start
 Temporary breakpoint 1 at 0x400492: file main.c, line 11.
 Starting program: /home/pedro/gdb/tests/main64

 Temporary breakpoint 1, main (argc=1, argv=0x7fffffffdc48) at main.c:11
 11        return 0;

and then try to load the executable of an incompatible arch, like:

 (gdb) file ~/gdb/tests/main32
 A program is being debugged already.
 Are you sure you want to change the file? (y or n) y
 warning: Selected architecture i386 is not compatible with reported target architecture i386:x86-64
 Architecture of file not recognized.

then GDB doesn't switch the architecture to the file's architecture, it is still x86-64:

 (gdb) show architecture 
 The target architecture is set to "auto" (currently "i386:x86-64").

and it doesn't let me manually select an incompatible architecture either:

 (gdb) set architecture riscv
 warning: Selected architecture riscv is not compatible with reported target architecture i386:x86-64
 Architecture `riscv' not recognized.
 The target architecture is set to "auto" (currently "i386:x86-64").
 (gdb) 

It kind of sounds like if GDB detected the incompatibility when we first fetch the
target description, similarly to how we detect it in the cases above, and GDB picked
the target arch or errored out after detecting the incompatibility, then we wouldn't
need a patch like yours?  How come the mismatch isn't detected then (when we first
retrieve the target description) today?

> +
>  protected:
>    /* Unpush the target if it wasn't explicitly open with "target native"
>       and there are no live inferiors left.  Note: if calling this as a
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index e909d4d4c81..b8acf858b3a 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -413,6 +413,10 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how)
>    if (non_stop && !run_target->supports_non_stop ())
>      error (_("The target does not support running in non-stop mode."));
>  
> +  if (!run_target->supports_architecture_p (get_current_arch ()))
> +    error (_("The target does not support architecture \"%s\"."),
> +	   gdbarch_bfd_arch_info (get_current_arch ())->printable_name);
> +
>    /* Done.  Can now set breakpoints, change inferior args, etc.  */
>  
>    /* Insert temporary breakpoint in main function if requested.  */
> @@ -2590,6 +2594,10 @@ attach_command (const char *args, int from_tty)
>    if (non_stop && !attach_target->supports_non_stop ())
>      error (_("Cannot attach to this target in non-stop mode"));
>  
> +  if (!attach_target->supports_architecture_p (get_current_arch ()))
> +    error (_("The target does not support architecture \"%s\"."),
> +	   gdbarch_bfd_arch_info (get_current_arch ())->printable_name);
> +

Since the error is the same in both cases, I'd suggest putting it in
a shared routine, like:

 static void
 check_target_supports_current_arch (process_stratum_target *target)
 {
   if (!target->supports_architecture_p (get_current_arch ()))
     error (_("The target does not support architecture \"%s\"."),
 	   gdbarch_bfd_arch_info (get_current_arch ())->printable_name);
 }