RFR: 7904115: Fix for AIX test case failures due to incorrect alignment for double and pointer [v2]
Jorn Vernee
jvernee at openjdk.org
Fri Nov 21 15:43:26 UTC 2025
On Fri, 21 Nov 2025 12:57:39 GMT, Varada M <varadam at openjdk.org> wrote:
>> Total of 10 test failures observed on AIX:
>> jtreg/generator/nestedTypes/TestNestedTypesUnsupported.java
>> jtreg/generator/test8246400/LibTest8246400Test.java
>> jtreg/generator/test8258605/LibTest8258605Test.java
>> jtreg/generator/test8261511/Test8261511.java
>> jtreg/generator/testStruct/LibStructTest.java
>> testng/org/openjdk/jextract/test/toolprovider/ConstantsTest.java
>> testng/org/openjdk/jextract/test/toolprovider/IncompleteArrayTest.java
>> testng/org/openjdk/jextract/test/toolprovider/Test8240811.java
>> testng/org/openjdk/jextract/test/toolprovider/TestClassGeneration.java
>> testng/org/openjdk/jextract/test/toolprovider/nestedAnonOffset/TestNestedAnonOffset.java
>>
>> This PR fixes AIX specific layout generation issues related to incorrect alignment double and pointer types.
>> 1. Structs containing double fields fail with:
>> i. Unsupported layout: 4%D8
>> ii. Invalid alignment constraint for member layout
>> double in AIX structs has size 8 but alignment 4 (except as first field). AIX specific handling for C_DOUBLE computes the correct alignment.
>>
>> 2. Clang was detected as 32-bit due to missing -m64 during macro extraction, causing inconsistent macros. This caused jextract to interpret pointer constants incorrectly, leading to failures like:
>> expected [-1] but found [4294967295]
>>
>> 3. TestNestedAnonOffset.java test failed on AIX because it also expects more padding similar to platforms like windows and linux
>>
>>
>> After the patch jtreg tests passes successfully.
>>
>> JBS: [CODETOOLS-7904115](https://bugs.openjdk.org/browse/CODETOOLS-7904115)
>
> Varada M has updated the pull request incrementally with two additional commits since the last revision:
>
> - addition of -m64 at right place
> - addition of -m64 at right place
@varada1110
You could also try the following patch:
diff --git a/src/main/java/org/openjdk/jextract/impl/ClassSourceBuilder.java b/src/main/java/org/openjdk/jextract/impl/ClassSourceBuilder.java
index 7c1ae3a..a317dc5 100644
--- a/src/main/java/org/openjdk/jextract/impl/ClassSourceBuilder.java
+++ b/src/main/java/org/openjdk/jextract/impl/ClassSourceBuilder.java
@@ -30,7 +30,6 @@ import org.openjdk.jextract.Type.Declared;
import org.openjdk.jextract.Type.Delegated;
import org.openjdk.jextract.Type.Function;
import org.openjdk.jextract.Type.Primitive;
-import org.openjdk.jextract.impl.DeclarationImpl.ClangAlignOf;
import org.openjdk.jextract.impl.DeclarationImpl.ClangEnumType;
import org.openjdk.jextract.impl.DeclarationImpl.DeclarationString;
import org.openjdk.jextract.impl.DeclarationImpl.JavaName;
@@ -201,19 +200,25 @@ abstract class ClassSourceBuilder {
throw new IllegalArgumentException("Not handled: " + type);
}
+ private static final int NO_ALIGN_REQUIRED_MARKER = -1;
+
String layoutString(Type type) {
- return layoutString(type, Long.MAX_VALUE);
+ return fieldLayoutString(type, -1, NO_ALIGN_REQUIRED_MARKER);
}
- String layoutString(Type type, long align) {
+ String fieldLayoutString(Type type, long typeAlign, long expectedAlign) {
return switch (type) {
- case Primitive p -> primitiveLayoutString(p, align);
- case Declared d when Utils.isEnum(d) -> layoutString(ClangEnumType.get(d.tree()).get(), align);
- case Declared d when Utils.isStructOrUnion(d) -> alignIfNeeded(JavaName.getFullNameOrThrow(d.tree()) + ".layout()", ClangAlignOf.getOrThrow(d.tree()) / 8, align);
- case Delegated d when d.kind() == Delegated.Kind.POINTER -> alignIfNeeded(runtimeHelperName() + ".C_POINTER", 8, align);
- case Delegated d -> layoutString(d.type(), align);
- case Function _ -> alignIfNeeded(runtimeHelperName() + ".C_POINTER", 8, align);
- case Array a -> String.format("MemoryLayout.sequenceLayout(%1$d, %2$s)", a.elementCount().orElse(0L), layoutString(a.elementType(), align));
+ case Primitive p -> primitiveLayoutString(p, typeAlign, expectedAlign);
+ case Declared d when Utils.isEnum(d) ->
+ fieldLayoutString(ClangEnumType.get(d.tree()).get(), typeAlign, expectedAlign);
+ case Declared d when Utils.isStructOrUnion(d) ->
+ alignIfNeeded(JavaName.getFullNameOrThrow(d.tree()) + ".layout()", typeAlign, expectedAlign);
+ case Delegated d when d.kind() == Delegated.Kind.POINTER ->
+ alignIfNeeded(runtimeHelperName() + ".C_POINTER", typeAlign, expectedAlign);
+ case Delegated d -> fieldLayoutString(d.type(), typeAlign, expectedAlign);
+ case Function _ -> alignIfNeeded(runtimeHelperName() + ".C_POINTER", typeAlign, expectedAlign);
+ case Array a -> String.format("MemoryLayout.sequenceLayout(%1$d, %2$s)", a.elementCount().orElse(0L),
+ fieldLayoutString(a.elementType(), typeAlign, expectedAlign));
default -> throw new UnsupportedOperationException();
};
}
@@ -250,18 +255,18 @@ abstract class ClassSourceBuilder {
return " ".repeat(size * 4);
}
- private String primitiveLayoutString(Primitive primitiveType, long align) {
+ private String primitiveLayoutString(Primitive primitiveType, long defaultAlign, long expectedAlign) {
return switch (primitiveType.kind()) {
- case Bool -> runtimeHelperName() + ".C_BOOL";
- case Char -> runtimeHelperName() + ".C_CHAR";
- case Short -> alignIfNeeded(runtimeHelperName() + ".C_SHORT", 2, align);
- case Int -> alignIfNeeded(runtimeHelperName() + ".C_INT", 4, 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 Bool -> alignIfNeeded(runtimeHelperName() + ".C_BOOL", defaultAlign, expectedAlign);
+ case Char -> alignIfNeeded(runtimeHelperName() + ".C_CHAR", defaultAlign, expectedAlign);
+ case Short -> alignIfNeeded(runtimeHelperName() + ".C_SHORT", defaultAlign, expectedAlign);
+ case Int -> alignIfNeeded(runtimeHelperName() + ".C_INT", defaultAlign, expectedAlign);
+ case Long -> alignIfNeeded(runtimeHelperName() + ".C_LONG", defaultAlign, expectedAlign);
+ case LongLong -> alignIfNeeded(runtimeHelperName() + ".C_LONG_LONG", defaultAlign, expectedAlign);
+ case Float -> alignIfNeeded(runtimeHelperName() + ".C_FLOAT", defaultAlign, expectedAlign);
+ case Double -> alignIfNeeded(runtimeHelperName() + ".C_DOUBLE", defaultAlign, expectedAlign);
case LongDouble -> TypeImpl.IS_WINDOWS ?
- alignIfNeeded(runtimeHelperName() + ".C_LONG_DOUBLE", 8, align) :
+ alignIfNeeded(runtimeHelperName() + ".C_LONG_DOUBLE", defaultAlign, expectedAlign) :
paddingLayoutString(8, 0);
case HalfFloat, Char16, WChar -> paddingLayoutString(2, 0); // unsupported
case Float128, Int128 -> paddingLayoutString(16, 0); // unsupported
@@ -269,8 +274,8 @@ abstract class ClassSourceBuilder {
};
}
- private String alignIfNeeded(String layoutPrefix, long align, long expectedAlign) {
- return align > expectedAlign ?
+ private String alignIfNeeded(String layoutPrefix, long defaultAlign, long expectedAlign) {
+ return expectedAlign != NO_ALIGN_REQUIRED_MARKER && defaultAlign != expectedAlign ?
String.format("%1$s.align(%2$s, %3$d)", runtimeHelperName(), layoutPrefix, expectedAlign) :
layoutPrefix;
}
diff --git a/src/main/java/org/openjdk/jextract/impl/StructBuilder.java b/src/main/java/org/openjdk/jextract/impl/StructBuilder.java
index dedad14..3415996 100644
--- a/src/main/java/org/openjdk/jextract/impl/StructBuilder.java
+++ b/src/main/java/org/openjdk/jextract/impl/StructBuilder.java
@@ -460,7 +460,7 @@ final class StructBuilder extends ClassSourceBuilder implements OutputFactory.Bu
boolean isStruct = scoped.kind() == Scoped.Kind.STRUCT;
- long align = ClangAlignOf.getOrThrow(scoped) / 8;
+ long scopedTypeAlign = ClangAlignOf.getOrThrow(scoped) / 8;
long offset = base;
long size = 0L; // bits
@@ -477,7 +477,16 @@ final class StructBuilder extends ClassSourceBuilder implements OutputFactory.Bu
}
String memberLayout;
if (member instanceof Variable var) {
- memberLayout = layoutString(var.type(), align);
+ // FIXME we can not handle hyper-aligned fields here since clang doesn't attach the
+ // alignment specified by a field alignment specifier to the field declaration cursor.
+ //
+ // struct foo { // ClangAlignOf == 8
+ // _Alignas(8) int x; // ClangAlignOf == 4
+ // };
+ long fieldTypeAlign = ClangAlignOf.getOrThrow(var) / 8;
+ // if struct is packed, adjust expected alignment down
+ long expectedAlign = Math.min(scopedTypeAlign, fieldTypeAlign);
+ memberLayout = fieldLayoutString(var.type(), fieldTypeAlign, expectedAlign);
memberLayout = String.format("%1$s%2$s.withName("%3$s")", indentString(indent + 1), memberLayout, member.name());
} else {
// anon struct
And then I think you should be able to just adjust `expectedAlign` in `StructBuilder` for what AIX needs.
-------------
PR Comment: https://git.openjdk.org/jextract/pull/296#issuecomment-3563559127
More information about the jextract-dev
mailing list