Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: "Maciej W. Rozycki" <macro@imgtec.com>
Cc: John Baldwin <jhb@freebsd.org>,
	Luis Machado <lgustavo@codesourcery.com>,
	gdb-patches@sourceware.org
Subject: Re: [PATCH 4/4] Don't throw an error in 'info registers' for unavailable MIPS GP registers.
Date: Fri, 28 Apr 2017 13:51:00 -0000	[thread overview]
Message-ID: <b34f8a52-c9d7-1507-ce76-f9339300deb1@redhat.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1704271952290.25796@tp.orcam.me.uk>

On 04/27/2017 08:37 PM, Maciej W. Rozycki wrote:
> On Thu, 27 Apr 2017, Pedro Alves wrote:
> 
>>> 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.
>>
>> FWIW, I'd find using anything but <unavailable> when that's what
>> GDB means to be making the port be gratuitously different.
>> "<unavailable>" always means the same thing in gdb -- gdb knows
>> the value materially exists, but it can't get at it for some
>> reason (e.g., ptrace does not expose it, a core dump is trimmed, value
>> not collect in a trace frame, etc.).
> 
>  If you think keeping the word the same across ports is essential, then 
> `<unavl>' would be my second choice, at some aesthetical cost.

I'm not sure that trying to come up with short
variants of these special values manually is the best option.

Another output you could see here is "<not saved>".
GDB prints that if GDB figures out the register is not saved
in a frame (or if the DWARF indicates that, via DW_CFA_undefined).

E.g., with the gdb.mi/mi-reg-undefined.exp testcase binary:

(gdb) b stop_frame
Breakpoint 1 at 0x40059a: file mi-reg-undefined.c, line 21.
(gdb) r
Breakpoint 1, stop_frame () at mi-reg-undefined.c:21
(gdb) up
#1  0x00000000004005aa in first_frame () at mi-reg-undefined.c:26
26      in mi-reg-undefined.c
(gdb) info registers
rax            <not saved>
...

We don't have any arch-independent test for this unfortunately.

Maybe a two-pass approach where the first pass prints
each register to a buffer to find the largest width, and then
use that for column width would be more future proof.  It's also
work better if these magic values are ever translated (i18n).

But TBC, I won't object whatever output you decide is best for MIPS,
and I certainly don't want to impose extra work on John -- getting rid
of that error call is the most important here.
I'm just trying to expose a concern so you can have a more
informed decision.

> 
>> Also, I'm not absolutely sure I'm following the code correctly, but ISTM
>> that the current mips table printing code can already print strings larger
>> than unavailable, like "<invalid float>" in mips_print_fp_register?
> 
>  For obvious reasons FP registers are formatted differently:

OK.  It wasn't actually obvious to me that each FP register was getting
its own column.

>>       if (mips_float_register_p (gdbarch, regnum))
>> 	break;			/* End the row: reached FP register.  */
>>       /* Large registers are handled separately.  */
>>       if (register_size (gdbarch, regnum) > mips_abi_regsize (gdbarch))
>> 	{
>> ...
>> 	  /* Print this register on a row by itself.  */
>>
>> supposedly to avoid column misalignment.  Maybe <unavailable> registers could
>> be printed on a row by themselves too?
> 
>  Hmm, I've investigated the origin of this piece and it has turned out to 
> be quite recent in MIPS port's history, introduced as a hack with commit 

I like how 10 years ago is considered recent.  :-)

Thanks,
Pedro Alves


  reply	other threads:[~2017-04-28 13:51 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-12 18:37 [PATCH 0/4] Cleanups for MIPS registers (mostly FreeBSD-specific) John Baldwin
2017-04-12 18:37 ` [PATCH 1/4] Cleanups to FreeBSD/mips native register operations John Baldwin
2017-04-13 15:48   ` Luis Machado
2017-04-14 18:02     ` John Baldwin
2017-04-12 18:37 ` [PATCH 2/4] Use mips_regnum instead of constants for FreeBSD/mips " John Baldwin
2017-04-13 16:31   ` Luis Machado
2017-04-12 18:37 ` [PATCH 3/4] Consistently use fprintf_filtered when displaying MIPS registers John Baldwin
2017-04-13 16:29   ` Luis Machado
2017-04-27 16:05     ` Pedro Alves
2017-04-28 16:52       ` John Baldwin
2017-04-12 18:45 ` [PATCH 4/4] Don't throw an error in 'info registers' for unavailable MIPS GP registers John Baldwin
2017-04-13 16:37   ` Luis Machado
2017-04-14 18:02     ` John Baldwin
2017-04-15 16:02       ` Maciej W. Rozycki
2017-04-15 17:36         ` John Baldwin
2017-04-15 22:07           ` Maciej W. Rozycki
2017-04-17 18:27             ` John Baldwin
2017-04-18 21:33               ` Maciej W. Rozycki
2017-04-18 22:19                 ` John Baldwin
2017-04-25 21:02                   ` John Baldwin
2017-04-27  0:49                   ` Maciej W. Rozycki
2017-04-27 15:50                 ` Pedro Alves
2017-04-27 19:38                   ` Maciej W. Rozycki
2017-04-28 13:51                     ` Pedro Alves [this message]
2017-04-28 16:52                       ` John Baldwin
2017-05-05 19:51                         ` John Baldwin
2017-05-05 20:08                           ` Maciej W. Rozycki
2017-06-12 18:47                             ` John Baldwin
2017-06-15 23:50                               ` Maciej W. Rozycki
2017-06-16 16:17                                 ` John Baldwin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b34f8a52-c9d7-1507-ce76-f9339300deb1@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=jhb@freebsd.org \
    --cc=lgustavo@codesourcery.com \
    --cc=macro@imgtec.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox