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 08:10:35AM -0600, Christian Biesinger wrote:
> On Fri, Feb 28, 2020 at 8:08 AM Shahab Vahedi <shahab.vahedi@gmail.com> wrote:
> >
> > 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.
> 
> Maybe we can do something like: if there is no register with
> group=general, print all registers w/o group. Otherwise, only print
> registers with group=general.
> 
> Christian

I like that. We must update the documentation to something like:

* no gorup=... mentioned is the same as group="general" --> default
* group="" explicitly means that the register does not belong to any group.

And make the code act accordingly.


Cheers,
Shahab