Fix the alignment issue with long & pointer on AIX

[ChengJin01]

The changes aim to restore the original alignment setting for long and pointer on AIX given the padding is required in native in the case of struct [int/float,long] and [int/float,pointer].

Signed-off-by: Cheng Jin jincheng@ca.ibm.com

[ChengJin01]

The changes revert part of fix specific to long(C_LONG_LONG) and pointer(ADDRESS) on AIX at #15, which was captured in jtreg test for upcall (also verified in native code) given the alignment for long and pointer are 64bit rather than 32bit on AIX.

Reviewer: @tajila
FYI: @pshipton, @keithc-ca

[keithc-ca]

So, on AIX, double is the only special case (where the alignment is less than the size)?

[ChengJin01]

So, on AIX, double is the only special case (where the alignment is less than the size)?

Yes. the previous alignment adjustment only applies to double on AIX.

[keithc-ca]

So, on AIX, double is the only special case (where the alignment is less than the size)?

Yes. the previous alignment adjustment only applies to double on AIX.

I don't understand your use of the word "previous" here. I am trying to understand what the alignment rules on AIX should be. Consider these C structs:

struct sample_Z { char fill; bool       field; };
struct sample_C { char fill; char       field; };
struct sample_S { char fill; short      field; };
struct sample_I { char fill; int        field; };
struct sample_L { char fill; long       field; };
struct sample_J { char fill; long long  field; };
struct sample_F { char fill; float      field; };
struct sample_D { char fill; double     field; };
struct sample_P { char fill; void     * field; };
struct sample_V { char fill; va_list    field; };

The bottom line is that we want to know what offset C compilers assign to field in each structure.
This change suggests we used to believe that the offsets in sample_J and sample_P were 4 (when they should have been 8), but we still believe that the offset in sample_D is 4. Do I understand correctly?

[ChengJin01]

This change suggests we used to believe that the offsets in sample_J and sample_P were 4 (when they should have been 8), but we still believe that the offset in sample_D is 4. Do I understand correctly?

Yes, that's what happens in such case. The offset should be 8 for sample_J and sample_P in that the compiling option must be -q64 for 64bit mode while the default option is -q32 for 32bit mode on AIX. That is to say, there are 4-byte alignment padding for sample_J and sample_P in 64bit mode but not required for sample_D.

[keithc-ca]

It's my understanding that we only intend to produce 64-bit builds for AIX, so the discussion of -q32 and 32-bit mode is irrelevant.

None of the structures should have 4-byte padding - that would leave field at offset 5. For types that tolerate odd alignment, offset 1 would suffice, that is no padding. You say I understand, but your response muddies the water. It still feels inconsistent to me: I would expect double, long long and void * all to require 8-byte alignment.

[keithc-ca]

That is to say, there are 4-byte alignment padding for sample_J and sample_P in 64bit mode but not required for sample_D.

Re-reading your comment, I realize there's another way to take it: that is "4-byte alignment padding" means the following field must be at a byte offset that is a multiple of 4. Perhaps you meant "8-byte alignment padding", otherwise the code before this change is what we want. Please clarify, paying attention to the details and be precise.

[ChengJin01]

I think the debugging results better explain what I meant:

here the debug result on these struct


(dbx) p a <--- {char, long long}
(fill = 'a', field = 123456789)
(dbx) p &a
0x0ffffffffffff9d8
(dbx) p sizeof(a)
16  <------------- 7-byte padding is required between char and long long.
(dbx) 0x0ffffffffffff9d8/20x
0x0ffffffffffff9d8:  6100 0000 0000 0000 0000 0000 075b cd15
0x0ffffffffffff9e8:  6100 0000 0000 0000 6100 0000 0000 0000
0x0ffffffffffff9f8:  0fff ffff ffff f9e8


(dbx) p  db  <--- {char, void *}
(fill = 'a', field = 0x0ffffffffffff9e8)
(dbx) p sizeof(db)
16  <------------- 7-byte padding is required between char and voi.
(dbx) p &db
0x0ffffffffffff9f0
(dbx) 0x0ffffffffffff9f0/20x
0x0ffffffffffff9f0:  6100 0000 0000 0000 0fff ffff ffff f9e8
0x0ffffffffffffa00:  badc 0ffe e0dd f00d badc 0ffe e0dd f00d
0x0ffffffffffffa10:  badc 0ffe e0dd f00d


(dbx) p b  <--- {char, double}
(fill = 'a', field = 153.666)
(dbx) p sizeof(b)
12 <------------- the char needs to be at offset that is a multiple of 4.
(dbx) p &b
0x0ffffffffffff9c8
(dbx) 0x0ffffffffffff9c8/20x
0x0ffffffffffff9c8:  6100 0000 4063 354f df3b 645a f000 0090
0x0ffffffffffff9d8:  6100 0000 0000 0000 0000 0000 075b cd15
0x0ffffffffffff9e8:  6100 0000 0000 0000

[ChengJin01]

As discussed with @keithc-ca offline, the intention of this PR is to correct what we used to do for long long/pointer to ensure the 8-byte alignment applies to long long/pointer.

[tajila]

@ChengJin01 do the tests verify this behaviour?

[ChengJin01]

@ChengJin01 do the tests verify this behaviour?

Yes, this behaviour was verified with all downcall & upcall jtreg test suites, including the failing tests in https://github.com/ibmruntimes/openj9-openjdk-jdk19/blob/openj9/test/jdk/java/foreign/TestUpcallScope.java and https://github.com/ibmruntimes/openj9-openjdk-jdk19/blob/openj9/test/jdk/java/foreign/TestUpcallStack.java where this issue was captured on AIX, plus our own FFI specific tests in https://github.com/eclipse-openj9/openj9/tree/master/test/functional/Java19andUp/src/org/openj9/test/jep424.

[tajila]

jenkins test sanity aix jdk19