Re: [PATCH 4/4] Don't throw an error in 'info registers' for unavailable MIPS GP registers.

[John Baldwin <jhb@freebsd.org>]

On Tuesday, April 18, 2017 10:33:28 PM Maciej W. Rozycki wrote:
> On Mon, 17 Apr 2017, John Baldwin wrote:
> 
> > >  That needs to be fixed then.  Previously there was no need to handle FIR 
> > > specially.  Overall we need to handle configurations without FPU as well.
> > 
> > That might be true, but that is a larger patch (and it doesn't help with my
> > kernel target case where only a subset of GPRs are valid).
> 
>  Minimising changes is not our goal though, unlike making them correct.  
> And I think we need to tell apart a situation where a register (FIR) is 
> invalid according to the OS ABI and where a subset of registers may not 
> always be accessible.

For FreeBSD/mips in particular I will probably fix the FIR issue by fixing
FreeBSD to export FIR.  That said, 'info registers' on other architectures
is consistent in that they do not throw an error for an unavailable register
but annotate it as such.  The goal of this patch was to align MIPS with
other architectures in terms of that behavior.  Other architectures also
permit registers to be unavailable without requiring a custom target
description FWIW (e.g. the segment registers on x86 are effectively
"optional" and not always supplied by a target).

> > >  I'm somewhat put off by the truncated message in the 32-bit case though 
> > > -- unless a better proposition comes up, then how about using `xxxxxxxx'
> > > and `xxxxxxxxxxxxxxxx' for the 32-bit and 64-bit case respectively as with 
> > > some previous effort?  What do other target backends do?
> > 
> > I don't disagree with the 32-bit format and am certainly open to other
> > options.  Other architectures don't generally use a table and use the full
> > "<unavailable>" string, e.g. on a FreeBSD 64-bit x86 kernel (which uses the
> > "stock" method to print registers rather than a gdbarch-specific one):
> > 
> > (kgdb) info registers 
> > rax            <unavailable>
> > rbx            0xfffff800085cb500       -8795952728832
> > rcx            <unavailable>
> > rdx            <unavailable>
> > rsi            <unavailable>
> > rdi            <unavailable>
> > rbp            0xfffffe04674819c0       0xfffffe04674819c0
> > rsp            0xfffffe0467481968       0xfffffe0467481968
> > r8             <unavailable>
> > r9             <unavailable>
> > r10            <unavailable>
> > r11            <unavailable>
> > r12            0xffffffff80d43b00       -2133574912
> > r13            0xfffff801fb2f3000       -8787583881216
> > r14            0x0      0
> > r15            0xffffffff80d43b00       -2133574912
> > rip            0xffffffff8058bb12       0xffffffff8058bb12 <sched_switch+1218>
> > eflags         <unavailable>
> > cs             0x20     32
> > ss             0x28     40
> > ds             <unavailable>
> > es             <unavailable>
> > fs             <unavailable>
> > gs             <unavailable>
> 
>  Thanks for checking that.  NB I find output above quite messy, especially 
> the lack of column alignment, e.g. `r14' vs `r15'.  It makes it hard to 
> read for me.

I don't disagree with the note on alignment.  That is probably worth fixing
in a separate change.

>  I've given your proposal some thought and I'm of mixed minds between two 
> possibilites I have come up with for the condensed format, specifically 
> `xxxxxxxx' vs `<absent>', which I think are both suitable without being 
> messy:
> 
> [snip]
> 
> The use of angle brackets with the latter variant is consistent with other 
> targets, so I might have just a slight preference for it, but I'll be 
> happy to accept input from other people.

I like <absent> and prefer it for similar reasons (consistency).

>  Where space is at disposal full `<unavailable>' could be printed, e.g:
> 
> (gdb) info registers s3 s4 s5
> s3: <unavailable>
> s4: <unavailable>
> s5: 0x4f0000
> (gdb)
> 
> or:
> 
> (gdb) info registers
>                   zero               at               v0               v1
>  R0   0000000000000000 fffffffffffffff0   <unavailable>  ffffffffc0000000
>                     a0               a1               a2               a3
>  R4     <unavailable>  0000000000000001 000000fffffffbb8 000000fffffffbc8
>                     a4               a5               a6               a7
>  R8     <unavailable>  0000000000000000 8101010101010100 2f2f2f2f2f2f2f2f
>                     t0               t1               t2               t3
>  R12  0000000000000000 0000000000000001 ffffffff8122f5d8   <unavailable>
>                     s0               s1               s2               s3
>  R16  000000fff8006000 0000000120000970 000000fff8006000 0000000000500000
>                     s4               s5               s6               s7
>  R20  0000000000521ec8 0000000000522608 0000000000000000 0000000000000000
>                     t8               t9               k0               k1
>  R24  000000000000161a 0000000120000970 0000000000000000 0000000000000000
>                     gp               sp               s8               ra
>  R28  000000fff8006000 000000fffffffbb0 0000000000000000   <unavailable>
>                 status               lo               hi         badvaddr
>       0000000000109cf3 000000000d35ec75 0000000000000000   <unavailable>
>                  cause               pc
>       0000000000800024 0000000120000970
>                   fcsr              fir          restart
>               00000000         00f30000 0000000000000000
> (gdb)
> 
>  Comments?  Thoughts?

I'm generally fine with using <absent> in the 32-bit table and <unavailable>
otherwise.  I can rework the patch to do that and come back with a V2.

-- 
John Baldwin