This is an automated email from the ASF dual-hosted git repository.

tqchen pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tvm-ffi.git


The following commit(s) were added to refs/heads/main by this push:
     new 644fbf79 [FIX][Rust] Raise panics at FFI boundaries as InternalError 
(#814)
644fbf79 is described below

commit 644fbf79112d8919add902d74be20b86a22df26b
Author: Gabriel Wu <[email protected]>
AuthorDate: Sat Sep 26 07:34:15 2026 +0800

    [FIX][Rust] Raise panics at FFI boundaries as InternalError (#814)
    
    ## Why
    
    A Rust function called through the tvm-ffi C ABI runs inside an `extern
    "C"` function, whether it is exported with
    `tvm_ffi_dll_export_typed_func!` or wrapped by `Function::from_typed` /
    `Function::from_packed`. A panic there cannot unwind into the caller, so
    the process aborts. This includes panics from indexing, `unwrap`, or
    checks such as `TensorView::strides`.
    
    Functions should still report errors by returning them. Catching panics
    is a safety net, not a way to raise errors.
    
    ## What
    
    - Both boundaries wrap the call in `catch_unwind` and keep their
    original `match` on the result. A caught panic becomes an error through
    the non-generic `function_internal::panic_to_error` and is raised as an
    `InternalError` with the panic message (`panicked: <message>`). The
    function stays usable after a panic.
    - `INTERNAL_ERROR` joins the other error kinds, matching the
    `InternalError` of C++ `TVM_FFI_ICHECK`.
    - The docs of the export macro and of `Function::from_packed` /
    `Function::from_typed` say to report errors by returning them, and that
    panicking is discouraged.
---
 rust/tvm-ffi/src/error.rs             |  1 +
 rust/tvm-ffi/src/function.rs          | 13 +++++++++-
 rust/tvm-ffi/src/function_internal.rs | 18 +++++++++++++-
 rust/tvm-ffi/src/lib.rs               |  2 +-
 rust/tvm-ffi/src/macros.rs            | 12 +++++++++-
 rust/tvm-ffi/tests/test_function.rs   | 45 +++++++++++++++++++++++++++++++++++
 6 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/rust/tvm-ffi/src/error.rs b/rust/tvm-ffi/src/error.rs
index f8b7e524..f75f18c3 100644
--- a/rust/tvm-ffi/src/error.rs
+++ b/rust/tvm-ffi/src/error.rs
@@ -47,6 +47,7 @@ pub const RUNTIME_ERROR: ErrorKind = 
ErrorKind("RuntimeError");
 pub const ATTRIBUTE_ERROR: ErrorKind = ErrorKind("AttributeError");
 pub const KEY_ERROR: ErrorKind = ErrorKind("KeyError");
 pub const INDEX_ERROR: ErrorKind = ErrorKind("IndexError");
+pub const INTERNAL_ERROR: ErrorKind = ErrorKind("InternalError");
 
 /// error object
 #[repr(C)]
diff --git a/rust/tvm-ffi/src/function.rs b/rust/tvm-ffi/src/function.rs
index d87e72a9..2c7b1827 100644
--- a/rust/tvm-ffi/src/function.rs
+++ b/rust/tvm-ffi/src/function.rs
@@ -81,7 +81,12 @@ impl<F: Fn(&[AnyView]) -> Result<Any> + 'static> 
CallbackFunctionObjImpl<F> {
     ) -> i32 {
         let this = &*(handle as *mut Self);
         let packed_args = std::slice::from_raw_parts(args as *const AnyView, 
num_args as usize);
-        let ret_value = (this.callback)(packed_args);
+        let ret_value = match 
std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+            (this.callback)(packed_args)
+        })) {
+            Ok(ret_value) => ret_value,
+            Err(payload) => 
Err(crate::function_internal::panic_to_error(payload)),
+        };
         match ret_value {
             Ok(value) => {
                 *result = Any::into_raw_ffi_any(value);
@@ -317,6 +322,9 @@ impl Function {
     ///
     /// # Returns
     /// * `Function` - The function
+    ///
+    /// Report errors by returning them. A panic in `func` is caught and raised
+    /// to the caller as an `InternalError`, but panicking is discouraged.
     pub fn from_packed<F>(func: F) -> Self
     where
         F: Fn(&[AnyView]) -> Result<Any> + 'static,
@@ -336,6 +344,9 @@ impl Function {
     ///
     /// # Returns
     /// * `Function` - The function
+    ///
+    /// Report errors by returning them. A panic in `func` is caught and raised
+    /// to the caller as an `InternalError`, but panicking is discouraged.
     pub fn from_typed<F, I, O>(func: F) -> Self
     where
         F: AsPackedCallable<I, O> + 'static,
diff --git a/rust/tvm-ffi/src/function_internal.rs 
b/rust/tvm-ffi/src/function_internal.rs
index fae1b46c..328766c7 100644
--- a/rust/tvm-ffi/src/function_internal.rs
+++ b/rust/tvm-ffi/src/function_internal.rs
@@ -17,7 +17,7 @@
  * under the License.
  */
 use crate::any::{Any, AnyView, ArgTryFromAnyView};
-use crate::error::Result;
+use crate::error::{Error, Result, INTERNAL_ERROR};
 use crate::object::ObjectRefCore;
 use crate::rvalue_ref::RValueRef;
 use crate::string::{Bytes, String};
@@ -26,6 +26,22 @@ use crate::type_traits::{AnyCompatible, ContainerElement};
 //------------------------------------------------------------------------
 // PackedCallable
 //------------------------------------------------------------------------
+/// The error to raise for a panic caught at an FFI boundary, where unwinding
+/// into the caller would abort the process.
+#[doc(hidden)]
+pub fn panic_to_error(payload: Box<dyn std::any::Any + Send>) -> Error {
+    let message = payload
+        .downcast_ref::<&str>()
+        .copied()
+        .or_else(|| {
+            payload
+                .downcast_ref::<std::string::String>()
+                .map(|s| s.as_str())
+        })
+        .unwrap_or("unknown payload");
+    Error::new(INTERNAL_ERROR, &format!("panicked: {message}"), "")
+}
+
 pub trait AsPackedCallable<I, O> {
     // Call the function in packed convention
     fn call_packed(&self, packed_args: &[AnyView]) -> Result<Any>;
diff --git a/rust/tvm-ffi/src/lib.rs b/rust/tvm-ffi/src/lib.rs
index 2bc2bb81..a4ddfbda 100644
--- a/rust/tvm-ffi/src/lib.rs
+++ b/rust/tvm-ffi/src/lib.rs
@@ -45,7 +45,7 @@ pub use crate::device::{current_stream, with_stream};
 pub use crate::dtype::DLDataTypeExt;
 pub use crate::error::{Error, ErrorKind, Result};
 pub use crate::error::{
-    ATTRIBUTE_ERROR, INDEX_ERROR, KEY_ERROR, RUNTIME_ERROR, TYPE_ERROR, 
VALUE_ERROR,
+    ATTRIBUTE_ERROR, INDEX_ERROR, INTERNAL_ERROR, KEY_ERROR, RUNTIME_ERROR, 
TYPE_ERROR, VALUE_ERROR,
 };
 pub use crate::extra::module::Module;
 pub use crate::extra::structural_mutate::{
diff --git a/rust/tvm-ffi/src/macros.rs b/rust/tvm-ffi/src/macros.rs
index 27cf5250..5f32ecbf 100644
--- a/rust/tvm-ffi/src/macros.rs
+++ b/rust/tvm-ffi/src/macros.rs
@@ -308,6 +308,11 @@ macro_rules! impl_arg_into_ref {
 
 /// Macro to export a typed function as a C symbol that follows the tvm-ffi ABI
 ///
+/// An error the function returns is raised to the caller. Report errors by
+/// returning them; a panic is also caught and raised, as an `InternalError`,
+/// since unwinding into the caller would abort the process, but panicking
+/// is discouraged.
+///
 /// # Arguments
 /// * `$name` - The name of the function
 /// * `$func` - The function to export
@@ -348,7 +353,12 @@ macro_rules! tvm_ffi_dll_export_typed_func {
             ) -> i32 {
                 let packed_args =
                     std::slice::from_raw_parts(args as *const 
$crate::any::AnyView, num_args as usize);
-                let ret_value = 
$crate::function_internal::call_packed_callable($func, packed_args);
+                let ret_value = match 
::std::panic::catch_unwind(::std::panic::AssertUnwindSafe(|| {
+                    $crate::function_internal::call_packed_callable($func, 
packed_args)
+                })) {
+                    Ok(ret_value) => ret_value,
+                    Err(payload) => 
Err($crate::function_internal::panic_to_error(payload)),
+                };
                 match ret_value {
                     Ok(value) => {
                         *result = $crate::any::Any::into_raw_ffi_any(value);
diff --git a/rust/tvm-ffi/tests/test_function.rs 
b/rust/tvm-ffi/tests/test_function.rs
index 07ed6779..77e1d60e 100644
--- a/rust/tvm-ffi/tests/test_function.rs
+++ b/rust/tvm-ffi/tests/test_function.rs
@@ -290,3 +290,48 @@ fn test_function_call_tensor_fn() {
     assert_eq!(y_data[2], 3.0);
     assert_eq!(y_data[3], 4.0);
 }
+
+// Functions should report errors by returning them; these panic only to test
+// that a panic is still raised to the caller instead of aborting.
+fn testing_panic(x: i32) -> Result<i32> {
+    if x < 0 {
+        panic!("negative input {x}");
+    }
+    Ok(x)
+}
+tvm_ffi_dll_export_typed_func!(testing_panic, testing_panic);
+
+#[test]
+fn test_function_exported_panic_raises_internal_error() {
+    // SAFETY: null handle is valid for testing_panic which doesn't use the 
handle.
+    let f = unsafe { Function::from_extern_c(std::ptr::null_mut(), 
__tvm_ffi_testing_panic, None) };
+    let typed = into_typed_fn!(f, Fn(i32) -> Result<i32>);
+    assert_eq!(typed(1).unwrap(), 1);
+    let error = typed(-1).unwrap_err();
+    assert_eq!(error.kind(), INTERNAL_ERROR);
+    assert!(
+        error.message().contains("negative input -1"),
+        "{}",
+        error.message()
+    );
+}
+
+#[test]
+fn test_function_callback_panic_raises_internal_error() {
+    let f = Function::from_typed(|x: i32| -> Result<i32> {
+        if x < 0 {
+            panic!("callback got {x}");
+        }
+        Ok(x)
+    });
+    let typed = into_typed_fn!(f, Fn(i32) -> Result<i32>);
+    let error = typed(-2).unwrap_err();
+    assert_eq!(error.kind(), INTERNAL_ERROR);
+    assert!(
+        error.message().contains("callback got -2"),
+        "{}",
+        error.message()
+    );
+    // The function stays usable after a panic.
+    assert_eq!(typed(3).unwrap(), 3);
+}

Reply via email to