runtime/thread: fix unsoundness in native method calls

Author: Serial-ATA

Cargo.toml

@@ -100,7 +100,7 @@ vm_symbols = { path = "generators/vm_symbols" }
 native-macros = { path = "native/macros" }
 libc = "0.2"
 windows = "0.61.3"
-libffi = "4.1.1"
+libffi = "5.0.0"
 
 byteorder = "1.5.0"
 byte-slice-cast = "1.2.2"

runtime/src/objects/method/mod.rs

@@ -660,19 +660,19 @@ impl Method {
 }
 
 #[cfg(feature = "libffi")]
-pub struct PreparedCfi {
+pub struct PreparedCfi<'a> {
 	pub cfi: libffi::middle::Cif,
-	pub args: Vec<libffi::middle::Arg>,
+	pub args: Vec<libffi::middle::Arg<'a>>,
 }
 
 impl Method {
 	#[cfg(feature = "libffi")]
-	pub fn prepare_cfi(
+	pub fn prepare_cfi<'a>(
 		&'static self,
-		env: *mut jni::sys::JNIEnv,
-		class: jni::sys::jclass,
-		locals: &crate::stack::local_stack::LocalStack,
-	) -> PreparedCfi {
+		env: &'a *mut jni::sys::JNIEnv,
+		receiver: libffi::middle::Arg<'a>,
+		locals: &mut crate::stack::local_stack::LocalStackIter<'a>,
+	) -> PreparedCfi<'a> {
 		trait IntoFfiType {
 			fn into_ffi_type(&self) -> Type;
 		}
@@ -694,6 +694,23 @@ impl Method {
 			}
 		}
 
+		trait OperandArgExt {
+			fn as_arg(&self) -> Arg<'_>;
+		}
+
+		impl OperandArgExt for Operand<Reference> {
+			fn as_arg(&self) -> Arg<'_> {
+				match self {
+					Operand::Int(val) => Arg::new(val),
+					Operand::Float(val) => Arg::new(val),
+					Operand::Double(val) => Arg::new(val),
+					Operand::Long(val) => Arg::new(val),
+					Operand::Reference(val) => Arg::new(val),
+					Operand::Empty => unreachable!(),
+				}
+			}
+		}
+
 		use libffi::middle::{Arg, Type};
 
 		let num_args = self.descriptor.parameters.len() + if self.is_static() { 2 } else { 1 };
@@ -702,56 +719,50 @@ impl Method {
 		let mut args = Vec::with_capacity(num_args);
 
 		// env
-		args.push(Arg::new(&env));
+		args.push(Arg::new(env));
+		args_types.push(Type::pointer());
+
+		// jclass or jobject
+		args.push(receiver);
 		args_types.push(Type::pointer());
-		if self.is_static() {
-			// jclass
-			args.push(Arg::new(&class));
-			args_types.push(Type::pointer());
-		}
 
-		for (arg_ty, arg) in self.descriptor.parameters.iter().zip(locals.iter()) {
+		for (arg_ty, operand) in self.descriptor.parameters.iter().zip(locals) {
 			match arg_ty {
+				FieldType::Void => {
+					args_types.push(Type::void());
+					continue;
+				},
+
 				FieldType::Byte => {
-					args.push(Arg::new(&(arg.expect_int() as i8)));
 					args_types.push(Type::i8());
 				},
 				FieldType::Character => {
-					args.push(Arg::new(&(arg.expect_int() as u16)));
 					args_types.push(Type::u16());
 				},
 				FieldType::Double => {
-					args.push(Arg::new(&(arg.expect_double())));
 					args_types.push(Type::f64());
 				},
 				FieldType::Float => {
-					args.push(Arg::new(&(arg.expect_float())));
 					args_types.push(Type::f32());
 				},
 				FieldType::Integer => {
-					args.push(Arg::new(&(arg.expect_int())));
 					args_types.push(Type::i32());
 				},
 				FieldType::Long => {
-					args.push(Arg::new(&(arg.expect_long())));
 					args_types.push(Type::i64());
 				},
 				FieldType::Short => {
-					args.push(Arg::new(&(arg.expect_int() as i16)));
 					args_types.push(Type::i16());
 				},
 				FieldType::Boolean => {
-					args.push(Arg::new(&(arg.expect_int() != 0)));
 					args_types.push(Type::u8());
 				},
 				FieldType::Object(_) | FieldType::Array(_) => {
-					args.push(Arg::new(&(unsafe { arg.expect_reference().raw() })));
 					args_types.push(Type::pointer());
 				},
-				FieldType::Void => {
-					args_types.push(Type::void());
-				},
 			}
+
+			args.push(operand.as_arg());
 		}
 
 		let ret_ty = self.descriptor.return_type.into_ffi_type();

runtime/src/stack/local_stack.rs

@@ -68,7 +68,7 @@ impl LocalStack {
 }
 
 impl<'a> IntoIterator for &'a LocalStack {
-	type Item = Operand<Reference>;
+	type Item = &'a Operand<Reference>;
 	type IntoIter = LocalStackIter<'a>;
 
 	fn into_iter(self) -> Self::IntoIter {
@@ -84,8 +84,8 @@ pub struct LocalStackIter<'a> {
 	remaining: usize,
 }
 
-impl Iterator for LocalStackIter<'_> {
-	type Item = Operand<Reference>;
+impl<'a> Iterator for LocalStackIter<'a> {
+	type Item = &'a Operand<Reference>;
 
 	fn next(&mut self) -> Option<Self::Item> {
 		match self.inner.next() {
@@ -98,7 +98,7 @@ impl Iterator for LocalStackIter<'_> {
 				}
 
 				self.remaining -= 1;
-				Some(operand.clone())
+				Some(operand)
 			},
 		}
 	}

runtime/src/thread/mod.rs

@@ -24,6 +24,7 @@ use crate::thread::frame::native::NativeFrame;
 use crate::{classes, globals, java_call};
 
 use std::cell::{Cell, SyncUnsafeCell, UnsafeCell};
+use std::mem::MaybeUninit;
 use std::sync::atomic::{AtomicBool, AtomicIsize, Ordering};
 use std::thread::JoinHandle;
 
@@ -477,8 +478,29 @@ impl JavaThread {
 					jboolean, jbyte, jchar, jdouble, jfloat, jint, jlong, jobject, jshort,
 				};
 				use libffi::low::CodePtr;
+				use libffi::middle::Arg;
 
-				let cfi = method.prepare_cfi(self.env().raw(), method.class().into_jni(), &locals);
+				let env = self.env().raw();
+				let target_class = method.class().into_jni();
+
+				let mut locals = locals.iter();
+
+				// To keep the receiver live
+				let mut this = MaybeUninit::uninit();
+
+				let receiver;
+				if method.is_static() {
+					receiver = Arg::new(&target_class);
+				} else {
+					let this_ref = locals
+						.next()
+						.expect("should have a receiver")
+						.expect_reference();
+					this.write(unsafe { this_ref.raw() });
+					receiver = Arg::new(&this);
+				}
+
+				let cfi = method.prepare_cfi(&env, receiver, &mut locals);
 				ret = unsafe {
 					match method.descriptor.return_type {
 						FieldType::Byte => Some(Operand::Int(