[ver,3] PowerPC: fix _Float128 type output string

Message ID fc915fe8e911dac1f96cfc738a0af6c802d7ad86.camel@us.ibm.com
State New
Headers
Series [ver,3] PowerPC: fix _Float128 type output string |

Commit Message

Carl Love April 17, 2023, 3:45 p.m. UTC
  GDB maintainers, Tom:

The comment in gdb/gdbarch_components.py has been updated to be more
clear.  Hopefully that addresses the comments from Tom.

The function linux_dwarf2_omit_typedef_p has been made static per the
comments from Tom.

There is some confusion on how the patch works.  There have been
several emails discussing specifically what the patch does and why. 
Hopefully the discussion has clarified what the GCC hack does and how
this patch "fixes" the GCC hack to ensure the expected name is printed
by the ptype command.

Please let me know if the updated patch is acceptable.  Thanks.

                     Carl 


------------------------------------
PowerPC: fix _Float128 type output string

PowerPC supports two 128-bit floating point formats, the IBM long double
and IEEE 128-bit float.  The issue is the DWARF information does not
distinguish between the two.  There have been proposals of how to extend
the DWARF information as discussed in

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104194

but has not been fully implemented.

GCC introduced the _Float128 internal type as a work around for the issue.
The workaround is not transparent to GDB.  The internal _Float128 type
name is printed rather then the user specified long double type.  This
patch adds a new gdbarch method to allow PowerPC to detect the GCC
workaround.  The workaround checks for "_Float128" name when reading the
base typedef from the die_info.  If the workaround is detected, the type
and format fields from the _Float128 typedef are copied to the long
double typedef.  The same is done for the complex long double typedef.

This patch fixes 74 regression test failures in
gdb.base/whatis-ptype-typedefs.exp on PowerPC with IEEE float 128 as the
default on GCC.  It fixes one regression test failure in
gdb.base/complex-parts.exp.

The patch has been tested on Power 10 where GCC defaults to IEEE Float
128-bit and on Power 10 where GCC defaults to the IBM 128-bit float.  The
patch as also been tested on X86-64 with no new regression failures.
---
 gdb/arch-utils.c          |  7 +++++++
 gdb/arch-utils.h          |  5 +++++
 gdb/dwarf2/read.c         | 23 +++++++++++++++++++----
 gdb/gdbarch-gen.h         |  8 ++++++++
 gdb/gdbarch.c             | 22 ++++++++++++++++++++++
 gdb/gdbarch_components.py | 23 +++++++++++++++++++++++
 gdb/ppc-linux-tdep.c      | 36 ++++++++++++++++++++++++++++++++++++
 7 files changed, 120 insertions(+), 4 deletions(-)
  

Comments

Ulrich Weigand April 18, 2023, 10:18 a.m. UTC | #1
Carl Love <cel@us.ibm.com> wrote:

>Please let me know if the updated patch is acceptable.  Thanks.

This version is OK.

Thanks,
Ulrich
  

Patch

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index e3af9ce2dbc..997a292e9ef 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -1098,6 +1098,13 @@  default_get_return_buf_addr (struct type *val_type, frame_info_ptr cur_frame)
   return 0;
 }
 
+bool
+default_dwarf2_omit_typedef_p (struct type *target_type, const char *producer,
+			       const char *name)
+{
+  return false;
+}
+
 /* Non-zero if we want to trace architecture code.  */
 
 #ifndef GDBARCH_DEBUG
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index 56690f0fd43..fc0c0b16793 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -309,6 +309,11 @@  extern void default_read_core_file_mappings
 extern CORE_ADDR default_get_return_buf_addr (struct type *val_typegdbarch,
 					      frame_info_ptr cur_frame);
 
+/* Default implementation of gdbaarch default_dwarf2_omit_typedef_p method.  */
+extern bool default_dwarf2_omit_typedef_p (struct type *target_type,
+					   const char *producer,
+					   const char *name);
+
 extern enum return_value_convention default_gdbarch_return_value
      (struct gdbarch *gdbarch, struct value *function, struct type *valtype,
       struct regcache *regcache, struct value **read_value,
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index c9208a097bf..19654f408ee 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14702,14 +14702,29 @@  static struct type *
 read_typedef (struct die_info *die, struct dwarf2_cu *cu)
 {
   struct objfile *objfile = cu->per_objfile->objfile;
-  const char *name = NULL;
-  struct type *this_type, *target_type;
+  const char *name = dwarf2_full_name (NULL, die, cu);
+  struct type *this_type;
+  struct gdbarch *gdbarch = objfile->arch ();
+  struct type *target_type = die_type (die, cu);
+
+  if (gdbarch_dwarf2_omit_typedef_p (gdbarch, target_type, cu->producer, name))
+    {
+      /* The long double is defined as a base type in C.  GCC creates a long
+	 double typedef with target-type _Float128 for the long double to
+	 identify it as the IEEE Float128 value.  This is a GCC hack since the
+	 DWARF doesn't distinquish between the IBM long double and IEEE
+	 128-bit float.	 Replace the GCC workaround for the long double
+	 typedef with the actual type information copied from the target-type
+	 with the correct long double base type name.  */
+      this_type = copy_type (target_type);
+      this_type->set_name (name);
+      set_die_type (die, this_type, cu);
+      return this_type;
+    }
 
-  name = dwarf2_full_name (NULL, die, cu);
   this_type = type_allocator (objfile).new_type (TYPE_CODE_TYPEDEF, 0, name);
   this_type->set_target_is_stub (true);
   set_die_type (die, this_type, cu);
-  target_type = die_type (die, cu);
   if (target_type != this_type)
     this_type->set_target_type (target_type);
   else
diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
index a3fc0b9272b..3d8aafd5ea6 100644
--- a/gdb/gdbarch-gen.h
+++ b/gdb/gdbarch-gen.h
@@ -468,6 +468,14 @@  typedef CORE_ADDR (gdbarch_get_return_buf_addr_ftype) (struct type *val_type, fr
 extern CORE_ADDR gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type *val_type, frame_info_ptr cur_frame);
 extern void set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, gdbarch_get_return_buf_addr_ftype *get_return_buf_addr);
 
+/* Return true if the typedef record needs to be replaced.".
+
+   Return 0 by default */
+
+typedef bool (gdbarch_dwarf2_omit_typedef_p_ftype) (struct type *target_type, const char *producer, const char *name);
+extern bool gdbarch_dwarf2_omit_typedef_p (struct gdbarch *gdbarch, struct type *target_type, const char *producer, const char *name);
+extern void set_gdbarch_dwarf2_omit_typedef_p (struct gdbarch *gdbarch, gdbarch_dwarf2_omit_typedef_p_ftype *dwarf2_omit_typedef_p);
+
 /* Return true if the return value of function is stored in the first hidden
    parameter.  In theory, this feature should be language-dependent, specified
    by language and its ABI, such as C++.  Unfortunately, compiler may
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index b676e346fd0..00e7191653a 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -114,6 +114,7 @@  struct gdbarch
   gdbarch_return_value_ftype *return_value = nullptr;
   gdbarch_return_value_as_value_ftype *return_value_as_value = default_gdbarch_return_value;
   gdbarch_get_return_buf_addr_ftype *get_return_buf_addr = default_get_return_buf_addr;
+  gdbarch_dwarf2_omit_typedef_p_ftype *dwarf2_omit_typedef_p = default_dwarf2_omit_typedef_p;
   gdbarch_return_in_first_hidden_param_p_ftype *return_in_first_hidden_param_p = default_return_in_first_hidden_param_p;
   gdbarch_skip_prologue_ftype *skip_prologue = nullptr;
   gdbarch_skip_main_prologue_ftype *skip_main_prologue = nullptr;
@@ -370,6 +371,7 @@  verify_gdbarch (struct gdbarch *gdbarch)
   if ((gdbarch->return_value_as_value == default_gdbarch_return_value) == (gdbarch->return_value == nullptr))
     log.puts ("\n\treturn_value_as_value");
   /* Skip verify of get_return_buf_addr, invalid_p == 0 */
+  /* Skip verify of dwarf2_omit_typedef_p, invalid_p == 0 */
   /* Skip verify of return_in_first_hidden_param_p, invalid_p == 0 */
   if (gdbarch->skip_prologue == 0)
     log.puts ("\n\tskip_prologue");
@@ -788,6 +790,9 @@  gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
   gdb_printf (file,
 	      "gdbarch_dump: get_return_buf_addr = <%s>\n",
 	      host_address_to_string (gdbarch->get_return_buf_addr));
+  gdb_printf (file,
+	      "gdbarch_dump: dwarf2_omit_typedef_p = <%s>\n",
+	      host_address_to_string (gdbarch->dwarf2_omit_typedef_p));
   gdb_printf (file,
 	      "gdbarch_dump: return_in_first_hidden_param_p = <%s>\n",
 	      host_address_to_string (gdbarch->return_in_first_hidden_param_p));
@@ -2617,6 +2622,23 @@  set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch,
   gdbarch->get_return_buf_addr = get_return_buf_addr;
 }
 
+bool
+gdbarch_dwarf2_omit_typedef_p (struct gdbarch *gdbarch, struct type *target_type, const char *producer, const char *name)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->dwarf2_omit_typedef_p != NULL);
+  if (gdbarch_debug >= 2)
+    gdb_printf (gdb_stdlog, "gdbarch_dwarf2_omit_typedef_p called\n");
+  return gdbarch->dwarf2_omit_typedef_p (target_type, producer, name);
+}
+
+void
+set_gdbarch_dwarf2_omit_typedef_p (struct gdbarch *gdbarch,
+				   gdbarch_dwarf2_omit_typedef_p_ftype dwarf2_omit_typedef_p)
+{
+  gdbarch->dwarf2_omit_typedef_p = dwarf2_omit_typedef_p;
+}
+
 int
 gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch, struct type *type)
 {
diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
index 2b1a2b4f602..59c895d3645 100644
--- a/gdb/gdbarch_components.py
+++ b/gdb/gdbarch_components.py
@@ -901,6 +901,29 @@  May return 0 when unable to determine that address.""",
     invalid=False,
 )
 
+
+# The DWARF info currently does not distinquish between IEEE 128-bit floating
+# point values and the IBM 128-bit floating point format.  GCC has an internal
+# hack to identify the IEEE 128-bit floating point value.  The long double is a
+# defined base type in C.  The GCC hack uses a typedef for long double to
+# reference_Float128 base to identify the long double as and IEEE 128-bit
+# value.  The following method is used to "fix" the long double type to be a
+# base type with the IEEE float format info from the _Float128 basetype and
+# the long double name.  With the fix, the proper name is printed for the
+# GDB typedef command.
+Function(
+    comment="""
+Return true if the typedef record needs to be replaced.".
+
+Return 0 by default""",
+    type="bool",
+    name="dwarf2_omit_typedef_p",
+    params=[("struct type *", "target_type"), ("const char *", "producer"),
+            ("const char *", "name")],
+    predefault="default_dwarf2_omit_typedef_p",
+    invalid=False,
+)
+
 Method(
     comment="""
 Return true if the return value of function is stored in the first hidden
diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index fcddb2008a0..784dafa59db 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -62,6 +62,7 @@ 
 #include "user-regs.h"
 #include <ctype.h>
 #include "elf-bfd.h"
+#include "producer.h"
 
 #include "features/rs6000/powerpc-32l.c"
 #include "features/rs6000/powerpc-altivec32l.c"
@@ -2006,6 +2007,38 @@  ppc_floatformat_for_type (struct gdbarch *gdbarch,
   return default_floatformat_for_type (gdbarch, name, len);
 }
 
+static bool
+linux_dwarf2_omit_typedef_p (struct type *target_type,
+			     const char *producer, const char *name)
+{
+  int gcc_major, gcc_minor;
+
+  if (producer_is_gcc (producer, &gcc_major, &gcc_minor))
+    {
+      if ((target_type->code () == TYPE_CODE_FLT
+	   || target_type->code () == TYPE_CODE_COMPLEX)
+	  && (strcmp (name, "long double") == 0
+	      || strcmp (name, "complex long double") == 0))
+	{
+	  /* IEEE 128-bit floating point and IBM long double are two
+	     encodings for 128-bit values.  The DWARF debug data can't
+	     distinguish between them.  See bugzilla:
+	     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104194
+
+	     A GCC hack was introduced to still allow the debugger to identify
+	     the case where "long double" uses the IEEE 128-bit floating point
+	     format: GCC will emit a bogus DWARF type record pretending that
+	     "long double" is a typedef alias for the _Float128 type.
+
+	     This hack should not be visible to the GDB user, so we replace
+	     this bogus typedef by a normal floating-point type, copying the
+	     format information from the target type of the bogus typedef.  */
+	  return true;
+	}
+    }
+  return false;
+}
+
 /* Specify the powerpc64le target triplet.
    This can be variations of
 	ppc64le-{distro}-linux-gcc
@@ -2083,6 +2116,9 @@  ppc_linux_init_abi (struct gdbarch_info info,
   /* Support for floating-point data type variants.  */
   set_gdbarch_floatformat_for_type (gdbarch, ppc_floatformat_for_type);
 
+  /* Support for replacing typedef record.  */
+  set_gdbarch_dwarf2_omit_typedef_p (gdbarch, linux_dwarf2_omit_typedef_p);
+
   /* Handle inferior calls during interrupted system calls.  */
   set_gdbarch_write_pc (gdbarch, ppc_linux_write_pc);