Add workaround for JNA bug when passing small structs by value.

The current version of JNA appears to have a bug when passing small structs
as arguments by value, on arm64 platforms, when the function call has too
many arguments to pass the struct in registers. It turns out that our generated
code does this a lot thanks to the way we pass around `RustBuffer` structs.

This commit adds a temporary workaround for the issue, by padding our structs
to be larger than 16 bytes, which causes them to be passed as pointers rather
than by value.

See mozilla/application-services#3646 for investigation
of the JNA issue, and #334 to track
the work of removing this workaround once a fix is released in upstream JNA.

Author: rfk

uniffi/src/ffi/foreignbytes.rs

@@ -31,9 +31,30 @@ pub struct ForeignBytes {
     len: i32,
     /// The pointer to the foreign-owned bytes.
     data: *const u8,
+    /// This forces the struct to be larger than 16 bytes, as a temporary workaround for a bug in JNA.
+    /// See https://github.com/mozilla/uniffi-rs/issues/334 for details.
+    padding: i64,
+    padding2: i32,
 }
 
 impl ForeignBytes {
+    /// Creates a `ForeignBytes` from its constituent fields.
+    ///
+    /// This is intended mainly as an internal convenience function and should not
+    /// be used outside of this module.
+    ///
+    /// # Safety
+    ///
+    /// You must ensure that the raw parts uphold the documented invariants of this class.
+    pub unsafe fn from_raw_parts(data: *const u8, len: i32) -> Self {
+        Self {
+            len,
+            data,
+            padding: 0,
+            padding2: 0,
+        }
+    }
+
     /// View the foreign bytes as a `&[u8]`.
     ///
     /// # Panics
@@ -72,52 +93,37 @@ mod test {
     #[test]
     fn test_foreignbytes_access() {
         let v = vec![1u8, 2, 3];
-        let fbuf = ForeignBytes {
-            len: 3,
-            data: v.as_ptr(),
-        };
+        let fbuf = unsafe { ForeignBytes::from_raw_parts(v.as_ptr(), 3) };
         assert_eq!(fbuf.len(), 3);
         assert_eq!(fbuf.as_slice(), &[1u8, 2, 3]);
     }
 
     #[test]
     fn test_foreignbytes_empty() {
         let v = Vec::<u8>::new();
-        let fbuf = ForeignBytes {
-            len: 0,
-            data: v.as_ptr(),
-        };
+        let fbuf = unsafe { ForeignBytes::from_raw_parts(v.as_ptr(), 0) };
         assert_eq!(fbuf.len(), 0);
         assert_eq!(fbuf.as_slice(), &[0u8; 0]);
     }
 
     #[test]
     fn test_foreignbytes_null_means_empty() {
-        let fbuf = ForeignBytes {
-            len: 0,
-            data: std::ptr::null_mut(),
-        };
+        let fbuf = unsafe { ForeignBytes::from_raw_parts(std::ptr::null_mut(), 0) };
         assert_eq!(fbuf.as_slice(), &[0u8; 0]);
     }
 
     #[test]
     #[should_panic]
     fn test_foreignbytes_null_must_have_zero_length() {
-        let fbuf = ForeignBytes {
-            len: 12,
-            data: std::ptr::null_mut(),
-        };
+        let fbuf = unsafe { ForeignBytes::from_raw_parts(std::ptr::null_mut(), 12) };
         fbuf.as_slice();
     }
 
     #[test]
     #[should_panic]
     fn test_foreignbytes_provided_len_must_be_non_negative() {
         let v = vec![0u8, 1, 2];
-        let fbuf = ForeignBytes {
-            len: -1,
-            data: v.as_ptr(),
-        };
+        let fbuf = unsafe { ForeignBytes::from_raw_parts(v.as_ptr(), -1) };
         fbuf.as_slice();
     }
 }

uniffi/src/ffi/rustbuffer.rs

@@ -58,6 +58,9 @@ pub struct RustBuffer {
     len: i32,
     /// The pointer to the allocated buffer of the `Vec<u8>`.
     data: *mut u8,
+    /// This forces the struct to be larger than 16 bytes, as a temporary workaround for a bug in JNA.
+    /// See https://github.com/mozilla/uniffi-rs/issues/334 for details.
+    padding: i64,
 }
 
 impl RustBuffer {
@@ -69,6 +72,23 @@ impl RustBuffer {
         Self::from_vec(Vec::new())
     }
 
+    /// Creates a `RustBuffer` from its constituent fields.
+    ///
+    /// This is intended mainly as an internal convenience function and should not
+    /// be used outside of this module.
+    ///
+    /// # Safety
+    ///
+    /// You must ensure that the raw parts uphold the documented invariants of this class.
+    pub unsafe fn from_raw_parts(data: *mut u8, len: i32, capacity: i32) -> Self {
+        Self {
+            capacity,
+            len,
+            data,
+            padding: 0,
+        }
+    }
+
     /// Get the current length of the buffer, as a `usize`.
     ///
     /// This is mostly a helper function to convert the `i32` length field
@@ -119,11 +139,7 @@ impl RustBuffer {
         let capacity = i32::try_from(v.capacity()).expect("buffer capacity cannot fit into a i32.");
         let len = i32::try_from(v.len()).expect("buffer length cannot fit into a i32.");
         let mut v = std::mem::ManuallyDrop::new(v);
-        Self {
-            capacity,
-            len,
-            data: v.as_mut_ptr(),
-        }
+        unsafe { Self::from_raw_parts(v.as_mut_ptr(), len, capacity) }
     }
 
     /// Converts this `RustBuffer` back into an owned `Vec<u8>`.
@@ -177,11 +193,7 @@ impl Default for RustBuffer {
 unsafe impl crate::deps::ffi_support::IntoFfi for RustBuffer {
     type Value = Self;
     fn ffi_default() -> Self {
-        Self {
-            capacity: 0,
-            len: 0,
-            data: std::ptr::null_mut(),
-        }
+        unsafe { Self::from_raw_parts(std::ptr::null_mut(), 0, 0) }
     }
     fn into_ffi_value(self) -> Self::Value {
         self
@@ -220,34 +232,22 @@ mod test {
     #[test]
     fn test_rustbuffer_null_means_empty() {
         // This is how foreign-language code might cheaply indicate an empty buffer.
-        let rbuf = RustBuffer {
-            capacity: 0,
-            len: 0,
-            data: std::ptr::null_mut(),
-        };
+        let rbuf = unsafe { RustBuffer::from_raw_parts(std::ptr::null_mut(), 0, 0) };
         assert_eq!(rbuf.destroy_into_vec().as_slice(), &[0u8; 0]);
     }
 
     #[test]
     #[should_panic]
     fn test_rustbuffer_null_must_have_no_capacity() {
         // We guard against foreign-language code providing this kind of invalid struct.
-        let rbuf = RustBuffer {
-            capacity: 1,
-            len: 0,
-            data: std::ptr::null_mut(),
-        };
+        let rbuf = unsafe { RustBuffer::from_raw_parts(std::ptr::null_mut(), 0, 1) };
         rbuf.destroy_into_vec();
     }
     #[test]
     #[should_panic]
     fn test_rustbuffer_null_must_have_zero_length() {
         // We guard against foreign-language code providing this kind of invalid struct.
-        let rbuf = RustBuffer {
-            capacity: 0,
-            len: 12,
-            data: std::ptr::null_mut(),
-        };
+        let rbuf = unsafe { RustBuffer::from_raw_parts(std::ptr::null_mut(), 12, 0) };
         rbuf.destroy_into_vec();
     }
 
@@ -256,11 +256,7 @@ mod test {
     fn test_rustbuffer_provided_capacity_must_be_non_negative() {
         // We guard against foreign-language code providing this kind of invalid struct.
         let mut v = vec![0u8, 1, 2];
-        let rbuf = RustBuffer {
-            capacity: -7,
-            len: 3,
-            data: v.as_mut_ptr(),
-        };
+        let rbuf = unsafe { RustBuffer::from_raw_parts(v.as_mut_ptr(), 3, -7) };
         rbuf.destroy_into_vec();
     }
 
@@ -269,11 +265,7 @@ mod test {
     fn test_rustbuffer_provided_len_must_be_non_negative() {
         // We guard against foreign-language code providing this kind of invalid struct.
         let mut v = vec![0u8, 1, 2];
-        let rbuf = RustBuffer {
-            capacity: 3,
-            len: -1,
-            data: v.as_mut_ptr(),
-        };
+        let rbuf = unsafe { RustBuffer::from_raw_parts(v.as_mut_ptr(), -1, 3) };
         rbuf.destroy_into_vec();
     }
 
@@ -282,11 +274,7 @@ mod test {
     fn test_rustbuffer_provided_len_must_not_exceed_capacity() {
         // We guard against foreign-language code providing this kind of invalid struct.
         let mut v = vec![0u8, 1, 2];
-        let rbuf = RustBuffer {
-            capacity: 2,
-            len: 3,
-            data: v.as_mut_ptr(),
-        };
+        let rbuf = unsafe { RustBuffer::from_raw_parts(v.as_mut_ptr(), 3, 2) };
         rbuf.destroy_into_vec();
     }
 

uniffi_bindgen/src/bindings/kotlin/templates/ErrorTemplate.kt

@@ -113,7 +113,7 @@ internal open class {{e.name()}} : RustError() {
             {% for value in e.values() -%}
             {{loop.index}} -> return {{e.name()}}Exception.{{value}}(message) as E
             {% endfor -%}
-            else -> throw RuntimeException("Invalid error received...")
+            else -> throw RuntimeException("Invalid error received: $code, $message")
         }
     }
 }

uniffi_bindgen/src/bindings/kotlin/templates/RustBufferTemplate.kt

@@ -2,11 +2,13 @@
 // A rust-owned buffer is represented by its capacity, its current length, and a
 // pointer to the underlying data.
 
-@Structure.FieldOrder("capacity", "len", "data")
+@Structure.FieldOrder("capacity", "len", "data", "padding")
 open class RustBuffer : Structure() {
     @JvmField var capacity: Int = 0
     @JvmField var len: Int = 0
     @JvmField var data: Pointer? = null
+    // Ref https://github.com/mozilla/uniffi-rs/issues/334 for this weird "padding" field.
+    @JvmField var padding: Long = 0
 
     class ByValue : RustBuffer(), Structure.ByValue
 
@@ -37,10 +39,13 @@ open class RustBuffer : Structure() {
 // then we might as well copy it into a `RustBuffer`. But it's here for API
 // completeness.
 
-@Structure.FieldOrder("len", "data")
+@Structure.FieldOrder("len", "data", "padding", "padding2")
 open class ForeignBytes : Structure() {
     @JvmField var len: Int = 0
     @JvmField var data: Pointer? = null
+    // Ref https://github.com/mozilla/uniffi-rs/issues/334 for these weird "padding" fields.
+    @JvmField var padding: Long = 0
+    @JvmField var padding2: Int = 0
 
     class ByValue : ForeignBytes(), Structure.ByValue
 }
@@ -137,4 +142,4 @@ class RustBufferBuilder() {
             bbuf.put(v)
         }
     }
-}
+}

uniffi_bindgen/src/bindings/python/templates/RustBufferTemplate.py

@@ -4,6 +4,8 @@ class RustBuffer(ctypes.Structure):
         ("capacity", ctypes.c_int32),
         ("len", ctypes.c_int32),
         ("data", ctypes.POINTER(ctypes.c_char)),
+        # Ref https://github.com/mozilla/uniffi-rs/issues/334 for this weird "padding" field.
+        ("padding", ctypes.c_int64),
     ]
 
     @staticmethod
@@ -139,7 +141,10 @@ class ForeignBytes(ctypes.Structure):
     _fields_ = [
         ("len", ctypes.c_int32),
         ("data", ctypes.POINTER(ctypes.c_char)),
+        # Ref https://github.com/mozilla/uniffi-rs/issues/334 for these weird "padding" fields.
+        ("padding", ctypes.c_int64),
+        ("padding2", ctypes.c_int32),
     ]
 
     def __str__(self):
-        return "ForeignBytes(len={}, data={})".format(self.len, self.data[0:self.len])
+        return "ForeignBytes(len={}, data={})".format(self.len, self.data[0:self.len])

uniffi_bindgen/src/bindings/swift/templates/RustBufferTemplate.swift

@@ -6,7 +6,8 @@ extension RustBuffer {
                 {{ ci.ffi_rustbuffer_from_bytes().name() }}(ForeignBytes(bufferPointer: ptr), err)
             }
         }
-        self.init(capacity: rbuf.capacity, len: rbuf.len, data: rbuf.data)
+        // Ref https://github.com/mozilla/uniffi-rs/issues/334 for the extra "padding" arg.
+        self.init(capacity: rbuf.capacity, len: rbuf.len, data: rbuf.data, padding: 0)
     }
 
     // Frees the buffer in place.
@@ -20,6 +21,7 @@ extension RustBuffer {
 
 extension ForeignBytes {
     init(bufferPointer: UnsafeBufferPointer<UInt8>) {
-        self.init(len: Int32(bufferPointer.count), data: bufferPointer.baseAddress)
+        // Ref https://github.com/mozilla/uniffi-rs/issues/334 for the extra "padding" args.
+        self.init(len: Int32(bufferPointer.count), data: bufferPointer.baseAddress, padding: 0, padding2: 0)
     }
-}
+}

uniffi_bindgen/src/bindings/swift/templates/Template-Bridging-Header.h

@@ -11,12 +11,17 @@ typedef struct RustBuffer
     int32_t capacity;
     int32_t len;
     uint8_t *_Nullable data;
+    // Ref https://github.com/mozilla/uniffi-rs/issues/334 for this weird "padding" field.
+    int64_t padding;
 } RustBuffer;
 
 typedef struct ForeignBytes
 {
     int32_t len;
     const uint8_t *_Nullable data;
+    // Ref https://github.com/mozilla/uniffi-rs/issues/334 for these weird "padding" fields.
+    int64_t padding;
+    int32_t padding2;
 } ForeignBytes;
 
 // Error definitions