[RFA 2/2] Remove BITS_IN_BYTES define

[RFA 2/2] Remove BITS_IN_BYTES define

[Tom Tromey]

While working on the previous patch, I noticed that BITS_IN_BYTES can be
replaced by HOST_CHAR_BIT, which is used more widely in gdb.

2017-07-13  Tom Tromey  <tom@tromey.com>

	* valprint.c (print_octal_chars): Use HOST_CHAR_BIT.
	(print_binary_chars): Likewise.
	(BITS_IN_BYTES): Remove.
---
 gdb/ChangeLog  | 6 ++++++
 gdb/printcmd.c | 4 ++--
 gdb/valprint.c | 9 +++------
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 5ac5bab..bc68b67 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2017-07-13  Tom Tromey  <tom@tromey.com>
 
+	* valprint.c (print_octal_chars): Use HOST_CHAR_BIT.
+	(print_binary_chars): Likewise.
+	(BITS_IN_BYTES): Remove.
+
+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.
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index cd615ec..179c22c 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -432,8 +432,8 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
     case 'd':
     case 'u':
       {
-	bool is_signed = options->format != 'u' || !TYPE_UNSIGNED (type);
-	print_decimal_chars (stream, valaddr, len, is_signed, byte_order);
+	bool is_unsigned = options->format == 'u' || TYPE_UNSIGNED (type);
+	print_decimal_chars (stream, valaddr, len, !is_unsigned, byte_order);
       }
       break;
     case 0:
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 9e216cf..eef99b1 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -1490,9 +1490,6 @@ void
 print_binary_chars (struct ui_file *stream, const gdb_byte *valaddr,
 		    unsigned len, enum bfd_endian byte_order, bool zero_pad)
 {
-
-#define BITS_IN_BYTES 8
-
   const gdb_byte *p;
   unsigned int i;
   int b;
@@ -1512,7 +1509,7 @@ print_binary_chars (struct ui_file *stream, const gdb_byte *valaddr,
 	  /* Every byte has 8 binary characters; peel off
 	     and print from the MSB end.  */
 
-	  for (i = 0; i < (BITS_IN_BYTES * sizeof (*p)); i++)
+	  for (i = 0; i < (HOST_CHAR_BIT * sizeof (*p)); i++)
 	    {
 	      if (*p & (mask >> i))
 		b = '1';
@@ -1532,7 +1529,7 @@ print_binary_chars (struct ui_file *stream, const gdb_byte *valaddr,
 	   p >= valaddr;
 	   p--)
 	{
-	  for (i = 0; i < (BITS_IN_BYTES * sizeof (*p)); i++)
+	  for (i = 0; i < (HOST_CHAR_BIT * sizeof (*p)); i++)
 	    {
 	      if (*p & (mask >> i))
 		b = '1';
@@ -1612,7 +1609,7 @@ print_octal_chars (struct ui_file *stream, const gdb_byte *valaddr,
   /* 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.  */
 
-  cycle = (len * BITS_IN_BYTES) % BITS_IN_OCTAL;
+  cycle = (len * HOST_CHAR_BIT) % BITS_IN_OCTAL;
   carry = 0;
 
   fputs_filtered ("0", stream);
-- 
2.9.4

Re: [RFA 2/2] Remove BITS_IN_BYTES define

[Pedro Alves]

On 07/13/2017 01:34 PM, Tom Tromey wrote:
> @@ -432,8 +432,8 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
>      case 'd':
>      case 'u':
>        {
> -	bool is_signed = options->format != 'u' || !TYPE_UNSIGNED (type);
> -	print_decimal_chars (stream, valaddr, len, is_signed, byte_order);
> +	bool is_unsigned = options->format == 'u' || TYPE_UNSIGNED (type);
> +	print_decimal_chars (stream, valaddr, len, !is_unsigned, byte_order);
>        }
>        break;
>      case 0:

Ah, looks like you meant to squash this hunk in the previous patch...

The actual BITS_IN_BYTES bits are fine.

Thanks,
Pedro Alves

Re: [RFA 2/2] Remove BITS_IN_BYTES define

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> On 07/13/2017 01:34 PM, Tom Tromey wrote:
>> @@ -432,8 +432,8 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
>> case 'd':
>> case 'u':
>> {
>> -	bool is_signed = options->format != 'u' || !TYPE_UNSIGNED (type);
>> -	print_decimal_chars (stream, valaddr, len, is_signed, byte_order);
>> +	bool is_unsigned = options->format == 'u' || TYPE_UNSIGNED (type);
>> +	print_decimal_chars (stream, valaddr, len, !is_unsigned, byte_order);
>> }
>> break;
>> case 0:

Pedro> Ah, looks like you meant to squash this hunk in the previous patch...

Oops, sorry about this.  I'll fix it up.

Tom

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

[Tom Tromey]

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.  */
-- 
2.9.4

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

[Pedro Alves]

On 07/13/2017 01:33 PM, 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.

I'm open to changing the behavior, if we can come up with something
that is useful and generally makes more sense.

The manual does say, for "/x":

  Regard the bits of the value as an integer, and print the integer in
  hexadecimal.

and for "/f":

  Regard the bits of the value as a floating point number and print
  using typical floating point syntax.

which seems to suggest the intention was to reinterpret the
variable's raw contents as integer.  However, I suspect this was written
with the "x" command in mind though, where you're reinterpreting raw
memory disregarding anything about the types of the objects that may
exist on the examined memory (and also where you can also request a
specific width).  A question like "how to print a float type
object in hexadecimal format" just doesn't really leave much
doubt for the "x" command.  There, it's clearly raw bits
interpretation you want, so you can do things like:

 float global_f = 3.14f;

 (gdb) x /4xb &global_f
 0x601038 <global_f>:    0xc3    0xf5    0x48    0x40
 (gdb) x /fw &global_f
 0x601038 <global_f>:    3.1400001

"print" is different because it's working with objects with
size and type, and can print aggregates/structs with subobjects
of different types, even.

Playing devil's advocate, I could see some justification for the
converting behavior if you look at /x /d /u as behaving like a
printf %x/%d/%u format strings with an implicit cast.  That view
doesn't work with the current implementation of "%f" though, which
reinterprets with print, instead of converting, though with such
a view, that would be seen as a bug...  What a mess.  :-/

Meanwhile, it's good to avoid behavior changes until we have a
clear plan.

> 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.
> 


> @@ -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);

I think you meant "&&" instead of "||".

(
Maybe checking against 'd' would be clearer:
	bool is_signed = options->format == 'd' && !TYPE_UNSIGNED (type);
or even separate case switches.
)

As is, you'll get this:

 (gdb) p /u -1
 $1 = -1

instead of current master's output:

 (gdb) p /u -1
 $1 = 4294967295

which doesn't look right.

Also, I'm also not quite sure about the TYPE_UNSIGNED check.
The manual says:

 @item d
 Print as integer in signed decimal.

 @item u
 Print as integer in unsigned decimal.

And I see this with a gdb from before the recent print_scalar_formatted
changes:

 (gdb) p /d (unsigned long long) -1
 $1 = -1

while we see this with either current master, or your patch:

 (gdb) p /d (unsigned long long) -1
 $1 = 18446744073709551615

which also doesn't look right to me.

And here:

 (gdb) p /d (unsigned) -1
 $2 = 4294967295

I'd expect "-1", but we don't get it with any gdb version (before
original print_scalar_formatted changes, or current master, or
your current patch), which also looks like a bug to me.

I think this would all be fixed by simply having separate
'u'/'d' cases with fixed signness:

    case 'u':
      print_decimal_chars (stream, valaddr, len, false, byte_order);
      break;
    case 'd':
      print_decimal_chars (stream, valaddr, len, true, byte_order);
      break;

> +	print_decimal_chars (stream, valaddr, len, is_signed, byte_order);

Thanks,
Pedro Alves

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

[Eli Zaretskii]

> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 14 Jul 2017 15:56:27 +0100
> 
> The manual says:
> 
>  @item d
>  Print as integer in signed decimal.
> 
>  @item u
>  Print as integer in unsigned decimal.
> 
> And I see this with a gdb from before the recent print_scalar_formatted
> changes:
> 
>  (gdb) p /d (unsigned long long) -1
>  $1 = -1
> 
> while we see this with either current master, or your patch:
> 
>  (gdb) p /d (unsigned long long) -1
>  $1 = 18446744073709551615
> 
> which also doesn't look right to me.
> 
> And here:
> 
>  (gdb) p /d (unsigned) -1
>  $2 = 4294967295
> 
> I'd expect "-1", but we don't get it with any gdb version (before
> original print_scalar_formatted changes, or current master, or
> your current patch), which also looks like a bug to me.

I think I agree: we should produce what the format says, not what the
cast says.

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

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I think this would all be fixed by simply having separate
Pedro> 'u'/'d' cases with fixed signness:

Pedro>     case 'u':
Pedro>       print_decimal_chars (stream, valaddr, len, false, byte_order);
Pedro>       break;
Pedro>     case 'd':
Pedro>       print_decimal_chars (stream, valaddr, len, true, byte_order);
Pedro>       break;

I'm testing this.  Thanks for the feedback.

One early note is that this changes the expected output for this test:

FAIL: gdb.dwarf2/var-access.exp: verify re-initialized s2

Now it says:

print/d s2
$9 = {a = -65, b = 73, c = -25, d = 123}

but the test wants:

gdb_test "print/d s2"  " = \\{a = 191, b = 73, c = 231, d = 123\\}" \
    "verify re-initialized s2"

Tom

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

[Pedro Alves]

On 07/14/2017 05:49 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> I think this would all be fixed by simply having separate
> Pedro> 'u'/'d' cases with fixed signness:
> 
> Pedro>     case 'u':
> Pedro>       print_decimal_chars (stream, valaddr, len, false, byte_order);
> Pedro>       break;
> Pedro>     case 'd':
> Pedro>       print_decimal_chars (stream, valaddr, len, true, byte_order);
> Pedro>       break;
> 
> I'm testing this.  Thanks for the feedback.
> 
> One early note is that this changes the expected output for this test:
> 
> FAIL: gdb.dwarf2/var-access.exp: verify re-initialized s2
> 
> Now it says:
> 
> print/d s2
> $9 = {a = -65, b = 73, c = -25, d = 123}
> 
> but the test wants:
> 
> gdb_test "print/d s2"  " = \\{a = 191, b = 73, c = 231, d = 123\\}" \
>     "verify re-initialized s2"
> 

Yeah, that seems OK to me GDB-output-wise.  "You get what you ask for".

Now, the test is for "# Byte-aligned register- and memory pieces.", and
is treating the bytes as raw bytes, even though the fields are
defined as "char".

We see just above that a test setting the fields using values over 0x7f:

 gdb_test_no_output "set var s2 = {191, 73, 231, 123}" \
     "re-initialize s2"
 gdb_test "print/d s2"  " = \\{a = 191, b = 73, c = 231, d = 123\\}" \
     "verify re-initialized s2"

The /d was surely as a convenience to avoid printing the bytes
in character format.  I'd just change it to /u.  Same for the related
tests just above.

Thanks,
Pedro Alves

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

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Yeah, that seems OK to me GDB-output-wise.  "You get what you ask
Pedro> for".

Here is an updated version of this patch, that (I think) implements what
was discussed in this thread.

I regtested it on the buildbot.

Let me know what you think.

Tom

commit 32d9b636591021a7d2a31ce53da6ba1db9f85689
Author: Tom Tromey <tom@tromey.com>
Date:   Tue Jul 11 06:40:40 2017 -0600

    Fix two regressions in scalar printing
    
    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 "/d" causes a floating
    point number to first be cast to a signed integer.  This patch restores
    this behavior.
    
    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.
    
    gdb/ChangeLog
    2017-07-31  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' case.
    
    gdb/testsuite/ChangeLog
    2017-07-31  Tom Tromey  <tom@tromey.com>
    
            PR gdb/21675:
            * gdb.base/printcmds.exp (test_radices): New function.
            * gdb.dwarf2/var-access.exp: Use p/u, not p/d.
            * gdb.base/sizeof.exp (check_valueof): Use p/d.
            * lib/gdb.exp (get_integer_valueof): Use p/d.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index dd66a45..afdfb16 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2017-07-31  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' case.
+
 2017-07-31  Xavier Roirand  <roirand@adacore.com>
 
 	* solib-darwin.c (DYLD_VERSION_MAX): Increase value.
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index a8cc052..f5ed513 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,13 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
     case 'o':
       print_octal_chars (stream, valaddr, len, byte_order);
       break;
+    case 'd':
+      print_decimal_chars (stream, valaddr, len, true, byte_order);
+      break;
     case 'u':
       print_decimal_chars (stream, valaddr, len, false, 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 2a777a8..f763661 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2017-07-31  Tom Tromey  <tom@tromey.com>
+
+	PR gdb/21675:
+	* gdb.base/printcmds.exp (test_radices): New function.
+	* gdb.dwarf2/var-access.exp: Use p/u, not p/d.
+	* gdb.base/sizeof.exp (check_valueof): Use p/d.
+	* lib/gdb.exp (get_integer_valueof): Use p/d.
+
 2017-07-26  Yao Qi  <yao.qi@linaro.org>
 
 	* gdb.gdb/unittest.exp: Invoke command
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/testsuite/gdb.base/sizeof.exp b/gdb/testsuite/gdb.base/sizeof.exp
index d7ada65..5d89407 100644
--- a/gdb/testsuite/gdb.base/sizeof.exp
+++ b/gdb/testsuite/gdb.base/sizeof.exp
@@ -81,7 +81,7 @@ check_sizeof "long double" ${sizeof_long_double}
 
 proc check_valueof { exp val } {
     gdb_test "next" "" ""
-    gdb_test "p value" " = ${val}" "check valueof \"$exp\""
+    gdb_test "p /d value" " = ${val}" "check valueof \"$exp\""
 }
 
 # Check that GDB and the target agree over the sign of a character.
diff --git a/gdb/testsuite/gdb.dwarf2/var-access.exp b/gdb/testsuite/gdb.dwarf2/var-access.exp
index 8ebad6a..9180c88 100644
--- a/gdb/testsuite/gdb.dwarf2/var-access.exp
+++ b/gdb/testsuite/gdb.dwarf2/var-access.exp
@@ -282,16 +282,16 @@ gdb_test_no_output "set var \$[lindex $regname 0] = 81" \
     "init reg for s2.a"
 gdb_test_no_output "set var \$[lindex $regname 1] = 28" \
     "init reg for s2.c"
-gdb_test "print/d s2" " = \\{a = 81, b = 4, c = 28, d = 5\\}" \
+gdb_test "print/u s2" " = \\{a = 81, b = 4, c = 28, d = 5\\}" \
     "initialized s2 from mem and regs"
 gdb_test_no_output "set var s2.c += s2.a + s2.b - s2.d"
-gdb_test "print/d s2" " = \\{a = 81, b = 4, c = 108, d = 5\\}" \
+gdb_test "print/u s2" " = \\{a = 81, b = 4, c = 108, d = 5\\}" \
     "verify s2.c"
-gdb_test "print/d \$[lindex $regname 1]" " = 108" \
+gdb_test "print/u \$[lindex $regname 1]" " = 108" \
     "verify s2.c through reg"
 gdb_test_no_output "set var s2 = {191, 73, 231, 123}" \
     "re-initialize s2"
-gdb_test "print/d s2"  " = \\{a = 191, b = 73, c = 231, d = 123\\}" \
+gdb_test "print/u s2"  " = \\{a = 191, b = 73, c = 231, d = 123\\}" \
     "verify re-initialized s2"
 
 # Unaligned bitfield access through byte-aligned pieces.
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.  */

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

[Pedro Alves]

On 07/31/2017 11:03 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> Yeah, that seems OK to me GDB-output-wise.  "You get what you ask
> Pedro> for".
> 
> Here is an updated version of this patch, that (I think) implements what
> was discussed in this thread.
> 
> I regtested it on the buildbot.
> 
> Let me know what you think.

Sorry for dropping the ball on this.  I wanted to be sure that
we have tests for the

 (gdb) p /u -1
 $1 = 4294967295
 (gdb) p /d (unsigned long long) -1
 $2 = -1

etc. issues discussed earlier.  Do you know whether there's some tests for
that already somewhere, but might have simply been missed before for
running both patches together?

> +# 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\[^.\]"

What's the reason for the "\[^.\]" part of the regexes?
What's that trying to match?  Why not simply " = 1" ?

> +}
> +
>  proc test_print_all_chars {} {
>      global gdb_prompt

Thanks,
Pedro Alves

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

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Sorry for dropping the ball on this.  I wanted to be sure that
Pedro> we have tests for the

Pedro>  (gdb) p /u -1
Pedro>  $1 = 4294967295
Pedro>  (gdb) p /d (unsigned long long) -1
Pedro>  $2 = -1

Pedro> etc. issues discussed earlier.  Do you know whether there's some tests for
Pedro> that already somewhere, but might have simply been missed before for
Pedro> running both patches together?

Good question.  I think what happened is that this change had some
fallout elsewhere, which is why there are also some test changes in the
patch.

However, these are kind of indirect, so I added tests for
"print/d (unsigned char) -1" and "print/u (char) -1".

>> +# 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\[^.\]"

Pedro> What's the reason for the "\[^.\]" part of the regexes?
Pedro> What's that trying to match?  Why not simply " = 1" ?

I was worried that this would erroneously match "1.5" or the like; but I
tried it and I see my fears are unfounded.  So, I changed these to " = 1".

Tom

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

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> However, these are kind of indirect, so I added tests for
Tom> "print/d (unsigned char) -1" and "print/u (char) -1".

Here's the updated patch.

Tom

commit e2575fc9ea66582b71d4bccee0a76f772558be40
Author: Tom Tromey <tom@tromey.com>
Date:   Tue Jul 11 06:40:40 2017 -0600

    Fix two regressions in scalar printing
    
    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 "/d" causes a floating
    point number to first be cast to a signed integer.  This patch restores
    this behavior.
    
    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-08-14  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' case.
    
    testsuite/ChangeLog
    2017-08-14  Tom Tromey  <tom@tromey.com>
    
            PR gdb/21675:
            * gdb.base/printcmds.exp (test_radices): New function.
            * gdb.dwarf2/var-access.exp: Use p/u, not p/d.
            * gdb.base/sizeof.exp (check_valueof): Use p/d.
            * lib/gdb.exp (get_integer_valueof): Use p/d.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index dcb591d..fd0f959 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2017-08-14  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' case.
+
 2017-08-11  Tom Tromey  <tom@tromey.com>
 
 	* symfile.c (add_symbol_file_command): Use std::vector.
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index d5c83f0..a1231d4 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,13 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
     case 'o':
       print_octal_chars (stream, valaddr, len, byte_order);
       break;
+    case 'd':
+      print_decimal_chars (stream, valaddr, len, true, byte_order);
+      break;
     case 'u':
       print_decimal_chars (stream, valaddr, len, false, 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 23658b0..6db249b 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2017-08-14  Tom Tromey  <tom@tromey.com>
+
+	PR gdb/21675:
+	* gdb.base/printcmds.exp (test_radices): New function.
+	* gdb.dwarf2/var-access.exp: Use p/u, not p/d.
+	* gdb.base/sizeof.exp (check_valueof): Use p/d.
+	* lib/gdb.exp (get_integer_valueof): Use p/d.
+
 2017-08-12  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* lib/gdb.exp (get_valueof): Don't capture end-of-line
diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
index 323ca73..9409c6a 100644
--- a/gdb/testsuite/gdb.base/printcmds.exp
+++ b/gdb/testsuite/gdb.base/printcmds.exp
@@ -155,6 +155,16 @@ 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"
+
+    gdb_test "print/u (char) -1" " = 255"
+    gdb_test "print/d (unsigned char) -1" " = -1"
+}
+
 proc test_print_all_chars {} {
     global gdb_prompt
 
@@ -981,3 +991,4 @@ test_printf
 test_printf_with_dfp
 test_print_symbol
 test_repeat_bytes
+test_radices
diff --git a/gdb/testsuite/gdb.base/sizeof.exp b/gdb/testsuite/gdb.base/sizeof.exp
index d7ada65..5d89407 100644
--- a/gdb/testsuite/gdb.base/sizeof.exp
+++ b/gdb/testsuite/gdb.base/sizeof.exp
@@ -81,7 +81,7 @@ check_sizeof "long double" ${sizeof_long_double}
 
 proc check_valueof { exp val } {
     gdb_test "next" "" ""
-    gdb_test "p value" " = ${val}" "check valueof \"$exp\""
+    gdb_test "p /d value" " = ${val}" "check valueof \"$exp\""
 }
 
 # Check that GDB and the target agree over the sign of a character.
diff --git a/gdb/testsuite/gdb.dwarf2/var-access.exp b/gdb/testsuite/gdb.dwarf2/var-access.exp
index 8ebad6a..9180c88 100644
--- a/gdb/testsuite/gdb.dwarf2/var-access.exp
+++ b/gdb/testsuite/gdb.dwarf2/var-access.exp
@@ -282,16 +282,16 @@ gdb_test_no_output "set var \$[lindex $regname 0] = 81" \
     "init reg for s2.a"
 gdb_test_no_output "set var \$[lindex $regname 1] = 28" \
     "init reg for s2.c"
-gdb_test "print/d s2" " = \\{a = 81, b = 4, c = 28, d = 5\\}" \
+gdb_test "print/u s2" " = \\{a = 81, b = 4, c = 28, d = 5\\}" \
     "initialized s2 from mem and regs"
 gdb_test_no_output "set var s2.c += s2.a + s2.b - s2.d"
-gdb_test "print/d s2" " = \\{a = 81, b = 4, c = 108, d = 5\\}" \
+gdb_test "print/u s2" " = \\{a = 81, b = 4, c = 108, d = 5\\}" \
     "verify s2.c"
-gdb_test "print/d \$[lindex $regname 1]" " = 108" \
+gdb_test "print/u \$[lindex $regname 1]" " = 108" \
     "verify s2.c through reg"
 gdb_test_no_output "set var s2 = {191, 73, 231, 123}" \
     "re-initialize s2"
-gdb_test "print/d s2"  " = \\{a = 191, b = 73, c = 231, d = 123\\}" \
+gdb_test "print/u s2"  " = \\{a = 191, b = 73, c = 231, d = 123\\}" \
     "verify re-initialized s2"
 
 # Unaligned bitfield access through byte-aligned pieces.
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.  */

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

[Pedro Alves]

On 08/14/2017 04:43 PM, Tom Tromey wrote:
>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
> 
> Tom> However, these are kind of indirect, so I added tests for
> Tom> "print/d (unsigned char) -1" and "print/u (char) -1".
> 
> Here's the updated patch.

OK, please push.

Thanks,
Pedro Alves

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

[Tom Tromey]

Tom> Here's the updated patch.

My latest try run on the buildbot shows this regressing again.
I will try to fix it soon.

Tom

Regression on 32-bit: gdb.guile/scm-ports.exp [Re: [RFA 1/2] Fix two regressions in scalar printing]

[Jan Kratochvil]

On Thu, 17 Aug 2017 04:23:49 +0200, Tom Tromey wrote:
> Tom> Here's the updated patch.
> 
> My latest try run on the buildbot shows this regressing again.
> I will try to fix it soon.

I am not sure which regression did you mean here but:

d6382fffde99214ce4aee99a208ddb703c647008 is the first bad commit
commit d6382fffde99214ce4aee99a208ddb703c647008
Author: Tom Tromey <tom@tromey.com>
Date:   Tue Jul 11 06:40:40 2017 -0600
    Fix two regressions in scalar printing

Running /home/jkratoch/redhat/gdb-test-guile/gdb/testsuite/gdb.guile/scm-ports.exp ...
FAIL: gdb.guile/scm-ports.exp: buffered: seek to $sp
FAIL: gdb.guile/scm-ports.exp: buffered: seek to $sp for restore
FAIL: gdb.guile/scm-ports.exp: unbuffered: seek to $sp
FAIL: gdb.guile/scm-ports.exp: unbuffered: seek to $sp for restore

Tested on Fedora Rawhide i386.  It happens also on x86_64 with -m32.


Jan


 guile (print (seek rw-mem-port (value->integer sp-reg) SEEK_SET))^M
-= 4294949960^M
-(gdb) PASS: gdb.guile/scm-ports.exp: buffered: seek to $sp
+= 4294949832^M
+(gdb) FAIL: gdb.guile/scm-ports.exp: buffered: seek to $sp

 guile (print (seek rw-mem-port (value->integer sp-reg) SEEK_SET))^M
-= 4294949960^M
-(gdb) PASS: gdb.guile/scm-ports.exp: buffered: seek to $sp for restore
+= 4294949832^M
+(gdb) FAIL: gdb.guile/scm-ports.exp: buffered: seek to $sp for restore

 guile (print (seek rw-mem-port (value->integer sp-reg) SEEK_SET))^M
-= 4294949960^M
-(gdb) PASS: gdb.guile/scm-ports.exp: unbuffered: seek to $sp
+= 4294949832^M
+(gdb) FAIL: gdb.guile/scm-ports.exp: unbuffered: seek to $sp

 guile (print (seek rw-mem-port (value->integer sp-reg) SEEK_SET))^M
-= 4294949960^M
-(gdb) PASS: gdb.guile/scm-ports.exp: unbuffered: seek to $sp for restore
+= 4294949832^M
+(gdb) FAIL: gdb.guile/scm-ports.exp: unbuffered: seek to $sp for restore

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

[Jonah Graham]

On 13 July 2017 at 13:33, Tom Tromey <tom@tromey.com> 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.

While Eclipse CDT is testing for this format, speaking as an Eclipse
CDT committer, the Eclipse CDT community should be happy to change the
behaviour if the GDB community is. The tests purpose in Eclipse CDT is
to ensure that there is proper communication between CDT and GDB over
MI. Personally I would prefer changes like this to be on major version
number changes, but I am not intimately familiar with what GDB version
numbers are intending to convey to users.

> 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.

In filing gdb/21675 I seem to have combined two issues that I thought
were the same cause (due to having both been triggered by the same GDB
change). This part of the bug is the problematic one.

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

[Thomas Preudhomme]

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.  */
>