On 6/1/22 16:32, Pedro Alves wrote:
> 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.
> 

Hi,

thanks for the review.

Done.

>> +
>>   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).
> 

Done.


>>   	     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").
> 

Done, thanks for catching that.

How does this look?

Thanks,
- Tom