From 24f28bb6050455bee1a0586de4327bee107a21bf Mon Sep 17 00:00:00 2001 From: chyyran Date: Sun, 23 Apr 2023 02:01:31 -0400 Subject: [PATCH] capi: better clarify story around panic safety - catches panics for all functions now except frame for performance reasons --- librashader-capi/src/ffi.rs | 150 ++++++++++++++---- .../src/runtime/d3d11/filter_chain.rs | 2 +- .../src/runtime/d3d12/filter_chain.rs | 2 +- .../src/runtime/gl/filter_chain.rs | 2 +- .../src/runtime/vk/filter_chain.rs | 2 +- 5 files changed, 126 insertions(+), 32 deletions(-) diff --git a/librashader-capi/src/ffi.rs b/librashader-capi/src/ffi.rs index 552d796..4937f19 100644 --- a/librashader-capi/src/ffi.rs +++ b/librashader-capi/src/ffi.rs @@ -1,6 +1,31 @@ macro_rules! ffi_body { + (nopanic $body:block) => { + { + let result: Result<(), $crate::error::LibrashaderError> = try { + $body + }; + + let Err(e) = result else { + return $crate::error::LibrashaderError::ok() + }; + e.export() + } + }; ($body:block) => { { + let result = ::std::panic::catch_unwind(::std::panic::AssertUnwindSafe(|| { + $crate::ffi::ffi_body!(nopanic $body) + })); + + result.unwrap_or_else(|e| $crate::error::LibrashaderError::UnknownError(e).export()) + } + }; + (nopanic |$($ref_capture:ident),*|; mut |$($mut_capture:ident),*| $body:block) => { + { + $($crate::error::assert_non_null!($ref_capture);)* + $(let $ref_capture = unsafe { &*$ref_capture };)* + $($crate::error::assert_non_null!($mut_capture);)* + $(let $mut_capture = unsafe { &mut *$mut_capture };)* let result: Result<(), $crate::error::LibrashaderError> = try { $body }; @@ -13,8 +38,15 @@ macro_rules! ffi_body { }; (|$($ref_capture:ident),*|; mut |$($mut_capture:ident),*| $body:block) => { { - $($crate::error::assert_non_null!($ref_capture);)* - $(let $ref_capture = unsafe { &*$ref_capture };)* + let result = ::std::panic::catch_unwind(::std::panic::AssertUnwindSafe(|| { + $crate::ffi::ffi_body!(nopanic |$($ref_capture),*|; mut |$($mut_capture),*| $body) + })); + + result.unwrap_or_else(|e| $crate::error::LibrashaderError::UnknownError(e).export()) + } + }; + (nopanic mut |$($mut_capture:ident),*| $body:block) => { + { $($crate::error::assert_non_null!($mut_capture);)* $(let $mut_capture = unsafe { &mut *$mut_capture };)* let result: Result<(), $crate::error::LibrashaderError> = try { @@ -28,20 +60,15 @@ macro_rules! ffi_body { } }; (mut |$($mut_capture:ident),*| $body:block) => { - { - $($crate::error::assert_non_null!($mut_capture);)* - $(let $mut_capture = unsafe { &mut *$mut_capture };)* - let result: Result<(), $crate::error::LibrashaderError> = try { - $body - }; + { + let result = ::std::panic::catch_unwind(::std::panic::AssertUnwindSafe(|| { + $crate::ffi::ffi_body!(nopanic mut |$($mut_capture),*| $body) + })); - let Err(e) = result else { - return $crate::error::LibrashaderError::ok() - }; - e.export() + result.unwrap_or_else(|e| $crate::error::LibrashaderError::UnknownError(e).export()) } }; - (|$($ref_capture:ident),*| $body:block) => { + (nopanic |$($ref_capture:ident),*| $body:block) => { { $($crate::error::assert_non_null!($ref_capture);)* $(let $ref_capture = unsafe { &*$ref_capture };)* @@ -54,24 +81,20 @@ macro_rules! ffi_body { }; e.export() } - } + }; + (|$($ref_capture:ident),*| $body:block) => { + { + let result = ::std::panic::catch_unwind(::std::panic::AssertUnwindSafe(|| { + $crate::ffi::ffi_body!(nopanic |$($ref_capture),*| $body) + })); + + result.unwrap_or_else(|e| $crate::error::LibrashaderError::UnknownError(e).export()) + } + }; } macro_rules! extern_fn { - ($(#[$($attrss:tt)*])* fn $func_name:ident ($($arg_name:ident : $arg_ty:ty),* $(,)?) $body:block) => { - ::paste::paste! { - /// Function pointer definition for - #[doc = ::std::stringify!($func_name)] - pub type [] = unsafe extern "C" fn($($arg_name: $arg_ty,)*) -> $crate::ctypes::libra_error_t; - } - - #[no_mangle] - $(#[$($attrss)*])* - pub unsafe extern "C" fn $func_name($($arg_name: $arg_ty,)*) -> $crate::ctypes::libra_error_t { - $crate::ffi::ffi_body!($body) - } - }; - + // raw doesn't wrap in ffi_body ($(#[$($attrss:tt)*])* raw fn $func_name:ident ($($arg_name:ident : $arg_ty:ty),* $(,)?) $body:block) => { ::paste::paste! { /// Function pointer definition for @@ -86,6 +109,21 @@ macro_rules! extern_fn { } }; + // ffi_body but panic-safe + ($(#[$($attrss:tt)*])* fn $func_name:ident ($($arg_name:ident : $arg_ty:ty),* $(,)?) $body:block) => { + ::paste::paste! { + /// Function pointer definition for + #[doc = ::std::stringify!($func_name)] + pub type [] = unsafe extern "C" fn($($arg_name: $arg_ty,)*) -> $crate::ctypes::libra_error_t; + } + + #[no_mangle] + $(#[$($attrss)*])* + pub unsafe extern "C" fn $func_name($($arg_name: $arg_ty,)*) -> $crate::ctypes::libra_error_t { + $crate::ffi::ffi_body!($body) + } + }; + ($(#[$($attrss:tt)*])* fn $func_name:ident ($($arg_name:ident : $arg_ty:ty),* $(,)?) |$($ref_capture:ident),*|; mut |$($mut_capture:ident),*| $body:block) => { ::paste::paste! { /// Function pointer definition for @@ -126,6 +164,62 @@ macro_rules! extern_fn { $crate::ffi::ffi_body!(|$($ref_capture),*| $body) } }; + + // nopanic variants that are UB if panics + ($(#[$($attrss:tt)*])* nopanic fn $func_name:ident ($($arg_name:ident : $arg_ty:ty),* $(,)?) $body:block) => { + ::paste::paste! { + /// Function pointer definition for + #[doc = ::std::stringify!($func_name)] + pub type [] = unsafe extern "C" fn($($arg_name: $arg_ty,)*) -> $crate::ctypes::libra_error_t; + } + + #[no_mangle] + $(#[$($attrss)*])* + pub unsafe extern "C" fn $func_name($($arg_name: $arg_ty,)*) -> $crate::ctypes::libra_error_t { + $crate::ffi::ffi_body!(nopanic $body) + } + }; + + ($(#[$($attrss:tt)*])* nopanic fn $func_name:ident ($($arg_name:ident : $arg_ty:ty),* $(,)?) |$($ref_capture:ident),*|; mut |$($mut_capture:ident),*| $body:block) => { + ::paste::paste! { + /// Function pointer definition for + #[doc = ::std::stringify!($func_name)] + pub type [] = unsafe extern "C" fn($($arg_name: $arg_ty,)*) -> $crate::ctypes::libra_error_t; + } + + #[no_mangle] + $(#[$($attrss)*])* + pub unsafe extern "C" fn $func_name($($arg_name: $arg_ty,)*) -> $crate::ctypes::libra_error_t { + $crate::ffi::ffi_body!(nopanic |$($ref_capture),*|; mut |$($mut_capture),*| $body) + } + }; + + ($(#[$($attrss:tt)*])* nopanic fn $func_name:ident ($($arg_name:ident : $arg_ty:ty),* $(,)?) mut |$($mut_capture:ident),*| $body:block) => { + ::paste::paste! { + /// Function pointer definition for + #[doc = ::std::stringify!($func_name)] + pub type [] = unsafe extern "C" fn($($arg_name: $arg_ty,)*) -> $crate::ctypes::libra_error_t; + } + + #[no_mangle] + $(#[$($attrss)*])* + pub unsafe extern "C" fn $func_name($($arg_name: $arg_ty,)*) -> $crate::ctypes::libra_error_t { + $crate::ffi::ffi_body!(nopanic mut |$($mut_capture),*| $body) + } + }; + ($(#[$($attrss:tt)*])* nopanic fn $func_name:ident ($($arg_name:ident : $arg_ty:ty),* $(,)?) |$($ref_capture:ident),*| $body:block) => { + ::paste::paste! { + /// Function pointer definition for + #[doc = ::std::stringify!($func_name)] + pub type [] = unsafe extern "C" fn($($arg_name: $arg_ty,)*) -> $crate::ctypes::libra_error_t; + } + + #[no_mangle] + $(#[$($attrss)*])* + pub unsafe extern "C" fn $func_name($($arg_name: $arg_ty,)*) -> $crate::ctypes::libra_error_t { + $crate::ffi::ffi_body!(nopanic |$($ref_capture),*| $body) + } + }; } pub(crate) use extern_fn; diff --git a/librashader-capi/src/runtime/d3d11/filter_chain.rs b/librashader-capi/src/runtime/d3d11/filter_chain.rs index 3ec208e..b76c92f 100644 --- a/librashader-capi/src/runtime/d3d11/filter_chain.rs +++ b/librashader-capi/src/runtime/d3d11/filter_chain.rs @@ -216,7 +216,7 @@ extern_fn! { /// the filter chain was created with. /// - You must ensure that only one thread has access to `chain` before you call this function. Only one /// thread at a time may call this function. - fn libra_d3d11_filter_chain_frame( + nopanic fn libra_d3d11_filter_chain_frame( chain: *mut libra_d3d11_filter_chain_t, // cbindgen can't discover that ID3D11DeviceContext has the niche optimization // so ManuallyDrop> doesn't generate correct bindings. diff --git a/librashader-capi/src/runtime/d3d12/filter_chain.rs b/librashader-capi/src/runtime/d3d12/filter_chain.rs index 449619b..21cfe10 100644 --- a/librashader-capi/src/runtime/d3d12/filter_chain.rs +++ b/librashader-capi/src/runtime/d3d12/filter_chain.rs @@ -226,7 +226,7 @@ extern_fn! { /// and must be associated with the `ID3D12Device` this filter chain was created with. /// - You must ensure that only one thread has access to `chain` before you call this function. Only one /// thread at a time may call this function. - fn libra_d3d12_filter_chain_frame( + nopanic fn libra_d3d12_filter_chain_frame( chain: *mut libra_d3d12_filter_chain_t, command_list: ManuallyDrop, frame_count: usize, diff --git a/librashader-capi/src/runtime/gl/filter_chain.rs b/librashader-capi/src/runtime/gl/filter_chain.rs index e60d8fb..c73ee6d 100644 --- a/librashader-capi/src/runtime/gl/filter_chain.rs +++ b/librashader-capi/src/runtime/gl/filter_chain.rs @@ -172,7 +172,7 @@ extern_fn! { /// thread at a time may call this function. The thread `libra_gl_filter_chain_frame` is called from /// must have its thread-local OpenGL context initialized with the same context used to create /// the filter chain. - fn libra_gl_filter_chain_frame( + nopanic fn libra_gl_filter_chain_frame( chain: *mut libra_gl_filter_chain_t, frame_count: usize, image: libra_source_image_gl_t, diff --git a/librashader-capi/src/runtime/vk/filter_chain.rs b/librashader-capi/src/runtime/vk/filter_chain.rs index 7cbbaa6..efe9323 100644 --- a/librashader-capi/src/runtime/vk/filter_chain.rs +++ b/librashader-capi/src/runtime/vk/filter_chain.rs @@ -246,7 +246,7 @@ extern_fn! { /// struct. /// - You must ensure that only one thread has access to `chain` before you call this function. Only one /// thread at a time may call this function. - fn libra_vk_filter_chain_frame( + nopanic fn libra_vk_filter_chain_frame( chain: *mut libra_vk_filter_chain_t, command_buffer: vk::CommandBuffer, frame_count: usize,