Skip to content

7904115: Fix for AIX test case failures due to incorrect alignment for double and pointer #296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
varada1110 wants to merge 11 commits into
base: master
Choose a base branch
from
Open

7904115: Fix for AIX test case failures due to incorrect alignment for double and pointer #296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
97b3e87
testcase fixes for aix
varada1110 Nov 11, 2025
fdaf568
testcase fixes for aix
varada1110 Nov 11, 2025
996537e
testcase fixes for aix
varada1110 Nov 19, 2025
d4a0f64
testcase fixes for aix
varada1110 Nov 19, 2025
12b42e8
addition of -m64 at right place
varada1110 Nov 21, 2025
7f1983b
addition of -m64 at right place
varada1110 Nov 21, 2025
f43b66f
reverted the C_DOUBLE alignment fix
varada1110 Nov 25, 2025
56d5b88
added field alignment and natural alignment argument
varada1110 Dec 12, 2025
32846ad
restoring the changes
varada1110 Dec 17, 2025
a798d54
7904115: Fix for AIX test case failures due to incorrect alignment fo…
varada1110 Dec 18, 2025
a3dbfaf
7904115: Fix for AIX test case failures due to incorrect alignment fo…
varada1110 Dec 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/main/java/org/openjdk/jextract/JextractTool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,12 @@ private int run(String[] args) {

builder.addClangArg("-I" + System.getProperty("user.dir"));

// 64 bit mode on AIX
if (System.getProperty("os.name").toLowerCase().contains("aix")) {
builder.addClangArg("-m64");
builder.addClangArg("-DAIX_NATURAL_ALIGN=1");
}

if (optionSet.nonOptionArguments().isEmpty()) {
printOptionError(logger.format("expected.atleast.one.header"));
return OPTION_ERROR;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/openjdk/jextract/impl/ClassSourceBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ private String primitiveLayoutString(Primitive primitiveType, long align) {
case Long -> alignIfNeeded(runtimeHelperName() + ".C_LONG", TypeImpl.IS_WINDOWS ? 4 : 8, align);
case LongLong -> alignIfNeeded(runtimeHelperName() + ".C_LONG_LONG", 8, align);
case Float -> alignIfNeeded(runtimeHelperName() + ".C_FLOAT", 4, align);
case Double -> alignIfNeeded(runtimeHelperName() + ".C_DOUBLE", 8, align);
case Double -> alignIfNeeded(runtimeHelperName() + ".C_DOUBLE", TypeImpl.IS_AIX ? 4 : 8, align);
Copy link
Member

@JornVernee JornVernee Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With natural alignment, double is aligned to 8 bytes, so this change shouldn't be needed?

Copy link
Author

@varada1110 varada1110 Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m seeing two test failures reporting unsupported layout 4%D(8) and expected [D8(d)] but found [4%D8(d)]. In these cases, it is reported as 4 byte alignment for double. The 4 byte handling resolves these failures

Copy link
Member

@JornVernee JornVernee Dec 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that the first number here is the actual alignment of the C_DOUBLE layout. We get this from the canonical layout map of the system's native linker. On AIX this is just the layout of JAVA_DOUBLE [1], [2], which has an alignment of 8 bytes [3]. So, the first number here should really be 8, even on AIX.

If changing this is making the tests pass, it indicates that there's an issue elsewhere. Is it possible these tests are missing #pragma align(natural)?

Copy link
Author

@varada1110 varada1110 Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tried adding #pragma align(natural) to the failing test structs, but still reported 4 byte alignment for subsequent double. Another alternative that fixed the tests is when added attribute((aligned(8))) for the double field.

Copy link
Member

@JornVernee JornVernee Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which tests in particular are failing? Did you look at the Java code generated by jextract?

case LongDouble -> TypeImpl.IS_WINDOWS ?
alignIfNeeded(runtimeHelperName() + ".C_LONG_DOUBLE", 8, align) :
paddingLayoutString(8, 0);
Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/openjdk/jextract/impl/TypeImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
public abstract class TypeImpl implements Type {

public static final boolean IS_WINDOWS = System.getProperty("os.name").startsWith("Windows");
public static final boolean IS_AIX = System.getProperty("os.name").startsWith("AIX");

@Override
public boolean isErroneous() {
Expand Down
4 changes: 4 additions & 0 deletions test/jtreg/generator/testStruct/struct.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ extern "C" {
#define EXPORT
#endif

#ifdef AIX_NATURAL_ALIGN
#pragma align(natural)
#endif

typedef struct Point {
int x;
int y;
Expand Down
1 change: 1 addition & 0 deletions test/lib/testlib/JextractToolRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public class JextractToolRunner {
public static final boolean IS_WINDOWS = System.getProperty("os.name").startsWith("Windows");
public static final boolean IS_LINUX = System.getProperty("os.name").equals("Linux");
public static final boolean IS_AARCH64 = System.getProperty("os.arch").equals("aarch64");
public static final boolean IS_AIX = System.getProperty("os.name").equals("AIX");

public static final ValueLayout.OfBoolean C_BOOL = ValueLayout.JAVA_BOOLEAN;
public static final ValueLayout.OfByte C_CHAR = ValueLayout.JAVA_BYTE;
Expand Down
6 changes: 5 additions & 1 deletion test/testng/org/openjdk/jextract/test/toolprovider/examples.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@
#define EXPORT
#endif

#ifdef AIX_NATURAL_ALIGN
#pragma align(natural)
#endif

#define macro_byte (char) 1
#define macro_short (short) 1
#define macro_int 1
Expand Down Expand Up @@ -79,4 +83,4 @@ enum {
enum_anon_0,
enum_anon_1,
enum_anon_2,
};
};
2 changes: 1 addition & 1 deletion .../testng/org/openjdk/jextract/test/toolprovider/nestedAnonOffset/TestNestedAnonOffset.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public void testBaz() {
StructLayout layout = (StructLayout) findLayout(baz);
assertEquals(layout.memberLayouts().get(0), C_CHAR.withName("c"));
// Note here: on some platforms, the bitfield needs to be aligned and requires more padding
int paddingBytes = (IS_WINDOWS || (IS_LINUX && IS_AARCH64)) ? 11 : 8;
int paddingBytes = (IS_WINDOWS || (IS_LINUX && IS_AARCH64) || IS_AIX) ? 11 : 8;
assertEquals(layout.memberLayouts().get(1), MemoryLayout.paddingLayout(paddingBytes));
}

Expand Down