Re: [PATCH][gdb] Fix warning in foreach_arch selftests

[Pedro Alves <pedro@palves.net>]

On 2022-06-01 11:41, Tom de Vries via Gdb-patches wrote:
> Hi,
> 
> When running the selftests, I run into:
> ...
> $ gdb -q -batch -ex "maint selftest"
>   ...
> Running selftest execute_cfa_program::aarch64:ilp32.
> warning: A handler for the OS ABI "GNU/Linux" is not built into this
> configuration of GDB.  Attempting to continue with the default aarch64:ilp32
> settings.
> ...
> and likewise for execute_cfa_program::i8086 and
> execute_cfa_program::ia64-elf32.
> 
> The warning can easily be reproduced outside the selftests by doing:
> ...
> $ gdb -q -batch -ex "set arch aarch64:ilp32"
> ...
> and can be prevented by first doing "set osabi none".
> 
> Fix the warning by setting osabi to none while doing selftests that iterate
> over all architectures.
> 
> Tested on x86_64-linux.
> 
> Any comments?
> 
> Thanks,
> - Tom
> 
> [gdb] Fix warning in foreach_arch selftests
> 
> ---
>  gdb/osabi.c         | 13 +++++++++++++
>  gdb/osabi.h         |  6 ++++++
>  gdb/selftest-arch.c | 15 ++++++++++++++-
>  3 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/osabi.c b/gdb/osabi.c
> index bbd7635532f..c3f221df969 100644
> --- a/gdb/osabi.c
> +++ b/gdb/osabi.c
> @@ -633,6 +633,19 @@ set_osabi (const char *args, int from_tty, struct cmd_list_element *c)
>      internal_error (__FILE__, __LINE__, _("Updating OS ABI failed."));
>  }
>  
> +void
> +set_osabi (const char *arg)
> +{
> +  set_osabi_string = arg;
> +  set_osabi (NULL, 0, NULL);
> +}
> +
> +const char *
> +get_osabi ()
> +{
> +  return set_osabi_string;
> +}

Missing usual "see foo.h" comments.

> +
>  static void
>  show_osabi (struct ui_file *file, int from_tty, struct cmd_list_element *c,
>  	    const char *value)
> diff --git a/gdb/osabi.h b/gdb/osabi.h
> index be016732cbc..eb5d88699e7 100644
> --- a/gdb/osabi.h
> +++ b/gdb/osabi.h
> @@ -89,4 +89,10 @@ const char *osabi_triplet_regexp (enum gdb_osabi osabi);
>  void generic_elf_osabi_sniff_abi_tag_sections (bfd *, asection *,
>  					       enum gdb_osabi *);
>  
> +/* Set osabi to ARG.  */
> +extern void set_osabi (const char *arg);
> +
> +/* Return current osabi setting.  */
> +extern const char *get_osabi ();
> +
>  #endif /* OSABI_H */
> diff --git a/gdb/selftest-arch.c b/gdb/selftest-arch.c
> index f434da718d5..a631f52e31e 100644
> --- a/gdb/selftest-arch.c
> +++ b/gdb/selftest-arch.c
> @@ -66,12 +66,25 @@ foreach_arch_test_generator (const std::string &name,
>        auto test_fn
>  	= ([=] ()
>  	   {
> +	     /* Prevent warnings when setting architecture with current osabi
> +		settings, like:
> +		  A handler for the OS ABI "GNU/Linux" is not built into this
> +		  configuration of GDB.  Attempting to continue with the
> +		  default aarch64:ilp32 settings.  */
> +	     const char *save_osabi = get_osabi ();
> +	     set_osabi ("none");
> +

A bit of an odd API to have to pass the string name in.  I'd think an API that
takes an enum gdb_osabi would be better, as that way you don't have to worry
about either passing the wrong string, or even worry about whether it's the
string contents that counts (whether the function internally uses strcmp),
or the string's address (the function internally compares pointers, assuming
the only strings that will be passed down are the ones in the command's enum
strings array).

>  	     struct gdbarch_info info;
>  	     info.bfd_arch_info = bfd_scan_arch (arch);
>  	     struct gdbarch *gdbarch = gdbarch_find_by_info (info);
>  	     SELF_CHECK (gdbarch != NULL);
> +
>  	     function (gdbarch);
> -	     reset ();
> +
> +	     SCOPE_EXIT {
> +	       reset ();
> +	       set_osabi (save_osabi);
> +	     };

Please format as:

	     SCOPE_EXIT 
               {
	         reset ();
	         set_osabi (save_osabi);
	       };

And you'll also need to move this to right after the set_osabi("none") call.  Doing it
here is too late.  SCOPE_EXIT creates a RAII object on the stack, which needs to exist
before any of the code that may throw right after set_osabi("none").


>  	   });
>  
>        tests.emplace_back (string_printf ("%s::%s", name.c_str (), arch),