[PATCH 2/3] gdb/arm: Use type_align instead of arm_type_align

[PATCH 2/3] gdb/arm: Use type_align instead of arm_type_align

[Andrew Burgess]

Replaces use of arm_type_align with common type_align function.

Doing this fixes a bug in arm_type_align where static fields are
considered as part of the alignment calculation of a struct, which
results in arguments passed on the stack being misaligned, this bug
was causing a failure in gdb.cp/many-args.exp.

Part of the old arm_type_align is retained and used as the gdbarch
type align callback in order to correctly align vectors.

gdb/ChangeLog:

	* arm-tdep.c (arm_type_align): Only handle vector override case.
	(arm_push_dummy_call): Use type_align.
	(arm_gdbarch_init): Register arm_type_align gdbarch function.
---
 gdb/ChangeLog  |  6 ++++++
 gdb/arm-tdep.c | 68 +++++++++++++++-------------------------------------------
 2 files changed, 23 insertions(+), 51 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 599f785b349..742bfa57069 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -3314,62 +3314,25 @@ pop_stack_item (struct stack_item *si)
   return si;
 }
 
+/* Implement the gdbarch type alignment method, overrides the generic
+   alignment algorithm for anything that is arm specific.  */
 
-/* Return the alignment (in bytes) of the given type.  */
-
-static int
-arm_type_align (struct type *t)
+static ULONGEST
+arm_type_align (gdbarch *gdbarch, struct type *t)
 {
-  int n;
-  int align;
-  int falign;
-
   t = check_typedef (t);
-  switch (TYPE_CODE (t))
+  if (TYPE_CODE (t) == TYPE_CODE_ARRAY && TYPE_VECTOR (t))
     {
-    default:
-      /* Should never happen.  */
-      internal_error (__FILE__, __LINE__, _("unknown type alignment"));
-      return 4;
-
-    case TYPE_CODE_PTR:
-    case TYPE_CODE_ENUM:
-    case TYPE_CODE_INT:
-    case TYPE_CODE_FLT:
-    case TYPE_CODE_SET:
-    case TYPE_CODE_RANGE:
-    case TYPE_CODE_REF:
-    case TYPE_CODE_RVALUE_REF:
-    case TYPE_CODE_CHAR:
-    case TYPE_CODE_BOOL:
-      return TYPE_LENGTH (t);
-
-    case TYPE_CODE_ARRAY:
-      if (TYPE_VECTOR (t))
-	{
-	  /* Use the natural alignment for vector types (the same for
-	     scalar type), but the maximum alignment is 64-bit.  */
-	  if (TYPE_LENGTH (t) > 8)
-	    return 8;
-	  else
-	    return TYPE_LENGTH (t);
-	}
+      /* Use the natural alignment for vector types (the same for
+	 scalar type), but the maximum alignment is 64-bit.  */
+      if (TYPE_LENGTH (t) > 8)
+	return 8;
       else
-	return arm_type_align (TYPE_TARGET_TYPE (t));
-    case TYPE_CODE_COMPLEX:
-      return arm_type_align (TYPE_TARGET_TYPE (t));
-
-    case TYPE_CODE_STRUCT:
-    case TYPE_CODE_UNION:
-      align = 1;
-      for (n = 0; n < TYPE_NFIELDS (t); n++)
-	{
-	  falign = arm_type_align (TYPE_FIELD_TYPE (t, n));
-	  if (falign > align)
-	    align = falign;
-	}
-      return align;
+	return TYPE_LENGTH (t);
     }
+
+  /* Allow the common code to calculate the alignment.  */
+  return 0;
 }
 
 /* Possible base types for a candidate for passing and returning in
@@ -3715,7 +3678,7 @@ arm_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
       typecode = TYPE_CODE (arg_type);
       val = value_contents (args[argnum]);
 
-      align = arm_type_align (arg_type);
+      align = type_align (arg_type);
       /* Round alignment up to a whole number of words.  */
       align = (align + INT_REGISTER_SIZE - 1) & ~(INT_REGISTER_SIZE - 1);
       /* Different ABIs have different maximum alignments.  */
@@ -9309,6 +9272,9 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   else
     set_gdbarch_wchar_signed (gdbarch, 1);
 
+  /* Compute type alignment.  */
+  set_gdbarch_type_align (gdbarch, arm_type_align);
+
   /* Note: for displaced stepping, this includes the breakpoint, and one word
      of additional scratch space.  This setting isn't used for anything beside
      displaced stepping at present.  */
-- 
2.14.5

[PATCH 0/3] More use of type_align function

[Andrew Burgess]

After converting RISC-V to use type_align, this series converts
aarch64, arm, and nds32.

I've done very limited testing for aarch64 and arm - I'm struggling to
get the testsuite runnninng against the simulator I have to hand right
now, so only a small amount of manual testing has been done.

I've done no testing of nds32 at all.

The new test passes on X86-64/GNU-Linux.

--

Andrew Burgess (3):
  gdb/aarch64: Use type_align instead of aarch64_type_align
  gdb/arm: Use type_align instead of arm_type_align
  gdb/nds32: Use type_align instead of nds32_type_align

 gdb/ChangeLog                      | 19 ++++++++++
 gdb/aarch64-tdep.c                 | 66 ++++++++-------------------------
 gdb/arm-tdep.c                     | 68 +++++++++-------------------------
 gdb/nds32-tdep.c                   | 49 +-----------------------
 gdb/testsuite/ChangeLog            |  5 +++
 gdb/testsuite/gdb.cp/many-args.cc  | 76 ++++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.cp/many-args.exp | 37 +++++++++++++++++++
 7 files changed, 171 insertions(+), 149 deletions(-)
 create mode 100644 gdb/testsuite/gdb.cp/many-args.cc
 create mode 100644 gdb/testsuite/gdb.cp/many-args.exp

-- 
2.14.5

[PATCH 1/3] gdb/aarch64: Use type_align instead of aarch64_type_align

[Andrew Burgess]

Replaces use of aarch64_type_align with common type_align function.

Doing this fixes a bug in aarch64_type_align where static fields are
considered as part of the alignment calculation of a struct, which
results in arguments passed on the stack being misaligned.  This bug
is exposed in the new test gdb.cp/many-args.exp.

Part of the old aarch64_type_align is retained and used as the gdbarch
type align callback in order to correctly align vectors.

gdb/ChangeLog:

	* aarch64-tdep.c (aarch64_type_align): Only handle vector override
	case.
	(pass_on_stack): Use type_align.
	(aarch64_gdbarch_init): Register aarch64_type_align gdbarch
	function.

gdb/testsuite/ChangeLog:

	* gdb.cp/many-args.cc: New file.
	* gdb.cp/many-args.exp: New file.
---
 gdb/ChangeLog                      |  8 ++++
 gdb/aarch64-tdep.c                 | 66 ++++++++-------------------------
 gdb/testsuite/ChangeLog            |  5 +++
 gdb/testsuite/gdb.cp/many-args.cc  | 76 ++++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.cp/many-args.exp | 37 +++++++++++++++++++
 5 files changed, 141 insertions(+), 51 deletions(-)
 create mode 100644 gdb/testsuite/gdb.cp/many-args.cc
 create mode 100644 gdb/testsuite/gdb.cp/many-args.exp

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 1b3977bfaf2..8e44442c33a 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -1218,62 +1218,25 @@ typedef struct
 
 DEF_VEC_O (stack_item_t);
 
-/* Return the alignment (in bytes) of the given type.  */
+/* Implement the gdbarch type alignment method, overrides the generic
+   alignment algorithm for anything that is aarch64 specific.  */
 
-static int
-aarch64_type_align (struct type *t)
+static ULONGEST
+aarch64_type_align (gdbarch *gdbarch, struct type *t)
 {
-  int n;
-  int align;
-  int falign;
-
   t = check_typedef (t);
-  switch (TYPE_CODE (t))
+  if (TYPE_CODE (t) == TYPE_CODE_ARRAY && TYPE_VECTOR (t))
     {
-    default:
-      /* Should never happen.  */
-      internal_error (__FILE__, __LINE__, _("unknown type alignment"));
-      return 4;
-
-    case TYPE_CODE_PTR:
-    case TYPE_CODE_ENUM:
-    case TYPE_CODE_INT:
-    case TYPE_CODE_FLT:
-    case TYPE_CODE_SET:
-    case TYPE_CODE_RANGE:
-    case TYPE_CODE_BITSTRING:
-    case TYPE_CODE_REF:
-    case TYPE_CODE_RVALUE_REF:
-    case TYPE_CODE_CHAR:
-    case TYPE_CODE_BOOL:
-      return TYPE_LENGTH (t);
-
-    case TYPE_CODE_ARRAY:
-      if (TYPE_VECTOR (t))
-	{
-	  /* Use the natural alignment for vector types (the same for
-	     scalar type), but the maximum alignment is 128-bit.  */
-	  if (TYPE_LENGTH (t) > 16)
-	    return 16;
-	  else
-	    return TYPE_LENGTH (t);
-	}
+      /* Use the natural alignment for vector types (the same for
+	 scalar type), but the maximum alignment is 128-bit.  */
+      if (TYPE_LENGTH (t) > 16)
+	return 16;
       else
-	return aarch64_type_align (TYPE_TARGET_TYPE (t));
-    case TYPE_CODE_COMPLEX:
-      return aarch64_type_align (TYPE_TARGET_TYPE (t));
-
-    case TYPE_CODE_STRUCT:
-    case TYPE_CODE_UNION:
-      align = 1;
-      for (n = 0; n < TYPE_NFIELDS (t); n++)
-	{
-	  falign = aarch64_type_align (TYPE_FIELD_TYPE (t, n));
-	  if (falign > align)
-	    align = falign;
-	}
-      return align;
+	return TYPE_LENGTH (t);
     }
+
+  /* Allow the common code to calculate the alignment.  */
+  return 0;
 }
 
 /* Worker function for aapcs_is_vfp_call_or_return_candidate.
@@ -1540,7 +1503,7 @@ pass_on_stack (struct aarch64_call_info *info, struct type *type,
 
   info->argnum++;
 
-  align = aarch64_type_align (type);
+  align = type_align (type);
 
   /* PCS C.17 Stack should be aligned to the larger of 8 bytes or the
      Natural alignment of the argument's type.  */
@@ -3370,6 +3333,7 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_float_format (gdbarch, floatformats_ieee_single);
   set_gdbarch_double_format (gdbarch, floatformats_ieee_double);
   set_gdbarch_long_double_format (gdbarch, floatformats_ia64_quad);
+  set_gdbarch_type_align (gdbarch, aarch64_type_align);
 
   /* Internal <-> external register number maps.  */
   set_gdbarch_dwarf2_reg_to_regnum (gdbarch, aarch64_dwarf_reg_to_regnum);
diff --git a/gdb/testsuite/gdb.cp/many-args.cc b/gdb/testsuite/gdb.cp/many-args.cc
new file mode 100644
index 00000000000..389a69ae32b
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/many-args.cc
@@ -0,0 +1,76 @@
+/* Copyright 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <string.h>
+
+/* Vector type will align on a 16-byte boundary.  */
+typedef int v4si __attribute__ ((vector_size (16)));
+
+struct ss
+{
+  static v4si static_field;
+
+  unsigned char aa;
+
+  bool operator== (const ss &rhs)
+  {
+    return (memcmp (&this->static_field, &rhs.static_field,
+		    sizeof (this->static_field)) == 0
+            && this->aa == rhs.aa);
+  }
+};
+
+v4si ss::static_field = { 1, 2, 3, 4 };
+
+ss ref_val = { 'a' };
+
+bool
+check_val (ss v1, ss v2, ss v3, ss v4, ss v5, ss v6, ss v7, ss v8,
+           ss v9, ss v10, ss v11, ss v12, ss v13, ss v14, ss v15,
+           ss v16, ss v17, ss v18, ss v19, ss v20, ss v21, ss v22,
+           ss v23, ss v24, ss v25, ss v26, ss v27, ss v28, ss v29,
+           ss v30, ss v31, ss v32, ss v33, ss v34, ss v35, ss v36,
+           ss v37, ss v38, ss v39, ss v40)
+{
+  return (v1 == ref_val && v2 == ref_val && v3 == ref_val && v4 == ref_val
+          && v5 == ref_val && v6 == ref_val && v7 == ref_val
+          && v8 == ref_val && v9 == ref_val && v10 == ref_val
+          && v11 == ref_val && v12 == ref_val && v13 == ref_val
+          && v14 == ref_val && v15 == ref_val && v16 == ref_val
+          && v17 == ref_val && v18 == ref_val && v19 == ref_val
+          && v20 == ref_val && v21 == ref_val && v22 == ref_val
+          && v23 == ref_val && v24 == ref_val && v25 == ref_val
+          && v26 == ref_val && v27 == ref_val && v28 == ref_val
+          && v29 == ref_val && v30 == ref_val && v31 == ref_val
+          && v32 == ref_val && v33 == ref_val && v34 == ref_val
+          && v35 == ref_val && v36 == ref_val && v37 == ref_val
+          && v38 == ref_val && v39 == ref_val && v40 == ref_val);
+}
+
+int
+main ()
+{
+  bool flag = check_val (ref_val, ref_val, ref_val, ref_val, ref_val,
+			 ref_val, ref_val, ref_val, ref_val, ref_val,
+			 ref_val, ref_val, ref_val, ref_val, ref_val,
+			 ref_val, ref_val, ref_val, ref_val, ref_val,
+			 ref_val, ref_val, ref_val, ref_val, ref_val,
+			 ref_val, ref_val, ref_val, ref_val, ref_val,
+			 ref_val, ref_val, ref_val, ref_val, ref_val,
+			 ref_val, ref_val, ref_val, ref_val, ref_val);
+  return (flag ? 0 : 1);	/* break-here */
+}
diff --git a/gdb/testsuite/gdb.cp/many-args.exp b/gdb/testsuite/gdb.cp/many-args.exp
new file mode 100644
index 00000000000..57da1748397
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/many-args.exp
@@ -0,0 +1,37 @@
+# Copyright 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+# It tests various aspects of iostream that have caused problems for gdb.
+
+if { [skip_cplus_tests] } { continue }
+
+standard_testfile .cc
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
+    return -1
+}
+
+if ![runto_main] {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+gdb_breakpoint [gdb_get_line_number "break-here"]
+gdb_continue_to_breakpoint "break-here"
+
+gdb_test "p check_val (ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val, ref_val)" \
+    "= true" \
+    "check passing many structures"
-- 
2.14.5

[PATCH 3/3] gdb/nds32: Use type_align instead of nds32_type_align

[Andrew Burgess]

The general type_align method should be a suitable alternative to
nds32_type_align, so switch to use that.

The only change this will introduce is related to static fields in a
struct or union, the existing code doesn't take account of static
fields when computing the alignment for structs of unions, though this
is probably a bug - which would probably be exposed by the test case
gdb.cp/many-args.exp, though I don't have any way to test this target
right now.

gdb/ChangeLog:

	* nds32-tdep.c (nds32_type_align): Delete.
	(nds32_push_dummy_call): Use type_align instead.
---
 gdb/ChangeLog    |  5 +++++
 gdb/nds32-tdep.c | 49 ++-----------------------------------------------
 2 files changed, 7 insertions(+), 47 deletions(-)

diff --git a/gdb/nds32-tdep.c b/gdb/nds32-tdep.c
index 801b2dadcae..d3481e2000b 100644
--- a/gdb/nds32-tdep.c
+++ b/gdb/nds32-tdep.c
@@ -1439,51 +1439,6 @@ nds32_check_calling_use_fpr (struct type *type)
   return typecode == TYPE_CODE_FLT;
 }
 
-/* Return the alignment (in bytes) of the given type.  */
-
-static int
-nds32_type_align (struct type *type)
-{
-  int n;
-  int align;
-  int falign;
-
-  type = check_typedef (type);
-  switch (TYPE_CODE (type))
-    {
-    default:
-      /* Should never happen.  */
-      internal_error (__FILE__, __LINE__, _("unknown type alignment"));
-      return 4;
-
-    case TYPE_CODE_PTR:
-    case TYPE_CODE_ENUM:
-    case TYPE_CODE_INT:
-    case TYPE_CODE_FLT:
-    case TYPE_CODE_SET:
-    case TYPE_CODE_RANGE:
-    case TYPE_CODE_REF:
-    case TYPE_CODE_CHAR:
-    case TYPE_CODE_BOOL:
-      return TYPE_LENGTH (type);
-
-    case TYPE_CODE_ARRAY:
-    case TYPE_CODE_COMPLEX:
-      return nds32_type_align (TYPE_TARGET_TYPE (type));
-
-    case TYPE_CODE_STRUCT:
-    case TYPE_CODE_UNION:
-      align = 1;
-      for (n = 0; n < TYPE_NFIELDS (type); n++)
-	{
-	  falign = nds32_type_align (TYPE_FIELD_TYPE (type, n));
-	  if (falign > align)
-	    align = falign;
-	}
-      return align;
-    }
-}
-
 /* Implement the "push_dummy_call" gdbarch method.  */
 
 static CORE_ADDR
@@ -1522,7 +1477,7 @@ nds32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   for (i = 0; i < nargs; i++)
     {
       struct type *type = value_type (args[i]);
-      int align = nds32_type_align (type);
+      int align = type_align (type);
 
       /* If align is zero, it may be an empty struct.
 	 Just ignore the argument of empty struct.  */
@@ -1548,7 +1503,7 @@ nds32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
       type = value_type (args[i]);
       calling_use_fpr = nds32_check_calling_use_fpr (type);
       len = TYPE_LENGTH (type);
-      align = nds32_type_align (type);
+      align = type_align (type);
       val = value_contents (args[i]);
 
       /* The size of a composite type larger than 4 bytes will be rounded
-- 
2.14.5

Re: [PATCH 3/3] gdb/nds32: Use type_align instead of nds32_type_align

[Kevin Buettner]

On Sat, 13 Apr 2019 00:25:34 +0100
Andrew Burgess <andrew.burgess@embecosm.com> wrote:

> The general type_align method should be a suitable alternative to
> nds32_type_align, so switch to use that.
> 
> The only change this will introduce is related to static fields in a
> struct or union, the existing code doesn't take account of static
> fields when computing the alignment for structs of unions, though this
> is probably a bug - which would probably be exposed by the test case
> gdb.cp/many-args.exp, though I don't have any way to test this target
> right now.
> 
> gdb/ChangeLog:
> 
> 	* nds32-tdep.c (nds32_type_align): Delete.
> 	(nds32_push_dummy_call): Use type_align instead.

I think that nds32_type_align needs to be registered in
nds32_gdbarch_init().

Otherwise, LGTM.

Kevin

Re: [PATCH 1/3] gdb/aarch64: Use type_align instead of aarch64_type_align

[Kevin Buettner]

On Sat, 13 Apr 2019 00:25:32 +0100
Andrew Burgess <andrew.burgess@embecosm.com> wrote:

> Replaces use of aarch64_type_align with common type_align function.
> 
> Doing this fixes a bug in aarch64_type_align where static fields are
> considered as part of the alignment calculation of a struct, which
> results in arguments passed on the stack being misaligned.  This bug
> is exposed in the new test gdb.cp/many-args.exp.
> 
> Part of the old aarch64_type_align is retained and used as the gdbarch
> type align callback in order to correctly align vectors.
> 
> gdb/ChangeLog:
> 
> 	* aarch64-tdep.c (aarch64_type_align): Only handle vector override
> 	case.
> 	(pass_on_stack): Use type_align.
> 	(aarch64_gdbarch_init): Register aarch64_type_align gdbarch
> 	function.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.cp/many-args.cc: New file.
> 	* gdb.cp/many-args.exp: New file.

LGTM.

Kevin

Re: [PATCH 2/3] gdb/arm: Use type_align instead of arm_type_align

[Kevin Buettner]

On Sat, 13 Apr 2019 00:25:33 +0100
Andrew Burgess <andrew.burgess@embecosm.com> wrote:

> Replaces use of arm_type_align with common type_align function.
> 
> Doing this fixes a bug in arm_type_align where static fields are
> considered as part of the alignment calculation of a struct, which
> results in arguments passed on the stack being misaligned, this bug
> was causing a failure in gdb.cp/many-args.exp.
> 
> Part of the old arm_type_align is retained and used as the gdbarch
> type align callback in order to correctly align vectors.
> 
> gdb/ChangeLog:
> 
> 	* arm-tdep.c (arm_type_align): Only handle vector override case.
> 	(arm_push_dummy_call): Use type_align.
> 	(arm_gdbarch_init): Register arm_type_align gdbarch function.

LGTM.

Kevin

Re: [PATCH 3/3] gdb/nds32: Use type_align instead of nds32_type_align

[Andrew Burgess]

* Kevin Buettner <kevinb@redhat.com> [2019-04-14 11:53:52 -0700]:

> On Sat, 13 Apr 2019 00:25:34 +0100
> Andrew Burgess <andrew.burgess@embecosm.com> wrote:
> 
> > The general type_align method should be a suitable alternative to
> > nds32_type_align, so switch to use that.
> > 
> > The only change this will introduce is related to static fields in a
> > struct or union, the existing code doesn't take account of static
> > fields when computing the alignment for structs of unions, though this
> > is probably a bug - which would probably be exposed by the test case
> > gdb.cp/many-args.exp, though I don't have any way to test this target
> > right now.
> > 
> > gdb/ChangeLog:
> > 
> > 	* nds32-tdep.c (nds32_type_align): Delete.
> > 	(nds32_push_dummy_call): Use type_align instead.
> 
> I think that nds32_type_align needs to be registered in
> nds32_gdbarch_init().

No, I deleted nds32_type_align completely.  It doesn't have any
special vector type handling, so the default type_align should be
fine.

The one change that will be seen is that the old nds32_type_align
counts static fields within structs when computing the alignment.  I
haven't tried any testing, but I would guess this was a bug.  Unless
someone has the ability to test the target I'll probably just push
this change, and if it turns out the static field handling is wrong,
then it's easy enough to fix later.

Thanks,
Andrew


> 
> Otherwise, LGTM.
> 
> Kevin

Re: [PATCH 3/3] gdb/nds32: Use type_align instead of nds32_type_align

[Kevin Buettner]

On Wed, 17 Apr 2019 21:59:46 +0100
Andrew Burgess <andrew.burgess@embecosm.com> wrote:

> * Kevin Buettner <kevinb@redhat.com> [2019-04-14 11:53:52 -0700]:
> 
> > On Sat, 13 Apr 2019 00:25:34 +0100
> > Andrew Burgess <andrew.burgess@embecosm.com> wrote:
> >   
> > > The general type_align method should be a suitable alternative to
> > > nds32_type_align, so switch to use that.
> > > 
> > > The only change this will introduce is related to static fields in a
> > > struct or union, the existing code doesn't take account of static
> > > fields when computing the alignment for structs of unions, though this
> > > is probably a bug - which would probably be exposed by the test case
> > > gdb.cp/many-args.exp, though I don't have any way to test this target
> > > right now.
> > > 
> > > gdb/ChangeLog:
> > > 
> > > 	* nds32-tdep.c (nds32_type_align): Delete.
> > > 	(nds32_push_dummy_call): Use type_align instead.  
> > 
> > I think that nds32_type_align needs to be registered in
> > nds32_gdbarch_init().  
> 
> No, I deleted nds32_type_align completely.  It doesn't have any
> special vector type handling, so the default type_align should be
> fine.

I see.  This is okay then.  (Sorry for not reading it more closely.)

> The one change that will be seen is that the old nds32_type_align
> counts static fields within structs when computing the alignment.  I
> haven't tried any testing, but I would guess this was a bug.  Unless
> someone has the ability to test the target I'll probably just push
> this change, and if it turns out the static field handling is wrong,
> then it's easy enough to fix later.

I think that's fine.

Kevin

Re: [PATCH 0/3] More use of type_align function

[Andrew Burgess]

* Andrew Burgess <andrew.burgess@embecosm.com> [2019-04-13 00:25:31 +0100]:

> After converting RISC-V to use type_align, this series converts
> aarch64, arm, and nds32.
> 
> I've done very limited testing for aarch64 and arm - I'm struggling to
> get the testsuite runnninng against the simulator I have to hand right
> now, so only a small amount of manual testing has been done.
> 
> I've done no testing of nds32 at all.
> 
> The new test passes on X86-64/GNU-Linux.
> 
> --
> 
> Andrew Burgess (3):
>   gdb/aarch64: Use type_align instead of aarch64_type_align
>   gdb/arm: Use type_align instead of arm_type_align
>   gdb/nds32: Use type_align instead of nds32_type_align

I have now pushed this series.

Thanks,
Andrew


> 
>  gdb/ChangeLog                      | 19 ++++++++++
>  gdb/aarch64-tdep.c                 | 66 ++++++++-------------------------
>  gdb/arm-tdep.c                     | 68 +++++++++-------------------------
>  gdb/nds32-tdep.c                   | 49 +-----------------------
>  gdb/testsuite/ChangeLog            |  5 +++
>  gdb/testsuite/gdb.cp/many-args.cc  | 76 ++++++++++++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.cp/many-args.exp | 37 +++++++++++++++++++
>  7 files changed, 171 insertions(+), 149 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.cp/many-args.cc
>  create mode 100644 gdb/testsuite/gdb.cp/many-args.exp
> 
> -- 
> 2.14.5
>