Re: [PATCH] Do not print empty-group regs when printing general ones

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Shahab Vahedi <shahab.vahedi@gmail.com> [2020-01-20 16:53:15 +0100]:

> From: Shahab Vahedi <shahab@synopsys.com>
> 
> When the command "info registers" (same as "info registers general"),
> is issued, _all_ the registers from a tdesc XML are printed. This
> includes the registers with empty register groups (set as "") which
> are supposed to be only printed by "info registers all" (or "info
> all-registers").
> 
> This bug got introduced after all the overhauls that the
> tdesc_register_in_reggroup_p() went through. You can see that the
> logic of tdesc_register_in_reggroup_p() did NOT remain the same after
> all those changes:
> 
>   git difftool c9c895b9666..HEAD -- gdb/target-descriptions.c
> 
> With the current implementation, when the reg->group is an empty
> string, this function returns -1, while in the working revision
> (c9c895b9666), it returned 0. This patch makes sure that the 0 is
> returned again.
> 
> The old implementation of tdesc_register_in_reggroup_p() returned
> -1 when "reggroup" was set to "all_reggroups" at line 4 below:
> 
> 1  tdesc_register_reggroup_p (...)
> 2  {
> 3   ...
> 4   ret = tdesc_register_in_reggroup_p (gdbarch, regno, reggroup);
> 5   if (ret != -1)
> 6     return ret;
> 7
> 8   return default_register_reggroup_p (gdbarch, regno, reggroup);
> 9  }
> 
> As a result, the execution continued at line 8 and the
> default_register_reggroup_p(..., reggroup=all_reggroups) would
> return 1. However, with the current implementation of
> tdesc_register_in_reggroup_p() that allows checking against any
> arbitrary group name, it returns 0 when comparing the "reg->group"
> against the string "all" which is the group name for "all_reggroups".
> I have added a special check to cover this case and
> "info all-registers" works as expected.
> 
> gdb/ChangeLog:
> 2020-01-20  Shahab Vahedi  <shahab@synopsys.com>
> 
> 	* target-descriptions.c (tdesc_register_in_reggroup_p):
> 	Return 0 when reg->group is empty and reggroup is not.

Looks good to me.

Thanks,
Andrew



> ---
>  gdb/target-descriptions.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index 04711ba2fa5..937210cf25e 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -977,13 +977,15 @@ tdesc_register_in_reggroup_p (struct gdbarch *gdbarch, int regno,
>  {
>    struct tdesc_reg *reg = tdesc_find_register (gdbarch, regno);
>  
> -  if (reg != NULL && !reg->group.empty ()
> -      && (reg->group == reggroup_name (reggroup)))
> +  if (reg != NULL)
> +    {
> +      if (reggroup == all_reggroup)
>  	return 1;
> -
> -  if (reg != NULL
> -      && (reggroup == save_reggroup || reggroup == restore_reggroup))
> -    return reg->save_restore;
> +      else if (reggroup == save_reggroup || reggroup == restore_reggroup)
> +	return reg->save_restore;
> +      else
> +	return (int) (reg->group == reggroup_name (reggroup));
> +    }
>  
>    return -1;
>  }
> -- 
> 2.25.0
>