Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: gdb-patches@sourceware.org
Cc: Joel Brobecker <brobecker@adacore.com>
Subject: [RFA] convert_doublest_to_floatformat: handle off-range values.
Date: Thu, 14 Jun 2012 00:01:00 -0000	[thread overview]
Message-ID: <1339632037-5252-1-git-send-email-brobecker@adacore.com> (raw)

Hello,

This is a patch where I would definitely appreciate some feedback
from the floating-point gurus out there.

I initially noticed the problem on x86_64-linux cross AVR, but
I eventually managed to reproduce the problem on x86_64-linux
native. I won't reproduce on x86-linux hosts, for reasons that
are going to be apparent later.

Here is a transcript showing the source of the problem:

    (gdb) p 1.6e309
    $5 = -4.9509536758121243e-308
    (gdb) p 1.6e-309
    $6 = -5.1707209714097603e+307

What happens is that GDB is trying to convert the value it read
(as a host "long double") into a target "double" value. The routine
performing the conversion does not realize that 1.6e309 is just
too large to fit in a double. Similarly, it does not notice that
1.6e-309 is too small to be represented.

This patch enhances convert_doublest_to_floatformat to both handle
floats that are too small and too large.

The hesitation I have is that I am wondering whether the checks
I am making might be making a few too many assumptions (maybe
IEEE-centric?). The goal is to compute e_min and e_max, where
e_min and e_max are the minimum and maximum acceptable values for
the exponent. In IEEE floating-point values, the exponent is
encoded as a biased unsigned integer.

What if the exponent was encoded differently? For instance,
I am wondering how things would work if the exponent was represented
using a two's complement representation.  My patch as it is would
probably break. But, on the other hand, I don't see how non-IEEE
representations could be handled with the current struct floatformat
description we have.

gdb/ChangeLog:

        * doublest.c (convert_doublest_to_floatformat): If the exponent
        is too small, treat the value as zero.  If the exponent is too
        large, treat the value as infinity.

Tested on avr with AdaCore's testsuite. Tested on x86_64-linux.
No regression. Does it look OK for now?

As for the testcase, I not sure what to do, because reproducing
the problem depends on the host and target. I think that the simplest
is to choose values that should trigger the problem on most 64bit
targets, assuming that "double" is a 64bit IEEE floating point,
and that "long double" has a bigger exponent size. The trick is
to find an exponent that is large enough to exceed a double's
capacity, while at the same not being so big that it exceeds the
capacity of a long double. Hence the choice of 309 in the example
at the start of this message, which once the decimal representation
is represented in binary, becomes an exponent slightly greater than
1023, which is the e_max for doubles on x86_64-linux.

---
 gdb/doublest.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/gdb/doublest.c b/gdb/doublest.c
index c8c9e05..f683002 100644
--- a/gdb/doublest.c
+++ b/gdb/doublest.c
@@ -472,6 +472,28 @@ convert_doublest_to_floatformat (CONST struct floatformat *fmt,
   mant = frexp (dfrom, &exponent);
 #endif
 
+  if (exponent + fmt->exp_bias <= 0)
+    {
+      /* The value is too small to be expressed in the destination
+	 type (not enough bits in the exponent.  Treat as 0.  */
+      put_field (uto, order, fmt->totalsize, fmt->exp_start,
+		 fmt->exp_len, 0);
+      put_field (uto, order, fmt->totalsize, fmt->man_start,
+		 fmt->man_len, 0);
+      goto finalize_byteorder;
+    }
+
+  if (exponent + fmt->exp_bias >= (1 << fmt->exp_len) - 1)
+    {
+      /* The value is too large to fit into the destination.
+	 Treat as infinity.  */
+      put_field (uto, order, fmt->totalsize, fmt->exp_start,
+		 fmt->exp_len, fmt->exp_nan);
+      put_field (uto, order, fmt->totalsize, fmt->man_start,
+		 fmt->man_len, 0);
+      goto finalize_byteorder;
+    }
+
   put_field (uto, order, fmt->totalsize, fmt->exp_start, fmt->exp_len,
 	     exponent + fmt->exp_bias - 1);
 
-- 
1.7.1


             reply	other threads:[~2012-06-14  0:01 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-14  0:01 Joel Brobecker [this message]
2012-06-14  0:51 ` Joel Brobecker
2012-07-25 18:31 ` Joel Brobecker

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=1339632037-5252-1-git-send-email-brobecker@adacore.com \
    --to=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    /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