public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Change how "print/x" displays floating-point value
@ 2022-02-17 21:29 Tom Tromey
  2022-02-17 23:17 ` Andrew Burgess
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2022-02-17 21:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Currently, "print/x" will display a floating-point value by first
casting it to an integer type.  This yields weird results like:

    (gdb) print/x 1.5
    $1 = 0x1

This has confused users multiple times -- see PR gdb/16242, where
there are several dups.  I've also seen some confusion from this
internally at AdaCore.

The manual says:

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

... which seems more useful.  So, perhaps what happened is that this
was incorrectly implemented (or maybe correctly implemented and then
regressed, as there don't seem to be any tests).

This patch fixes the bug.

There was a previous discussion where we agreed to preserve the old
behavior:

    https://sourceware.org/legacy-ml/gdb-patches/2017-06/msg00314.html

However, I think it makes more sense to follow the manual.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16242
---
 gdb/printcmd.c                            | 9 ++-------
 gdb/testsuite/gdb.base/printcmds.exp      | 7 +++++--
 gdb/testsuite/gdb.base/return-nodebug.exp | 7 ++++++-
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 6f9be820b0c..30de1927d39 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -426,19 +426,14 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
       len = newlen;
     }
 
-  /* Historically gdb has printed floats by first casting them to a
-     long, and then printing the long.  PR cli/16242 suggests changing
-     this to using C-style hex float format.
-
-     Biased range types and sub-word scalar types must also be handled
+  /* Biased range types and sub-word scalar types must be handled
      here; the value is correctly computed by unpack_long.  */
   gdb::byte_vector converted_bytes;
   /* Some cases below will unpack the value again.  In the biased
      range case, we want to avoid this, so we store the unpacked value
      here for possible use later.  */
   gdb::optional<LONGEST> val_long;
-  if (((type->code () == TYPE_CODE_FLT
-	|| is_fixed_point_type (type))
+  if ((is_fixed_point_type (type)
        && (options->format == 'o'
 	   || options->format == 'x'
 	   || options->format == 't'
diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
index 37632985a07..2d0aafd6d8b 100644
--- a/gdb/testsuite/gdb.base/printcmds.exp
+++ b/gdb/testsuite/gdb.base/printcmds.exp
@@ -158,8 +158,11 @@ proc test_float_rejected {} {
 # 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"
+
+    # See PR gdb/16242 for this.
+    gdb_test "print/d 1.5f" " = 1069547520"
+    gdb_test "print/u 1.5f" " = 1069547520"
+    gdb_test "print/x 1.5f" " = 0x3fc00000"
 
     gdb_test "print/u (char) -1" " = 255"
     gdb_test "print/d (unsigned char) -1" " = -1"
diff --git a/gdb/testsuite/gdb.base/return-nodebug.exp b/gdb/testsuite/gdb.base/return-nodebug.exp
index 6fd41bee884..1cce09d2fc4 100644
--- a/gdb/testsuite/gdb.base/return-nodebug.exp
+++ b/gdb/testsuite/gdb.base/return-nodebug.exp
@@ -38,7 +38,12 @@ proc do_test {type} {
 		"advance to marker"
 
 	    # And if it returned the full width of the result.
-	    gdb_test "print /d t" " = -1" "full width of the returned result"
+	    if {$type == "float" || $type == "double"} {
+		set flag ""
+	    } else {
+		set flag "/d"
+	    }
+	    gdb_test "print $flag t" " = -1" "full width of the returned result"
 	}
     }
 }
-- 
2.31.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Change how "print/x" displays floating-point value
  2022-02-17 21:29 [PATCH] Change how "print/x" displays floating-point value Tom Tromey
@ 2022-02-17 23:17 ` Andrew Burgess
  2022-02-18 16:44   ` Tom Tromey
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2022-02-17 23:17 UTC (permalink / raw)
  To: Tom Tromey via Gdb-patches, gdb-patches; +Cc: Tom Tromey

Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

> Currently, "print/x" will display a floating-point value by first
> casting it to an integer type.  This yields weird results like:
>
>     (gdb) print/x 1.5
>     $1 = 0x1
>
> This has confused users multiple times -- see PR gdb/16242, where
> there are several dups.  I've also seen some confusion from this
> internally at AdaCore.
>
> The manual says:
>
>     'x'
> 	 Regard the bits of the value as an integer, and print the integer
> 	 in hexadecimal.
>
> ... which seems more useful.  So, perhaps what happened is that this
> was incorrectly implemented (or maybe correctly implemented and then
> regressed, as there don't seem to be any tests).
>
> This patch fixes the bug.
>
> There was a previous discussion where we agreed to preserve the old
> behavior:
>
>     https://sourceware.org/legacy-ml/gdb-patches/2017-06/msg00314.html
>
> However, I think it makes more sense to follow the manual.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16242
> ---
>  gdb/printcmd.c                            | 9 ++-------
>  gdb/testsuite/gdb.base/printcmds.exp      | 7 +++++--
>  gdb/testsuite/gdb.base/return-nodebug.exp | 7 ++++++-
>  3 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index 6f9be820b0c..30de1927d39 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -426,19 +426,14 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
>        len = newlen;
>      }
>  
> -  /* Historically gdb has printed floats by first casting them to a
> -     long, and then printing the long.  PR cli/16242 suggests changing
> -     this to using C-style hex float format.
> -
> -     Biased range types and sub-word scalar types must also be handled
> +  /* Biased range types and sub-word scalar types must be handled
>       here; the value is correctly computed by unpack_long.  */
>    gdb::byte_vector converted_bytes;
>    /* Some cases below will unpack the value again.  In the biased
>       range case, we want to avoid this, so we store the unpacked value
>       here for possible use later.  */
>    gdb::optional<LONGEST> val_long;
> -  if (((type->code () == TYPE_CODE_FLT
> -	|| is_fixed_point_type (type))
> +  if ((is_fixed_point_type (type)
>         && (options->format == 'o'
>  	   || options->format == 'x'
>  	   || options->format == 't'
> diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
> index 37632985a07..2d0aafd6d8b 100644
> --- a/gdb/testsuite/gdb.base/printcmds.exp
> +++ b/gdb/testsuite/gdb.base/printcmds.exp
> @@ -158,8 +158,11 @@ proc test_float_rejected {} {
>  # 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"
> +
> +    # See PR gdb/16242 for this.
> +    gdb_test "print/d 1.5f" " = 1069547520"
> +    gdb_test "print/u 1.5f" " = 1069547520"
> +    gdb_test "print/x 1.5f" " = 0x3fc00000"

I noticed the reference to PR gdb/21675 at the top of this hunk, which
was an interesting read.

Would it be possible to increase the testing to cover printing doubles
as well as floats, and also to test /o and /t format?

FWIW, I think this change makes sense.  Though I wonder if it is worth
having a NEWS entry as this might look like a bug to someone who is used
to the old behaviour?

Also, maybe I'm just overly paranoid, but I wonder if it is worth really
stressing the point in the manual (@node Output Format) that the value
is reinterpreted, rather than cast.  Especially for /d /u /o, etc the
manual is rather vague on how a float will be handled, e.g.:

  @item d
  Print as integer in signed decimal.

Which doesn't really tell me much...

Thanks,
Andrew




>  
>      gdb_test "print/u (char) -1" " = 255"
>      gdb_test "print/d (unsigned char) -1" " = -1"
> diff --git a/gdb/testsuite/gdb.base/return-nodebug.exp b/gdb/testsuite/gdb.base/return-nodebug.exp
> index 6fd41bee884..1cce09d2fc4 100644
> --- a/gdb/testsuite/gdb.base/return-nodebug.exp
> +++ b/gdb/testsuite/gdb.base/return-nodebug.exp
> @@ -38,7 +38,12 @@ proc do_test {type} {
>  		"advance to marker"
>  
>  	    # And if it returned the full width of the result.
> -	    gdb_test "print /d t" " = -1" "full width of the returned result"
> +	    if {$type == "float" || $type == "double"} {
> +		set flag ""
> +	    } else {
> +		set flag "/d"
> +	    }
> +	    gdb_test "print $flag t" " = -1" "full width of the returned result"
>  	}
>      }
>  }
> -- 
> 2.31.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Change how "print/x" displays floating-point value
  2022-02-17 23:17 ` Andrew Burgess
@ 2022-02-18 16:44   ` Tom Tromey
  2022-02-18 17:02     ` Eli Zaretskii
  2022-02-18 17:41     ` Andreas Schwab
  0 siblings, 2 replies; 14+ messages in thread
From: Tom Tromey @ 2022-02-18 16:44 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: Tom Tromey via Gdb-patches, Tom Tromey

Andrew> Would it be possible to increase the testing to cover printing doubles
Andrew> as well as floats, and also to test /o and /t format?

Did it.

Andrew> FWIW, I think this change makes sense.  Though I wonder if it is worth
Andrew> having a NEWS entry as this might look like a bug to someone who is used
Andrew> to the old behaviour?

I had actually considered this, so I since you also asked, I think it's
a good idea.

Andrew> Also, maybe I'm just overly paranoid, but I wonder if it is worth really
Andrew> stressing the point in the manual (@node Output Format) that the value
Andrew> is reinterpreted, rather than cast.  Especially for /d /u /o, etc the
Andrew> manual is rather vague on how a float will be handled, e.g.:

I did this too.

Let me know what you think of this.

Tom

commit bb4117440a91bff97483eaab4d12543c9663de09
Author: Tom Tromey <tromey@adacore.com>
Date:   Thu Feb 17 13:43:59 2022 -0700

    Change how "print/x" displays floating-point value
    
    Currently, "print/x" will display a floating-point value by first
    casting it to an integer type.  This yields weird results like:
    
        (gdb) print/x 1.5
        $1 = 0x1
    
    This has confused users multiple times -- see PR gdb/16242, where
    there are several dups.  I've also seen some confusion from this
    internally at AdaCore.
    
    The manual says:
    
        'x'
             Regard the bits of the value as an integer, and print the integer
             in hexadecimal.
    
    ... which seems more useful.  So, perhaps what happened is that this
    was incorrectly implemented (or maybe correctly implemented and then
    regressed, as there don't seem to be any tests).
    
    This patch fixes the bug.
    
    There was a previous discussion where we agreed to preserve the old
    behavior:
    
        https://sourceware.org/legacy-ml/gdb-patches/2017-06/msg00314.html
    
    However, I think it makes more sense to follow the manual.
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16242

diff --git a/gdb/NEWS b/gdb/NEWS
index 9da74e71796..544efa3fe74 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -97,6 +97,12 @@ show style disassembler enabled
 
 * Changed commands
 
+print
+  Printing of floating-point values with base-modifying formats like
+  /x has been changed to display the underlying bytes of the value in
+  the desired base.  This was GDB's documented behavior, but was never
+  implemented correctly.
+
 maint packet
   This command can now print a reply, if the reply includes
   non-printable characters.  Any non-printable characters are printed
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a68cf31dcf3..88121c9d4c5 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -10758,16 +10758,20 @@ Regard the bits of the value as an integer, and print the integer in
 hexadecimal.
 
 @item d
-Print as integer in signed decimal.
+Regard the bits of the value as an integer, and print the integer in
+decimal.
 
 @item u
-Print as integer in unsigned decimal.
+Regard the bits of the value as an integer, and print the integer in
+unsigned decimal.
 
 @item o
-Print as integer in octal.
+Regard the bits of the value as an integer, and print the integer in
+octal.
 
 @item t
-Print as integer in binary.  The letter @samp{t} stands for ``two''.
+Regard the bits of the value as an integer, and print the integer in
+binary.  The letter @samp{t} stands for ``two''.
 @footnote{@samp{b} cannot be used because these format letters are also
 used with the @code{x} command, where @samp{b} stands for ``byte'';
 see @ref{Memory,,Examining Memory}.}
@@ -10789,10 +10793,11 @@ The command @code{info symbol 0x54320} yields similar results.
 @xref{Symbols, info symbol}.
 
 @item c
-Regard as an integer and print it as a character constant.  This
-prints both the numerical value and its character representation.  The
-character representation is replaced with the octal escape @samp{\nnn}
-for characters outside the 7-bit @sc{ascii} range.
+Cast the value to an integer (unlike other formats, this does not just
+reinterpret the underlying bits) and print it as a character constant.
+This prints both the numerical value and its character representation.
+The character representation is replaced with the octal escape
+@samp{\nnn} for characters outside the 7-bit @sc{ascii} range.
 
 Without this format, @value{GDBN} displays @code{char},
 @w{@code{unsigned char}}, and @w{@code{signed char}} data as character
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 6f9be820b0c..30de1927d39 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -426,19 +426,14 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
       len = newlen;
     }
 
-  /* Historically gdb has printed floats by first casting them to a
-     long, and then printing the long.  PR cli/16242 suggests changing
-     this to using C-style hex float format.
-
-     Biased range types and sub-word scalar types must also be handled
+  /* Biased range types and sub-word scalar types must be handled
      here; the value is correctly computed by unpack_long.  */
   gdb::byte_vector converted_bytes;
   /* Some cases below will unpack the value again.  In the biased
      range case, we want to avoid this, so we store the unpacked value
      here for possible use later.  */
   gdb::optional<LONGEST> val_long;
-  if (((type->code () == TYPE_CODE_FLT
-	|| is_fixed_point_type (type))
+  if ((is_fixed_point_type (type)
        && (options->format == 'o'
 	   || options->format == 'x'
 	   || options->format == 't'
diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
index 37632985a07..78a2147017d 100644
--- a/gdb/testsuite/gdb.base/printcmds.exp
+++ b/gdb/testsuite/gdb.base/printcmds.exp
@@ -158,8 +158,14 @@ proc test_float_rejected {} {
 # 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"
+
+    # See PR gdb/16242 for this.
+    gdb_test "print/d 1.5f" " = 1069547520"
+    gdb_test "print/u 1.5f" " = 1069547520"
+    gdb_test "print/x 1.5f" " = 0x3fc00000"
+    gdb_test "print/t 1.5f" " = 111111110000000000000000000000"
+    gdb_test "print/o 1.5f" " = 07760000000"
+    gdb_test "print/c 65.0f" " = 65 'A'"
 
     gdb_test "print/u (char) -1" " = 255"
     gdb_test "print/d (unsigned char) -1" " = -1"
diff --git a/gdb/testsuite/gdb.base/return-nodebug.exp b/gdb/testsuite/gdb.base/return-nodebug.exp
index 6fd41bee884..1cce09d2fc4 100644
--- a/gdb/testsuite/gdb.base/return-nodebug.exp
+++ b/gdb/testsuite/gdb.base/return-nodebug.exp
@@ -38,7 +38,12 @@ proc do_test {type} {
 		"advance to marker"
 
 	    # And if it returned the full width of the result.
-	    gdb_test "print /d t" " = -1" "full width of the returned result"
+	    if {$type == "float" || $type == "double"} {
+		set flag ""
+	    } else {
+		set flag "/d"
+	    }
+	    gdb_test "print $flag t" " = -1" "full width of the returned result"
 	}
     }
 }

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Change how "print/x" displays floating-point value
  2022-02-18 16:44   ` Tom Tromey
@ 2022-02-18 17:02     ` Eli Zaretskii
  2022-03-10 17:27       ` Tom Tromey
  2022-02-18 17:41     ` Andreas Schwab
  1 sibling, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2022-02-18 17:02 UTC (permalink / raw)
  To: Tom Tromey; +Cc: aburgess, gdb-patches

> Date: Fri, 18 Feb 2022 09:44:24 -0700
> From: Tom Tromey via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Tom Tromey <tromey@adacore.com>,
>  Tom Tromey via Gdb-patches <gdb-patches@sourceware.org>
> 
> Let me know what you think of this.
> 
> Tom
> 
> commit bb4117440a91bff97483eaab4d12543c9663de09
> Author: Tom Tromey <tromey@adacore.com>
> Date:   Thu Feb 17 13:43:59 2022 -0700

Thanks.  I find the documentation confusing:

>  @item d
> -Print as integer in signed decimal.
> +Regard the bits of the value as an integer, and print the integer in
> +decimal.

What does it mean to "regard the bits of the value as an integer" when
the actual value is not an integer, but, say, a C 'double'?  I don't
think I understand what to expect from that, by just reading this
description.  Would it be possible to have a more clear text?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Change how "print/x" displays floating-point value
  2022-02-18 16:44   ` Tom Tromey
  2022-02-18 17:02     ` Eli Zaretskii
@ 2022-02-18 17:41     ` Andreas Schwab
  2022-02-19 10:05       ` Andrew Burgess
  2022-03-10 17:30       ` Tom Tromey
  1 sibling, 2 replies; 14+ messages in thread
From: Andreas Schwab @ 2022-02-18 17:41 UTC (permalink / raw)
  To: Tom Tromey via Gdb-patches; +Cc: Andrew Burgess, Tom Tromey

On Feb 18 2022, Tom Tromey via Gdb-patches wrote:

> diff --git a/gdb/NEWS b/gdb/NEWS
> index 9da74e71796..544efa3fe74 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -97,6 +97,12 @@ show style disassembler enabled
>  
>  * Changed commands
>  
> +print
> +  Printing of floating-point values with base-modifying formats like
> +  /x has been changed to display the underlying bytes of the value in
> +  the desired base.  This was GDB's documented behavior, but was never
> +  implemented correctly.

How does that handle padding?

> diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
> index 37632985a07..78a2147017d 100644
> --- a/gdb/testsuite/gdb.base/printcmds.exp
> +++ b/gdb/testsuite/gdb.base/printcmds.exp
> @@ -158,8 +158,14 @@ proc test_float_rejected {} {
>  # 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"
> +
> +    # See PR gdb/16242 for this.
> +    gdb_test "print/d 1.5f" " = 1069547520"
> +    gdb_test "print/u 1.5f" " = 1069547520"
> +    gdb_test "print/x 1.5f" " = 0x3fc00000"
> +    gdb_test "print/t 1.5f" " = 111111110000000000000000000000"
> +    gdb_test "print/o 1.5f" " = 07760000000"

That assumes a particular representation of the float type that depends
on the target.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Change how "print/x" displays floating-point value
  2022-02-18 17:41     ` Andreas Schwab
@ 2022-02-19 10:05       ` Andrew Burgess
  2022-03-10 17:30         ` Tom Tromey
  2022-03-10 17:30       ` Tom Tromey
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2022-02-19 10:05 UTC (permalink / raw)
  To: Andreas Schwab, Tom Tromey via Gdb-patches; +Cc: Tom Tromey

Andreas Schwab <schwab@linux-m68k.org> writes:

> On Feb 18 2022, Tom Tromey via Gdb-patches wrote:
>
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index 9da74e71796..544efa3fe74 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -97,6 +97,12 @@ show style disassembler enabled
>>  
>>  * Changed commands
>>  
>> +print
>> +  Printing of floating-point values with base-modifying formats like
>> +  /x has been changed to display the underlying bytes of the value in
>> +  the desired base.  This was GDB's documented behavior, but was never
>> +  implemented correctly.
>
> How does that handle padding?
>
>> diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
>> index 37632985a07..78a2147017d 100644
>> --- a/gdb/testsuite/gdb.base/printcmds.exp
>> +++ b/gdb/testsuite/gdb.base/printcmds.exp
>> @@ -158,8 +158,14 @@ proc test_float_rejected {} {
>>  # 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"
>> +
>> +    # See PR gdb/16242 for this.
>> +    gdb_test "print/d 1.5f" " = 1069547520"
>> +    gdb_test "print/u 1.5f" " = 1069547520"
>> +    gdb_test "print/x 1.5f" " = 0x3fc00000"
>> +    gdb_test "print/t 1.5f" " = 111111110000000000000000000000"
>> +    gdb_test "print/o 1.5f" " = 07760000000"
>
> That assumes a particular representation of the float type that depends
> on the target.

Maybe we can harden the tests in this case with some logic like
(psuedo-code):

  ## Assuming printcmds.c contains:
  ##  float f_var = 1.5f;

  if { sizeof(int) == sizeof(float) } {
    set expected_value [valueof "p/d *((int *) &f_var)"]
    gdb_test "print/d f_var" "$expected_value"
  }

As I think these two methods of printing are now the same, right?

Thanks,
Andrew


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Change how "print/x" displays floating-point value
  2022-02-18 17:02     ` Eli Zaretskii
@ 2022-03-10 17:27       ` Tom Tromey
  2022-03-10 17:54         ` Eli Zaretskii
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2022-03-10 17:27 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Tom Tromey, aburgess, gdb-patches

>> @item d
>> -Print as integer in signed decimal.
>> +Regard the bits of the value as an integer, and print the integer in
>> +decimal.

Eli> What does it mean to "regard the bits of the value as an integer" when
Eli> the actual value is not an integer, but, say, a C 'double'?  I don't
Eli> think I understand what to expect from that, by just reading this
Eli> description.  Would it be possible to have a more clear text?

Here I just copied some existing text in the manual.  I'm definitely
open to other suggestions.

The meaning of it is to take the bytes that constitute the
floating-point value, then print those as if they constituted an integer
value with the same number of bytes.  That is, it shows the raw bytes of
the floating-point value.

Would it be more clear to say

    Print the underlying bytes of the value as if they were an integer?

Tom

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Change how "print/x" displays floating-point value
  2022-02-18 17:41     ` Andreas Schwab
  2022-02-19 10:05       ` Andrew Burgess
@ 2022-03-10 17:30       ` Tom Tromey
  2022-03-10 18:23         ` Andreas Schwab
  1 sibling, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2022-03-10 17:30 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Tom Tromey via Gdb-patches, Andrew Burgess, Tom Tromey

>>>>> "Andreas" == Andreas Schwab <schwab@linux-m68k.org> writes:

>> +print
>> +  Printing of floating-point values with base-modifying formats like
>> +  /x has been changed to display the underlying bytes of the value in
>> +  the desired base.  This was GDB's documented behavior, but was never
>> +  implemented correctly.

Andreas> How does that handle padding?

I don't know, can you say more?

>> +    # See PR gdb/16242 for this.
>> +    gdb_test "print/d 1.5f" " = 1069547520"
>> +    gdb_test "print/u 1.5f" " = 1069547520"
>> +    gdb_test "print/x 1.5f" " = 0x3fc00000"
>> +    gdb_test "print/t 1.5f" " = 111111110000000000000000000000"
>> +    gdb_test "print/o 1.5f" " = 07760000000"

Andreas> That assumes a particular representation of the float type that depends
Andreas> on the target.

Yeah.  I don't know what targets this will fail for.  I could restrict
it to x86 or skip it on some, if I knew which ones mattered.

Tom

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Change how "print/x" displays floating-point value
  2022-02-19 10:05       ` Andrew Burgess
@ 2022-03-10 17:30         ` Tom Tromey
  0 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2022-03-10 17:30 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: Andreas Schwab, Tom Tromey via Gdb-patches, Tom Tromey

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew>   if { sizeof(int) == sizeof(float) } {
Andrew>     set expected_value [valueof "p/d *((int *) &f_var)"]
Andrew>     gdb_test "print/d f_var" "$expected_value"
Andrew>   }

Andrew> As I think these two methods of printing are now the same, right?

Nice, I'll do this.

Tom

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Change how "print/x" displays floating-point value
  2022-03-10 17:27       ` Tom Tromey
@ 2022-03-10 17:54         ` Eli Zaretskii
  2022-03-10 19:19           ` Tom Tromey
  0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2022-03-10 17:54 UTC (permalink / raw)
  To: Tom Tromey; +Cc: aburgess, gdb-patches

> From: Tom Tromey <tromey@adacore.com>
> Cc: Tom Tromey <tromey@adacore.com>,  aburgess@redhat.com,
>   gdb-patches@sourceware.org
> Date: Thu, 10 Mar 2022 10:27:21 -0700
> 
> The meaning of it is to take the bytes that constitute the
> floating-point value, then print those as if they constituted an integer
> value with the same number of bytes.  That is, it shows the raw bytes of
> the floating-point value.
> 
> Would it be more clear to say
> 
>     Print the underlying bytes of the value as if they were an integer?

Yes.  I think I would even lose the "as if they were an integer" part.

Or maybe something like this:

  Print the binary representation of the value in decimal.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Change how "print/x" displays floating-point value
  2022-03-10 17:30       ` Tom Tromey
@ 2022-03-10 18:23         ` Andreas Schwab
  2022-03-10 20:39           ` Tom Tromey
  0 siblings, 1 reply; 14+ messages in thread
From: Andreas Schwab @ 2022-03-10 18:23 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Tom Tromey via Gdb-patches, Andrew Burgess

On Mär 10 2022, Tom Tromey wrote:

>>>>>> "Andreas" == Andreas Schwab <schwab@linux-m68k.org> writes:
>
>>> +print
>>> +  Printing of floating-point values with base-modifying formats like
>>> +  /x has been changed to display the underlying bytes of the value in
>>> +  the desired base.  This was GDB's documented behavior, but was never
>>> +  implemented correctly.
>
> Andreas> How does that handle padding?
>
> I don't know, can you say more?

It is part of the underlying bytes.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Change how "print/x" displays floating-point value
  2022-03-10 17:54         ` Eli Zaretskii
@ 2022-03-10 19:19           ` Tom Tromey
  0 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2022-03-10 19:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Tom Tromey, aburgess, gdb-patches

Eli> Or maybe something like this:

Eli>   Print the binary representation of the value in decimal.

Ok, I used this.  I'll send the updated patch soon.

Tom

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Change how "print/x" displays floating-point value
  2022-03-10 18:23         ` Andreas Schwab
@ 2022-03-10 20:39           ` Tom Tromey
  2022-03-10 20:53             ` Andreas Schwab
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2022-03-10 20:39 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Tom Tromey, Tom Tromey via Gdb-patches, Andrew Burgess

Andreas> How does that handle padding?

>> I don't know, can you say more?

Andreas> It is part of the underlying bytes.

If you mean if there's a float format that doesn't use all the bits in
the underlying bytes, then they'll just be printed along with everything
else.

Tom

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Change how "print/x" displays floating-point value
  2022-03-10 20:39           ` Tom Tromey
@ 2022-03-10 20:53             ` Andreas Schwab
  0 siblings, 0 replies; 14+ messages in thread
From: Andreas Schwab @ 2022-03-10 20:53 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Tom Tromey via Gdb-patches, Andrew Burgess

On Mär 10 2022, Tom Tromey wrote:

> Andreas> How does that handle padding?
>
>>> I don't know, can you say more?
>
> Andreas> It is part of the underlying bytes.
>
> If you mean if there's a float format that doesn't use all the bits in
> the underlying bytes, then they'll just be printed along with everything
> else.

That's not how all other print formats work.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-03-10 20:53 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-17 21:29 [PATCH] Change how "print/x" displays floating-point value Tom Tromey
2022-02-17 23:17 ` Andrew Burgess
2022-02-18 16:44   ` Tom Tromey
2022-02-18 17:02     ` Eli Zaretskii
2022-03-10 17:27       ` Tom Tromey
2022-03-10 17:54         ` Eli Zaretskii
2022-03-10 19:19           ` Tom Tromey
2022-02-18 17:41     ` Andreas Schwab
2022-02-19 10:05       ` Andrew Burgess
2022-03-10 17:30         ` Tom Tromey
2022-03-10 17:30       ` Tom Tromey
2022-03-10 18:23         ` Andreas Schwab
2022-03-10 20:39           ` Tom Tromey
2022-03-10 20:53             ` Andreas Schwab

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).