Skip to content

Commit

Permalink
Handle arrays in structures with differing size
Browse files Browse the repository at this point in the history
libffi uses ffi_type structures to describe arrays and structures passed
to native. JNA builds these structures and caches them using the
associated classes as keys. For arrays this leads to the situation where
only a single ffi_type was stored for all sizes of the same base type.
This in turn causes wrong behavior in libffi.
  • Loading branch information
matthiasblaesing committed May 11, 2022
1 parent 19daca8 commit ed019cc
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 28 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Expand Up @@ -12,6 +12,7 @@ Features

Bug Fixes
---------
* [#1438](https://github.com/java-native-access/jna/pull/1438): Handle arrays in structures with differing size - [@matthiasblaesing](https://github.com/matthiasblaesing).

Important Changes
-----------------
Expand Down
20 changes: 20 additions & 0 deletions native/testlib.c
Expand Up @@ -1058,6 +1058,26 @@ returnClass(JNIEnv *env, jobject arg) {
return (*env)->GetObjectClass(env, arg);
}

typedef struct{
double t1[3];
double t2[4];
double t3[5];
} DemoStructureDifferentArrayLengths;

EXPORT DemoStructureDifferentArrayLengths
returnLastElementOfComponentsDSDAL(DemoStructureDifferentArrayLengths ts, int debug) {
DemoStructureDifferentArrayLengths result;
result.t1[2] = ts.t1[2];
result.t2[3] = ts.t2[3];
result.t3[4] = ts.t3[4];
if(debug) {
printf("DemoStructureDifferentArrayLengths.t1[2]: %1.3f\n", result.t1[2]);
printf("DemoStructureDifferentArrayLengths.t2[3]: %1.3f\n", result.t2[3]);
printf("DemoStructureDifferentArrayLengths.t3[4]: %1.3f\n", result.t3[4]);
}
return result;
}

#ifdef __cplusplus
}
#endif
82 changes: 54 additions & 28 deletions src/com/sun/jna/Structure.java
Expand Up @@ -1955,7 +1955,7 @@ public static class size_t extends IntegerType {
public size_t(long value) { super(Native.SIZE_T_SIZE, value); }
}

private static final Map<Class, FFIType> typeInfoMap = new WeakHashMap<Class, FFIType>();
private static final Map<Class, Map<Integer,FFIType>> typeInfoMap = new WeakHashMap<Class, Map<Integer,FFIType>>();
private static final Map<Class, FFIType> unionHelper = new WeakHashMap<Class, FFIType>();
private static final Map<Pointer, FFIType> ffiTypeInfo = new HashMap<Pointer, FFIType>();

Expand Down Expand Up @@ -2018,29 +2018,29 @@ private static boolean isFloatType(FFIType type) {
for(FFIType f: ffiTypeInfo.values()) {
f.read();
}
typeInfoMap.put(void.class, ffiTypeInfo.get(FFITypes.ffi_type_void));
typeInfoMap.put(Void.class, ffiTypeInfo.get(FFITypes.ffi_type_void));
typeInfoMap.put(float.class, ffiTypeInfo.get(FFITypes.ffi_type_float));
typeInfoMap.put(Float.class, ffiTypeInfo.get(FFITypes.ffi_type_float));
typeInfoMap.put(double.class, ffiTypeInfo.get(FFITypes.ffi_type_double));
typeInfoMap.put(Double.class, ffiTypeInfo.get(FFITypes.ffi_type_double));
typeInfoMap.put(long.class, ffiTypeInfo.get(FFITypes.ffi_type_sint64));
typeInfoMap.put(Long.class, ffiTypeInfo.get(FFITypes.ffi_type_sint64));
typeInfoMap.put(int.class, ffiTypeInfo.get(FFITypes.ffi_type_sint32));
typeInfoMap.put(Integer.class, ffiTypeInfo.get(FFITypes.ffi_type_sint32));
typeInfoMap.put(short.class, ffiTypeInfo.get(FFITypes.ffi_type_sint16));
typeInfoMap.put(Short.class, ffiTypeInfo.get(FFITypes.ffi_type_sint16));
storeTypeInfo(void.class, ffiTypeInfo.get(FFITypes.ffi_type_void));
storeTypeInfo(Void.class, ffiTypeInfo.get(FFITypes.ffi_type_void));
storeTypeInfo(float.class, ffiTypeInfo.get(FFITypes.ffi_type_float));
storeTypeInfo(Float.class, ffiTypeInfo.get(FFITypes.ffi_type_float));
storeTypeInfo(double.class, ffiTypeInfo.get(FFITypes.ffi_type_double));
storeTypeInfo(Double.class, ffiTypeInfo.get(FFITypes.ffi_type_double));
storeTypeInfo(long.class, ffiTypeInfo.get(FFITypes.ffi_type_sint64));
storeTypeInfo(Long.class, ffiTypeInfo.get(FFITypes.ffi_type_sint64));
storeTypeInfo(int.class, ffiTypeInfo.get(FFITypes.ffi_type_sint32));
storeTypeInfo(Integer.class, ffiTypeInfo.get(FFITypes.ffi_type_sint32));
storeTypeInfo(short.class, ffiTypeInfo.get(FFITypes.ffi_type_sint16));
storeTypeInfo(Short.class, ffiTypeInfo.get(FFITypes.ffi_type_sint16));
FFIType ctype = Native.WCHAR_SIZE == 2
? ffiTypeInfo.get(FFITypes.ffi_type_uint16) : ffiTypeInfo.get(FFITypes.ffi_type_uint32);
typeInfoMap.put(char.class, ctype);
typeInfoMap.put(Character.class, ctype);
typeInfoMap.put(byte.class, ffiTypeInfo.get(FFITypes.ffi_type_sint8));
typeInfoMap.put(Byte.class, ffiTypeInfo.get(FFITypes.ffi_type_sint8));
typeInfoMap.put(Pointer.class, ffiTypeInfo.get(FFITypes.ffi_type_pointer));
typeInfoMap.put(String.class, ffiTypeInfo.get(FFITypes.ffi_type_pointer));
typeInfoMap.put(WString.class, ffiTypeInfo.get(FFITypes.ffi_type_pointer));
typeInfoMap.put(boolean.class, ffiTypeInfo.get(FFITypes.ffi_type_uint32));
typeInfoMap.put(Boolean.class, ffiTypeInfo.get(FFITypes.ffi_type_uint32));
storeTypeInfo(char.class, ctype);
storeTypeInfo(Character.class, ctype);
storeTypeInfo(byte.class, ffiTypeInfo.get(FFITypes.ffi_type_sint8));
storeTypeInfo(Byte.class, ffiTypeInfo.get(FFITypes.ffi_type_sint8));
storeTypeInfo(Pointer.class, ffiTypeInfo.get(FFITypes.ffi_type_pointer));
storeTypeInfo(String.class, ffiTypeInfo.get(FFITypes.ffi_type_pointer));
storeTypeInfo(WString.class, ffiTypeInfo.get(FFITypes.ffi_type_pointer));
storeTypeInfo(boolean.class, ffiTypeInfo.get(FFITypes.ffi_type_uint32));
storeTypeInfo(Boolean.class, ffiTypeInfo.get(FFITypes.ffi_type_uint32));
}
// From ffi.h
private static final int FFI_TYPE_STRUCT = 13;
Expand Down Expand Up @@ -2139,7 +2139,9 @@ private void init(Pointer[] els) {
/** Obtain a pointer to the native FFI type descriptor for the given object. */
static FFIType get(Object obj) {
if (obj == null)
return typeInfoMap.get(Pointer.class);
synchronized (typeInfoMap) {
return getTypeInfo(Pointer.class, 0);
}
if (obj instanceof Class)
return get(null, (Class<?>)obj);
return get(obj, obj.getClass());
Expand All @@ -2154,23 +2156,23 @@ private static FFIType get(Object obj, Class<?> cls) {
}
}
synchronized(typeInfoMap) {
FFIType o = typeInfoMap.get(cls);
FFIType o = getTypeInfo(cls, cls.isArray() ? Array.getLength(obj) : 0);
if (o != null) {
return o;
}
if ((Platform.HAS_BUFFERS && Buffer.class.isAssignableFrom(cls))
|| Callback.class.isAssignableFrom(cls)) {
typeInfoMap.put(cls, typeInfoMap.get(Pointer.class));
return typeInfoMap.get(Pointer.class);
return typeInfoMap.get(Pointer.class).get(0);
}
if (Structure.class.isAssignableFrom(cls)) {
if (obj == null) obj = newInstance((Class<? extends Structure>) cls, PLACEHOLDER_MEMORY);
if (ByReference.class.isAssignableFrom(cls)) {
typeInfoMap.put(cls, typeInfoMap.get(Pointer.class));
return typeInfoMap.get(Pointer.class);
return typeInfoMap.get(Pointer.class).get(0);
}
FFIType type = new FFIType((Structure)obj);
typeInfoMap.put(cls, type);
storeTypeInfo(cls, type);
return type;
}
if (NativeMapped.class.isAssignableFrom(cls)) {
Expand All @@ -2180,12 +2182,36 @@ private static FFIType get(Object obj, Class<?> cls) {
if (cls.isArray()) {
FFIType type = new FFIType(obj, cls);
// Store it in the map to prevent premature GC of type info
typeInfoMap.put(cls, type);
storeTypeInfo(cls, Array.getLength(obj), type);
return type;
}
throw new IllegalArgumentException("Unsupported type " + cls);
}
}

private static FFIType getTypeInfo(Class clazz, int elementCount) {
Map<Integer,FFIType> typeMap = typeInfoMap.get(clazz);
if(typeMap != null) {
return typeMap.get(elementCount);
} else {
return null;
}
}

private static void storeTypeInfo(Class clazz, FFIType type) {
storeTypeInfo(clazz, 0, type);
}

private static void storeTypeInfo(Class clazz, int elementCount, FFIType type) {
synchronized (typeInfoMap) {
Map<Integer,FFIType> typeMap = typeInfoMap.get(clazz);
if(typeMap == null) {
typeMap = new HashMap<Integer,FFIType>();
typeInfoMap.put(clazz, typeMap);
}
typeMap.put(elementCount, type);
}
}
}

private static class AutoAllocated extends Memory {
Expand Down
1 change: 1 addition & 0 deletions test/com/sun/jna/DirectStructureByValueTest.java
Expand Up @@ -36,6 +36,7 @@ public static class DirectTestLibrary implements TestLibrary {
public native int testStructureByValueArgument32(ByValue32 arg);
public native long testStructureByValueArgument64(ByValue64 arg);
public native long testStructureByValueArgument128(ByValue128 arg);
public native DemoStructureDifferentArrayLengths.ByValue returnLastElementOfComponentsDSDAL(DemoStructureDifferentArrayLengths.ByValue arg, int debug);
static {
Native.register("testlib");
}
Expand Down
30 changes: 30 additions & 0 deletions test/com/sun/jna/StructureByValueTest.java
Expand Up @@ -43,6 +43,17 @@ protected List<String> getFieldOrder() {
return FIELDS;
}
}

@Structure.FieldOrder({"t1", "t2", "t3"})
public static class DemoStructureDifferentArrayLengths extends Structure {

public static class ByValue extends DemoStructureDifferentArrayLengths implements Structure.ByValue {
}
public double t1[] = new double[3];
public double t2[] = new double[4];
public double t3[] = new double[5];
}

public void testNativeMappedInByValue() {
new TestNativeMappedInStructure.ByValue();
}
Expand All @@ -53,6 +64,7 @@ public interface TestLibrary extends Library {
int testStructureByValueArgument32(ByValue32 arg);
long testStructureByValueArgument64(ByValue64 arg);
long testStructureByValueArgument128(ByValue128 arg);
DemoStructureDifferentArrayLengths.ByValue returnLastElementOfComponentsDSDAL(DemoStructureDifferentArrayLengths.ByValue ts, int debug);
}

TestLibrary lib;
Expand Down Expand Up @@ -145,4 +157,22 @@ public void testStructureArgByValue128() {
assertEquals("Failed to pass 128-bit struct by value",
2*DATA, lib.testStructureByValueArgument128(data));
}

public void testStructureDifferentArrayLengths() {
// returnLastElementOfComponentsDSDAL copies the last element of the
// components of t1, t2 and t3, which are double arrays with lengths
// 3, 4 and 5. In the observed case JNA created ffi_types for primitive
// arrays with wrong element count (the first array definition was used)

DemoStructureDifferentArrayLengths.ByValue ts = new DemoStructureDifferentArrayLengths.ByValue();
ts.t1 = new double[]{1, 1, 1};
ts.t2 = new double[]{2, 2, 2, 2};
ts.t3 = new double[]{3, 3, 3, 3, 3};

DemoStructureDifferentArrayLengths.ByValue result = lib.returnLastElementOfComponentsDSDAL(ts, 0);

assertEquals(1.0d, result.t1[2], 0.1d);
assertEquals(2.0d, result.t2[3], 0.1d);
assertEquals(3.0d, result.t3[4], 0.1d);
}
}

0 comments on commit ed019cc

Please sign in to comment.