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);
+}