From ed019cc53a805f97ff7d83bd80235f05f7e2c6a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Bl=C3=A4sing?= Date: Tue, 10 May 2022 22:31:33 +0200 Subject: [PATCH] Handle arrays in structures with differing size 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. --- CHANGES.md | 1 + native/testlib.c | 20 +++++ src/com/sun/jna/Structure.java | 82 ++++++++++++------- .../sun/jna/DirectStructureByValueTest.java | 1 + test/com/sun/jna/StructureByValueTest.java | 30 +++++++ 5 files changed, 106 insertions(+), 28 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index fef9d18c10..470456a22a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -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 ----------------- diff --git a/native/testlib.c b/native/testlib.c index 4195d88c3a..d438ffd750 100644 --- a/native/testlib.c +++ b/native/testlib.c @@ -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 diff --git a/src/com/sun/jna/Structure.java b/src/com/sun/jna/Structure.java index def60d7c17..b2457f69d7 100644 --- a/src/com/sun/jna/Structure.java +++ b/src/com/sun/jna/Structure.java @@ -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 typeInfoMap = new WeakHashMap(); + private static final Map> typeInfoMap = new WeakHashMap>(); private static final Map unionHelper = new WeakHashMap(); private static final Map ffiTypeInfo = new HashMap(); @@ -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; @@ -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()); @@ -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) 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)) { @@ -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 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 typeMap = typeInfoMap.get(clazz); + if(typeMap == null) { + typeMap = new HashMap(); + typeInfoMap.put(clazz, typeMap); + } + typeMap.put(elementCount, type); + } + } } private static class AutoAllocated extends Memory { diff --git a/test/com/sun/jna/DirectStructureByValueTest.java b/test/com/sun/jna/DirectStructureByValueTest.java index 6cc1c09d54..ab04f14e59 100644 --- a/test/com/sun/jna/DirectStructureByValueTest.java +++ b/test/com/sun/jna/DirectStructureByValueTest.java @@ -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"); } diff --git a/test/com/sun/jna/StructureByValueTest.java b/test/com/sun/jna/StructureByValueTest.java index 6f1c47db72..497ed68462 100644 --- a/test/com/sun/jna/StructureByValueTest.java +++ b/test/com/sun/jna/StructureByValueTest.java @@ -43,6 +43,17 @@ protected List 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(); } @@ -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; @@ -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); + } } \ No newline at end of file