[binutils-gdb] Target FP printing: Simplify and fix print_floating

[Ulrich Weigand <uweigand at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=fdf0cbc2b710cb5e01249e18f5a377a55eddc39b

commit fdf0cbc2b710cb5e01249e18f5a377a55eddc39b
Author: Ulrich Weigand <ulrich.weigand@de.ibm.com>
Date:   Tue Oct 24 17:59:22 2017 +0200

    Target FP printing: Simplify and fix print_floating
    
    The print_floating routine currently makes a lot of assumptions about host
    and target floating point formats.  This patch cleans up many of those.
    
    One problem is that print_floating may currently be called with types
    that are not actually floating-point types, and it tries hard to output
    those as floating-point values anyway.  However, there is only one single
    caller of print_floating where this can ever happen: print_scalar_formatted.
    And in fact, it is much simpler to handle the case where the value to be
    printed is not already of floating-point type right there.
    
    So this patch changes print_scalar_formatted to handle the 'f' format
    as follows:
    
    - If the value to be printed is already of floating-point type, just
      call print_floating on it.
    
    - Otherwise, if there is a standard target floating-point type of
      the same size as the value, call print_floating using that type.
    
    - Otherwise, just print the value as if the 'f' format had not been
      specified at all.
    
    This has the overall effect to printing everything the same way as
    the old code did, but is overall a lot simpler.  (Also, it would
    allow us to change the above strategy more easily, if that might
    be a more intuitive user interface.  For example, in the third
    case above, maybe an error would be more appropriate?)
    
    Given that change, print_floating can become much simpler.  In particular,
    we now always have a floating-point format that we can consult.  This
    means we can use the floating-point format to programmatically determine
    the number of digits necessary to print the value.
    
    The current code uses a hard-coded value of 9, 17, or 35 digits.  Note
    that this matches the DECIMAL_DIG values for IEEE-32, IEEE-64, and
    IEEE-128.  (Actually, for IEEE-128 the correct value is 36 -- the 35
    seems to be an oversight.)  The DECIMAL_DIG value is defined to be
    the smallest number so that any number in the target format, when
    printed to this number of digits and then scanned back into a binary
    floating-point number, will result in the original value.
    
    Now that we always have a FP format, we can just compute the DECIMAL_DIG
    value using the formula from the C standard.  This will be correct for
    *all* FP formats, not just the above list, and it will be correct (as
    opposed to current code) if the target formats differ from the host ones.
    
    The patch moves the new logic to a new floatformat_to_string routine
    (analogous to the existing decimal_to_string).  The print_floating
    routine now calls floatformat_to_string or decimal_to_string, making
    the separate print_decimal_floating and generic_val_print_decfloat routines
    unnecessary.
    
    gdb/ChangeLog:
    2017-10-24  Ulrich Weigand  <uweigand@de.ibm.com>
    
    	* doublest.c (floatformat_precision): New routine.
    	(floatformat_to_string): Likewise.
    	* doublest.c (floatformat_to_string): Add prototype.
    
    	* printcmd.c (print_scalar_formatted): Only call print_floating
    	on floating-point types.
    	* valprint.c: Do not include "floatformat.h".
    	(generic_val_print_decfloat): Remove.
    	(generic_val_print): Call generic_val_print_float for both
    	TYPE_CODE_FLT and TYPE_CODE_DECFLOAT.
    	(print_floating): Use floatformat_to_string.  Handle decimal float.
    	(print_decimal_floating): Remove, merge into floatformat_to_string.
    	* value.h (print_decimal_floating): Remove.
    
    	* Makefile.in: Do not build doublest.c with -Wformat-nonliteral.

Diff:
---
 gdb/ChangeLog   |  18 +++++++++
 gdb/Makefile.in |   5 +++
 gdb/doublest.c  |  64 +++++++++++++++++++++++++++++++
 gdb/doublest.h  |   3 ++
 gdb/printcmd.c  |  16 ++++++--
 gdb/valprint.c  | 115 ++++++--------------------------------------------------
 gdb/value.h     |   3 --
 7 files changed, 114 insertions(+), 110 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b44349f..aeb3f99 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,23 @@
 2017-10-24  Ulrich Weigand  <uweigand@de.ibm.com>
 
+	* doublest.c (floatformat_precision): New routine.
+	(floatformat_to_string): Likewise.
+	* doublest.c (floatformat_to_string): Add prototype.
+
+	* printcmd.c (print_scalar_formatted): Only call print_floating
+	on floating-point types.
+	* valprint.c: Do not include "floatformat.h".
+	(generic_val_print_decfloat): Remove.
+	(generic_val_print): Call generic_val_print_float for both
+	TYPE_CODE_FLT and TYPE_CODE_DECFLOAT.
+	(print_floating): Use floatformat_to_string.  Handle decimal float.
+	(print_decimal_floating): Remove, merge into floatformat_to_string.
+	* value.h (print_decimal_floating): Remove.
+
+	* Makefile.in: Do not build doublest.c with -Wformat-nonliteral.
+
+2017-10-24  Ulrich Weigand  <uweigand@de.ibm.com>
+
 	* buildsym.c (end_symtab_get_static_block): Use std::stable_sort.
 
 2017-10-21  Simon Marchi  <simon.marchi@ericsson.com>
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index e21ea55..6cb0970 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -2733,6 +2733,11 @@ printcmd.o: $(srcdir)/printcmd.c
 		$(COMPILE.post) $(srcdir)/printcmd.c
 	$(POSTCOMPILE)
 
+# Same for "doublest.c".
+doublest.o: $(srcdir)/doublest.c
+	$(COMPILE.pre) $(INTERNAL_CFLAGS) $(GDB_WARN_CFLAGS_NO_FORMAT) \
+		$(COMPILE.post) $(srcdir)/doublest.c
+
 # ada-exp.c can appear in srcdir, for releases; or in ., for
 # development builds.
 ADA_EXP_C = `if test -f ada-exp.c; then echo ada-exp.c; else echo $(srcdir)/ada-exp.c; fi`
diff --git a/gdb/doublest.c b/gdb/doublest.c
index 2047087..cafa7d3 100644
--- a/gdb/doublest.c
+++ b/gdb/doublest.c
@@ -680,6 +680,25 @@ floatformat_mantissa (const struct floatformat *fmt,
   return res;
 }
 
+/* Return the precision of the floating point format FMT.  */
+
+static int
+floatformat_precision (const struct floatformat *fmt)
+{
+  /* Assume the precision of and IBM long double is twice the precision
+     of the underlying double.  This matches what GCC does.  */
+  if (fmt->split_half)
+    return 2 * floatformat_precision (fmt->split_half);
+
+  /* Otherwise, the precision is the size of mantissa in bits,
+     including the implicit bit if present.  */
+  int prec = fmt->man_len;
+  if (fmt->intbit == floatformat_intbit_no)
+    prec++;
+
+  return prec;
+}
+
 
 /* Convert TO/FROM target to the hosts DOUBLEST floating-point format.
 
@@ -769,6 +788,51 @@ floatformat_from_doublest (const struct floatformat *fmt,
     convert_doublest_to_floatformat (fmt, in, out);
 }
 
+/* Convert the byte-stream ADDR, interpreted as floating-point format FMT,
+   to a string.  */
+std::string
+floatformat_to_string (const struct floatformat *fmt,
+		       const gdb_byte *in)
+{
+  /* Detect invalid representations.  */
+  if (!floatformat_is_valid (fmt, in))
+    return "<invalid float value>";
+
+  /* Handle NaN and Inf.  */
+  enum float_kind kind = floatformat_classify (fmt, in);
+  if (kind == float_nan)
+    {
+      const char *sign = floatformat_is_negative (fmt, in)? "-" : "";
+      const char *mantissa = floatformat_mantissa (fmt, in);
+      return string_printf ("%snan(0x%s)", sign, mantissa);
+    }
+  else if (kind == float_infinite)
+    {
+      const char *sign = floatformat_is_negative (fmt, in)? "-" : "";
+      return string_printf ("%sinf", sign);
+    }
+
+  /* Print the number using a format string where the precision is set
+     to the DECIMAL_DIG value for the given floating-point format.
+     This value is computed as
+
+		ceil(1 + p * log10(b)),
+
+     where p is the precision of the floating-point format in bits, and
+     b is the base (which is always 2 for the formats we support).  */
+  const double log10_2 = .30102999566398119521;
+  double d_decimal_dig = 1 + floatformat_precision (fmt) * log10_2;
+  int decimal_dig = d_decimal_dig;
+  if (decimal_dig < d_decimal_dig)
+    decimal_dig++;
+
+  std::string host_format
+    = string_printf ("%%.%d" DOUBLEST_PRINT_FORMAT, decimal_dig);
+
+  DOUBLEST doub;
+  floatformat_to_doublest (fmt, in, &doub);
+  return string_printf (host_format.c_str (), doub);
+}
 
 /* Extract a floating-point number of type TYPE from a target-order
    byte-stream at ADDR.  Returns the value as type DOUBLEST.  */
diff --git a/gdb/doublest.h b/gdb/doublest.h
index 74d0572..7fdd0ab 100644
--- a/gdb/doublest.h
+++ b/gdb/doublest.h
@@ -71,6 +71,9 @@ extern enum float_kind floatformat_classify (const struct floatformat *,
 extern const char *floatformat_mantissa (const struct floatformat *,
 					 const bfd_byte *);
 
+extern std::string floatformat_to_string (const struct floatformat *fmt,
+					  const gdb_byte *in);
+
 /* Return the floatformat's total size in host bytes.  */
 
 extern size_t floatformat_totalsize_bytes (const struct floatformat *fmt);
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 77a05e5..b2b7994 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -420,7 +420,18 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
       valaddr = converted_float_bytes.data ();
     }
 
-  switch (options->format)
+  /* Printing a non-float type as 'f' will interpret the data as if it were
+     of a floating-point type of the same length, if that exists.  Otherwise,
+     the data is printed as integer.  */
+  char format = options->format;
+  if (format == 'f' && TYPE_CODE (type) != TYPE_CODE_FLT)
+    {
+      type = float_type_from_length (type);
+      if (TYPE_CODE (type) != TYPE_CODE_FLT)
+        format = 0;
+    }
+
+  switch (format)
     {
     case 'o':
       print_octal_chars (stream, valaddr, len, byte_order);
@@ -440,7 +451,6 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
 	}
       /* FALLTHROUGH */
     case 'f':
-      type = float_type_from_length (type);
       print_floating (valaddr, type, stream);
       break;
 
@@ -478,7 +488,7 @@ print_scalar_formatted (const gdb_byte *valaddr, struct type *type,
       break;
 
     default:
-      error (_("Undefined output format \"%c\"."), options->format);
+      error (_("Undefined output format \"%c\"."), format);
     }
 }
 
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 2dd383f..9ea8d9c 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -27,7 +27,6 @@
 #include "language.h"
 #include "annotate.h"
 #include "valprint.h"
-#include "floatformat.h"
 #include "doublest.h"
 #include "dfp.h"
 #include "extension.h"
@@ -816,7 +815,7 @@ generic_val_print_char (struct type *type, struct type *unresolved_type,
     }
 }
 
-/* generic_val_print helper for TYPE_CODE_FLT.  */
+/* generic_val_print helper for TYPE_CODE_FLT and TYPE_CODE_DECFLOAT.  */
 
 static void
 generic_val_print_float (struct type *type,
@@ -840,29 +839,6 @@ generic_val_print_float (struct type *type,
     }
 }
 
-/* generic_val_print helper for TYPE_CODE_DECFLOAT.  */
-
-static void
-generic_val_print_decfloat (struct type *type,
-			    int embedded_offset, struct ui_file *stream,
-			    struct value *original_value,
-			    const struct value_print_options *options)
-{
-  struct gdbarch *gdbarch = get_type_arch (type);
-  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);
-
-  if (options->format)
-    val_print_scalar_formatted (type, embedded_offset, original_value,
-				options, 0, stream);
-  else
-    {
-      const gdb_byte *valaddr = value_contents_for_printing (original_value);
-
-      print_decimal_floating (valaddr + embedded_offset * unit_size, type,
-			      stream);
-    }
-}
-
 /* generic_val_print helper for TYPE_CODE_COMPLEX.  */
 
 static void
@@ -986,15 +962,11 @@ generic_val_print (struct type *type,
       break;
 
     case TYPE_CODE_FLT:
+    case TYPE_CODE_DECFLOAT:
       generic_val_print_float (type, embedded_offset, stream,
 			       original_value, options);
       break;
 
-    case TYPE_CODE_DECFLOAT:
-      generic_val_print_decfloat (type, embedded_offset, stream,
-				  original_value, options);
-      break;
-
     case TYPE_CODE_VOID:
       fputs_filtered (decorations->void_name, stream);
       break;
@@ -1387,90 +1359,25 @@ longest_to_int (LONGEST arg)
   return (rtnval);
 }
 
-/* Print a floating point value of type TYPE (not always a
-   TYPE_CODE_FLT), pointed to in GDB by VALADDR, on STREAM.  */
+/* Print a floating point value of floating-point type TYPE,
+   pointed to in GDB by VALADDR, on STREAM.  */
 
 void
 print_floating (const gdb_byte *valaddr, struct type *type,
 		struct ui_file *stream)
 {
-  DOUBLEST doub;
-  int inv;
-  const struct floatformat *fmt = NULL;
-  unsigned len = TYPE_LENGTH (type);
-  enum float_kind kind;
-
-  /* If it is a floating-point, check for obvious problems.  */
+  std::string str;
   if (TYPE_CODE (type) == TYPE_CODE_FLT)
-    fmt = floatformat_from_type (type);
-  if (fmt != NULL)
     {
-      kind = floatformat_classify (fmt, valaddr);
-      if (kind == float_nan)
-	{
-	  if (floatformat_is_negative (fmt, valaddr))
-	    fprintf_filtered (stream, "-");
-	  fprintf_filtered (stream, "nan(");
-	  fputs_filtered ("0x", stream);
-	  fputs_filtered (floatformat_mantissa (fmt, valaddr), stream);
-	  fprintf_filtered (stream, ")");
-	  return;
-	}
-      else if (kind == float_infinite)
-	{
-	  if (floatformat_is_negative (fmt, valaddr))
-	    fputs_filtered ("-", stream);
-	  fputs_filtered ("inf", stream);
-	  return;
-	}
+      const struct floatformat *fmt = floatformat_from_type (type);
+      str = floatformat_to_string (fmt, valaddr);
     }
-
-  /* NOTE: cagney/2002-01-15: The TYPE passed into print_floating()
-     isn't necessarily a TYPE_CODE_FLT.  Consequently, unpack_double
-     needs to be used as that takes care of any necessary type
-     conversions.  Such conversions are of course direct to DOUBLEST
-     and disregard any possible target floating point limitations.
-     For instance, a u64 would be converted and displayed exactly on a
-     host with 80 bit DOUBLEST but with loss of information on a host
-     with 64 bit DOUBLEST.  */
-
-  doub = unpack_double (type, valaddr, &inv);
-  if (inv)
+  else
     {
-      fprintf_filtered (stream, "<invalid float value>");
-      return;
+      enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
+      unsigned len = TYPE_LENGTH (type);
+      str = decimal_to_string (valaddr, len, byte_order);
     }
-
-  /* FIXME: kettenis/2001-01-20: The following code makes too much
-     assumptions about the host and target floating point format.  */
-
-  /* NOTE: cagney/2002-02-03: Since the TYPE of what was passed in may
-     not necessarily be a TYPE_CODE_FLT, the below ignores that and
-     instead uses the type's length to determine the precision of the
-     floating-point value being printed.  */
-
-  if (len < sizeof (double))
-      fprintf_filtered (stream, "%.9g", (double) doub);
-  else if (len == sizeof (double))
-      fprintf_filtered (stream, "%.17g", (double) doub);
-  else
-#ifdef PRINTF_HAS_LONG_DOUBLE
-    fprintf_filtered (stream, "%.35Lg", doub);
-#else
-    /* This at least wins with values that are representable as
-       doubles.  */
-    fprintf_filtered (stream, "%.17g", (double) doub);
-#endif
-}
-
-void
-print_decimal_floating (const gdb_byte *valaddr, struct type *type,
-			struct ui_file *stream)
-{
-  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
-  unsigned len = TYPE_LENGTH (type);
-
-  std::string str = decimal_to_string (valaddr, len, byte_order);
   fputs_filtered (str.c_str (), stream);
 }
 
diff --git a/gdb/value.h b/gdb/value.h
index fb6f81f..cfc8cae 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -1065,9 +1065,6 @@ extern void print_longest (struct ui_file *stream, int format,
 extern void print_floating (const gdb_byte *valaddr, struct type *type,
 			    struct ui_file *stream);
 
-extern void print_decimal_floating (const gdb_byte *valaddr, struct type *type,
-				    struct ui_file *stream);
-
 extern void value_print (struct value *val, struct ui_file *stream,
 			 const struct value_print_options *options);