diff --git a/src/java.base/share/classes/java/lang/foreign/Linker.java b/src/java.base/share/classes/java/lang/foreign/Linker.java index 8a57f51d150..9862ded081d 100644 --- a/src/java.base/share/classes/java/lang/foreign/Linker.java +++ b/src/java.base/share/classes/java/lang/foreign/Linker.java @@ -365,11 +365,41 @@ import java.util.stream.Stream; * *

Variadic functions

* - * Variadic functions (e.g. a C function declared with a trailing ellipses {@code ...} at the end of the formal parameter - * list or with an empty formal parameter list) are not supported directly by the native linker. However, it is still possible - * to link a variadic function by using a specialized function descriptor, together with a - * {@linkplain Linker.Option#firstVariadicArg(int) a linker option} which indicates the position of the first variadic argument - * in that specialized descriptor. + * Variadic functions are C functions which can accept a variable number and type of arguments. They are declared: + *
    + *
  1. With a trailing ellipsis ({@code ...}) at the end of the formal parameter list, such as: {@code void foo(int x, ...);}
    2. + *
  2. With an empty formal parameter list, called a prototype-less function, such as: {@code void foo();}
    3. + *
+ * The arguments passed in place of the ellipsis, or the arguments passed to a prototype-less function are called + * variadic arguments. Variadic functions are, essentially, templates that can be specialized into multiple + * non-variadic functions by replacing the {@code ...} or empty formal parameter list with a list of variadic parameters + * of a fixed number and type. + *

+ * It should be noted that values passed as variadic arguments undergo default argument promotion in C. For instance, the + * following argument promotions are applied: + *

+ * whereby the signed-ness of the source type corresponds to the signed-ness of the promoted type. The complete process + * of default argument promotion is described in the C specification. In effect these promotions place limits on the + * specialized form of a variadic function, as the variadic parameters of the specialized form will always have a promoted + * type. + *

+ * The native linker only supports linking the specialized form of a variadic function. A variadic function in its specialized + * form can be linked using a function descriptor describing the specialized form. Additionally, the + * {@link Linker.Option#firstVariadicArg(int)} linker option must be provided to indicate the first variadic parameter in + * the parameter list. The corresponding argument layout, and all following argument layouts in the specialized function + * descriptor, are called variadic argument layouts. For a prototype-less function, the index passed to + * {@link Linker.Option#firstVariadicArg(int)} should always be {@code 0}. + *

+ * The native linker will reject an attempt to link a specialized function descriptor with any variadic argument layouts + * corresponding to a C type that would be subject to default argument promotion (as described above). Exactly which layouts + * will be rejected is platform specific, but as an example: on Linux/x64 the layouts {@link ValueLayout#JAVA_BOOLEAN}, + * {@link ValueLayout#JAVA_BYTE}, {@link ValueLayout#JAVA_CHAR}, {@link ValueLayout#JAVA_SHORT}, and + * {@link ValueLayout#JAVA_FLOAT} will be rejected. *

* A well-known variadic function is the {@code printf} function, defined in the C standard library: * diff --git a/src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java b/src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java index 268fb8def43..b1847791751 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java +++ b/src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java @@ -90,6 +90,7 @@ public abstract sealed class AbstractLinker implements Linker permits LinuxAArch checkLayouts(function); function = stripNames(function); LinkerOptions optionSet = LinkerOptions.forDowncall(function, options); + validateVariadicLayouts(function, optionSet); return DOWNCALL_CACHE.get(new LinkRequest(function, optionSet), linkRequest -> { FunctionDescriptor fd = linkRequest.descriptor(); @@ -134,6 +135,28 @@ public abstract sealed class AbstractLinker implements Linker permits LinuxAArch /** {@return byte order used by this linker} */ protected abstract ByteOrder linkerByteOrder(); + // C spec mandates that variadic arguments smaller than int are promoted to int, + // and float is promoted to double + // See: https://en.cppreference.com/w/c/language/conversion#Default_argument_promotions + // We reject the corresponding layouts here, to avoid issues where unsigned values + // are sign extended when promoted. (as we don't have a way to unambiguously represent signed-ness atm). + private void validateVariadicLayouts(FunctionDescriptor function, LinkerOptions optionSet) { + if (optionSet.isVariadicFunction()) { + List argumentLayouts = function.argumentLayouts(); + List variadicLayouts = argumentLayouts.subList(optionSet.firstVariadicArgIndex(), argumentLayouts.size()); + + for (MemoryLayout variadicLayout : variadicLayouts) { + if (variadicLayout.equals(ValueLayout.JAVA_BOOLEAN) + || variadicLayout.equals(ValueLayout.JAVA_BYTE) + || variadicLayout.equals(ValueLayout.JAVA_CHAR) + || variadicLayout.equals(ValueLayout.JAVA_SHORT) + || variadicLayout.equals(ValueLayout.JAVA_FLOAT)) { + throw new IllegalArgumentException("Invalid variadic argument layout: " + variadicLayout); + } + } + } + } + private void checkLayouts(FunctionDescriptor descriptor) { descriptor.returnLayout().ifPresent(this::checkLayout); descriptor.argumentLayouts().forEach(this::checkLayout); diff --git a/src/java.base/share/classes/jdk/internal/foreign/abi/LinkerOptions.java b/src/java.base/share/classes/jdk/internal/foreign/abi/LinkerOptions.java index 28cde93090e..2c2bcbcc163 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/abi/LinkerOptions.java +++ b/src/java.base/share/classes/jdk/internal/foreign/abi/LinkerOptions.java @@ -93,6 +93,10 @@ public class LinkerOptions { return fva != null; } + public int firstVariadicArgIndex() { + return getOption(FirstVariadicArg.class).index(); + } + public boolean isTrivial() { IsTrivial it = getOption(IsTrivial.class); return it != null; diff --git a/src/java.base/share/classes/jdk/internal/foreign/abi/fallback/FallbackLinker.java b/src/java.base/share/classes/jdk/internal/foreign/abi/fallback/FallbackLinker.java index 72a9399360d..541557b47e7 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/abi/fallback/FallbackLinker.java +++ b/src/java.base/share/classes/jdk/internal/foreign/abi/fallback/FallbackLinker.java @@ -81,7 +81,7 @@ public final class FallbackLinker extends AbstractLinker { @Override protected MethodHandle arrangeDowncall(MethodType inferredMethodType, FunctionDescriptor function, LinkerOptions options) { - MemorySegment cif = makeCif(inferredMethodType, function, FFIABI.DEFAULT, Arena.ofAuto()); + MemorySegment cif = makeCif(inferredMethodType, function, options, Arena.ofAuto()); int capturedStateMask = options.capturedCallState() .mapToInt(CapturableState::mask) @@ -107,7 +107,7 @@ public final class FallbackLinker extends AbstractLinker { @Override protected UpcallStubFactory arrangeUpcall(MethodType targetType, FunctionDescriptor function, LinkerOptions options) { - MemorySegment cif = makeCif(targetType, function, FFIABI.DEFAULT, Arena.ofAuto()); + MemorySegment cif = makeCif(targetType, function, options, Arena.ofAuto()); UpcallData invData = new UpcallData(function.returnLayout().orElse(null), function.argumentLayouts(), cif); MethodHandle doUpcallMH = MethodHandles.insertArguments(MH_DO_UPCALL, 3, invData); @@ -123,7 +123,9 @@ public final class FallbackLinker extends AbstractLinker { return ByteOrder.nativeOrder(); } - private static MemorySegment makeCif(MethodType methodType, FunctionDescriptor function, FFIABI abi, Arena scope) { + private static MemorySegment makeCif(MethodType methodType, FunctionDescriptor function, LinkerOptions options, Arena scope) { + FFIABI abi = FFIABI.DEFAULT; + MemorySegment argTypes = scope.allocate(function.argumentLayouts().size() * ADDRESS.byteSize()); List argLayouts = function.argumentLayouts(); for (int i = 0; i < argLayouts.size(); i++) { @@ -134,7 +136,14 @@ public final class FallbackLinker extends AbstractLinker { MemorySegment returnType = methodType.returnType() != void.class ? FFIType.toFFIType(function.returnLayout().orElseThrow(), abi, scope) : LibFallback.voidType(); - return LibFallback.prepCif(returnType, argLayouts.size(), argTypes, abi, scope); + + if (options.isVariadicFunction()) { + int numFixedArgs = options.firstVariadicArgIndex(); + int numTotalArgs = argLayouts.size(); + return LibFallback.prepCifVar(returnType, numFixedArgs, numTotalArgs, argTypes, abi, scope); + } else { + return LibFallback.prepCif(returnType, argLayouts.size(), argTypes, abi, scope); + } } private record DowncallData(MemorySegment cif, MemoryLayout returnLayout, List argLayouts, diff --git a/src/java.base/share/classes/jdk/internal/foreign/abi/fallback/LibFallback.java b/src/java.base/share/classes/jdk/internal/foreign/abi/fallback/LibFallback.java index c434dbcef59..6ba81263d5c 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/abi/fallback/LibFallback.java +++ b/src/java.base/share/classes/jdk/internal/foreign/abi/fallback/LibFallback.java @@ -104,6 +104,27 @@ final class LibFallback { return cif; } + /** + * Wrapper for {@code ffi_prep_cif_var}. The variadic version of prep_cif + * + * @param returnType a pointer to an @{code ffi_type} describing the return type + * @param numFixedArgs the number of fixed arguments + * @param numTotalArgs the number of total arguments + * @param paramTypes a pointer to an array of pointers, which each point to an {@code ffi_type} describing a + * parameter type + * @param abi the abi to be used + * @param scope the scope into which to allocate the returned {@code ffi_cif} struct + * @return a pointer to a prepared {@code ffi_cif} struct + * + * @throws IllegalStateException if the call to {@code ffi_prep_cif} returns a non-zero status code + */ + static MemorySegment prepCifVar(MemorySegment returnType, int numFixedArgs, int numTotalArgs, MemorySegment paramTypes, FFIABI abi, + Arena scope) throws IllegalStateException { + MemorySegment cif = scope.allocate(NativeConstants.SIZEOF_CIF); + checkStatus(ffi_prep_cif_var(cif.address(), abi.value(), numFixedArgs, numTotalArgs, returnType.address(), paramTypes.address())); + return cif; + } + /** * Create an upcallStub-style closure. This method wraps the {@code ffi_closure_alloc} * and {@code ffi_prep_closure_loc} functions. @@ -177,6 +198,7 @@ final class LibFallback { private static native void doDowncall(long cif, long fn, long rvalue, long avalues, long capturedState, int capturedStateMask); private static native int ffi_prep_cif(long cif, int abi, int nargs, long rtype, long atypes); + private static native int ffi_prep_cif_var(long cif, int abi, int nfixedargs, int ntotalargs, long rtype, long atypes); private static native int ffi_get_struct_offsets(int abi, long type, long offsets); private static native int ffi_default_abi(); diff --git a/src/java.base/share/native/libfallbackLinker/fallbackLinker.c b/src/java.base/share/native/libfallbackLinker/fallbackLinker.c index d2058ce0899..7f554d49737 100644 --- a/src/java.base/share/native/libfallbackLinker/fallbackLinker.c +++ b/src/java.base/share/native/libfallbackLinker/fallbackLinker.c @@ -58,6 +58,10 @@ Java_jdk_internal_foreign_abi_fallback_LibFallback_ffi_1prep_1cif(JNIEnv* env, j return ffi_prep_cif(jlong_to_ptr(cif), (ffi_abi) abi, (unsigned int) nargs, jlong_to_ptr(rtype), jlong_to_ptr(atypes)); } JNIEXPORT jint JNICALL +Java_jdk_internal_foreign_abi_fallback_LibFallback_ffi_1prep_1cif_1var(JNIEnv* env, jclass cls, jlong cif, jint abi, jint nfixedargs, jint ntotalargs, jlong rtype, jlong atypes) { + return ffi_prep_cif_var(jlong_to_ptr(cif), (ffi_abi) abi, (unsigned int) nfixedargs, (unsigned int) ntotalargs, jlong_to_ptr(rtype), jlong_to_ptr(atypes)); +} +JNIEXPORT jint JNICALL Java_jdk_internal_foreign_abi_fallback_LibFallback_ffi_1get_1struct_1offsets(JNIEnv* env, jclass cls, jint abi, jlong type, jlong offsets) { return ffi_get_struct_offsets((ffi_abi) abi, jlong_to_ptr(type), jlong_to_ptr(offsets)); } diff --git a/test/jdk/java/foreign/StdLibTest.java b/test/jdk/java/foreign/StdLibTest.java index a66dc1e9b9c..360bb501bbb 100644 --- a/test/jdk/java/foreign/StdLibTest.java +++ b/test/jdk/java/foreign/StdLibTest.java @@ -378,13 +378,10 @@ public class StdLibTest extends NativeTestHelper { } enum PrintfArg { - - INTEGRAL(int.class, C_INT, "%d", arena -> 42, 42), - STRING(MemorySegment.class, C_POINTER, "%s", arena -> { - return arena.allocateUtf8String("str"); - }, "str"), - CHAR(byte.class, C_CHAR, "%c", arena -> (byte) 'h', 'h'), - DOUBLE(double.class, C_DOUBLE, "%.4f", arena ->1.2345d, 1.2345d); + INT(int.class, C_INT, "%d", arena -> 42, 42), + LONG(long.class, C_LONG_LONG, "%d", arena -> 84L, 84L), + DOUBLE(double.class, C_DOUBLE, "%.4f", arena -> 1.2345d, 1.2345d), + STRING(MemorySegment.class, C_POINTER, "%s", arena -> arena.allocateUtf8String("str"), "str"); final Class carrier; final ValueLayout layout; diff --git a/test/jdk/java/foreign/TestIllegalLink.java b/test/jdk/java/foreign/TestIllegalLink.java index ebaf3ab221d..9360e6f8559 100644 --- a/test/jdk/java/foreign/TestIllegalLink.java +++ b/test/jdk/java/foreign/TestIllegalLink.java @@ -60,9 +60,9 @@ public class TestIllegalLink extends NativeTestHelper { private static final Linker ABI = Linker.nativeLinker(); @Test(dataProvider = "types") - public void testIllegalLayouts(FunctionDescriptor desc, String expectedExceptionMessage) { + public void testIllegalLayouts(FunctionDescriptor desc, Linker.Option[] options, String expectedExceptionMessage) { try { - ABI.downcallHandle(DUMMY_TARGET, desc); + ABI.downcallHandle(DUMMY_TARGET, desc, options); fail("Expected IllegalArgumentException was not thrown"); } catch (IllegalArgumentException e) { assertTrue(e.getMessage().contains(expectedExceptionMessage), @@ -108,25 +108,31 @@ public class TestIllegalLink extends NativeTestHelper { @DataProvider public static Object[][] types() { + Linker.Option[] NO_OPTIONS = new Linker.Option[0]; List cases = new ArrayList<>(Arrays.asList(new Object[][]{ { FunctionDescriptor.of(MemoryLayout.sequenceLayout(2, C_INT)), + NO_OPTIONS, "Unsupported layout: [2:i4]" }, { FunctionDescriptor.ofVoid(MemoryLayout.sequenceLayout(2, C_INT)), + NO_OPTIONS, "Unsupported layout: [2:i4]" }, { FunctionDescriptor.ofVoid(C_INT.withByteAlignment(2)), + NO_OPTIONS, "Layout alignment must be natural alignment" }, { FunctionDescriptor.ofVoid(C_POINTER.withByteAlignment(2)), + NO_OPTIONS, "Layout alignment must be natural alignment" }, { FunctionDescriptor.ofVoid(ValueLayout.JAVA_CHAR.withByteAlignment(4)), + NO_OPTIONS, "Layout alignment must be natural alignment" }, { @@ -135,6 +141,7 @@ public class TestIllegalLink extends NativeTestHelper { C_SHORT.withName("y").withByteAlignment(1), C_INT.withName("z").withByteAlignment(1) ).withByteAlignment(1)), + NO_OPTIONS, "Layout alignment must be natural alignment" }, { @@ -144,6 +151,7 @@ public class TestIllegalLink extends NativeTestHelper { C_SHORT.withName("y").withByteAlignment(1), C_INT.withName("z").withByteAlignment(1) ))), + NO_OPTIONS, "Layout alignment must be natural alignment" }, { @@ -151,6 +159,7 @@ public class TestIllegalLink extends NativeTestHelper { MemoryLayout.sequenceLayout( C_INT.withByteAlignment(1) ))), + NO_OPTIONS, "Layout alignment must be natural alignment" }, { @@ -158,40 +167,55 @@ public class TestIllegalLink extends NativeTestHelper { ValueLayout.JAVA_INT, MemoryLayout.paddingLayout(4), // no excess padding ValueLayout.JAVA_INT)), + NO_OPTIONS, "unexpected offset" }, { FunctionDescriptor.of(C_INT.withOrder(nonNativeOrder())), + NO_OPTIONS, "Layout does not have the right byte order" }, { FunctionDescriptor.of(MemoryLayout.structLayout(C_INT.withOrder(nonNativeOrder()))), + NO_OPTIONS, "Layout does not have the right byte order" }, { FunctionDescriptor.of(MemoryLayout.structLayout(MemoryLayout.sequenceLayout(C_INT.withOrder(nonNativeOrder())))), + NO_OPTIONS, "Layout does not have the right byte order" }, { FunctionDescriptor.ofVoid(MemoryLayout.structLayout( ValueLayout.JAVA_LONG, ValueLayout.JAVA_INT)), // missing trailing padding + NO_OPTIONS, "has unexpected size" }, { FunctionDescriptor.ofVoid(MemoryLayout.structLayout( ValueLayout.JAVA_INT, MemoryLayout.paddingLayout(4))), // too much trailing padding + NO_OPTIONS, "has unexpected size" }, })); + for (ValueLayout illegalLayout : List.of(C_CHAR, ValueLayout.JAVA_CHAR, C_BOOL, C_SHORT, C_FLOAT)) { + cases.add(new Object[]{ + FunctionDescriptor.ofVoid(C_INT, illegalLayout), + new Linker.Option[]{Linker.Option.firstVariadicArg(1)}, + "Invalid variadic argument layout" + }); + } + if (IS_SYSV) { cases.add(new Object[] { FunctionDescriptor.ofVoid(MemoryLayout.structLayout( MemoryLayout.sequenceLayout( C_INT ))), + NO_OPTIONS, "GroupLayout is too large" }); } diff --git a/test/jdk/java/foreign/TestIntrinsics.java b/test/jdk/java/foreign/TestIntrinsics.java index 09d629f8742..95f98b50e7e 100644 --- a/test/jdk/java/foreign/TestIntrinsics.java +++ b/test/jdk/java/foreign/TestIntrinsics.java @@ -108,8 +108,8 @@ public class TestIntrinsics extends NativeTestHelper { { // identity_va MemorySegment ma = findNativeOrThrow("identity_va"); FunctionDescriptor fd = FunctionDescriptor.of(C_INT, C_INT, - C_DOUBLE, C_INT, C_FLOAT, C_LONG_LONG); - tests.add(abi.downcallHandle(ma, fd, firstVariadicArg(1)), 1, 1, 10D, 2, 3F, 4L); + C_DOUBLE, C_INT, C_DOUBLE, C_LONG_LONG); + tests.add(abi.downcallHandle(ma, fd, firstVariadicArg(1)), 1, 1, 10D, 2, 3D, 4L); } { // high_arity diff --git a/test/jdk/java/foreign/TestVarArgs.java b/test/jdk/java/foreign/TestVarArgs.java index f7c63c0c96b..8f1ef4e862b 100644 --- a/test/jdk/java/foreign/TestVarArgs.java +++ b/test/jdk/java/foreign/TestVarArgs.java @@ -34,6 +34,7 @@ import java.lang.foreign.Arena; import java.lang.foreign.Linker; import java.lang.foreign.FunctionDescriptor; import java.lang.foreign.MemoryLayout; +import java.lang.foreign.ValueLayout; import java.lang.foreign.MemorySegment; import org.testng.annotations.DataProvider; @@ -161,6 +162,9 @@ public class TestVarArgs extends CallGeneratorHelper { List args = new ArrayList<>(); for (ParamType pType : paramTypes) { MemoryLayout layout = pType.layout(fields); + if (layout instanceof ValueLayout.OfFloat) { + layout = C_DOUBLE; // promote to double, per C spec + } TestValue testValue = genTestValue(layout, arena); Arg.NativeType type = Arg.NativeType.of(pType.type(fields)); args.add(pType == ParamType.STRUCT @@ -232,7 +236,6 @@ public class TestVarArgs extends CallGeneratorHelper { enum NativeType { INT, - FLOAT, DOUBLE, POINTER, S_I, @@ -325,7 +328,7 @@ public class TestVarArgs extends CallGeneratorHelper { public static NativeType of(String type) { return NativeType.valueOf(switch (type) { case "int" -> "INT"; - case "float" -> "FLOAT"; + case "float" -> "DOUBLE"; // promote case "double" -> "DOUBLE"; case "void*" -> "POINTER"; default -> type.substring("struct ".length()); diff --git a/test/jdk/java/foreign/libVarArgs.c b/test/jdk/java/foreign/libVarArgs.c index a3d0938ae29..e1a14e5afb7 100644 --- a/test/jdk/java/foreign/libVarArgs.c +++ b/test/jdk/java/foreign/libVarArgs.c @@ -41,7 +41,6 @@ typedef struct { enum NativeType { T_INT, - T_FLOAT, T_DOUBLE, T_POINTER, T_S_I, @@ -141,7 +140,6 @@ EXPORT void varargs(call_info* info, int num, ...) { int id = info->argids[i]; switch (id) { CASE(T_INT, int) - CASE(T_FLOAT, double) // vararg float is promoted to double per C spec CASE(T_DOUBLE, double) CASE(T_POINTER, void*) CASE(T_S_I, struct S_I)