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

[Shahab Vahedi <shahab.vahedi@gmail.com>]

On Fri, Feb 28, 2020 at 10:50:54AM -0300, Luis Machado wrote:
> On 2/28/20 10:36 AM, Shahab Vahedi wrote:
> > On Fri, Feb 28, 2020 at 10:31:26AM -0300, Luis Machado wrote:
> > > 
> > > 
> > > On 2/28/20 10:22 AM, Christian Biesinger wrote:
> > > > On Fri, Feb 28, 2020 at 7:08 AM Luis Machado <luis.machado@linaro.org> wrote:
> > > > > 
> > > > > On 1/31/20 7:34 AM, Shahab Vahedi wrote:
> > > > > > This patch was reviewed once (as OK):
> > > > > > https://sourceware.org/ml/gdb-patches/2020-01/msg00613.html
> > > > > > 
> > > > > > Could someone review/merge it?
> > > > > > 
> > > > > > 
> > > > > > Cheers,
> > > > > > Shahab
> > > > > > 
> > > > > 
> > > > > FTR, this has broken general register printing for ARM/AArch64. Now
> > > > > "info reg" shows nothing.
> > > > > 
> > > > > Given there are already remote stubs, probes and gdbservers running out
> > > > > there, this is an undesirable change to have.
> > > > > 
> > > > > I had an IRC chat with Christian and he pointed me at some documentation
> > > > > stating empty-group registers should not be printed, but i think this is
> > > > > a case where the implementation has diverged from the documentation.
> > > > > 
> > > > > https://sourceware.org/gdb/current/onlinedocs/gdb/Target-Description-Format.html#Target-Description-Format
> > > > > 
> > > > > We could probably patch up any non-standard target description XML's
> > > > > from now on, but the existing behavior may have to be preserved.
> > > > 
> > > > Most targets under features/ do not specify group="general" in their
> > > > XML files for anything (only S/390 does); it seems like that should
> > > > maybe be fixed either way?
> > > 
> > > I agree.
> > 
> > The documentation [1] says:
> > If no group is specified, GDB will not display the register in info registers
> > 
> > [1]
> > https://sourceware.org/gdb/onlinedocs/gdb/Target-Description-Format.html#Target-Description-Format
> > 
> 
> That's valid, but unfortunately it doesn't change the fact the existing code
> is breaking backwards compatibility with the installed base.
> 
> As we discussed on IRC, this is code put together in early 2007 and hasn't
> been touched since, apart from a small change in 2017 to cope with arbitrary
> group strings. Plus we have plenty of existing target descriptions that do
> not honor explicitly setting a register group.
> 
> With that said, i think the documentation would have a lower priority in
> this regard. We should fix the existing target descriptions to be more
> strict with the group names, but the old behavior would have to be honored
> IMO.

I agree. The point in the patch was to make extra registers go away. However,
it apparently eliminated the output of "info registers" for other targets and
that is not OK. No matter sticking to the documentation or not. Feel free to
revert the patch.

Ideally I'd like a solution that:

1) "info registers": must not print the non-default (non-general) registers
   as it was the case with c9c895b9666

2) "info registers": should only print "general" group registers. This requires
   adding 'group="general"' to every XML features out there. So I don't know
   how realistic it is.


Cheers,
Shahab