Re: [PATCH 1/5] gdb/arm: move fetch of arm_gdbarch_tdep to a more inner scope

[Luis Machado <luis.machado@arm.com>]

On 5/31/22 15:30, Andrew Burgess via Gdb-patches wrote:
> This is a small refactor to resolve an issue before it becomes a
> problem in a later commit.
> 
> Move the fetching of an arm_gdbarch_tdep into a more inner scope
> within two functions in arm-tdep.c.
> 
> The problem with the current code is that the functions in question
> are used as the callbacks for two set/show parameters.  These set/show
> parameters are available no matter the current architecture, but are
> really about controlling an ARM architecture specific setting.  And
> so, if I build GDB for all targets on an x86-64/GNU-Linux system, I
> can still do this:
> 
>    (gdb) show arm fpu
>    (gdb) show arm abi
> 
> After these calls we end up in show_fp_model and arm_show_abi
> respectively, where we unconditionally do this:
> 
>    arm_gdbarch_tdep *tdep
>      = (arm_gdbarch_tdep *) gdbarch_tdep (target_gdbarch ());
> 
> However, the gdbarch_tdep() result will only be a arm_gdbarch_tdep if
> the current architecture is ARM, otherwise the result will actually be
> of some other type.
> 
> This isn't actually a problem, as in both cases the use of tdep is
> guarded by a later check that the gdbarch architecture is
> bfd_arch_arm.
> 
> This commit just moves the call to gdbarch_tdep() after the
> architecture check.
> 
> In a later commit gdbarch_tdep() will be able to spot when we are
> casting the result to the wrong type, and this function will trigger
> assertion failures if things are not fixed.
> 
> There should be not user visible changes after this commit.
> ---
>   gdb/arm-tdep.c | 24 ++++++++++++++----------
>   1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 49664093f00..d39342e19c6 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -9015,14 +9015,16 @@ static void
>   show_fp_model (struct ui_file *file, int from_tty,
>   	       struct cmd_list_element *c, const char *value)
>   {
> -  arm_gdbarch_tdep *tdep
> -    = (arm_gdbarch_tdep *) gdbarch_tdep (target_gdbarch ());
> -
>     if (arm_fp_model == ARM_FLOAT_AUTO
>         && gdbarch_bfd_arch_info (target_gdbarch ())->arch == bfd_arch_arm)
> -    gdb_printf (file, _("\
> +    {
> +      arm_gdbarch_tdep *tdep
> +	= (arm_gdbarch_tdep *) gdbarch_tdep (target_gdbarch ());
> +
> +      gdb_printf (file, _("\
>   The current ARM floating point model is \"auto\" (currently \"%s\").\n"),
> -		fp_model_strings[tdep->fp_model]);
> +		  fp_model_strings[tdep->fp_model]);
> +    }
>     else
>       gdb_printf (file, _("\
>   The current ARM floating point model is \"%s\".\n"),
> @@ -9053,14 +9055,16 @@ static void
>   arm_show_abi (struct ui_file *file, int from_tty,
>   	     struct cmd_list_element *c, const char *value)
>   {
> -  arm_gdbarch_tdep *tdep
> -    = (arm_gdbarch_tdep *) gdbarch_tdep (target_gdbarch ());
> -
>     if (arm_abi_global == ARM_ABI_AUTO
>         && gdbarch_bfd_arch_info (target_gdbarch ())->arch == bfd_arch_arm)
> -    gdb_printf (file, _("\
> +    {
> +      arm_gdbarch_tdep *tdep
> +	= (arm_gdbarch_tdep *) gdbarch_tdep (target_gdbarch ());
> +
> +      gdb_printf (file, _("\
>   The current ARM ABI is \"auto\" (currently \"%s\").\n"),
> -		arm_abi_strings[tdep->arm_abi]);
> +		  arm_abi_strings[tdep->arm_abi]);
> +    }
>     else
>       gdb_printf (file, _("The current ARM ABI is \"%s\".\n"),
>   		arm_abi_string);

LGTM