[PATCH] PowerPC: fix _Float128 type output string

[PATCH] PowerPC: fix _Float128 type output string

[Carl Love]

GDB maintainers:

PowerPC supports the IBM and IEEE 128-bit floating point formats. 
However the DWARF information does not distinguish between the two. 
The PowerPC GCC support created a workaround to identify the two
formats.  Unfortunately, the workaround is not transparent to GDB when
printing the name of the typedef.  The issue occurs on systems where
GCC is now set to generate the IEEE 128-bit format by default.

This patch fixes the printing of the typedef.  It fixes 74 test
failures in gdb.base/whatis-ptype-typedefs.exp and 1 failure in
gdb.base/complex-parts.exp.

The patch has been tested on a Power 10 platform where GCC generates
the IEEE 128-bit floats by default and on a Power 10 platform where the
IBM 128-bit floating point format is used by default.  Additionally,
the patch has been tested on X86-64.  The patch passes the regression
runs with no new regressions.

Please let me know if the patch is acceptable for mainline.  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         | 16 ++++++++++++----
 gdb/gdbarch-gen.h         |  8 ++++++++
 gdb/gdbarch.c             | 22 ++++++++++++++++++++++
 gdb/gdbarch_components.py | 19 +++++++++++++++++++
 gdb/ppc-linux-tdep.c      | 38 +++++++++++++++++++++++++++++++++++++-
 gdb/ppc-tdep.h            |  3 +++
 8 files changed, 113 insertions(+), 5 deletions(-)

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..fa319e346c0 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14702,14 +14702,22 @@ 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))
+    {
+      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..7a64d9e006b 100644
--- a/gdb/gdbarch_components.py
+++ b/gdb/gdbarch_components.py
@@ -901,6 +901,25 @@ 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 that uses the _Float128 base typdef for IEEE 128-bit float values.  The
+# following method is used to "fix" the long double typedef so the _Float128
+# name is not printed.
+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..830a60fd49b 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -1,4 +1,4 @@
-/* Target-dependent code for GDB, the GNU debugger.
+* Target-dependent code for GDB, the GNU debugger.
 
    Copyright (C) 1986-2023 Free Software Foundation, Inc.
 
@@ -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);
 }
 
+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);
 
diff --git a/gdb/ppc-tdep.h b/gdb/ppc-tdep.h
index db4e53205a6..f078b3d45ab 100644
--- a/gdb/ppc-tdep.h
+++ b/gdb/ppc-tdep.h
@@ -176,6 +176,9 @@ extern void ppc_collect_vsxregset (const struct regset *regset,
 				  int regnum, void *vsxregs, size_t len);
 
 extern CORE_ADDR ppc64_sysv_get_return_buf_addr (type*, frame_info_ptr);
+extern bool linux_dwarf2_omit_typedef_p (struct type *target_type,
+					 const char *producer,
+					 const char *name);
 
 /* Private data that this module attaches to struct gdbarch.  */
 
-- 
2.37.2

Re: [PATCH] PowerPC: fix _Float128 type output string

[Carl Love]

GDB maintainers:

Update, fix a typo in the patch.  In file gdb/ppc-linux-tdep.c  I
accidentally, deleted the / at the beginning of the header comment.

                 Carl
------------------------------------

On Wed, 2023-04-05 at 08:28 -0700, Carl Love wrote:
> GDB maintainers:
> 
> PowerPC supports the IBM and IEEE 128-bit floating point formats. 
> However the DWARF information does not distinguish between the two. 
> The PowerPC GCC support created a workaround to identify the two
> formats.  Unfortunately, the workaround is not transparent to GDB
> when
> printing the name of the typedef.  The issue occurs on systems where
> GCC is now set to generate the IEEE 128-bit format by default.
> 
> This patch fixes the printing of the typedef.  It fixes 74 test
> failures in gdb.base/whatis-ptype-typedefs.exp and 1 failure in
> gdb.base/complex-parts.exp.
> 
> The patch has been tested on a Power 10 platform where GCC generates
> the IEEE 128-bit floats by default and on a Power 10 platform where
> the
> IBM 128-bit floating point format is used by default.  Additionally,
> the patch has been tested on X86-64.  The patch passes the regression
> runs with no new regressions.
> 
> Please let me know if the patch is acceptable for mainline.  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         | 16 ++++++++++++----
 gdb/gdbarch-gen.h         |  8 ++++++++
 gdb/gdbarch.c             | 22 ++++++++++++++++++++++
 gdb/gdbarch_components.py | 19 +++++++++++++++++++
 gdb/ppc-linux-tdep.c      | 36 ++++++++++++++++++++++++++++++++++++
 gdb/ppc-tdep.h            |  3 +++
 8 files changed, 112 insertions(+), 4 deletions(-)

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..fa319e346c0 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14702,14 +14702,22 @@ 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))
+    {
+      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..7a64d9e006b 100644
--- a/gdb/gdbarch_components.py
+++ b/gdb/gdbarch_components.py
@@ -901,6 +901,25 @@ 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 that uses the _Float128 base typdef for IEEE 128-bit float values.  The
+# following method is used to "fix" the long double typedef so the _Float128
+# name is not printed.
+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..7f5f97b6510 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);
 }
 
+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);
 
diff --git a/gdb/ppc-tdep.h b/gdb/ppc-tdep.h
index db4e53205a6..f078b3d45ab 100644
--- a/gdb/ppc-tdep.h
+++ b/gdb/ppc-tdep.h
@@ -176,6 +176,9 @@ extern void ppc_collect_vsxregset (const struct regset *regset,
 				  int regnum, void *vsxregs, size_t len);
 
 extern CORE_ADDR ppc64_sysv_get_return_buf_addr (type*, frame_info_ptr);
+extern bool linux_dwarf2_omit_typedef_p (struct type *target_type,
+					 const char *producer,
+					 const char *name);
 
 /* Private data that this module attaches to struct gdbarch.  */
 
-- 
2.37.2

Re: [PATCH] PowerPC: fix _Float128 type output string

[Kevin Buettner]

Hi Carl,

On Wed, 05 Apr 2023 13:18:26 -0700
Carl Love via Gdb-patches <gdb-patches@sourceware.org> wrote:

> 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 approach sounds reasonable to me.

One nit though...

> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index c9208a097bf..fa319e346c0 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -14702,14 +14702,22 @@ 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))
> +    {
> +      this_type = copy_type (target_type);
> +      this_type->set_name (name);
> +      set_die_type (die, this_type, cu);
> +      return this_type;
> +    }

I'd like to see a comment before the if statement that you added above
which explains what's going on. 
>  
> -  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

RE: [PATCH] PowerPC: fix _Float128 type output string

[Carl Love]

On Fri, 2023-04-07 at 14:51 -0700, Kevin Buettner wrote:
> Hi Carl,
> 
> On Wed, 05 Apr 2023 13:18:26 -0700
> Carl Love via Gdb-patches <gdb-patches@sourceware.org> wrote:
> 
> > 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 approach sounds reasonable to me.
> 
> One nit though...
> 
> > diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> > index c9208a097bf..fa319e346c0 100644
> > --- a/gdb/dwarf2/read.c
> > +++ b/gdb/dwarf2/read.c
> > @@ -14702,14 +14702,22 @@ 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))
> > +    {
> > +      this_type = copy_type (target_type);
> > +      this_type->set_name (name);
> > +      set_die_type (die, this_type, cu);
> > +      return this_type;
> > +    }
> 
> I'd like to see a comment before the if statement that you added
> above
> which explains what's going on. 

OK, I added the following comment to version 2 of tha patch:

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

Hopefully that addresses you concern.

Thanks.

                            Carl 

Re: [PATCH ver 2] PowerPC: fix _Float128 type output string

[Carl Love]

Kevin, GDB maintainers:

I have updated the patch with an additional comment in function
read_type to explain what the if statement is doing per Kevin's
feedback.

Please let me know if the patch is acceptable for mainline.  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 | 19 +++++++++++++++++++
 gdb/ppc-linux-tdep.c      | 36 ++++++++++++++++++++++++++++++++++++
 gdb/ppc-tdep.h            |  3 +++
 8 files changed, 119 insertions(+), 4 deletions(-)

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..7a64d9e006b 100644
--- a/gdb/gdbarch_components.py
+++ b/gdb/gdbarch_components.py
@@ -901,6 +901,25 @@ 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 that uses the _Float128 base typdef for IEEE 128-bit float values.  The
+# following method is used to "fix" the long double typedef so the _Float128
+# name is not printed.
+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..7f5f97b6510 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);
 }
 
+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);
 
diff --git a/gdb/ppc-tdep.h b/gdb/ppc-tdep.h
index db4e53205a6..f078b3d45ab 100644
--- a/gdb/ppc-tdep.h
+++ b/gdb/ppc-tdep.h
@@ -176,6 +176,9 @@ extern void ppc_collect_vsxregset (const struct regset *regset,
 				  int regnum, void *vsxregs, size_t len);
 
 extern CORE_ADDR ppc64_sysv_get_return_buf_addr (type*, frame_info_ptr);
+extern bool linux_dwarf2_omit_typedef_p (struct type *target_type,
+					 const char *producer,
+					 const char *name);
 
 /* Private data that this module attaches to struct gdbarch.  */
 
-- 
2.37.2

Re: [PATCH ver 2] PowerPC: fix _Float128 type output string

[Carl Love]

Kevin, GDB maintainers:

I have updated the patch with an additional comment in function
read_type to explain what the if statement is doing per Kevin's
feedback.

Please let me know if the patch is acceptable for mainline.  Thanks.

                         Carl

P.S. I am resending this as the first message bounced on Kevin's
address.

-----------------------------------------------------
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 | 19 +++++++++++++++++++
 gdb/ppc-linux-tdep.c      | 36 ++++++++++++++++++++++++++++++++++++
 gdb/ppc-tdep.h            |  3 +++
 8 files changed, 119 insertions(+), 4 deletions(-)

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..7a64d9e006b 100644
--- a/gdb/gdbarch_components.py
+++ b/gdb/gdbarch_components.py
@@ -901,6 +901,25 @@ 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 that uses the _Float128 base typdef for IEEE 128-bit float values.  The
+# following method is used to "fix" the long double typedef so the _Float128
+# name is not printed.
+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..7f5f97b6510 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);
 }
 
+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);
 
diff --git a/gdb/ppc-tdep.h b/gdb/ppc-tdep.h
index db4e53205a6..f078b3d45ab 100644
--- a/gdb/ppc-tdep.h
+++ b/gdb/ppc-tdep.h
@@ -176,6 +176,9 @@ extern void ppc_collect_vsxregset (const struct regset *regset,
 				  int regnum, void *vsxregs, size_t len);
 
 extern CORE_ADDR ppc64_sysv_get_return_buf_addr (type*, frame_info_ptr);
+extern bool linux_dwarf2_omit_typedef_p (struct type *target_type,
+					 const char *producer,
+					 const char *name);
 
 /* Private data that this module attaches to struct gdbarch.  */
 
-- 
2.37.2

Re: [PATCH ver 2] PowerPC: fix _Float128 type output string

[Tom Tromey]

>>>>> "Carl" == Carl Love via Gdb-patches <gdb-patches@sourceware.org> writes:

Carl> PowerPC supports two 128-bit floating point formats, the IBM long double
Carl> and IEEE 128-bit float.  The issue is the DWARF information does not
Carl> distinguish between the two.  There have been proposals of how to extend
Carl> the DWARF information as discussed in
Carl> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104194
Carl> but has not been fully implemented.

Could it be?  I didn't read the issue but it's often better to put in
the effort to fix the problem in the compiler.  Normally once these
workarounds go into gdb, they can never be removed.

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

I don't really understand how this patch works.

It took me a while to understand that maybe the issue is that the
association between a gdb type and a float format is done by name and
size, and because this is a typedef, the association is done instead by
the underlying type -- which is then wrong.

However, doesn't copy_type also copy the TYPE_FLOATFORMAT field?
So where would this get reset?

Or maybe this isn't the problem at all, but then I don't understand what
it would be.

Carl> +# The DWARF info currently does not distinquish between IEEE 128-bit floating
Carl> +# point values and the IBM 128-bit floating point format.  GCC has an internal
Carl> +# hack that uses the _Float128 base typdef for IEEE 128-bit float values.  The
Carl> +# following method is used to "fix" the long double typedef so the _Float128
Carl> +# name is not printed.
Carl> +Function(
Carl> +    comment="""
Carl> +Return true if the typedef record needs to be replaced.".
Carl> +
Carl> +Return 0 by default""",

I think the comment field could be reworded to be more clear.
I guess the idea is that some typedefs are replaced by their target
type, but given the typedef's name.

Carl> +bool
Carl> +linux_dwarf2_omit_typedef_p (struct type *target_type,
Carl> +			     const char *producer, const char *name)

I think this can be 'static'.

Tom

RE: [PATCH ver 2] PowerPC: fix _Float128 type output string

[Carl Love]

On Thu, 2023-04-13 at 08:18 -0600, Tom Tromey wrote:
> > > > > > "Carl" == Carl Love via Gdb-patches <
> > > > > > gdb-patches@sourceware.org> writes:
> 
> Carl> PowerPC supports two 128-bit floating point formats, the IBM
> long double
> Carl> and IEEE 128-bit float.  The issue is the DWARF information
> does not
> Carl> distinguish between the two.  There have been proposals of how
> to extend
> Carl> the DWARF information as discussed in
> Carl> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104194
>  
> Carl> but has not been fully implemented.
> 
> Could it be?  I didn't read the issue but it's often better to put in
> the effort to fix the problem in the compiler.  Normally once these
> workarounds go into gdb, they can never be removed.

Yup, that is the problem right now with GCC.  Even if we could get it
fixed in GCC today, the GCC hack is already in released distro
versions.  GDB will have to deal with it for sometime after it gets
fixed in GCC.  This issue was addressed in the GCC community several
years ago from my understanding.  Bugs have been filed with regards to
the fact that the GCC hack is not transparent to GDB, but there doesn't
seem to be much traction to address the issue yet.
> 
> Carl> This patch fixes 74 regression test failures in
> Carl> gdb.base/whatis-ptype-typedefs.exp on PowerPC with IEEE float
> 128 as the
> Carl> default on GCC.  It fixes one regression test failure in
> Carl> gdb.base/complex-parts.exp.
> 
> I don't really understand how this patch works.
> 
> It took me a while to understand that maybe the issue is that the
> association between a gdb type and a float format is done by name and
> size, and because this is a typedef, the association is done instead
> by
> the underlying type -- which is then wrong.
> 
> However, doesn't copy_type also copy the TYPE_FLOATFORMAT field?
> So where would this get reset?
> 
> Or maybe this isn't the problem at all, but then I don't understand
> what
> it would be.

The following is from the private discussion I had with Ulrich while
developing the patch.  Perhaps it will help,

the GCC-generated dwarf info including the hack provides
the following two type records:

1) name: _Float128
   type: base floating-point type (TYPE_CODE_FLT)
     size: 16
     format: floatformats_ieee_quad

and

2) name: "long double"
   type: typedef (TYPE_CODE_TYPEDEF)
     target-type: _Float128 (i.e. type 1) above)

What the patch does is to keep 1) as-is, and replace 2) by

2') name: "long double"
    type: base floating-point type (TYPE_CODE_FLT)
      size: 16
      format: floatformats_ieee_quad

where the name is taken from 2), and the rest of the
record is taken from 1). 

- what the actual problem is, namely the presence of a record like
  2) above -which really cannot happen normally as "long double"
  is defined to be a base type according to the C language, so it
  can normally never be a typedef;
- and what the patch does, namely replacing that weird typedef
  with a "normal" type definition for "long double", where the
  actual type information is taken/copied from target type of
  the weird typedef.


> 
> Carl> +# The DWARF info currently does not distinquish between IEEE
> 128-bit floating
> Carl> +# point values and the IBM 128-bit floating point format.  GCC
> has an internal
> Carl> +# hack that uses the _Float128 base typdef for IEEE 128-bit
> float values.  The
> Carl> +# following method is used to "fix" the long double typedef so
> the _Float128
> Carl> +# name is not printed.
> Carl> +Function(
> Carl> +    comment="""
> Carl> +Return true if the typedef record needs to be replaced.".
> Carl> +
> Carl> +Return 0 by default""",
> 
> I think the comment field could be reworded to be more clear.
> I guess the idea is that some typedefs are replaced by their target
> type, but given the typedef's name.

The patch only replaces the typedef with information from the "target
type", which is a base type, when GCC has added the typedef to identify
the IEEE 128-bit floating point value.  That is the GCC hack so we can
identify an IEEE 128-bit value since there is no way to specify that in
the DWARY ifno at this time.

I will see if I can make that clearer in the comment. 
> 
> Carl> +bool
> Carl> +linux_dwarf2_omit_typedef_p (struct type *target_type,
> Carl> +			     const char *producer, const char
> *name)
> 
> I think this can be 'static'.

OK, I will take a look at that change.

Thanks for the input.

                           Carl 

RE: [PATCH ver 2] PowerPC: fix _Float128 type output string

[Carl Love]

Tom:

On Thu, 2023-04-13 at 09:13 -0700, Carl Love wrote:
> > Carl> +bool
> > Carl> +linux_dwarf2_omit_typedef_p (struct type *target_type,
> > Carl> +                            const char *producer, const char
> > *name)
> > 
> > I think this can be 'static'.
> 
> OK, I will take a look at that change.

The function is declared extern in gdb/arch-utils.h b/gdb/arch-utils.h. 
Thus it can not be declared static as well.

                    Carl 

Re: [PATCH ver 2] PowerPC: fix _Float128 type output string

[Tom Tromey]

>>>>> "Carl" == Carl Love <cel@us.ibm.com> writes:

Carl> The function is declared extern in gdb/arch-utils.h b/gdb/arch-utils.h. 
Carl> Thus it can not be declared static as well.

Surely only the default implementation has to be declared there?
IIUC this particular one is only used by ppc.

Tom

RE: [PATCH ver 2] PowerPC: fix _Float128 type output string

[Carl Love]

Tom:

On Thu, 2023-04-13 at 11:12 -0600, Tom Tromey wrote:
> Carl> The function is declared extern in gdb/arch-utils.h b/gdb/arch-
> utils.h. 
> Carl> Thus it can not be declared static as well.
> 
> Surely only the default implementation has to be declared there?
> IIUC this particular one is only used by ppc.

Sorry, I kept thinking of the default function.  Had that stuck in my
head.  

The extern definition for function linux_dwarf2_omit_typedef_p () in
gdb/ppc-tdep.h is not needed as it is only used for PowerPC.  

The function linux_dwarf2_omit_typedef_p() in gdb/ppc-linux-tdep.c can
be made static once the extern declaration in gdb/ppc-tdep.h is
removed.

Sorry for the confusion.

                  Carl 

Re: [PATCH ver 2] PowerPC: fix _Float128 type output string

[Tom Tromey]

>>>>> "Carl" == Carl Love via Gdb-patches <gdb-patches@sourceware.org> writes:

>> However, doesn't copy_type also copy the TYPE_FLOATFORMAT field?
>> So where would this get reset?

Carl> the GCC-generated dwarf info including the hack provides
Carl> the following two type records:

Carl> 1) name: _Float128
Carl>    type: base floating-point type (TYPE_CODE_FLT)
Carl>      size: 16
Carl>      format: floatformats_ieee_quad

Carl> and

Carl> 2) name: "long double"
Carl>    type: typedef (TYPE_CODE_TYPEDEF)
Carl>      target-type: _Float128 (i.e. type 1) above)

Carl> What the patch does is to keep 1) as-is, and replace 2) by

Carl> 2') name: "long double"
Carl>     type: base floating-point type (TYPE_CODE_FLT)
Carl>       size: 16
Carl>       format: floatformats_ieee_quad

Carl> where the name is taken from 2), and the rest of the
Carl> record is taken from 1). 

Sorry for going in circles on this, but I still don't really get it.

From what I can see, the difference between (2) and (2') is just that
one is a typedef and one is not.  Where does this matter?

Tom

RE: [PATCH ver 2] PowerPC: fix _Float128 type output string

[Carl Love]

Tom:

On Fri, 2023-04-14 at 07:44 -0600, Tom Tromey wrote:
> > > > > > "Carl" == Carl Love via Gdb-patches <
> > > > > > gdb-patches@sourceware.org> writes:
> > > However, doesn't copy_type also copy the TYPE_FLOATFORMAT field?
> > > So where would this get reset?
> 
> Carl> the GCC-generated dwarf info including the hack provides
> Carl> the following two type records:
> 
> Carl> 1) name: _Float128
> Carl>    type: base floating-point type (TYPE_CODE_FLT)
> Carl>      size: 16
> Carl>      format: floatformats_ieee_quad
> 
> Carl> and
> 
> Carl> 2) name: "long double"
> Carl>    type: typedef (TYPE_CODE_TYPEDEF)
> Carl>      target-type: _Float128 (i.e. type 1) above)
> 
> Carl> What the patch does is to keep 1) as-is, and replace 2) by
> 
> Carl> 2') name: "long double"
> Carl>     type: base floating-point type (TYPE_CODE_FLT)
> Carl>       size: 16
> Carl>       format: floatformats_ieee_quad
> 
> Carl> where the name is taken from 2), and the rest of the
> Carl> record is taken from 1). 
> 
> Sorry for going in circles on this, but I still don't really get it.
> 
> From what I can see, the difference between (2) and (2') is just that
> one is a typedef and one is not.  Where does this matter?
> 
OK, I think I know what you are not getting.  

When the user wants to know the type of a variable, it gets the name
from the base type.  For example in the test program
gdb/testsuite/gdb.base/whatis-ptype-typedefs.c, we have the
declaration:

   typedef long double long_double_typedef;

On PowerPC, if GCC is using IEEE float 128-bit for the long double
format, the test case uses the ptype command to get the type of the
variable long_double_typedef, i.e.

   ptype v_long_double_typedef2

without the patch, GDB returns the base type name, in this case GDB
prints:

   type = _Float128
   
Note the type the user specified of "long double" and the GDB test case
is expecting the result to be "long double".  This results in the 74
test failures on PowerPC with when GCC uses IEEE float 128-bit. 

The "_Float128" is printed because the "long double" is typedef
pointing at the base type "_Float 128".  GDB tells the user the type of
the variable is something that the user didn't specify which is very
confusing to them, and to the testcase.  This is where the GCC "hack"
for identifying the "long double" as an IEEE 128-bit float value rather
than the IBM 128-bit float value is not transparent to GDB.

The GCC hack created this typedef for "long double", which should never
be a typedef since it is a base type in C.  The patch basically undoes
the GCC hack by making the type for "long double" a proper base type
again using the info from the _Float128 base type and the correct name
"long double".  Now, the GDB ptype command prints the proper type name
and the rest of the type info is there that GDB needs.  The comment in
gdb/gdbarch_components.py mentions that with the patch the proper name
is printed, but yea I can see where that wasn't completely obvious to
you.

Does that help?

                               Carl 

Re: [PATCH ver 2] PowerPC: fix _Float128 type output string

[Ulrich Weigand]

Tom Tromey <tom@tromey.com> wrote:

>Sorry for going in circles on this, but I still don't really get it.
>
>From what I can see, the difference between (2) and (2') is just that
>one is a typedef and one is not.  Where does this matter?

This matters as it changes how GDB prints the type.   If your source
code has e.g. a variable definition like
  long double x;
and you do e.g. "ptype x" in GDB, you'd expect to see "long double".

However, with the GCC hack typedef in place, you actually see
"_Float128" instead.  This not only confuses the user, it actually
causes a bunch of GDB test suite failures currently.

To summarize the set of problems that were leading up to this
scenario:

1) The ppc64le platform is currently in the process of switching
   the format of the "long double" type from the special IBM
   double-double to the standard IEEE 128-bit format.  Because
   of this, "long double" may currently refer to either of
   the two formats, depending on compiler version and options.
   For GDB to handle this correctly, the compiler therefore needs
   some way of informing GDB of the format actually used in any
   specific executable.

2) Since both types are 16-byte floating-point types, there is
   no method in the current DWARF standard to distinguish between
   them.  Therefore, GCC developers have added a hack that no
   longer emits "long double" as a DWARF base floating-point
   type, but rather emits "long double" as if it were defined
   as a typedef (with "_Float128" as target type).  [This is done
   only in those configurations where "long double" actually uses
   the IEEE format, otherwise "long double" remains a base type.]

3) GDB already recognizes "_Float128" (by name) and knows it
   always uses the IEEE 128-bit format, so it will automatically
   use the same format now for "long double" as well, if that
   typedef hack is present.  This makes GDB handle that type
   correctly - for the most part.

4) The exception is when printing the *name* of type, in which
   case we get the name "_Float128" showing up unexpectedly,
   as discussed above.

5) Carl's patch now adds a hack to GDB to "cancel out" the GCC
   hack by removing that typedef hack again, and defining
   "long double" as a base type (instead of a typedef) again.
   Note that since this base type inherits the format from
   the typedef hack's target type (_Float128), the GCC hack
   still fulfils its purpose of informing GDB of the proper
   format to use for "long double" - but now also type name
   printing works correctly.

I agree this is all a bit awkward, but given the GCC hack is
already out in the field, I don't really any better option to
handle this in GDB.

All this could probably go away once:
- there's a new DWARF format (or extension) that allows providing
  multiple floating-point base types with the same size but using
  different formats;
and
- there are no executables left in the field that still have the
  GCC typedef hack in their debug info.

Bye,
Ulrich

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

[Carl Love]

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(-)

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

Re: [PATCH ver 2] PowerPC: fix _Float128 type output string

[Tom Tromey]

> This matters as it changes how GDB prints the type.   If your source
> code has e.g. a variable definition like
>   long double x;
> and you do e.g. "ptype x" in GDB, you'd expect to see "long double".

> However, with the GCC hack typedef in place, you actually see
> "_Float128" instead.  This not only confuses the user, it actually
> causes a bunch of GDB test suite failures currently.

Ok, I understand now.  Thank you, & thanks for your patience.
The patch is OK by me.

Tom

RE: [PATCH ver 2] PowerPC: fix _Float128 type output string

[Ulrich Weigand]

Tom Tromey <tom@tromey.com> wrote:
> >This matters as it changes how GDB prints the type.   If your source
> >code has e.g. a variable definition like
> >  long double x;
> >and you do e.g. "ptype x" in GDB, you'd expect to see "long double".
> >However, with the GCC hack typedef in place, you actually see
> >"_Float128" instead.  This not only confuses the user, it actually
> >causes a bunch of GDB test suite failures currently.
>
>Ok, I understand now.  Thank you, & thanks for your patience.
>The patch is OK by me.

Thanks, Tom!

Bye,
Ulrich

Re: [PATCH ver 3] PowerPC: fix _Float128 type output string

[Ulrich Weigand]

Carl Love <cel@us.ibm.com> wrote:

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

This version is OK.

Thanks,
Ulrich