diff mbox

s390: Implement 'type_align' gdbarch method

Message ID m3o90z7unb.fsf@oc0404454431.ibm.com
State New, archived
Headers

Commit Message

Andreas Arnez Aug. 8, 2019, 11:40 a.m. UTC 
  The align.exp test case yields many FAILs on s390x, since GDB's _Alignoff
doesn't always agree with the compiler's.  On s390x, the maximum alignment
is 8, but GDB returns an alignment of 16 for 16-byte data types such as
"long double".

This is fixed by implementing the type_align gdbarch method.  The new
method returns an alignment of 8 for all integer, floating-point, and
vector types larger than 8 bytes.  With this change, all align.exp tests
pass.

gdb/ChangeLog:

	* s390-tdep.c (s390_type_align): New function.
	(s390_gdbarch_init): Set it as type_align gdbarch method.
---
 gdb/s390-tdep.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

Comments

Tom Tromey Aug. 8, 2019, 5 p.m. UTC | #1 
>>>>> "Andreas" == Andreas Arnez <arnez@linux.ibm.com> writes:

Andreas> gdb/ChangeLog:

Andreas> 	* s390-tdep.c (s390_type_align): New function.
Andreas> 	(s390_gdbarch_init): Set it as type_align gdbarch method.

FWIW this looks reasonable to me.

Tom
Andreas Arnez Aug. 9, 2019, 6:32 p.m. UTC | #2 
On Thu, Aug 08 2019, Tom Tromey wrote:

>>>>>> "Andreas" == Andreas Arnez <arnez@linux.ibm.com> writes:
>
> Andreas> gdb/ChangeLog:
>
> Andreas> 	* s390-tdep.c (s390_type_align): New function.
> Andreas> 	(s390_gdbarch_init): Set it as type_align gdbarch method.
>
> FWIW this looks reasonable to me.

Thanks, pushed as git commit 1022c627db.

--
Andreas
Tom de Vries Aug. 27, 2019, 11:29 a.m. UTC | #3 
On 08-08-19 13:40, Andreas Arnez wrote:
> The align.exp test case yields many FAILs on s390x, since GDB's _Alignoff
> doesn't always agree with the compiler's.  On s390x, the maximum alignment
> is 8, but GDB returns an alignment of 16 for 16-byte data types such as
> "long double".
> 
> This is fixed by implementing the type_align gdbarch method.  The new
> method returns an alignment of 8 for all integer, floating-point, and
> vector types larger than 8 bytes.  With this change, all align.exp tests
> pass.
> 

Hi,

in the "zSeries ELF Application Binary Interface Supplement" document I
find long double listed with 16-byte size and alignment.

Likewise in the "IBM XL C/C++ for Linux on z Systems Optimization and
Programming Guide".

So I wonder, is this patch hardcoding the assumptions of a single
compiler implementation (gcc) in gdb, thereby possibly breaking
functionality in gdb when debugging executables generated by other
compilers?

If so, ISTM the correct way to fix this is to get gcc to emit the
non-standard type alignment in the debug info.

Thanks,
- Tom
diff mbox

Patch

diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c
index 4b931017a8..871efc59bc 100644
--- a/gdb/s390-tdep.c
+++ b/gdb/s390-tdep.c
@@ -52,6 +52,37 @@  constexpr gdb_byte s390_break_insn[] = { 0x0, 0x1 };
 
 typedef BP_MANIPULATION (s390_break_insn) s390_breakpoint;
 
+/* Types.  */
+
+/* Implement the gdbarch type alignment method.  */
+
+static ULONGEST
+s390_type_align (gdbarch *gdbarch, struct type *t)
+{
+  t = check_typedef (t);
+
+  if (TYPE_LENGTH (t) > 8)
+    {
+      switch (TYPE_CODE (t))
+	{
+	case TYPE_CODE_INT:
+	case TYPE_CODE_RANGE:
+	case TYPE_CODE_FLT:
+	case TYPE_CODE_ENUM:
+	case TYPE_CODE_CHAR:
+	case TYPE_CODE_BOOL:
+	case TYPE_CODE_DECFLOAT:
+	  return 8;
+
+	case TYPE_CODE_ARRAY:
+	  if (TYPE_VECTOR (t))
+	    return 8;
+	  break;
+	}
+    }
+  return 0;
+}
+
 /* Decoding S/390 instructions.  */
 
 /* Read a single instruction from address AT.  */
@@ -6944,6 +6975,8 @@  s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_long_double_bit (gdbarch, 128);
   set_gdbarch_long_double_format (gdbarch, floatformats_ia64_quad);
 
+  set_gdbarch_type_align (gdbarch, s390_type_align);
+
   /* Breakpoints.  */
   /* Amount PC must be decremented by after a breakpoint.  This is
      often the number of bytes returned by gdbarch_breakpoint_from_pc but not