Skip to content

Commit b0e7d4f

Browse files
authored
no-abort (#2)
_This is the first change in a series to replace abort on panic with exceptions._ This change replaces the usage of aborting `prevent_unwind` with new error-generating `catch_unwind` and `try_unwind` in ffi *rust functions* only (see the note below). It largerly uses existing setup for returning exceptions in case of `Result` return type by forecibly enabling it. Since handling exceptions in cxx requires passing return values as pointer arguments (see example), this transformation requires specificatin of return references lifetime (eforced by #3) There are plenty of remaining usages of `prevent_unwind` but they are all (mostly?) concerned generating various c++ operators (that we don't really use). I intend to fix them in follow ups. Internal PR 8720 ## Example shim For `fn r_return_identity(_: usize) -> usize;` ### Before ```rust #[export_name = "tests$cxxbridge1$r_return_identity"] unsafe extern "C" fn __r_return_identity(arg0: usize) -> usize { let __fn = "cxx_test_suite::ffi::r_return_identity"; fn __r_return_identity(arg0: usize) -> usize { super::r_return_identity(arg0) } ::cxx::private::prevent_unwind(__fn, move || __r_return_identity(arg0)) } ``` ```c++ ::std::size_t r_return_identity(::std::size_t arg0) noexcept { return tests$cxxbridge1$r_return_identity(arg0); } ``` ### After ```rust #[export_name = "tests$cxxbridge1$r_return_identity"] unsafe extern "C" fn __r_return_identity( arg0: usize, __return: *mut usize, ) -> ::cxx::private::Result { let __fn = "cxx_test_suite::ffi::r_return_identity"; fn __r_return_identity(arg0: usize) -> usize { super::r_return_identity(arg0) } ::cxx::private::catch_unwind( __fn, move || unsafe { ::cxx::core::ptr::write(__return, __r_return_identity(arg0)) }, ) } ``` ```c++ ::std::size_t r_return_identity(::std::size_t arg0) { ::rust::MaybeUninit<::std::size_t> return$; ::rust::repr::PtrLen error$ = tests$cxxbridge1$r_return_identity(arg0, &return$.value); if (error$.ptr) { throw ::rust::impl<::rust::Error>::error(error$); } return ::std::move(return$.value); } ```
1 parent a6804d6 commit b0e7d4f

File tree

7 files changed

+106
-67
lines changed

7 files changed

+106
-67
lines changed

gen/src/write.rs

Lines changed: 27 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use crate::syntax::map::UnorderedMap as Map;
88
use crate::syntax::set::UnorderedSet;
99
use crate::syntax::symbol::{self, Symbol};
1010
use crate::syntax::trivial::{self, TrivialReason};
11+
use crate::syntax::Lang;
1112
use crate::syntax::{
1213
derive, mangle, Api, Doc, Enum, EnumRepr, ExternFn, ExternType, Pair, Signature, Struct, Trait,
1314
Type, TypeAlias, Types, Var,
@@ -274,7 +275,8 @@ fn write_struct<'a>(out: &mut OutFile<'a>, strct: &'a Struct, methods: &[&Extern
274275
let sig = &method.sig;
275276
let local_name = method.name.cxx.to_string();
276277
let indirect_call = false;
277-
write_rust_function_shim_decl(out, &local_name, sig, indirect_call);
278+
let throws = sig.throws || method.lang == Lang::Rust;
279+
write_rust_function_shim_decl(out, &local_name, sig, indirect_call, throws);
278280
writeln!(out, ";");
279281
if !method.doc.is_empty() {
280282
out.next_section();
@@ -366,7 +368,8 @@ fn write_opaque_type<'a>(out: &mut OutFile<'a>, ety: &'a ExternType, methods: &[
366368
let sig = &method.sig;
367369
let local_name = method.name.cxx.to_string();
368370
let indirect_call = false;
369-
write_rust_function_shim_decl(out, &local_name, sig, indirect_call);
371+
let throws = sig.throws || method.lang == Lang::Rust;
372+
write_rust_function_shim_decl(out, &local_name, sig, indirect_call, throws);
370373
writeln!(out, ";");
371374
if !method.doc.is_empty() {
372375
out.next_section();
@@ -897,12 +900,11 @@ fn write_rust_function_decl_impl(
897900
indirect_call: bool,
898901
) {
899902
out.next_section();
900-
if sig.throws {
901-
out.builtin.ptr_len = true;
902-
write!(out, "::rust::repr::PtrLen ");
903-
} else {
904-
write_extern_return_type_space(out, &sig.ret);
905-
}
903+
904+
// rust functions always throw because of panic
905+
out.builtin.ptr_len = true;
906+
write!(out, "::rust::repr::PtrLen ");
907+
906908
write!(out, "{}(", link_name);
907909
let mut needs_comma = false;
908910
if let Some(receiver) = &sig.receiver {
@@ -924,7 +926,9 @@ fn write_rust_function_decl_impl(
924926
write_extern_arg(out, arg);
925927
needs_comma = true;
926928
}
927-
if indirect_return(sig, out.types) {
929+
// rust functions always use indirect return
930+
let indirect_return = sig.ret.is_some();
931+
if indirect_return {
928932
if needs_comma {
929933
write!(out, ", ");
930934
}
@@ -947,7 +951,7 @@ fn write_rust_function_decl_impl(
947951
}
948952
write!(out, "void *");
949953
}
950-
writeln!(out, ") noexcept;");
954+
writeln!(out, ");");
951955
}
952956

953957
fn write_rust_function_shim<'a>(out: &mut OutFile<'a>, efn: &'a ExternFn) {
@@ -971,6 +975,7 @@ fn write_rust_function_shim_decl(
971975
local_name: &str,
972976
sig: &Signature,
973977
indirect_call: bool,
978+
throws: bool,
974979
) {
975980
begin_function_definition(out);
976981
write_return_type(out, &sig.ret);
@@ -994,7 +999,7 @@ fn write_rust_function_shim_decl(
994999
write!(out, " const");
9951000
}
9961001
}
997-
if !sig.throws {
1002+
if !throws {
9981003
write!(out, " noexcept");
9991004
}
10001005
}
@@ -1015,7 +1020,7 @@ fn write_rust_function_shim_impl(
10151020
// Member functions already documented at their declaration.
10161021
write_doc(out, "", doc);
10171022
}
1018-
write_rust_function_shim_decl(out, local_name, sig, indirect_call);
1023+
write_rust_function_shim_decl(out, local_name, sig, indirect_call, true);
10191024
if out.header {
10201025
writeln!(out, ";");
10211026
return;
@@ -1031,7 +1036,8 @@ fn write_rust_function_shim_impl(
10311036
}
10321037
}
10331038
write!(out, " ");
1034-
let indirect_return = indirect_return(sig, out.types);
1039+
// rust functions always use indirect return
1040+
let indirect_return = sig.ret.is_some();
10351041
if indirect_return {
10361042
out.builtin.maybe_uninit = true;
10371043
write!(out, "::rust::MaybeUninit<");
@@ -1047,35 +1053,10 @@ fn write_rust_function_shim_impl(
10471053
}
10481054
writeln!(out, "> return$;");
10491055
write!(out, " ");
1050-
} else if let Some(ret) = &sig.ret {
1051-
write!(out, "return ");
1052-
match ret {
1053-
Type::RustBox(_) => {
1054-
write_type(out, ret);
1055-
write!(out, "::from_raw(");
1056-
}
1057-
Type::UniquePtr(_) => {
1058-
write_type(out, ret);
1059-
write!(out, "(");
1060-
}
1061-
Type::Ref(_) => write!(out, "*"),
1062-
Type::Str(_) => {
1063-
out.builtin.rust_str_new_unchecked = true;
1064-
write!(out, "::rust::impl<::rust::Str>::new_unchecked(");
1065-
}
1066-
Type::SliceRef(_) => {
1067-
out.builtin.rust_slice_new = true;
1068-
write!(out, "::rust::impl<");
1069-
write_type(out, ret);
1070-
write!(out, ">::slice(");
1071-
}
1072-
_ => {}
1073-
}
1074-
}
1075-
if sig.throws {
1076-
out.builtin.ptr_len = true;
1077-
write!(out, "::rust::repr::PtrLen error$ = ");
10781056
}
1057+
out.builtin.ptr_len = true;
1058+
write!(out, "::rust::repr::PtrLen error$ = ");
1059+
10791060
write!(out, "{}(", invoke);
10801061
let mut needs_comma = false;
10811062
if sig.receiver.is_some() {
@@ -1120,12 +1101,11 @@ fn write_rust_function_shim_impl(
11201101
}
11211102
}
11221103
writeln!(out, ";");
1123-
if sig.throws {
1124-
out.builtin.rust_error = true;
1125-
writeln!(out, " if (error$.ptr) {{");
1126-
writeln!(out, " throw ::rust::impl<::rust::Error>::error(error$);");
1127-
writeln!(out, " }}");
1128-
}
1104+
out.builtin.rust_error = true;
1105+
writeln!(out, " if (error$.ptr) {{");
1106+
writeln!(out, " throw ::rust::impl<::rust::Error>::error(error$);");
1107+
writeln!(out, " }}");
1108+
11291109
if indirect_return {
11301110
write!(out, " return ");
11311111
match sig.ret.as_ref().unwrap() {

macro/src/expand.rs

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,17 +1113,17 @@ fn expand_rust_function_shim_impl(
11131113
}
11141114
};
11151115

1116-
let mut outparam = None;
1117-
let indirect_return = indirect_return(sig, types);
1118-
if indirect_return {
1119-
let ret = expand_extern_type(sig.ret.as_ref().unwrap(), types, false);
1120-
outparam = Some(quote_spanned!(span=> __return: *mut #ret,));
1121-
}
1116+
// always indirect return when there's a return value
1117+
let out_param = sig.ret.as_ref().map(|ret| {
1118+
let ret = expand_extern_type(ret, types, false);
1119+
quote_spanned!(span=> __return: *mut #ret,)
1120+
});
1121+
let out = match sig.ret {
1122+
Some(_) => quote_spanned!(span=> __return),
1123+
None => quote_spanned!(span=> &mut ()),
1124+
};
1125+
let indirect_return = sig.ret.is_some();
11221126
if sig.throws {
1123-
let out = match sig.ret {
1124-
Some(_) => quote_spanned!(span=> __return),
1125-
None => quote_spanned!(span=> &mut ()),
1126-
};
11271127
requires_closure = true;
11281128
requires_unsafe = true;
11291129
expr = quote_spanned!(span=> ::cxx::private::r#try(#out, #expr));
@@ -1143,13 +1143,15 @@ fn expand_rust_function_shim_impl(
11431143
quote!(#local_name)
11441144
};
11451145

1146-
expr = quote_spanned!(span=> ::cxx::private::prevent_unwind(__fn, #closure));
1147-
1148-
let ret = if sig.throws {
1149-
quote!(-> ::cxx::private::Result)
1146+
if sig.throws {
1147+
// the function itself throws, wire through the exception
1148+
expr = quote_spanned!(span=> ::cxx::private::try_unwind(__fn, #closure));
11501149
} else {
1151-
expand_extern_return_type(&sig.ret, types, false)
1152-
};
1150+
// always protect against the panic
1151+
expr = quote_spanned!(span=> ::cxx::private::catch_unwind(__fn, #closure));
1152+
}
1153+
// rust functions always potentially throw
1154+
let ret = quote!(-> ::cxx::private::Result);
11531155

11541156
let pointer = match invoke {
11551157
None => Some(quote_spanned!(span=> __extern: *const ())),
@@ -1160,7 +1162,7 @@ fn expand_rust_function_shim_impl(
11601162
#attrs
11611163
#[doc(hidden)]
11621164
#[export_name = #link_name]
1163-
unsafe extern "C" fn #local_name #generics(#(#all_args,)* #outparam #pointer) #ret {
1165+
unsafe extern "C" fn #local_name #generics(#(#all_args,)* #out_param #pointer) #ret {
11641166
let __fn = ::cxx::private::concat!(::cxx::private::module_path!(), #prevent_unwind_label);
11651167
#wrap_super
11661168
#expr

src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,9 @@ pub mod private {
520520
pub use crate::shared_ptr::SharedPtrTarget;
521521
pub use crate::string::StackString;
522522
pub use crate::unique_ptr::UniquePtrTarget;
523+
pub use crate::unwind::catch_unwind;
523524
pub use crate::unwind::prevent_unwind;
525+
pub use crate::unwind::try_unwind;
524526
pub use crate::weak_ptr::WeakPtrTarget;
525527
pub use core::{concat, module_path};
526528
pub use cxxbridge_macro::type_id;

src/result.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,26 @@ pub union Result {
2323
ok: *const u8, // null
2424
}
2525

26+
impl Result {
27+
pub(crate) fn ok() -> Self {
28+
Result { ok: ptr::null() }
29+
}
30+
31+
pub(crate) fn error<E: Display>(err: E) -> Self {
32+
unsafe { to_c_error(err.to_string()) }
33+
}
34+
}
35+
2636
pub unsafe fn r#try<T, E>(ret: *mut T, result: StdResult<T, E>) -> Result
2737
where
2838
E: Display,
2939
{
3040
match result {
3141
Ok(ok) => {
3242
unsafe { ptr::write(ret, ok) }
33-
Result { ok: ptr::null() }
43+
Result::ok()
3444
}
35-
Err(err) => unsafe { to_c_error(err.to_string()) },
45+
Err(err) => Result::error(err),
3646
}
3747
}
3848

src/unwind.rs

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
#![allow(missing_docs)]
22

3-
use core::mem;
3+
use alloc::boxed::Box;
4+
use alloc::format;
5+
use core::{any::Any, mem};
6+
use std::panic;
47

58
pub fn prevent_unwind<F, R>(label: &'static str, foreign_call: F) -> R
69
where
@@ -37,3 +40,32 @@ impl Drop for Guard {
3740
panic!("panic in ffi function {}, aborting.", self.label);
3841
}
3942
}
43+
44+
/// Run the void `foreign_call`, intercepting panics and converting them to errors.
45+
pub fn catch_unwind<F>(label: &'static str, foreign_call: F) -> ::cxx::private::Result
46+
where
47+
F: FnOnce(),
48+
{
49+
match panic::catch_unwind(panic::AssertUnwindSafe(foreign_call)) {
50+
Ok(()) => ::cxx::private::Result::ok(),
51+
Err(err) => panic_result(label, &err),
52+
}
53+
}
54+
55+
/// Run the error-returning `foreign_call`, intercepting panics and converting them to errors.
56+
pub fn try_unwind<F>(label: &'static str, foreign_call: F) -> ::cxx::private::Result
57+
where
58+
F: FnOnce() -> ::cxx::private::Result,
59+
{
60+
match panic::catch_unwind(panic::AssertUnwindSafe(foreign_call)) {
61+
Ok(r) => r,
62+
Err(err) => panic_result(label, &err),
63+
}
64+
}
65+
66+
fn panic_result(label: &'static str, err: &Box<dyn Any + Send>) -> ::cxx::private::Result {
67+
if let Some(err) = err.downcast_ref::<alloc::string::String>() {
68+
return ::cxx::private::Result::error(format!("panic in {label}: {err}"));
69+
}
70+
::cxx::private::Result::error(format!("panic in {label}"))
71+
}

tests/ffi/lib.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,8 @@ pub mod ffi {
316316

317317
#[cxx_name = "rAliasedFunction"]
318318
fn r_aliased_function(x: i32) -> String;
319+
320+
fn r_panic(s: &str);
319321
}
320322

321323
struct Dag0 {
@@ -650,3 +652,7 @@ fn r_try_return_mutsliceu8(slice: &mut [u8]) -> Result<&mut [u8], Error> {
650652
fn r_aliased_function(x: i32) -> String {
651653
x.to_string()
652654
}
655+
656+
fn r_panic(s: &str) {
657+
panic!("{s}");
658+
}

tests/ffi/tests.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -904,6 +904,13 @@ extern "C" const char *cxx_run_test() noexcept {
904904
(void)rust::Vec<size_t>();
905905
(void)rust::Vec<rust::isize>();
906906

907+
try {
908+
r_panic("foobar");
909+
ASSERT(false);
910+
} catch (const rust::Error &e) {
911+
ASSERT(std::strcmp(e.what(), "panic in cxx_test_suite::ffi::r_panic: foobar") == 0);
912+
}
913+
907914
cxx_test_suite_set_correct();
908915
return nullptr;
909916
}

0 commit comments

Comments
 (0)