[powerpc][AIX] fix layout issues for nested struct types (#805)

* [powerpc][AIX] fix layout issues for nested struct types

On AIX under the default power alignment rules, the layout
of struct types which are nested inside other structs may
be different than the layout of those types on their own.

Specifically the first member double rules which would
apply an eight byte alignment if that type appears in the
first position of a struct:

 1) apply recursively if the struct appear in the first member
    of another struct
 2) do not apply if that struct is itself a member of another struct
    and not the first member.

The current implementation of the rules in libffi doesn't handle these
cases, causing a mismatch with the compiler and causing some crashes
we see when OpenJ9 is used with libffi on AIX.

This PR corrects this and adds some representative test cases.

* Fix code style

* Add a size check

* Add additional test

* Fix padding in internal structs

* Flip condition back to original form

* Add a comment

diff --git a/src/powerpc/ffi_darwin.c b/src/powerpc/ffi_darwin.c
index 64bb94dfae7e3d9a15aca95ce64282122bfcfa6a..afb6750a5bbfb5cb44208b42bbe5b0871bc79ee5 100644 (file)
--- a/src/powerpc/ffi_darwin.c
+++ b/src/powerpc/ffi_darwin.c
@@ -623,38 +623,50 @@ darwin_adjust_aggregate_sizes (ffi_type *s)
 }
 
 /* Adjust the size of S to be correct for AIX.
-   Word-align double unless it is the first member of a structure.  */
+   Word-align double unless it is the first member of a structure recursively.
+   Return non-zero if we found a recursive first member aggregate of interest. */
 
-static void
-aix_adjust_aggregate_sizes (ffi_type *s)
+static int
+aix_adjust_aggregate_sizes (ffi_type *s, int outer_most_type_or_first_member)
 {
-  int i;
+  int i, nested_first_member=0, final_align, rc=0;
 
   if (s->type != FFI_TYPE_STRUCT)
-    return;
+    return 0;
 
   s->size = 0;
   for (i = 0; s->elements[i] != NULL; i++)
     {
-      ffi_type *p;
+      ffi_type p;
       int align;
-      
-      p = s->elements[i];
-      aix_adjust_aggregate_sizes (p);
-      align = p->alignment;
-      if (i != 0 && p->type == FFI_TYPE_DOUBLE)
-       align = 4;
-      s->size = FFI_ALIGN(s->size, align) + p->size;
+
+      /* nested aggregates layout differently on AIX, so take a copy of the type */
+      p = *(s->elements[i]);
+      if (i == 0)
+        nested_first_member = aix_adjust_aggregate_sizes(&p, outer_most_type_or_first_member);
+      else
+        aix_adjust_aggregate_sizes(&p, 0);
+      align = p.alignment;
+      if (i != 0 && p.type == FFI_TYPE_DOUBLE)
+        align = 4;
+      s->size = FFI_ALIGN(s->size, align) + p.size;
     }
-  
-  s->size = FFI_ALIGN(s->size, s->alignment);
-  
-  if (s->elements[0]->type == FFI_TYPE_UINT64
-      || s->elements[0]->type == FFI_TYPE_SINT64
-      || s->elements[0]->type == FFI_TYPE_DOUBLE
-      || s->elements[0]->alignment == 8)
-    s->alignment = s->alignment > 8 ? s->alignment : 8;
-  /* Do not add additional tail padding.  */
+
+  final_align=s->alignment;
+  if ((s->elements[0]->type == FFI_TYPE_UINT64
+          || s->elements[0]->type == FFI_TYPE_SINT64
+          || s->elements[0]->type == FFI_TYPE_DOUBLE
+          || s->elements[0]->alignment == 8 || nested_first_member)) {
+      final_align = s->alignment > 8 ? s->alignment : 8;
+      rc=1;
+      /* still use the adjusted alignment to calculate tail padding, but don't adjust the types alignment if
+         we aren't in the recursive first position */
+      if (outer_most_type_or_first_member)
+        s->alignment=final_align;
+  }
+
+  s->size = FFI_ALIGN(s->size, final_align);
+  return rc;
 }
 
 /* Perform machine dependent cif processing.  */
@@ -682,9 +694,9 @@ ffi_prep_cif_machdep (ffi_cif *cif)
 
   if (cif->abi == FFI_AIX)
     {
-      aix_adjust_aggregate_sizes (cif->rtype);
+      aix_adjust_aggregate_sizes (cif->rtype, 1);
       for (i = 0; i < cif->nargs; i++)
-       aix_adjust_aggregate_sizes (cif->arg_types[i]);
+       aix_adjust_aggregate_sizes (cif->arg_types[i], 1);
     }
 
   /* Space for the frame pointer, callee's LR, CR, etc, and for

diff --git a/testsuite/libffi.call/callback.c b/testsuite/libffi.call/callback.c
new file mode 100644 (file)
index 0000000..5a803ed
--- /dev/null
+++ b/testsuite/libffi.call/callback.c
@@ -0,0 +1,99 @@
+/* Area:       ffi_call
+   Purpose:    Check structures with array and callback.
+   Limitations:        none.
+   PR:         none.
+   Originator: David Tenty <daltenty@ibm.com>  */
+
+/* { dg-do run } */
+#include "ffitest.h"
+
+int i=5;
+
+void callback() { i++; }
+
+typedef struct
+{
+  unsigned char c1;
+  double s[2];
+  unsigned char c2;
+} test_structure_12;
+
+static test_structure_12 ABI_ATTR struct12 (test_structure_12 ts, void (*func)(void))
+{
+  ts.c1 += 1;
+  ts.c2 += 1;
+  ts.s[0] += 1;
+  ts.s[1] += 1;
+
+  func();
+  return ts;
+}
+
+int main (void)
+{
+  ffi_cif cif;
+  ffi_type *args[MAX_ARGS];
+  void *values[MAX_ARGS];
+  ffi_type ts12_type,ts12a_type;
+  ffi_type *ts12_type_elements[4];
+  ffi_type *ts12a_type_elements[3];
+
+  test_structure_12 ts12_arg;
+  void (*ptr)(void)=&callback;
+
+  test_structure_12 *ts12_result =
+    (test_structure_12 *) malloc (sizeof(test_structure_12));
+
+  ts12a_type.size = 0;
+  ts12a_type.alignment = 0;
+  ts12a_type.type = FFI_TYPE_STRUCT;
+  ts12a_type.elements = ts12a_type_elements;
+  ts12a_type_elements[0] = &ffi_type_double;
+  ts12a_type_elements[1] = &ffi_type_double;
+  ts12a_type_elements[2] = NULL;
+
+  ts12_type.size = 0;
+  ts12_type.alignment = 0;
+  ts12_type.type = FFI_TYPE_STRUCT;
+  ts12_type.elements = ts12_type_elements;
+  ts12_type_elements[0] = &ffi_type_uchar;
+  ts12_type_elements[1] = &ts12a_type;
+  ts12_type_elements[2] = &ffi_type_uchar;
+  ts12_type_elements[3] = NULL;
+
+
+  args[0] = &ts12_type;
+  args[1] = &ffi_type_pointer;
+  values[0] = &ts12_arg;
+  values[1] = &ptr;
+
+  CHECK(ffi_prep_cif(&cif, ABI_NUM, 2, &ts12_type, args) == FFI_OK);
+
+  ts12_arg.c1 = 5;
+  ts12_arg.c2 = 6;
+  ts12_arg.s[0] = 7.77;
+  ts12_arg.s[1] = 8.88;
+
+  printf ("%u\n", ts12_arg.c1);
+  printf ("%u\n", ts12_arg.c2);
+  printf ("%g\n", ts12_arg.s[0]);
+  printf ("%g\n", ts12_arg.s[1]);
+  printf ("%d\n", i);
+
+  ffi_call(&cif, FFI_FN(struct12), ts12_result, values);
+
+  printf ("%u\n", ts12_result->c1);
+  printf ("%u\n", ts12_result->c2);
+  printf ("%g\n", ts12_result->s[0]);
+  printf ("%g\n", ts12_result->s[1]);
+  printf ("%d\n", i);
+  CHECK(ts12_result->c1 == 5 + 1);
+  CHECK(ts12_result->c2 == 6 + 1);
+  CHECK(ts12_result->s[0] == 7.77 + 1);
+  CHECK(ts12_result->s[1] == 8.88 + 1);
+  CHECK(i == 5 + 1);
+  CHECK(ts12_type.size == sizeof(test_structure_12));
+
+  free (ts12_result);
+  exit(0);
+}

diff --git a/testsuite/libffi.call/callback2.c b/testsuite/libffi.call/callback2.c
new file mode 100644 (file)
index 0000000..d061542
--- /dev/null
+++ b/testsuite/libffi.call/callback2.c
@@ -0,0 +1,108 @@
+/* Area:       ffi_call
+   Purpose:    Check structures with nested array and callback.
+   Limitations:        none.
+   PR:         none.
+   Originator: David Tenty <daltenty@ibm.com>  */
+
+/* { dg-do run } */
+#include "ffitest.h"
+
+int i=5;
+
+void callback() { i++; }
+
+typedef struct
+{
+  struct { double d; } s1;
+  double s[2];
+  unsigned char c2;
+} test_structure_12;
+
+static test_structure_12 ABI_ATTR struct12 (test_structure_12 ts, void (*func)(void))
+{
+  ts.s1.d += 1;
+  ts.c2 += 1;
+  ts.s[0] += 1;
+  ts.s[1] += 1;
+
+  func();
+  return ts;
+}
+
+int main (void)
+{
+  ffi_cif cif;
+  ffi_type *args[MAX_ARGS];
+  void *values[MAX_ARGS];
+  ffi_type ts12_type,ts12a_type, ts12b_type;
+  ffi_type *ts12_type_elements[4];
+  ffi_type *ts12a_type_elements[2];
+  ffi_type *ts12b_type_elements[3];
+
+
+  test_structure_12 ts12_arg;
+  void (*ptr)(void)=&callback;
+
+  test_structure_12 *ts12_result =
+    (test_structure_12 *) malloc (sizeof(test_structure_12));
+
+  ts12a_type.size = 0;
+  ts12a_type.alignment = 0;
+  ts12a_type.type = FFI_TYPE_STRUCT;
+  ts12a_type.elements = ts12a_type_elements;
+  ts12a_type_elements[0] = &ffi_type_double;
+  ts12a_type_elements[1] = NULL;
+
+  ts12b_type.size = 0;
+  ts12b_type.alignment = 0;
+  ts12b_type.type = FFI_TYPE_STRUCT;
+  ts12b_type.elements = ts12b_type_elements;
+  ts12b_type_elements[0] = &ffi_type_double;
+  ts12b_type_elements[1] = &ffi_type_double;
+  ts12b_type_elements[2] = NULL;
+
+  ts12_type.size = 0;
+  ts12_type.alignment = 0;
+  ts12_type.type = FFI_TYPE_STRUCT;
+  ts12_type.elements = ts12_type_elements;
+  ts12_type_elements[0] = &ts12a_type;
+  ts12_type_elements[1] = &ts12b_type;
+  ts12_type_elements[2] = &ffi_type_uchar;
+  ts12_type_elements[3] = NULL;
+
+
+  args[0] = &ts12_type;
+  args[1] = &ffi_type_pointer;
+  values[0] = &ts12_arg;
+  values[1] = &ptr;
+
+  CHECK(ffi_prep_cif(&cif, ABI_NUM, 2, &ts12_type, args) == FFI_OK);
+
+  ts12_arg.s1.d = 5.55;
+  ts12_arg.c2 = 6;
+  ts12_arg.s[0] = 7.77;
+  ts12_arg.s[1] = 8.88;
+
+  printf ("%g\n", ts12_arg.s1.d);
+  printf ("%u\n", ts12_arg.c2);
+  printf ("%g\n", ts12_arg.s[0]);
+  printf ("%g\n", ts12_arg.s[1]);
+  printf ("%d\n", i);
+
+  ffi_call(&cif, FFI_FN(struct12), ts12_result, values);
+
+  printf ("%g\n", ts12_result->s1.d);
+  printf ("%u\n", ts12_result->c2);
+  printf ("%g\n", ts12_result->s[0]);
+  printf ("%g\n", ts12_result->s[1]);
+  printf ("%d\n", i);
+  CHECK(ts12_result->s1.d == 5.55 + 1);
+  CHECK(ts12_result->c2 == 6 + 1);
+  CHECK(ts12_result->s[0] == 7.77 + 1);
+  CHECK(ts12_result->s[1] == 8.88 + 1);
+  CHECK(i == 5 + 1);
+  CHECK(ts12_type.size == sizeof(test_structure_12));
+
+  free (ts12_result);
+  exit(0);
+}

diff --git a/testsuite/libffi.call/callback3.c b/testsuite/libffi.call/callback3.c
new file mode 100644 (file)
index 0000000..5046b1e
--- /dev/null
+++ b/testsuite/libffi.call/callback3.c
@@ -0,0 +1,114 @@
+/* Area:       ffi_call
+   Purpose:    Check structures with array and callback.
+   Limitations:        none.
+   PR:         none.
+   Originator: David Tenty <daltenty@ibm.com>  */
+
+/* { dg-do run } */
+#include "ffitest.h"
+
+int i=5;
+
+void callback() { i++; }
+
+
+typedef struct
+{
+  struct { unsigned char c; double d; } s1;
+  double s[2];
+  unsigned char c2;
+} test_structure_12;
+
+static test_structure_12 ABI_ATTR struct12 (test_structure_12 ts, void (*func)(void))
+{
+  ts.s1.c += 1;
+  ts.s1.d += 1;
+  ts.c2 += 1;
+  ts.s[0] += 1;
+  ts.s[1] += 1;
+
+  func();
+  return ts;
+}
+
+int main (void)
+{
+  ffi_cif cif;
+  ffi_type *args[MAX_ARGS];
+  void *values[MAX_ARGS];
+  ffi_type ts12_type,ts12b_type, ts12a_type;
+  ffi_type *ts12_type_elements[4];
+  ffi_type *ts12b_type_elements[3];
+  ffi_type *ts12a_type_elements[3];
+
+  test_structure_12 ts12_arg;
+  void (*ptr)(void)=&callback;
+
+  test_structure_12 *ts12_result =
+    (test_structure_12 *) malloc (sizeof(test_structure_12));
+
+  ts12a_type.size = 0;
+  ts12a_type.alignment = 0;
+  ts12a_type.type = FFI_TYPE_STRUCT;
+  ts12a_type.elements = ts12a_type_elements;
+  ts12a_type_elements[0] = &ffi_type_uchar;
+  ts12a_type_elements[1] = &ffi_type_double;
+  ts12a_type_elements[2] = NULL;
+
+  ts12b_type.size = 0;
+  ts12b_type.alignment = 0;
+  ts12b_type.type = FFI_TYPE_STRUCT;
+  ts12b_type.elements = ts12b_type_elements;
+  ts12b_type_elements[0] = &ffi_type_double;
+  ts12b_type_elements[1] = &ffi_type_double;
+  ts12b_type_elements[2] = NULL;
+
+  ts12_type.size = 0;
+  ts12_type.alignment = 0;
+  ts12_type.type = FFI_TYPE_STRUCT;
+  ts12_type.elements = ts12_type_elements;
+  ts12_type_elements[0] = &ts12a_type;
+  ts12_type_elements[1] = &ts12b_type;
+  ts12_type_elements[2] = &ffi_type_uchar;
+  ts12_type_elements[3] = NULL;
+
+
+  args[0] = &ts12_type;
+  args[1] = &ffi_type_pointer;
+  values[0] = &ts12_arg;
+  values[1] = &ptr;
+
+  CHECK(ffi_prep_cif(&cif, ABI_NUM, 2, &ts12_type, args) == FFI_OK);
+
+  ts12_arg.s1.c = 5;
+  ts12_arg.s1.d = 5.55;
+  ts12_arg.c2 = 6;
+  ts12_arg.s[0] = 7.77;
+  ts12_arg.s[1] = 8.88;
+
+  printf ("%d\n", ts12_arg.s1.c);
+  printf ("%g\n", ts12_arg.s1.d);
+  printf ("%u\n", ts12_arg.c2);
+  printf ("%g\n", ts12_arg.s[0]);
+  printf ("%g\n", ts12_arg.s[1]);
+  printf ("%d\n", i);
+
+  ffi_call(&cif, FFI_FN(struct12), ts12_result, values);
+
+  printf ("%d\n", ts12_result->s1.c);
+  printf ("%g\n", ts12_result->s1.d);
+  printf ("%u\n", ts12_result->c2);
+  printf ("%g\n", ts12_result->s[0]);
+  printf ("%g\n", ts12_result->s[1]);
+  printf ("%d\n", i);
+  CHECK(ts12_result->s1.c == 5 + 1);
+  CHECK(ts12_result->s1.d == 5.55 + 1);
+  CHECK(ts12_result->c2 == 6 + 1);
+  CHECK(ts12_result->s[0] == 7.77 + 1);
+  CHECK(ts12_result->s[1] == 8.88 + 1);
+  CHECK(i == 5 + 1);
+  CHECK(ts12_type.size == sizeof(test_structure_12));
+
+  free (ts12_result);
+  exit(0);
+}

diff --git a/testsuite/libffi.call/callback4.c b/testsuite/libffi.call/callback4.c
new file mode 100644 (file)
index 0000000..5f191ef
--- /dev/null
+++ b/testsuite/libffi.call/callback4.c
@@ -0,0 +1,119 @@
+/* Area:       ffi_call
+   Purpose:    Check structures with array and callback.
+   Limitations:        none.
+   PR:         none.
+   Originator: David Tenty <daltenty@ibm.com>  */
+
+/* { dg-do run } */
+#include "ffitest.h"
+
+int i=5;
+
+void callback() { i++; }
+
+
+typedef struct
+{
+  unsigned char c1;
+  struct { double d; unsigned char c; } s[2];
+  unsigned char c2;
+} test_structure_12;
+
+static test_structure_12 ABI_ATTR struct12 (test_structure_12 ts, void (*func)(void))
+{
+  ts.c1 += 1;
+  ts.s[0].d += 1;
+  ts.s[0].c += 1;
+  ts.s[1].d += 1;
+  ts.s[1].c += 1;
+  ts.c2 += 1;
+
+  func();
+  return ts;
+}
+
+int main (void)
+{
+  ffi_cif cif;
+  ffi_type *args[MAX_ARGS];
+  void *values[MAX_ARGS];
+  ffi_type ts12_type,ts12b_type, ts12a_type;
+  ffi_type *ts12_type_elements[4];
+  ffi_type *ts12b_type_elements[3];
+  ffi_type *ts12a_type_elements[3];
+
+  test_structure_12 ts12_arg;
+  void (*ptr)(void)=&callback;
+
+  test_structure_12 *ts12_result =
+    (test_structure_12 *) malloc (sizeof(test_structure_12));
+
+  ts12a_type.size = 0;
+  ts12a_type.alignment = 0;
+  ts12a_type.type = FFI_TYPE_STRUCT;
+  ts12a_type.elements = ts12a_type_elements;
+  ts12a_type_elements[0] = &ffi_type_double;
+  ts12a_type_elements[1] = &ffi_type_uchar;
+  ts12a_type_elements[2] = NULL;
+
+  ts12b_type.size = 0;
+  ts12b_type.alignment = 0;
+  ts12b_type.type = FFI_TYPE_STRUCT;
+  ts12b_type.elements = ts12b_type_elements;
+  ts12b_type_elements[0] = &ts12a_type;
+  ts12b_type_elements[1] = &ts12a_type;
+  ts12b_type_elements[2] = NULL;
+
+  ts12_type.size = 0;
+  ts12_type.alignment = 0;
+  ts12_type.type = FFI_TYPE_STRUCT;
+  ts12_type.elements = ts12_type_elements;
+  ts12_type_elements[0] = &ffi_type_uchar;
+  ts12_type_elements[1] = &ts12b_type;
+  ts12_type_elements[2] = &ffi_type_uchar;
+  ts12_type_elements[3] = NULL;
+
+
+  args[0] = &ts12_type;
+  args[1] = &ffi_type_pointer;
+  values[0] = &ts12_arg;
+  values[1] = &ptr;
+
+  CHECK(ffi_prep_cif(&cif, ABI_NUM, 2, &ts12_type, args) == FFI_OK);
+
+  ts12_arg.c1 = 5;
+  ts12_arg.s[0].d = 5.55;
+  ts12_arg.s[0].c = 6;
+  ts12_arg.s[1].d = 7.77;
+  ts12_arg.s[1].c = 8;
+  ts12_arg.c2 = 9;
+
+  printf ("%u\n", ts12_arg.c1);
+  printf ("%g\n", ts12_arg.s[0].d);
+  printf ("%u\n", ts12_arg.s[0].c);
+  printf ("%g\n", ts12_arg.s[1].d);
+  printf ("%u\n", ts12_arg.s[1].c);
+  printf ("%u\n", ts12_arg.c2);
+  printf ("%d\n", i);
+
+  ffi_call(&cif, FFI_FN(struct12), ts12_result, values);
+
+  printf ("%u\n", ts12_result->c1);
+  printf ("%g\n", ts12_result->s[0].d);
+  printf ("%u\n", ts12_result->s[0].c);
+  printf ("%g\n", ts12_result->s[1].d);
+  printf ("%u\n", ts12_result->s[1].c);
+  printf ("%u\n", ts12_result->c2);
+  printf ("%d\n", i);
+  CHECK(ts12_result->c1 == 5 + 1);
+  CHECK(ts12_result->s[0].d == 5.55 + 1);
+  CHECK(ts12_result->s[0].c == 6 + 1);
+  CHECK(ts12_result->s[1].d == 7.77 + 1);
+  CHECK(ts12_result->s[1].c == 8 + 1);
+  CHECK(ts12_result->c2 == 9 + 1);
+  CHECK(i == 5 + 1);
+  CHECK(ts12_type.size == sizeof(test_structure_12));
+
+  free (ts12_result);
+  exit(0);
+}