Skip to content

Commit 75a14c2

Browse files
committed
[workerd-cxx] 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 For `fn r_return_identity(_: usize) -> usize;` ```rust 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); } ``` ```rust 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 43dacd0 commit 75a14c2

File tree

7 files changed

+111
-66
lines changed

7 files changed

+111
-66
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
@@ -1123,17 +1123,17 @@ fn expand_rust_function_shim_impl(
11231123
}
11241124
};
11251125

1126-
let mut outparam = None;
1127-
let indirect_return = indirect_return(sig, types);
1128-
if indirect_return {
1129-
let ret = expand_extern_type(sig.ret.as_ref().unwrap(), types, false);
1130-
outparam = Some(quote_spanned!(span=> __return: *mut #ret,));
1131-
}
1126+
// always indirect return when there's a return value
1127+
let out_param = sig.ret.as_ref().map(|ret| {
1128+
let ret = expand_extern_type(ret, types, false);
1129+
quote_spanned!(span=> __return: *mut #ret,)
1130+
});
1131+
let out = match sig.ret {
1132+
Some(_) => quote_spanned!(span=> __return),
1133+
None => quote_spanned!(span=> &mut ()),
1134+
};
1135+
let indirect_return = sig.ret.is_some();
11321136
if sig.throws {
1133-
let out = match sig.ret {
1134-
Some(_) => quote_spanned!(span=> __return),
1135-
None => quote_spanned!(span=> &mut ()),
1136-
};
11371137
requires_closure = true;
11381138
requires_unsafe = true;
11391139
expr = quote_spanned!(span=> ::cxx::private::r#try(#out, #expr));
@@ -1153,13 +1153,15 @@ fn expand_rust_function_shim_impl(
11531153
quote!(#local_name)
11541154
};
11551155

1156-
expr = quote_spanned!(span=> ::cxx::private::prevent_unwind(__fn, #closure));
1157-
1158-
let ret = if sig.throws {
1159-
quote!(-> ::cxx::private::Result)
1156+
if sig.throws {
1157+
// the function itself throws, wire through the exception
1158+
expr = quote_spanned!(span=> ::cxx::private::try_unwind(__fn, #closure));
11601159
} else {
1161-
expand_extern_return_type(&sig.ret, types, false)
1162-
};
1160+
// always protect against the panic
1161+
expr = quote_spanned!(span=> ::cxx::private::catch_unwind(__fn, #closure));
1162+
}
1163+
// rust functions always potentially throw
1164+
let ret = quote!(-> ::cxx::private::Result);
11631165

11641166
let pointer = match invoke {
11651167
None => Some(quote_spanned!(span=> __extern: *const ())),
@@ -1170,7 +1172,7 @@ fn expand_rust_function_shim_impl(
11701172
#attrs
11711173
#[doc(hidden)]
11721174
#[#UnsafeAttr(#ExportNameAttr = #link_name)]
1173-
unsafe extern "C" fn #local_name #generics(#(#all_args,)* #outparam #pointer) #ret {
1175+
unsafe extern "C" fn #local_name #generics(#(#all_args,)* #out_param #pointer) #ret {
11741176
let __fn = ::cxx::private::concat!(::cxx::private::module_path!(), #prevent_unwind_label);
11751177
#wrap_super
11761178
#expr

src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,12 @@ pub mod private {
507507
pub use crate::shared_ptr::SharedPtrTarget;
508508
pub use crate::string::StackString;
509509
pub use crate::unique_ptr::UniquePtrTarget;
510+
#[cfg(feature = "std")]
511+
pub use crate::unwind::catch_unwind;
512+
#[cfg(feature = "std")]
510513
pub use crate::unwind::prevent_unwind;
514+
#[cfg(feature = "std")]
515+
pub use crate::unwind::try_unwind;
511516
pub use crate::weak_ptr::WeakPtrTarget;
512517
pub use core::{concat, module_path};
513518
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: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#![allow(missing_docs)]
2+
#![allow(dead_code)]
23

34
use core::mem;
5+
use std::panic;
46

57
pub fn prevent_unwind<F, R>(label: &'static str, foreign_call: F) -> R
68
where
@@ -37,3 +39,36 @@ impl Drop for Guard {
3739
panic!("panic in ffi function {}, aborting.", self.label);
3840
}
3941
}
42+
43+
/// Run the void `foreign_call`, intercepting panics and converting them to errors.
44+
#[cfg(feature = "std")]
45+
pub fn catch_unwind<F>(label: &'static str, foreign_call: F) -> ::cxx::result::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+
56+
/// Run the error-returning `foreign_call`, intercepting panics and converting them to errors.
57+
#[cfg(feature = "std")]
58+
pub fn try_unwind<F>(label: &'static str, foreign_call: F) -> ::cxx::private::Result
59+
where
60+
F: FnOnce() -> ::cxx::private::Result,
61+
{
62+
match panic::catch_unwind(panic::AssertUnwindSafe(foreign_call)) {
63+
Ok(r) => r,
64+
Err(err) => panic_result(label, &err),
65+
}
66+
}
67+
68+
#[cfg(feature = "std")]
69+
fn panic_result(label: &'static str, err: &alloc::boxed::Box<dyn core::any::Any + Send>) -> ::cxx::private::Result {
70+
if let Some(err) = err.downcast_ref::<alloc::string::String>() {
71+
return ::cxx::private::Result::error(std::format!("panic in {label}: {err}"));
72+
}
73+
::cxx::private::Result::error(std::format!("panic in {label}"))
74+
}

tests/ffi/lib.rs

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

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

322324
struct Dag0 {
@@ -659,3 +661,7 @@ fn r_try_return_mutsliceu8(slice: &mut [u8]) -> Result<&mut [u8], Error> {
659661
fn r_aliased_function(x: i32) -> String {
660662
x.to_string()
661663
}
664+
665+
fn r_panic(s: &str) {
666+
panic!("{s}");
667+
}

tests/ffi/tests.cc

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

978+
try {
979+
r_panic("foobar");
980+
ASSERT(false);
981+
} catch (const rust::Error &e) {
982+
ASSERT(std::strcmp(e.what(), "panic in cxx_test_suite::ffi::r_panic: foobar") == 0);
983+
}
984+
978985
cxx_test_suite_set_correct();
979986
return nullptr;
980987
}

0 commit comments

Comments
 (0)