refactor(core/rust): propagate unsafety of raise_exception

core/embed/rust/src/micropython/runtime.rs

@@ -6,8 +6,10 @@ use super::ffi;
 
 /// Raise a micropython exception via NLR jump.
 /// Jumps directly out of the context without running any destructors,
-/// finalizers, etc. This is very likely to break a lot of Rust's assumptions.
-/// Should only be called in places where you really don't care anymore.
+/// finalizers, etc. This is very likely to break a lot of Rust's assumptions:
+/// in particular, _any_ jumping over Rust code is currently considered
+/// undefined. See full discussion at https://github.com/rust-lang/rfcs/issues/2625
+/// Should only be called at the boundary which would otherwise return to C.
 pub unsafe fn raise_exception(err: Error) -> ! {
     unsafe {
         // SAFETY:
@@ -88,7 +90,7 @@ mod tests {
 
     #[test]
     fn except_catches_raised() {
-        let result = catch_exception(|| raise_exception(Error::TypeError));
+        let result = catch_exception(|| unsafe { raise_exception(Error::TypeError) });
         assert!(result.is_err());
     }
 }

core/embed/rust/src/protobuf/decode.rs

@@ -17,27 +17,29 @@ use super::{
 
 #[no_mangle]
 pub extern "C" fn protobuf_type_for_name(name: Obj) -> Obj {
-    util::try_or_raise(|| {
+    let block = || {
         let name = Qstr::try_from(name)?;
         let def = MsgDef::for_name(name.to_u16()).ok_or_else(|| Error::KeyError(name.into()))?;
         let obj = MsgDefObj::alloc(def)?.into();
         Ok(obj)
-    })
+    };
+    unsafe { util::try_or_raise(block) }
 }
 
 #[no_mangle]
 pub extern "C" fn protobuf_type_for_wire(wire_id: Obj) -> Obj {
-    util::try_or_raise(|| {
+    let block = || {
         let wire_id = u16::try_from(wire_id)?;
         let def = MsgDef::for_wire_id(wire_id).ok_or_else(|| Error::KeyError(wire_id.into()))?;
         let obj = MsgDefObj::alloc(def)?.into();
         Ok(obj)
-    })
+    };
+    unsafe { util::try_or_raise(block) }
 }
 
 #[no_mangle]
 pub extern "C" fn protobuf_decode(buf: Obj, msg_def: Obj, enable_experimental: Obj) -> Obj {
-    util::try_or_raise(|| {
+    let block = || {
         let buf = Buffer::try_from(buf)?;
         let def = Gc::<MsgDefObj>::try_from(msg_def)?;
         let enable_experimental = bool::try_from(enable_experimental)?;
@@ -57,7 +59,8 @@ pub extern "C" fn protobuf_decode(buf: Obj, msg_def: Obj, enable_experimental: O
 
         let obj = decoder.message_from_stream(stream, def.msg())?;
         Ok(obj)
-    })
+    };
+    unsafe { util::try_or_raise(block) }
 }
 
 pub struct Decoder {

core/embed/rust/src/protobuf/encode.rs

@@ -22,7 +22,7 @@ use super::{
 
 #[no_mangle]
 pub extern "C" fn protobuf_len(obj: Obj) -> Obj {
-    util::try_or_raise(|| {
+    let block = || {
         let obj = Gc::<MsgObj>::try_from(obj)?;
 
         let stream = &mut CounterStream { len: 0 };
@@ -30,12 +30,13 @@ pub extern "C" fn protobuf_len(obj: Obj) -> Obj {
         Encoder.encode_message(stream, &obj.def(), &obj)?;
 
         Ok(stream.len.try_into()?)
-    })
+    };
+    unsafe { util::try_or_raise(block) }
 }
 
 #[no_mangle]
 pub extern "C" fn protobuf_encode(buf: Obj, obj: Obj) -> Obj {
-    util::try_or_raise(|| {
+    let block = || {
         let obj = Gc::<MsgObj>::try_from(obj)?;
 
         // We assume there are no other refs into `buf` at this point. This specifically
@@ -46,7 +47,8 @@ pub extern "C" fn protobuf_encode(buf: Obj, obj: Obj) -> Obj {
         Encoder.encode_message(stream, &obj.def(), &obj)?;
 
         Ok(stream.len().try_into()?)
-    })
+    };
+    unsafe { util::try_or_raise(block) }
 }
 
 pub struct Encoder;

core/embed/rust/src/protobuf/obj.rs

@@ -126,7 +126,7 @@ impl TryFrom<Obj> for Gc<MsgObj> {
 }
 
 unsafe extern "C" fn msg_obj_attr(self_in: Obj, attr: ffi::qstr, dest: *mut Obj) {
-    util::try_or_raise(|| {
+    let block = || {
         let mut this = Gc::<MsgObj>::try_from(self_in)?;
         let attr = Qstr::from_u16(attr as _);
 
@@ -142,7 +142,8 @@ unsafe extern "C" fn msg_obj_attr(self_in: Obj, attr: ffi::qstr, dest: *mut Obj)
             }
             Ok(())
         }
-    })
+    };
+    unsafe { util::try_or_raise(block) }
 }
 
 #[repr(C)]
@@ -200,7 +201,7 @@ impl TryFrom<Obj> for Gc<MsgDefObj> {
 }
 
 unsafe extern "C" fn msg_def_obj_attr(self_in: Obj, attr: ffi::qstr, dest: *mut Obj) {
-    util::try_or_raise(|| {
+    let block = || {
         let this = Gc::<MsgDefObj>::try_from(self_in)?;
         let attr = Qstr::from_u16(attr as _);
 
@@ -242,7 +243,8 @@ unsafe extern "C" fn msg_def_obj_attr(self_in: Obj, attr: ffi::qstr, dest: *mut
             }
         }
         Ok(())
-    });
+    };
+    unsafe { util::try_or_raise(block) }
 }
 
 unsafe extern "C" fn msg_def_obj_call(
@@ -251,25 +253,27 @@ unsafe extern "C" fn msg_def_obj_call(
     n_kw: usize,
     args: *const Obj,
 ) -> Obj {
-    util::try_with_args_and_kwargs_inline(n_args, n_kw, args, |_args, kwargs| {
+    let block = |_args: &[Obj], kwargs: &Map| {
         let this = Gc::<MsgDefObj>::try_from(self_in)?;
         let decoder = Decoder {
             enable_experimental: true,
         };
         let obj = decoder.message_from_values(kwargs, this.msg())?;
         Ok(obj)
-    })
+    };
+    unsafe { util::try_with_args_and_kwargs_inline(n_args, n_kw, args, block) }
 }
 
 unsafe extern "C" fn msg_def_obj_is_type_of(self_in: Obj, obj: Obj) -> Obj {
-    util::try_or_raise(|| {
+    let block = || {
         let this = Gc::<MsgDefObj>::try_from(self_in)?;
         let msg = Gc::<MsgObj>::try_from(obj);
         match msg {
             Ok(msg) if msg.msg_offset == this.def.offset => Ok(Obj::const_true()),
             _ => Ok(Obj::const_false()),
         }
-    })
+    };
+    unsafe { util::try_or_raise(block) }
 }
 
 static MSG_DEF_OBJ_IS_TYPE_OF_OBJ: ffi::mp_obj_fun_builtin_fixed_t =

core/embed/rust/src/util.rs

@@ -9,25 +9,44 @@ use crate::{
     },
 };
 
-pub fn try_or_raise<T>(func: impl FnOnce() -> Result<T, Error>) -> T {
-    func().unwrap_or_else(|err| raise_exception(err))
-}
-
-pub fn try_with_kwargs(kwargs: *const Map, func: impl FnOnce(&Map) -> Result<Obj, Error>) -> Obj {
-    try_or_raise(|| {
-        let kwargs = unsafe { kwargs.as_ref() }.ok_or(Error::MissingKwargs)?;
-
-        func(kwargs)
+/// Perform a call and convert errors into a raised micropython exception.
+/// Should only called when returning from Rust to C.
+/// See `raise_exception` for details.
+pub unsafe fn try_or_raise<T>(func: impl FnOnce() -> Result<T, Error>) -> T {
+    func().unwrap_or_else(|err| unsafe {
+        raise_exception(err);
     })
 }
 
-pub fn try_with_args_and_kwargs(
+/// Extract kwargs from a C call and pass them into Rust. Raise exception if an
+/// error occurs.
+/// TODO when does uPy call this?
+/// Should only called when returning from Rust to C.
+/// See `raise_exception` for details.
+pub unsafe fn try_with_kwargs(
+    kwargs: *const Map,
+    func: impl FnOnce(&Map) -> Result<Obj, Error>,
+) -> Obj {
+    let block = || {
+        let kwargs = unsafe { kwargs.as_ref() }.ok_or(Error::MissingKwargs)?;
+
+        func(kwargs)
+    };
+    unsafe { try_or_raise(block) }
+}
+
+/// Extract args and kwargs from a C call and pass them into Rust. Raise
+/// exception if an error occurs.
+/// TODO when would uPy call this?
+/// Should only called when returning from Rust to C.
+/// See `raise_exception` for details.
+pub unsafe fn try_with_args_and_kwargs(
     n_args: usize,
     args: *const Obj,
     kwargs: *const Map,
     func: impl FnOnce(&[Obj], &Map) -> Result<Obj, Error>,
 ) -> Obj {
-    try_or_raise(|| {
+    let block = || {
         let args = if args.is_null() {
             &[]
         } else {
@@ -36,16 +55,21 @@ pub fn try_with_args_and_kwargs(
         let kwargs = unsafe { kwargs.as_ref() }.ok_or(Error::MissingKwargs)?;
 
         func(args, kwargs)
-    })
+    };
+    unsafe { try_or_raise(block) }
 }
 
-pub fn try_with_args_and_kwargs_inline(
+/// Extract args and kwargs from a C call where args and kwargs are inlined, and
+/// pass them into Rust. Raise exception if an error occurs.
+/// Should only called when returning from Rust to C.
+/// See `raise_exception` for details.
+pub unsafe fn try_with_args_and_kwargs_inline(
     n_args: usize,
     n_kw: usize,
     args: *const Obj,
     func: impl FnOnce(&[Obj], &Map) -> Result<Obj, Error>,
 ) -> Obj {
-    try_or_raise(|| {
+    let block = || {
         let args_slice: &[Obj];
         let kwargs_slice: &[MapElem];
 
@@ -59,5 +83,6 @@ pub fn try_with_args_and_kwargs_inline(
 
         let kw_map = Map::from_fixed(kwargs_slice);
         func(args_slice, &kw_map)
-    })
+    };
+    unsafe { try_or_raise(block) }
 }