8308031: Linkers should reject unpromoted variadic parameters

Reviewed-by: mcimadamore
This commit is contained in:
Jorn Vernee 2023-06-07 12:14:55 +00:00
parent 16ebf47fe3
commit fa791119f0
11 changed files with 138 additions and 24 deletions

View File

@ -365,11 +365,41 @@ import java.util.stream.Stream;
*
* <h3 id="variadic-funcs">Variadic functions</h3>
*
* 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 <em>specialized</em> 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:
* <ol>
* <li>With a trailing ellipsis ({@code ...}) at the end of the formal parameter list, such as: {@code void foo(int x, ...);}</li>
* <li>With an empty formal parameter list, called a prototype-less function, such as: {@code void foo();}</li>
* </ol>
* The arguments passed in place of the ellipsis, or the arguments passed to a prototype-less function are called
* <em>variadic arguments</em>. Variadic functions are, essentially, templates that can be <em>specialized</em> into multiple
* non-variadic functions by replacing the {@code ...} or empty formal parameter list with a list of <em>variadic parameters</em>
* of a fixed number and type.
* <p>
* It should be noted that values passed as variadic arguments undergo default argument promotion in C. For instance, the
* following argument promotions are applied:
* <ul>
* <li>{@code _Bool} -> {@code unsigned int}</li>
* <li>{@code [signed] char} -> {@code [signed] int}</li>
* <li>{@code [signed] short} -> {@code [signed] int}</li>
* <li>{@code float} -> {@code double}</li>
* </ul>
* 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.
* <p>
* 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 <em>variadic argument layouts</em>. For a prototype-less function, the index passed to
* {@link Linker.Option#firstVariadicArg(int)} should always be {@code 0}.
* <p>
* 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.
* <p>
* A well-known variadic function is the {@code printf} function, defined in the C standard library:
*

View File

@ -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<MemoryLayout> argumentLayouts = function.argumentLayouts();
List<MemoryLayout> 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);

View File

@ -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;

View File

@ -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<MemoryLayout> 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<MemoryLayout> argLayouts,

View File

@ -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();

View File

@ -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));
}

View File

@ -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;

View File

@ -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<Object[]> 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"
});
}

View File

@ -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

View File

@ -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<Arg> 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());

View File

@ -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)