Skip to content

Commit dfd6648

Browse files
schreterschreter
committed
Fix handling of MSVC std::exception_ptr
Contrary to the most platforms, which just store a single pointer in `std::exception_ptr`, MSVC stores two. Add necessary code to handle this.
1 parent e7b7116 commit dfd6648

File tree

4 files changed

+115
-99
lines changed

4 files changed

+115
-99
lines changed

‎gen/src/builtin.rs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,11 +225,17 @@ pub(super) fn write(out: &mut OutFile) {
225225
writeln!(out, " void *ptr;");
226226
writeln!(out, " ::std::size_t len;");
227227
writeln!(out, "}};");
228+
writeln!(out, "struct CxxException final {{");
229+
writeln!(out, " void *repr_ptr;");
230+
writeln!(out, "#if _MSC_VER >= 1700");
231+
writeln!(out, " void *repr_ptr_2;");
232+
writeln!(out, "#endif");
233+
writeln!(out, "}};");
228234
writeln!(out, "struct CxxResult final {{");
229-
writeln!(out, " void *ptr;");
235+
writeln!(out, " CxxException exc;");
230236
writeln!(out, "}};");
231237
writeln!(out, "struct Exception final {{");
232-
writeln!(out, " CxxResult exc;");
238+
writeln!(out, " CxxResult res;");
233239
writeln!(out, " PtrLen msg;");
234240
writeln!(out, "}};");
235241
}

‎gen/src/write.rs‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -856,7 +856,9 @@ fn write_cxx_function_shim<'a>(out: &mut OutFile<'a>, efn: &'a ExternFn) {
856856
}
857857
writeln!(out, ";");
858858
if efn.throws {
859-
writeln!(out, " throw$.exc.ptr = nullptr;");
859+
writeln!(out, " throw$.res.exc.repr_ptr = nullptr;");
860+
#[cfg(target_env = "msvc")]
861+
writeln!(out, " throw$.res.exc.repr_ptr_2 = nullptr;");
860862
writeln!(out, " throw$.msg.ptr = nullptr;");
861863
writeln!(out, " }},");
862864
writeln!(out, " ::rust::detail::Fail(throw$));");
@@ -1124,9 +1126,12 @@ fn write_rust_function_shim_impl(
11241126
writeln!(out, ";");
11251127
if sig.throws {
11261128
out.builtin.rust_error = true;
1127-
writeln!(out, " if (error$.ptr) {{");
1128-
writeln!(out, " void *ppexc = &error$.ptr;");
1129-
writeln!(out, " std::rethrow_exception(std::move(*static_cast<std::exception_ptr*>(ppexc)));");
1129+
writeln!(out, " if (error$.exc.repr_ptr) {{");
1130+
writeln!(out, " void *ppexc = &error$.exc;");
1131+
writeln!(
1132+
out,
1133+
" std::rethrow_exception(std::move(*static_cast<std::exception_ptr*>(ppexc)));"
1134+
);
11301135
writeln!(out, " }}");
11311136
}
11321137
if indirect_return {

‎src/cxx.cc‎

Lines changed: 51 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -467,43 +467,6 @@ class impl<Error> final {
467467
};
468468
} // namespace
469469

470-
// Ensure statically that `std::exception_ptr` is really a pointer.
471-
static_assert(sizeof(std::exception_ptr) == sizeof(void *),
472-
"Unsupported std::exception_ptr size");
473-
static_assert(alignof(std::exception_ptr) == alignof(void *),
474-
"Unsupported std::exception_ptr alignment");
475-
476-
extern "C" {
477-
void *cxxbridge1$default_exception(const char *ptr, std::size_t len) noexcept {
478-
// Construct an `std::exception_ptr` for the default `rust::Error` in the
479-
// space provided by the pointer itself (placement new).
480-
//
481-
// The `std::exception_ptr` itself is just a pointer, so this effectively
482-
// converts it to the pointer.
483-
void *eptr;
484-
new (&eptr) std::exception_ptr(
485-
std::make_exception_ptr(impl<Error>::error_copy(ptr, len)));
486-
return eptr;
487-
}
488-
489-
void cxxbridge1$drop_exception(char *ptr) noexcept {
490-
// Implement the `drop` for `CxxException` on the Rust side, which is just a
491-
// pointer to the exception stored in `std::exception_ptr`.
492-
auto eptr = std::move(*reinterpret_cast<std::exception_ptr *>(&ptr));
493-
// eptr goes out of scope and deallocates `std::exception_ptr`.
494-
}
495-
496-
void *cxxbridge1$clone_exception(const char *ptr) noexcept {
497-
// Implement the `clone` for `CxxException` on the Rust side, which is just a
498-
// pointer to the exception stored in `std::exception_ptr`.
499-
void *eptr;
500-
const void *pptr = &ptr;
501-
new (&eptr)
502-
std::exception_ptr(*static_cast<const std::exception_ptr *>(pptr));
503-
return eptr;
504-
}
505-
} // extern "C"
506-
507470
Error::Error(const Error &other)
508471
: std::exception(other),
509472
msg(other.msg ? errorCopy(other.msg, other.len) : nullptr),
@@ -556,19 +519,64 @@ struct PtrLen final {
556519
void *ptr;
557520
std::size_t len;
558521
};
522+
struct CxxException final {
523+
void *repr_ptr;
524+
#if _MSC_VER >= 1700
525+
// NOTE: MSVC uses two pointers to store `std::exception_ptr`
526+
void *repr_ptr_2;
527+
#endif
528+
};
559529
struct CxxResult final {
560-
void *ptr;
530+
CxxException exc;
561531
};
562532
struct Exception final {
563-
CxxResult exc;
533+
CxxResult res;
564534
PtrLen msg;
565535
};
566536
} // namespace repr
567537

538+
// Ensure statically that `std::exception_ptr` is really a pointer.
539+
static_assert(sizeof(std::exception_ptr) == sizeof(repr::CxxException),
540+
"Unsupported std::exception_ptr size");
541+
static_assert(alignof(std::exception_ptr) == alignof(repr::CxxException),
542+
"Unsupported std::exception_ptr alignment");
543+
568544
extern "C" {
569545
repr::PtrLen cxxbridge1$exception(const char *, std::size_t len) noexcept;
546+
547+
repr::CxxException cxxbridge1$default_exception(const char *ptr,
548+
std::size_t len) noexcept {
549+
// Construct an `std::exception_ptr` for the default `rust::Error` in the
550+
// space provided by the pointer itself (placement new).
551+
//
552+
// The `std::exception_ptr` itself is just a pointer, so this effectively
553+
// converts it to the pointer.
554+
repr::CxxException eptr;
555+
new (&eptr) std::exception_ptr(
556+
std::make_exception_ptr(impl<Error>::error_copy(ptr, len)));
557+
return eptr;
570558
}
571559

560+
void cxxbridge1$drop_exception(repr::CxxException ptr) noexcept {
561+
// Implement the `drop` for `CxxException` on the Rust side, which is just a
562+
// pointer to the exception stored in `std::exception_ptr`.
563+
void *pptr = &ptr;
564+
std::exception_ptr eptr = std::move(*static_cast<std::exception_ptr *>(pptr));
565+
// eptr goes out of scope and deallocates `std::exception_ptr`.
566+
}
567+
568+
repr::CxxException
569+
cxxbridge1$clone_exception(repr::CxxException &ptr) noexcept {
570+
// Implement the `clone` for `CxxException` on the Rust side, which is just a
571+
// pointer to the exception stored in `std::exception_ptr`.
572+
repr::CxxException eptr;
573+
const void *pptr = &ptr;
574+
new (&eptr)
575+
std::exception_ptr(*static_cast<const std::exception_ptr *>(pptr));
576+
return eptr;
577+
}
578+
} // extern "C"
579+
572580
namespace detail {
573581
// On some platforms size_t is the same C++ type as one of the sized integer
574582
// types; on others it is a distinct type. Only in the latter case do we need to
@@ -593,16 +601,16 @@ class Fail final {
593601
};
594602

595603
void Fail::operator()(const char *catch$) noexcept {
596-
void *eptr;
604+
repr::CxxException eptr;
597605
new (&eptr)::std::exception_ptr(::std::current_exception());
598-
throw$.exc.ptr = eptr;
606+
throw$.res.exc = eptr;
599607
throw$.msg = cxxbridge1$exception(catch$, std::strlen(catch$));
600608
}
601609

602610
void Fail::operator()(const std::string &catch$) noexcept {
603-
void *eptr;
611+
repr::CxxException eptr;
604612
new (&eptr)::std::exception_ptr(::std::current_exception());
605-
throw$.exc.ptr = eptr;
613+
throw$.res.exc = eptr;
606614
throw$.msg = cxxbridge1$exception(catch$.data(), catch$.length());
607615
}
608616
} // namespace detail

‎src/result.rs‎

Lines changed: 47 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -19,47 +19,59 @@ pub(crate) struct PtrLen {
1919
pub len: usize,
2020
}
2121

22+
/// Representation of C++ `std::exception_ptr` for all targets except msvc.
23+
///
24+
/// This is a single pointer.
25+
#[repr(C)]
26+
#[derive(Copy, Clone)]
27+
#[cfg(not(target_env = "msvc"))]
28+
struct CxxExceptionRepr {
29+
ptr: NonNull<u8>,
30+
}
31+
32+
/// Representation of C++ `std::exception_ptr` for msvc.
33+
///
34+
/// Unfortunately, msvc uses EXCEPTION_
35+
#[repr(C)]
36+
#[derive(Copy, Clone)]
37+
#[cfg(target_env = "msvc")]
38+
struct CxxExceptionRepr {
39+
ptr: NonNull<u8>,
40+
}
41+
2242
extern "C" {
2343
/// Helper to construct the default exception from the error message.
2444
#[link_name = "cxxbridge1$default_exception"]
25-
fn default_exception(ptr: *const u8, len: usize) -> *mut u8;
45+
fn default_exception(ptr: *const u8, len: usize) -> CxxExceptionRepr;
2646
/// Helper to clone the instance of `std::exception_ptr` on the C++ side.
2747
#[link_name = "cxxbridge1$clone_exception"]
28-
fn clone_exception(ptr: *const u8) -> *mut u8;
48+
fn clone_exception(ptr: &CxxExceptionRepr) -> CxxExceptionRepr;
2949
/// Helper to drop the instance of `std::exception_ptr` on the C++ side.
3050
#[link_name = "cxxbridge1$drop_exception"]
31-
fn drop_exception(ptr: *mut u8);
51+
fn drop_exception(ptr: CxxExceptionRepr);
3252
}
3353

34-
/// C++ exception containing `std::exception_ptr`.
54+
/// C++ exception containing an `std::exception_ptr`.
3555
///
3656
/// This object is the Rust wrapper over `std::exception_ptr`, so it owns the exception pointer.
3757
/// I.e., the exception is either referenced by a `std::exception_ptr` on the C++ side or the
3858
/// reference is moved to this object on the Rust side.
3959
#[repr(C)]
4060
#[must_use]
41-
pub struct CxxException(NonNull<u8>);
61+
pub struct CxxException(CxxExceptionRepr);
4262

4363
impl CxxException {
4464
/// Construct the default `rust::Error` exception from the specified `exc_text`.
4565
fn new_default(exc_text: &str) -> Self {
46-
let exception_ptr = unsafe {
47-
default_exception(exc_text.as_ptr(), exc_text.len())
48-
};
49-
CxxException(
50-
NonNull::new(exception_ptr)
51-
.expect("Exception conversion returned a null pointer")
52-
)
66+
let exception_repr = unsafe { default_exception(exc_text.as_ptr(), exc_text.len()) };
67+
CxxException(exception_repr)
5368
}
5469
}
5570

5671
impl Clone for CxxException {
5772
fn clone(&self) -> Self {
58-
let clone_ptr = unsafe { clone_exception(self.0.as_ptr()) };
59-
Self(
60-
NonNull::new(clone_ptr)
61-
.expect("Exception cloning returned a null pointer")
62-
)
73+
let exception_repr = unsafe { clone_exception(&self.0) };
74+
Self(exception_repr)
6375
}
6476
}
6577

@@ -71,7 +83,7 @@ impl From<Exception> for CxxException {
7183

7284
impl Drop for CxxException {
7385
fn drop(&mut self) {
74-
unsafe { drop_exception(self.0.as_ptr()) };
86+
unsafe { drop_exception(self.0) };
7587
}
7688
}
7789

@@ -84,49 +96,34 @@ unsafe impl Sync for CxxException {}
8496

8597
/// C++ "result" containing `std::exception_ptr` or a `null`.
8698
#[repr(C)]
87-
pub struct CxxResult(*mut u8);
99+
pub struct CxxResult(Option<CxxException>);
88100

89101
impl From<CxxException> for CxxResult {
90102
fn from(value: CxxException) -> Self {
91-
let res = Self(value.0.as_ptr());
92-
// NOTE: we are copying the pointer, so we need to forget it here,
93-
// otherwise we'd double-free the `std::exception_ptr`.
94-
core::mem::forget(value);
95-
res
96-
}
97-
}
98-
99-
impl Drop for CxxResult {
100-
fn drop(&mut self) {
101-
if !self.0.is_null() {
102-
unsafe { drop_exception(self.0) };
103-
}
103+
Self(Some(value))
104104
}
105105
}
106106

107107
impl CxxResult {
108108
/// Construct an empty `Ok` result.
109109
pub fn new() -> Self {
110-
Self(core::ptr::null_mut())
110+
Self(None)
111111
}
112112
}
113113

114114
impl CxxResult {
115115
unsafe fn exception(self) -> Result<(), CxxException> {
116116
// SAFETY: We know that the `Result` can only contain a valid `std::exception_ptr` or null.
117-
match unsafe { self.0.as_mut() } {
117+
match self.0 {
118118
None => Ok(()),
119-
Some(ptr) => {
120-
let res = CxxException(NonNull::from(ptr));
121-
// NOTE: we are copying the pointer, so we need to forget this
122-
// object, otherwise we'd double-free the `std::exception_ptr`.
123-
core::mem::forget(self);
124-
Err(res)
125-
}
119+
Some(ptr) => Err(ptr),
126120
}
127121
}
128122
}
129123

124+
// Assert that the result is not larger than the exception (`Option` will use the niche).
125+
const _: () = assert!(core::mem::size_of::<CxxResult>() == core::mem::size_of::<CxxException>());
126+
130127
#[repr(C)]
131128
pub struct CxxResultWithMessage {
132129
pub(crate) res: CxxResult,
@@ -142,19 +139,20 @@ impl CxxResultWithMessage {
142139
// SAFETY: The message is always given for the exception and we constructed it in
143140
// a `Box` in `cxxbridge1$exception()`. We just reconstruct it here.
144141
let what = unsafe {
145-
str::from_utf8_unchecked_mut(
146-
slice::from_raw_parts_mut(self.msg.ptr.as_ptr(), self.msg.len))
142+
str::from_utf8_unchecked_mut(slice::from_raw_parts_mut(
143+
self.msg.ptr.as_ptr(),
144+
self.msg.len,
145+
))
147146
};
148147
Err(Exception {
149148
src,
150-
what: unsafe { Box::from_raw(what) }
149+
what: unsafe { Box::from_raw(what) },
151150
})
152151
}
153152
}
154153
}
155154
}
156155

157-
158156
/// Trait to convert an arbitrary Rust error into a C++ exception.
159157
///
160158
/// If an implementation of [`ToCxxException`] is explicitly provided for an `E`, then this
@@ -211,9 +209,8 @@ impl<T: Display> ToCxxExceptionDefault for &T {
211209
};
212210
// we have sufficient buffer size, just construct from the inplace
213211
// buffer
214-
let exc_text = unsafe {
215-
std::str::from_utf8_unchecked(&buffer.assume_init_ref()[0..size])
216-
};
212+
let exc_text =
213+
unsafe { std::str::from_utf8_unchecked(&buffer.assume_init_ref()[0..size]) };
217214
CxxException::new_default(exc_text)
218215
}
219216
#[cfg(not(feature = "std"))]
@@ -234,8 +231,8 @@ macro_rules! map_rust_error_to_cxx_exception {
234231
// the need for `specialization` feature. Namely, `ToCxxException` for `T` has higher
235232
// weight and is selected before `ToCxxExceptionDefault`, which is defined on `&T` (and
236233
// requires auto-deref). If it's not defined, then the default is used.
237-
use $crate::ToCxxExceptionDefault;
238234
use $crate::ToCxxException;
235+
use $crate::ToCxxExceptionDefault;
239236
(&$err).to_cxx_exception()
240237
};
241238
exc
@@ -250,7 +247,7 @@ macro_rules! map_rust_result_to_cxx_result {
250247
unsafe { ::core::ptr::write($ret_ptr, ok) };
251248
$crate::private::CxxResult::new()
252249
}
253-
Err(err) => $crate::private::CxxResult::from(err)
250+
Err(err) => $crate::private::CxxResult::from(err),
254251
}
255252
};
256253
}

0 commit comments

Comments
 (0)