Re: [RFA 1/2] Fix two regressions in scalar printing

[Thomas Preudhomme <thomas.preudhomme@foss.arm.com>]

Hi Tom,

As raised in [1], this patch creates a regression on arm targets in:

gdb.base/sizeof.exp: check valueof "'\377'"

I've taken the liberty of creating a bugzilla ticket [2] where I added some more 
info of things I've tried.

[1] https://sourceware.org/ml/gdb-testers/2017-q3/msg01944.html
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=22010

Please let me know if I can be of any help in fixing that bug.

Best regards,

Thomas


On 13/07/17 13:33, Tom Tromey wrote:
> PR gdb/21675 points out a few regressions in scalar printing.
> 
> One type of regression is due to not carrying over the old handling of
> floating point printing -- where a format like "/x" causes a floating
> point number to first be cast to integer.  While this behavior does not
> seem very useful to me, apparently at least one person is testing for
> it, and we did agree in the earlier thread to preserve this.  So, this
> patch extends this behavior to the 'd' and 'u' formats.
> 
> The other regression is a longstanding bug in print_octal_chars: one of
> the constants was wrong.  This patch fixes the constant and adds static
> asserts to help catch this sort of error.
> 
> 2017-07-13  Tom Tromey  <tom@tromey.com>
> 
> 	PR gdb/21675
> 	* valprint.c (LOW_ZERO): Change value to 034.
> 	(print_octal_chars): Add static_asserts for octal constants.
> 	* printcmd.c (print_scalar_formatted): Add 'd' and 'u' to special
> 	cases for float types.
> 
> 2017-07-13  Tom Tromey  <tom@tromey.com>
> 
> 	PR gdb/21675:
> 	* gdb.base/printcmds.exp (test_radices): New function.
> ---
>   gdb/ChangeLog                        |  8 ++++++++
>   gdb/printcmd.c                       | 11 ++++++++---
>   gdb/testsuite/ChangeLog              |  5 +++++
>   gdb/testsuite/gdb.base/printcmds.exp |  8 ++++++++
>   gdb/valprint.c                       |  8 +++++++-
>   5 files changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 012b3e4..5ac5bab 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,11 @@
> +2017-07-13  Tom Tromey  <tom@tromey.com>
> +
> +	PR gdb/21675
> +	* valprint.c (LOW_ZERO): Change value to 034.
> +	(print_octal_chars): Add static_asserts for octal constants.
> +	* printcmd.c (print_scalar_formatted): Add 'd' and 'u' to special
> +	cases for float types.
> +
>   2017-07-11  John Baldwin  <jhb@FreeBSD.org>
>   
>   	* amd64-bsd-nat.c (amd64bsd_fetch_inferior_registers): Use
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index a8cc052..cd615ec 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -413,7 +413,9 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
>         && (options->format == 'o'
>   	  || options->format == 'x'
>   	  || options->format == 't'
> -	  || options->format == 'z'))
> +	  || options->format == 'z'
> +	  || options->format == 'd'
> +	  || options->format == 'u'))
>       {
>         LONGEST val_long = unpack_long (type, valaddr);
>         converted_float_bytes.resize (TYPE_LENGTH (type));
> @@ -427,11 +429,14 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
>       case 'o':
>         print_octal_chars (stream, valaddr, len, byte_order);
>         break;
> +    case 'd':
>       case 'u':
> -      print_decimal_chars (stream, valaddr, len, false, byte_order);
> +      {
> +	bool is_signed = options->format != 'u' || !TYPE_UNSIGNED (type);
> +	print_decimal_chars (stream, valaddr, len, is_signed, byte_order);
> +      }
>         break;
>       case 0:
> -    case 'd':
>         if (TYPE_CODE (type) != TYPE_CODE_FLT)
>   	{
>   	  print_decimal_chars (stream, valaddr, len, !TYPE_UNSIGNED (type),
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index aa3dee3..0c8481b 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,8 @@
> +2017-07-13  Tom Tromey  <tom@tromey.com>
> +
> +	PR gdb/21675:
> +	* gdb.base/printcmds.exp (test_radices): New function.
> +
>   2017-07-11  Iain Buclaw  <ibuclaw@gdcproject.org>
>   
>   	* gdb.dlang/demangle.exp: Update for demangling changes.
> diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
> index 323ca73..03275c3 100644
> --- a/gdb/testsuite/gdb.base/printcmds.exp
> +++ b/gdb/testsuite/gdb.base/printcmds.exp
> @@ -155,6 +155,13 @@ proc test_float_rejected {} {
>       test_print_reject "p 1.1ll"
>   }
>   
> +# Regression test for PR gdb/21675
> +proc test_radices {} {
> +    gdb_test "print/o 16777211" " = 077777773"
> +    gdb_test "print/d 1.5" " = 1\[^.\]"
> +    gdb_test "print/u 1.5" " = 1\[^.\]"
> +}
> +
>   proc test_print_all_chars {} {
>       global gdb_prompt
>   
> @@ -981,3 +988,4 @@ test_printf
>   test_printf_with_dfp
>   test_print_symbol
>   test_repeat_bytes
> +test_radices
> diff --git a/gdb/valprint.c b/gdb/valprint.c
> index 1667882..9e216cf 100644
> --- a/gdb/valprint.c
> +++ b/gdb/valprint.c
> @@ -1593,15 +1593,21 @@ print_octal_chars (struct ui_file *stream, const gdb_byte *valaddr,
>      */
>   #define BITS_IN_OCTAL 3
>   #define HIGH_ZERO     0340
> -#define LOW_ZERO      0016
> +#define LOW_ZERO      0034
>   #define CARRY_ZERO    0003
> +  static_assert (HIGH_ZERO + LOW_ZERO + CARRY_ZERO == 0xff,
> +		 "cycle zero constants are wrong");
>   #define HIGH_ONE      0200
>   #define MID_ONE       0160
>   #define LOW_ONE       0016
>   #define CARRY_ONE     0001
> +  static_assert (HIGH_ONE + MID_ONE + LOW_ONE + CARRY_ONE == 0xff,
> +		 "cycle one constants are wrong");
>   #define HIGH_TWO      0300
>   #define MID_TWO       0070
>   #define LOW_TWO       0007
> +  static_assert (HIGH_TWO + MID_TWO + LOW_TWO == 0xff,
> +		 "cycle two constants are wrong");
>   
>     /* For 32 we start in cycle 2, with two bits and one bit carry;
>        for 64 in cycle in cycle 1, with one bit and a two bit carry.  */
>