public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH][gdb] Fix warning in foreach_arch selftests
@ 2022-06-01 10:41 Tom de Vries
  2022-06-01 14:32 ` Pedro Alves
  0 siblings, 1 reply; 9+ messages in thread
From: Tom de Vries @ 2022-06-01 10:41 UTC (permalink / raw)
  To: gdb-patches

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;
+}
+
 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");
+
 	     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);
+	     };
 	   });
 
       tests.emplace_back (string_printf ("%s::%s", name.c_str (), arch),

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][gdb] Fix warning in foreach_arch selftests
  2022-06-01 10:41 [PATCH][gdb] Fix warning in foreach_arch selftests Tom de Vries
@ 2022-06-01 14:32 ` Pedro Alves
  2022-06-01 16:36   ` Tom de Vries
  0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2022-06-01 14:32 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

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),


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][gdb] Fix warning in foreach_arch selftests
  2022-06-01 14:32 ` Pedro Alves
@ 2022-06-01 16:36   ` Tom de Vries
  2022-06-01 17:10     ` Pedro Alves
  2022-06-01 19:08     ` Simon Marchi
  0 siblings, 2 replies; 9+ messages in thread
From: Tom de Vries @ 2022-06-01 16:36 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 4326 bytes --]

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


[-- Attachment #2: 0002-gdb-Fix-warning-in-foreach_arch-selftests.patch --]
[-- Type: text/x-patch, Size: 5403 bytes --]

[gdb] Fix warning in foreach_arch selftests

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.

---
 gdb/osabi.c         | 50 ++++++++++++++++++++++++++++++++++++++------------
 gdb/osabi.h         | 13 +++++++++++++
 gdb/selftest-arch.c | 18 +++++++++++++++++-
 3 files changed, 68 insertions(+), 13 deletions(-)

diff --git a/gdb/osabi.c b/gdb/osabi.c
index bbd7635532f..776e62069e5 100644
--- a/gdb/osabi.c
+++ b/gdb/osabi.c
@@ -32,7 +32,7 @@
 #endif
 
 /* State for the "set osabi" command.  */
-static enum { osabi_auto, osabi_default, osabi_user } user_osabi_state;
+static enum gdb_osabi_mode user_osabi_state;
 static enum gdb_osabi user_selected_osabi;
 static const char *gdb_osabi_available_names[GDB_OSABI_INVALID + 3] = {
   "auto",
@@ -595,15 +595,48 @@ generic_elf_osabi_sniffer (bfd *abfd)
   return osabi;
 }
 \f
+/* See osabi.h.  */
+
+void
+set_osabi (enum gdb_osabi_mode mode, enum gdb_osabi osabi)
+{
+  if (mode == osabi_auto)
+    user_osabi_state = osabi_auto;
+  else if (mode == osabi_default)
+    {
+      user_selected_osabi = GDB_OSABI_DEFAULT;
+      user_osabi_state = osabi_user;
+    }
+  else
+    {
+      user_selected_osabi = osabi;
+      user_osabi_state = osabi_user;
+    }
+
+  /* NOTE: At some point (true multiple architectures) we'll need to be more
+     graceful here.  */
+  gdbarch_info info;
+  if (! gdbarch_update_p (info))
+    internal_error (__FILE__, __LINE__, _("Updating OS ABI failed."));
+}
+
+/* See osabi.h.  */
+
+void
+get_osabi (enum gdb_osabi_mode &mode, enum gdb_osabi &osabi)
+{
+  mode = user_osabi_state;
+  osabi = user_selected_osabi;
+}
+
 static void
 set_osabi (const char *args, int from_tty, struct cmd_list_element *c)
 {
   if (strcmp (set_osabi_string, "auto") == 0)
-    user_osabi_state = osabi_auto;
+    set_osabi (osabi_auto, GDB_OSABI_INVALID);
   else if (strcmp (set_osabi_string, "default") == 0)
     {
-      user_selected_osabi = GDB_OSABI_DEFAULT;
-      user_osabi_state = osabi_user;
+      set_osabi (osabi_default, GDB_OSABI_INVALID);
     }
   else
     {
@@ -615,8 +648,7 @@ set_osabi (const char *args, int from_tty, struct cmd_list_element *c)
 
 	  if (strcmp (set_osabi_string, gdbarch_osabi_name (osabi)) == 0)
 	    {
-	      user_selected_osabi = osabi;
-	      user_osabi_state = osabi_user;
+	      set_osabi (osabi_user, osabi);
 	      break;
 	    }
 	}
@@ -625,12 +657,6 @@ set_osabi (const char *args, int from_tty, struct cmd_list_element *c)
 			_("Invalid OS ABI \"%s\" passed to command handler."),
 			set_osabi_string);
     }
-
-  /* NOTE: At some point (true multiple architectures) we'll need to be more
-     graceful here.  */
-  gdbarch_info info;
-  if (! gdbarch_update_p (info))
-    internal_error (__FILE__, __LINE__, _("Updating OS ABI failed."));
 }
 
 static void
diff --git a/gdb/osabi.h b/gdb/osabi.h
index be016732cbc..3737a77d50e 100644
--- a/gdb/osabi.h
+++ b/gdb/osabi.h
@@ -50,6 +50,13 @@ enum gdb_osabi
   GDB_OSABI_INVALID		/* keep this last */
 };
 
+enum gdb_osabi_mode
+{
+  osabi_auto,
+  osabi_default,
+  osabi_user
+};
+
 /* Register an OS ABI sniffer.  Each arch/flavour may have more than
    one sniffer.  This is used to e.g. differentiate one OS's a.out from
    another.  The first sniffer to return something other than
@@ -89,4 +96,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 MODE/OSABI.  */
+extern void set_osabi (enum gdb_osabi_mode mode, enum gdb_osabi osabi);
+
+/* Return current osabi setting in MODE/OSABI.  */
+extern void get_osabi (enum gdb_osabi_mode &mode, enum gdb_osabi &osabi);
+
 #endif /* OSABI_H */
diff --git a/gdb/selftest-arch.c b/gdb/selftest-arch.c
index f434da718d5..a4d9de5a74d 100644
--- a/gdb/selftest-arch.c
+++ b/gdb/selftest-arch.c
@@ -66,12 +66,28 @@ 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.  */
+	     enum gdb_osabi_mode mode;
+	     enum gdb_osabi osabi;
+	     get_osabi (mode, osabi);
+
+	     set_osabi (osabi_user, GDB_OSABI_NONE);
+	     SCOPE_EXIT
+	       {
+		 reset ();
+		 set_osabi (mode, osabi);
+	       };
+
 	     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 ();
 	   });
 
       tests.emplace_back (string_printf ("%s::%s", name.c_str (), arch),

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][gdb] Fix warning in foreach_arch selftests
  2022-06-01 16:36   ` Tom de Vries
@ 2022-06-01 17:10     ` Pedro Alves
  2022-06-01 19:08     ` Simon Marchi
  1 sibling, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2022-06-01 17:10 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

On 2022-06-01 17:36, Tom de Vries wrote:

> How does this look?

LGTM.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][gdb] Fix warning in foreach_arch selftests
  2022-06-01 16:36   ` Tom de Vries
  2022-06-01 17:10     ` Pedro Alves
@ 2022-06-01 19:08     ` Simon Marchi
  2022-06-02 15:44       ` Tom de Vries
  1 sibling, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2022-06-01 19:08 UTC (permalink / raw)
  To: Tom de Vries, Pedro Alves, gdb-patches

On 6/1/22 12:36, Tom de Vries via Gdb-patches wrote:
> 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
> 

Hi Tom,

This introduces some failures for me:

Running selftest print_one_insn::A6.^M
Self test failed: Cannot access memory at address 0x0^M
Running selftest print_one_insn::A7.^M
Self test failed: Cannot access memory at address 0x0^M
Running selftest print_one_insn::ARC600.^M
Self test failed: Cannot access memory at address 0x0^M
Running selftest print_one_insn::ARC601.^M
Running selftest print_one_insn::ARC700.^M
Self test failed: Cannot access memory at address 0x0^M
Running selftest print_one_insn::ARCv2.^M
Self test failed: Cannot access memory at address 0x0^M
Running selftest print_one_insn::EM.^M
Self test failed: Cannot access memory at address 0x0^M
Running selftest print_one_insn::HS.^M
Self test failed: Cannot access memory at address 0x0^M
...
FAIL: gdb.gdb/unittest.exp: no executable loaded: maintenance selftest, failed none

Simon

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][gdb] Fix warning in foreach_arch selftests
  2022-06-01 19:08     ` Simon Marchi
@ 2022-06-02 15:44       ` Tom de Vries
  2022-06-02 17:44         ` Simon Marchi
  0 siblings, 1 reply; 9+ messages in thread
From: Tom de Vries @ 2022-06-02 15:44 UTC (permalink / raw)
  To: Simon Marchi, Pedro Alves, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 975 bytes --]

On 6/1/22 21:08, Simon Marchi wrote:
> Hi Tom,
> 
> This introduces some failures for me:
> 
> Running selftest print_one_insn::A6.^M
> Self test failed: Cannot access memory at address 0x0^M

Hi Simon,

sorry for not catching this, I forgot to test with --enable-targets=all, 
I'll try to fix the default in my build scripts.

I've done a new version of the existing patch (so it applies once the 
old one is reverted).

Mainly because I realized that there's an easier way to handle the osabi 
none setting:
...
@@ -68,6 +68,7 @@ foreach_arch_test_generator (const std::string &name,
            {
              struct gdbarch_info info;
              info.bfd_arch_info = bfd_scan_arch (arch);
+            info.osabi = GDB_OSABI_NONE;
              struct gdbarch *gdbarch = gdbarch_find_by_info (info);
              SELF_CHECK (gdbarch != NULL);
              function (gdbarch);
...

Currently regression testing, but at least fixes the ARC regression.

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-Fix-warning-in-foreach_arch-selftests.patch --]
[-- Type: text/x-patch, Size: 6578 bytes --]

[gdb] Fix warning in foreach_arch selftests

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.

This causes a regression in the print_one_insn selftests for the ARC
archictecture.

The problem is pre-existing, and can be demonstrated (already without this
patch) using:
...
$ gdb -q -batch -ex "set osabi none" -ex "maint selftest print_one_insn::A6"
Running selftest print_one_insn::A6.
Self test failed: Cannot access memory at address 0x0
Ran 1 unit tests, 1 failed
$
...

For ARC, we use the generic case in print_one_insn_test, containing this code:
...
       int kind = gdbarch_breakpoint_kind_from_pc (gdbarch, &pc);
       ...
       insn = gdbarch_sw_breakpoint_from_kind (gdbarch, kind, &bplen);
...

The problem is that with osabi linux we trigger:
...
static int
arc_linux_breakpoint_kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr)
{
  return trap_size;
}
...
but with osabi none:
...
arc_breakpoint_kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr)
{
  size_t length_with_limm = gdb_insn_length (gdbarch, *pcptr);
...
which needs access to memory, and will consequently fail.

Fix this in print_one_insn_test, in the default case, by iterating over
supported osabi's to makes sure we trigger arc_linux_breakpoint_kind_from_pc
which will give us a usable instruction to disassemble.

Tested on x86_64-linux.

---
 gdb/disasm-selftests.c | 47 ++++++++++++++++++++++++++++++++++++++++++++---
 gdb/osabi.c            | 35 +++++++++++++++++++++++++++++------
 gdb/osabi.h            |  3 +++
 gdb/selftest-arch.c    |  1 +
 4 files changed, 77 insertions(+), 9 deletions(-)

diff --git a/gdb/disasm-selftests.c b/gdb/disasm-selftests.c
index 928d26f7018..ccb60a0bc87 100644
--- a/gdb/disasm-selftests.c
+++ b/gdb/disasm-selftests.c
@@ -101,12 +101,53 @@ print_one_insn_test (struct gdbarch *gdbarch)
       {
 	/* Test disassemble breakpoint instruction.  */
 	CORE_ADDR pc = 0;
-	int kind = gdbarch_breakpoint_kind_from_pc (gdbarch, &pc);
+	int kind;
 	int bplen;
 
-	insn = gdbarch_sw_breakpoint_from_kind (gdbarch, kind, &bplen);
-	len = bplen;
+	const char *arch = gdbarch_bfd_arch_info (gdbarch)->printable_name;
+
+	enum gdb_osabi it;
+	bool found = false;
+	for (it = GDB_OSABI_UNKNOWN; it != GDB_OSABI_INVALID;
+	     it = static_cast<enum gdb_osabi>(static_cast<int>(it) + 1))
+	  {
+	    if (it == GDB_OSABI_UNKNOWN)
+	      continue;
+
+	    struct gdbarch_info info;
+	    info.bfd_arch_info = bfd_scan_arch (arch);
+	    info.osabi = it;
+
+	    if (it != GDB_OSABI_NONE)
+	      {
+		if (!has_gdb_osabi_handler (info))
+		  /* Unsupported.  Skip to prevent warnings like:
+		     A handler for the OS ABI <x> is not built into this
+		     configuration of GDB.  Attempting to continue with the
+		     default <y> settings.  */
+		  continue;
+	      }
+
+	    gdbarch = gdbarch_find_by_info (info);
+	    SELF_CHECK (gdbarch != NULL);
+
+	    try
+	      {
+		kind = gdbarch_breakpoint_kind_from_pc (gdbarch, &pc);
+		insn = gdbarch_sw_breakpoint_from_kind (gdbarch, kind, &bplen);
+	      }
+	    catch (...)
+	      {
+		continue;
+	      }
+	    found = true;
+	    break;
+	  }
+
+	/* Assert that we have found an instruction to disassemble.  */
+	SELF_CHECK (found);
 
+	len = bplen;
 	break;
       }
     }
diff --git a/gdb/osabi.c b/gdb/osabi.c
index bbd7635532f..d4cb7531c12 100644
--- a/gdb/osabi.c
+++ b/gdb/osabi.c
@@ -332,9 +332,10 @@ can_run_code_for (const struct bfd_arch_info *a, const struct bfd_arch_info *b)
   return (a == b || a->compatible (a, b) == a);
 }
 
+/* Return OS ABI handler for INFO.  */
 
-void
-gdbarch_init_osabi (struct gdbarch_info info, struct gdbarch *gdbarch)
+static struct gdb_osabi_handler *
+gdbarch_osabi_handler (struct gdbarch_info info)
 {
   struct gdb_osabi_handler *handler;
 
@@ -367,12 +368,28 @@ gdbarch_init_osabi (struct gdbarch_info info, struct gdbarch *gdbarch)
 	 ISA").  BFD doesn't normally consider 32-bit and 64-bit
 	 "compatible" so it doesn't succeed.  */
       if (can_run_code_for (info.bfd_arch_info, handler->arch_info))
-	{
-	  (*handler->init_osabi) (info, gdbarch);
-	  return;
-	}
+	return handler;
     }
 
+  return nullptr;
+}
+
+/* See osabi.h.  */
+
+bool
+has_gdb_osabi_handler (struct gdbarch_info info)
+{
+  return gdbarch_osabi_handler (info) != nullptr;
+}
+
+void
+gdbarch_init_osabi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  struct gdb_osabi_handler *handler;
+
+  gdb_assert (info.osabi != GDB_OSABI_UNKNOWN);
+  handler = gdbarch_osabi_handler (info);
+
   if (info.osabi == GDB_OSABI_NONE)
     {
       /* Don't complain about no OSABI.  Assume the user knows
@@ -380,6 +397,12 @@ gdbarch_init_osabi (struct gdbarch_info info, struct gdbarch *gdbarch)
       return;
     }
 
+  if (handler != nullptr)
+    {
+      (*handler->init_osabi) (info, gdbarch);
+      return;
+    }
+
   warning
     ("A handler for the OS ABI \"%s\" is not built into this configuration\n"
      "of GDB.  Attempting to continue with the default %s settings.\n",
diff --git a/gdb/osabi.h b/gdb/osabi.h
index be016732cbc..478a418aac2 100644
--- a/gdb/osabi.h
+++ b/gdb/osabi.h
@@ -74,6 +74,9 @@ enum gdb_osabi gdbarch_lookup_osabi (bfd *);
    string.  */
 enum gdb_osabi osabi_from_tdesc_string (const char *text);
 
+/* Return true if there's an OS ABI handler for INFO.  */
+bool has_gdb_osabi_handler (struct gdbarch_info info);
+
 /* Initialize the gdbarch for the specified OS ABI variant.  */
 void gdbarch_init_osabi (struct gdbarch_info, struct gdbarch *);
 
diff --git a/gdb/selftest-arch.c b/gdb/selftest-arch.c
index f434da718d5..1fe0b2d59b4 100644
--- a/gdb/selftest-arch.c
+++ b/gdb/selftest-arch.c
@@ -68,6 +68,7 @@ foreach_arch_test_generator (const std::string &name,
 	   {
 	     struct gdbarch_info info;
 	     info.bfd_arch_info = bfd_scan_arch (arch);
+	     info.osabi = GDB_OSABI_NONE;
 	     struct gdbarch *gdbarch = gdbarch_find_by_info (info);
 	     SELF_CHECK (gdbarch != NULL);
 	     function (gdbarch);

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][gdb] Fix warning in foreach_arch selftests
  2022-06-02 15:44       ` Tom de Vries
@ 2022-06-02 17:44         ` Simon Marchi
  2022-06-02 18:35           ` Tom de Vries
  0 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2022-06-02 17:44 UTC (permalink / raw)
  To: Tom de Vries, Pedro Alves, gdb-patches

On 6/2/22 11:44, Tom de Vries via Gdb-patches wrote:
> On 6/1/22 21:08, Simon Marchi wrote:
>> Hi Tom,
>>
>> This introduces some failures for me:
>>
>> Running selftest print_one_insn::A6.^M
>> Self test failed: Cannot access memory at address 0x0^M
> 
> Hi Simon,
> 
> sorry for not catching this, I forgot to test with --enable-targets=all, I'll try to fix the default in my build scripts.
> 
> I've done a new version of the existing patch (so it applies once the old one is reverted).
> 
> Mainly because I realized that there's an easier way to handle the osabi none setting:
> ...
> @@ -68,6 +68,7 @@ foreach_arch_test_generator (const std::string &name,
>            {
>              struct gdbarch_info info;
>              info.bfd_arch_info = bfd_scan_arch (arch);
> +            info.osabi = GDB_OSABI_NONE;
>              struct gdbarch *gdbarch = gdbarch_find_by_info (info);
>              SELF_CHECK (gdbarch != NULL);
>              function (gdbarch);
> ...
> 
> Currently regression testing, but at least fixes the ARC regression.

It fixes the gdb.gdb/unittest.exp failure, but introduces:

FAIL: gdb.arch/amd64-osabi.exp: arch=i386:x86-64: print sizeof (void*)
FAIL: gdb.arch/amd64-osabi.exp: arch=i386:x86-64: print sizeof (long double)
FAIL: gdb.arch/amd64-osabi.exp: arch=i386:x64-32: print sizeof (long double)

Simon

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][gdb] Fix warning in foreach_arch selftests
  2022-06-02 17:44         ` Simon Marchi
@ 2022-06-02 18:35           ` Tom de Vries
  2022-06-04  7:23             ` Tom de Vries
  0 siblings, 1 reply; 9+ messages in thread
From: Tom de Vries @ 2022-06-02 18:35 UTC (permalink / raw)
  To: Simon Marchi, Pedro Alves, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1646 bytes --]

On 6/2/22 19:44, Simon Marchi wrote:
> On 6/2/22 11:44, Tom de Vries via Gdb-patches wrote:
>> On 6/1/22 21:08, Simon Marchi wrote:
>>> Hi Tom,
>>>
>>> This introduces some failures for me:
>>>
>>> Running selftest print_one_insn::A6.^M
>>> Self test failed: Cannot access memory at address 0x0^M
>>
>> Hi Simon,
>>
>> sorry for not catching this, I forgot to test with --enable-targets=all, I'll try to fix the default in my build scripts.
>>
>> I've done a new version of the existing patch (so it applies once the old one is reverted).
>>
>> Mainly because I realized that there's an easier way to handle the osabi none setting:
>> ...
>> @@ -68,6 +68,7 @@ foreach_arch_test_generator (const std::string &name,
>>             {
>>               struct gdbarch_info info;
>>               info.bfd_arch_info = bfd_scan_arch (arch);
>> +            info.osabi = GDB_OSABI_NONE;
>>               struct gdbarch *gdbarch = gdbarch_find_by_info (info);
>>               SELF_CHECK (gdbarch != NULL);
>>               function (gdbarch);
>> ...
>>
>> Currently regression testing, but at least fixes the ARC regression.
> 
> It fixes the gdb.gdb/unittest.exp failure, but introduces:
> 
> FAIL: gdb.arch/amd64-osabi.exp: arch=i386:x86-64: print sizeof (void*)
> FAIL: gdb.arch/amd64-osabi.exp: arch=i386:x86-64: print sizeof (long double)
> FAIL: gdb.arch/amd64-osabi.exp: arch=i386:x64-32: print sizeof (long double)

Ack, I also encountered that during testing, that was a thinko in the 
refactoring of gdbarch_init_osabi, now fixed and fully tested.

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-Fix-warning-in-foreach_arch-selftests.patch --]
[-- Type: text/x-patch, Size: 6228 bytes --]

[gdb] Fix warning in foreach_arch selftests

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.

This causes a regression in the print_one_insn selftests for the ARC
archictecture.

The problem is pre-existing, and can be demonstrated (already without this
patch) using:
...
$ gdb -q -batch -ex "set osabi none" -ex "maint selftest print_one_insn::A6"
Running selftest print_one_insn::A6.
Self test failed: Cannot access memory at address 0x0
Ran 1 unit tests, 1 failed
$
...

For ARC, we use the generic case in print_one_insn_test, containing this code:
...
       int kind = gdbarch_breakpoint_kind_from_pc (gdbarch, &pc);
       ...
       insn = gdbarch_sw_breakpoint_from_kind (gdbarch, kind, &bplen);
...

The problem is that with osabi linux we trigger:
...
static int
arc_linux_breakpoint_kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr)
{
  return trap_size;
}
...
but with osabi none:
...
arc_breakpoint_kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr)
{
  size_t length_with_limm = gdb_insn_length (gdbarch, *pcptr);
...
which needs access to memory, and will consequently fail.

Fix this in print_one_insn_test, in the default case, by iterating over
supported osabi's to makes sure we trigger arc_linux_breakpoint_kind_from_pc
which will give us a usable instruction to disassemble.

Tested on x86_64-linux.

---
 gdb/disasm-selftests.c | 47 ++++++++++++++++++++++++++++++++++++++++++++---
 gdb/osabi.c            | 35 +++++++++++++++++++++++++++++------
 gdb/osabi.h            |  3 +++
 gdb/selftest-arch.c    |  1 +
 4 files changed, 77 insertions(+), 9 deletions(-)

diff --git a/gdb/disasm-selftests.c b/gdb/disasm-selftests.c
index 928d26f7018..ccb60a0bc87 100644
--- a/gdb/disasm-selftests.c
+++ b/gdb/disasm-selftests.c
@@ -101,12 +101,53 @@ print_one_insn_test (struct gdbarch *gdbarch)
       {
 	/* Test disassemble breakpoint instruction.  */
 	CORE_ADDR pc = 0;
-	int kind = gdbarch_breakpoint_kind_from_pc (gdbarch, &pc);
+	int kind;
 	int bplen;
 
-	insn = gdbarch_sw_breakpoint_from_kind (gdbarch, kind, &bplen);
-	len = bplen;
+	const char *arch = gdbarch_bfd_arch_info (gdbarch)->printable_name;
+
+	enum gdb_osabi it;
+	bool found = false;
+	for (it = GDB_OSABI_UNKNOWN; it != GDB_OSABI_INVALID;
+	     it = static_cast<enum gdb_osabi>(static_cast<int>(it) + 1))
+	  {
+	    if (it == GDB_OSABI_UNKNOWN)
+	      continue;
+
+	    struct gdbarch_info info;
+	    info.bfd_arch_info = bfd_scan_arch (arch);
+	    info.osabi = it;
+
+	    if (it != GDB_OSABI_NONE)
+	      {
+		if (!has_gdb_osabi_handler (info))
+		  /* Unsupported.  Skip to prevent warnings like:
+		     A handler for the OS ABI <x> is not built into this
+		     configuration of GDB.  Attempting to continue with the
+		     default <y> settings.  */
+		  continue;
+	      }
+
+	    gdbarch = gdbarch_find_by_info (info);
+	    SELF_CHECK (gdbarch != NULL);
+
+	    try
+	      {
+		kind = gdbarch_breakpoint_kind_from_pc (gdbarch, &pc);
+		insn = gdbarch_sw_breakpoint_from_kind (gdbarch, kind, &bplen);
+	      }
+	    catch (...)
+	      {
+		continue;
+	      }
+	    found = true;
+	    break;
+	  }
+
+	/* Assert that we have found an instruction to disassemble.  */
+	SELF_CHECK (found);
 
+	len = bplen;
 	break;
       }
     }
diff --git a/gdb/osabi.c b/gdb/osabi.c
index bbd7635532f..f1b7f227b87 100644
--- a/gdb/osabi.c
+++ b/gdb/osabi.c
@@ -332,9 +332,10 @@ can_run_code_for (const struct bfd_arch_info *a, const struct bfd_arch_info *b)
   return (a == b || a->compatible (a, b) == a);
 }
 
+/* Return OS ABI handler for INFO.  */
 
-void
-gdbarch_init_osabi (struct gdbarch_info info, struct gdbarch *gdbarch)
+static struct gdb_osabi_handler *
+gdbarch_osabi_handler (struct gdbarch_info info)
 {
   struct gdb_osabi_handler *handler;
 
@@ -367,10 +368,32 @@ gdbarch_init_osabi (struct gdbarch_info info, struct gdbarch *gdbarch)
 	 ISA").  BFD doesn't normally consider 32-bit and 64-bit
 	 "compatible" so it doesn't succeed.  */
       if (can_run_code_for (info.bfd_arch_info, handler->arch_info))
-	{
-	  (*handler->init_osabi) (info, gdbarch);
-	  return;
-	}
+	return handler;
+    }
+
+  return nullptr;
+}
+
+/* See osabi.h.  */
+
+bool
+has_gdb_osabi_handler (struct gdbarch_info info)
+{
+  return gdbarch_osabi_handler (info) != nullptr;
+}
+
+void
+gdbarch_init_osabi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  struct gdb_osabi_handler *handler;
+
+  gdb_assert (info.osabi != GDB_OSABI_UNKNOWN);
+  handler = gdbarch_osabi_handler (info);
+
+  if (handler != nullptr)
+    {
+      (*handler->init_osabi) (info, gdbarch);
+      return;
     }
 
   if (info.osabi == GDB_OSABI_NONE)
diff --git a/gdb/osabi.h b/gdb/osabi.h
index be016732cbc..478a418aac2 100644
--- a/gdb/osabi.h
+++ b/gdb/osabi.h
@@ -74,6 +74,9 @@ enum gdb_osabi gdbarch_lookup_osabi (bfd *);
    string.  */
 enum gdb_osabi osabi_from_tdesc_string (const char *text);
 
+/* Return true if there's an OS ABI handler for INFO.  */
+bool has_gdb_osabi_handler (struct gdbarch_info info);
+
 /* Initialize the gdbarch for the specified OS ABI variant.  */
 void gdbarch_init_osabi (struct gdbarch_info, struct gdbarch *);
 
diff --git a/gdb/selftest-arch.c b/gdb/selftest-arch.c
index f434da718d5..1fe0b2d59b4 100644
--- a/gdb/selftest-arch.c
+++ b/gdb/selftest-arch.c
@@ -68,6 +68,7 @@ foreach_arch_test_generator (const std::string &name,
 	   {
 	     struct gdbarch_info info;
 	     info.bfd_arch_info = bfd_scan_arch (arch);
+	     info.osabi = GDB_OSABI_NONE;
 	     struct gdbarch *gdbarch = gdbarch_find_by_info (info);
 	     SELF_CHECK (gdbarch != NULL);
 	     function (gdbarch);

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][gdb] Fix warning in foreach_arch selftests
  2022-06-02 18:35           ` Tom de Vries
@ 2022-06-04  7:23             ` Tom de Vries
  0 siblings, 0 replies; 9+ messages in thread
From: Tom de Vries @ 2022-06-04  7:23 UTC (permalink / raw)
  To: Simon Marchi, Pedro Alves, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 2413 bytes --]

On 6/2/22 20:35, Tom de Vries wrote:
> Subject:
> Re: [PATCH][gdb] Fix warning in foreach_arch selftests
> From:
> Tom de Vries <tdevries@suse.de>
> Date:
> 6/2/22, 20:35
> 
> To:
> Simon Marchi <simark@simark.ca>, Pedro Alves <pedro@palves.net>, 
> gdb-patches@sourceware.org
> 
> 
> On 6/2/22 19:44, Simon Marchi wrote:
>> On 6/2/22 11:44, Tom de Vries via Gdb-patches wrote:
>>> On 6/1/22 21:08, Simon Marchi wrote:
>>>> Hi Tom,
>>>>
>>>> This introduces some failures for me:
>>>>
>>>> Running selftest print_one_insn::A6.^M
>>>> Self test failed: Cannot access memory at address 0x0^M
>>>
>>> Hi Simon,
>>>
>>> sorry for not catching this, I forgot to test with 
>>> --enable-targets=all, I'll try to fix the default in my build scripts.
>>>
>>> I've done a new version of the existing patch (so it applies once the 
>>> old one is reverted).
>>>
>>> Mainly because I realized that there's an easier way to handle the 
>>> osabi none setting:
>>> ...
>>> @@ -68,6 +68,7 @@ foreach_arch_test_generator (const std::string &name,
>>>             {
>>>               struct gdbarch_info info;
>>>               info.bfd_arch_info = bfd_scan_arch (arch);
>>> +            info.osabi = GDB_OSABI_NONE;
>>>               struct gdbarch *gdbarch = gdbarch_find_by_info (info);
>>>               SELF_CHECK (gdbarch != NULL);
>>>               function (gdbarch);
>>> ...
>>>
>>> Currently regression testing, but at least fixes the ARC regression.
>>
>> It fixes the gdb.gdb/unittest.exp failure, but introduces:
>>
>> FAIL: gdb.arch/amd64-osabi.exp: arch=i386:x86-64: print sizeof (void*)
>> FAIL: gdb.arch/amd64-osabi.exp: arch=i386:x86-64: print sizeof (long 
>> double)
>> FAIL: gdb.arch/amd64-osabi.exp: arch=i386:x64-32: print sizeof (long 
>> double)
> 
> Ack, I also encountered that during testing, that was a thinko in the 
> refactoring of gdbarch_init_osabi, now fixed and fully tested.
> 

I've committed this version.

I ended up doing this slightly different:
...
	const char *arch = gdbarch_bfd_arch_info (gdbarch)->printable_name;
   ...
	    struct gdbarch_info info;
	    info.bfd_arch_info = bfd_scan_arch (arch);
	    info.osabi = it;
...
and used instead:
...
        struct gdbarch_info info; 

        info.bfd_arch_info = gdbarch_bfd_arch_info (gdbarch); 

   ...
	    info.osabi = it;
...

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-Fix-warning-in-foreach_arch-selftests.patch --]
[-- Type: text/x-patch, Size: 6159 bytes --]

[gdb] Fix warning in foreach_arch selftests

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.

This causes a regression in the print_one_insn selftests for the ARC
architecture.

The problem is pre-existing, and can be demonstrated (already without this
patch) using:
...
$ gdb -q -batch -ex "set osabi none" -ex "maint selftest print_one_insn::A6"
Running selftest print_one_insn::A6.
Self test failed: Cannot access memory at address 0x0
Ran 1 unit tests, 1 failed
$
...

For ARC, we use the generic case in print_one_insn_test, containing this code:
...
       int kind = gdbarch_breakpoint_kind_from_pc (gdbarch, &pc);
       ...
       insn = gdbarch_sw_breakpoint_from_kind (gdbarch, kind, &bplen);
...

The problem is that with osabi linux we trigger:
...
static int
arc_linux_breakpoint_kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr)
{
  return trap_size;
}
...
but with osabi none:
...
arc_breakpoint_kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr)
{
  size_t length_with_limm = gdb_insn_length (gdbarch, *pcptr);
...
which needs access to memory, and will consequently fail.

Fix this in print_one_insn_test, in the default case, by iterating over
supported osabi's to makes sure we trigger arc_linux_breakpoint_kind_from_pc
which will give us a usable instruction to disassemble.

Tested on x86_64-linux.

---
 gdb/disasm-selftests.c | 46 +++++++++++++++++++++++++++++++++++++++++++---
 gdb/osabi.c            | 35 +++++++++++++++++++++++++++++------
 gdb/osabi.h            |  3 +++
 gdb/selftest-arch.c    |  1 +
 4 files changed, 76 insertions(+), 9 deletions(-)

diff --git a/gdb/disasm-selftests.c b/gdb/disasm-selftests.c
index 928d26f7018..7daa0138f6f 100644
--- a/gdb/disasm-selftests.c
+++ b/gdb/disasm-selftests.c
@@ -101,12 +101,52 @@ print_one_insn_test (struct gdbarch *gdbarch)
       {
 	/* Test disassemble breakpoint instruction.  */
 	CORE_ADDR pc = 0;
-	int kind = gdbarch_breakpoint_kind_from_pc (gdbarch, &pc);
+	int kind;
 	int bplen;
 
-	insn = gdbarch_sw_breakpoint_from_kind (gdbarch, kind, &bplen);
-	len = bplen;
+	struct gdbarch_info info;
+	info.bfd_arch_info = gdbarch_bfd_arch_info (gdbarch);
+
+	enum gdb_osabi it;
+	bool found = false;
+	for (it = GDB_OSABI_UNKNOWN; it != GDB_OSABI_INVALID;
+	     it = static_cast<enum gdb_osabi>(static_cast<int>(it) + 1))
+	  {
+	    if (it == GDB_OSABI_UNKNOWN)
+	      continue;
+
+	    info.osabi = it;
+
+	    if (it != GDB_OSABI_NONE)
+	      {
+		if (!has_gdb_osabi_handler (info))
+		  /* Unsupported.  Skip to prevent warnings like:
+		     A handler for the OS ABI <x> is not built into this
+		     configuration of GDB.  Attempting to continue with the
+		     default <y> settings.  */
+		  continue;
+	      }
+
+	    gdbarch = gdbarch_find_by_info (info);
+	    SELF_CHECK (gdbarch != NULL);
+
+	    try
+	      {
+		kind = gdbarch_breakpoint_kind_from_pc (gdbarch, &pc);
+		insn = gdbarch_sw_breakpoint_from_kind (gdbarch, kind, &bplen);
+	      }
+	    catch (...)
+	      {
+		continue;
+	      }
+	    found = true;
+	    break;
+	  }
+
+	/* Assert that we have found an instruction to disassemble.  */
+	SELF_CHECK (found);
 
+	len = bplen;
 	break;
       }
     }
diff --git a/gdb/osabi.c b/gdb/osabi.c
index bbd7635532f..f1b7f227b87 100644
--- a/gdb/osabi.c
+++ b/gdb/osabi.c
@@ -332,9 +332,10 @@ can_run_code_for (const struct bfd_arch_info *a, const struct bfd_arch_info *b)
   return (a == b || a->compatible (a, b) == a);
 }
 
+/* Return OS ABI handler for INFO.  */
 
-void
-gdbarch_init_osabi (struct gdbarch_info info, struct gdbarch *gdbarch)
+static struct gdb_osabi_handler *
+gdbarch_osabi_handler (struct gdbarch_info info)
 {
   struct gdb_osabi_handler *handler;
 
@@ -367,10 +368,32 @@ gdbarch_init_osabi (struct gdbarch_info info, struct gdbarch *gdbarch)
 	 ISA").  BFD doesn't normally consider 32-bit and 64-bit
 	 "compatible" so it doesn't succeed.  */
       if (can_run_code_for (info.bfd_arch_info, handler->arch_info))
-	{
-	  (*handler->init_osabi) (info, gdbarch);
-	  return;
-	}
+	return handler;
+    }
+
+  return nullptr;
+}
+
+/* See osabi.h.  */
+
+bool
+has_gdb_osabi_handler (struct gdbarch_info info)
+{
+  return gdbarch_osabi_handler (info) != nullptr;
+}
+
+void
+gdbarch_init_osabi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  struct gdb_osabi_handler *handler;
+
+  gdb_assert (info.osabi != GDB_OSABI_UNKNOWN);
+  handler = gdbarch_osabi_handler (info);
+
+  if (handler != nullptr)
+    {
+      (*handler->init_osabi) (info, gdbarch);
+      return;
     }
 
   if (info.osabi == GDB_OSABI_NONE)
diff --git a/gdb/osabi.h b/gdb/osabi.h
index be016732cbc..478a418aac2 100644
--- a/gdb/osabi.h
+++ b/gdb/osabi.h
@@ -74,6 +74,9 @@ enum gdb_osabi gdbarch_lookup_osabi (bfd *);
    string.  */
 enum gdb_osabi osabi_from_tdesc_string (const char *text);
 
+/* Return true if there's an OS ABI handler for INFO.  */
+bool has_gdb_osabi_handler (struct gdbarch_info info);
+
 /* Initialize the gdbarch for the specified OS ABI variant.  */
 void gdbarch_init_osabi (struct gdbarch_info, struct gdbarch *);
 
diff --git a/gdb/selftest-arch.c b/gdb/selftest-arch.c
index f434da718d5..1fe0b2d59b4 100644
--- a/gdb/selftest-arch.c
+++ b/gdb/selftest-arch.c
@@ -68,6 +68,7 @@ foreach_arch_test_generator (const std::string &name,
 	   {
 	     struct gdbarch_info info;
 	     info.bfd_arch_info = bfd_scan_arch (arch);
+	     info.osabi = GDB_OSABI_NONE;
 	     struct gdbarch *gdbarch = gdbarch_find_by_info (info);
 	     SELF_CHECK (gdbarch != NULL);
 	     function (gdbarch);

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-06-04  7:23 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-01 10:41 [PATCH][gdb] Fix warning in foreach_arch selftests Tom de Vries
2022-06-01 14:32 ` Pedro Alves
2022-06-01 16:36   ` Tom de Vries
2022-06-01 17:10     ` Pedro Alves
2022-06-01 19:08     ` Simon Marchi
2022-06-02 15:44       ` Tom de Vries
2022-06-02 17:44         ` Simon Marchi
2022-06-02 18:35           ` Tom de Vries
2022-06-04  7:23             ` Tom de Vries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).