Improve traps #2305
Improve traps #2305
Conversation
syrusakbary
requested review from
Hywan,
MarkMcCaskey and
nlewycky
as code owners
|
bors try |
tryBuild failed: |
|
bors try |
| pub struct CallThreadState { | ||
| unwind: Cell<UnwindReason>, | ||
| pub struct CallThreadState<'a> { | ||
| unwind: UnsafeCell<MaybeUninit<UnwindReason>>, |
There was a problem hiding this comment.
Can we replace MaybeUninit with something safer, like Option? That way we can detect an unwind that happened without an unwnd_with, instead of just executing undefined behaviour in unsafe Rust?
There was a problem hiding this comment.
Unfortunately I actually tried with Option but it turned to be not trivial (and I couldn't solve it in a few hours, so I went with this).
There was a problem hiding this comment.
Maybe add an explicit "Poison" state to the UnwindReason and then initialize to that?
There was a problem hiding this comment.
I made a pidgin copy of the problem in the rust playground, this code works:
use std::cell::UnsafeCell;
pub struct Foo {
unwind: UnsafeCell<Option<UnwindReason>>,
}
enum UnwindReason {
Foo(u8),
Bar
}
impl Foo {
fn a(mut self) {
match unsafe { &*self.unwind.get() } {
Some(UnwindReason::Foo(_unwind)) => {},
Some(UnwindReason::Bar) => {},
None => {}
}
}
fn b(&self, reason: UnwindReason) {
*(unsafe { &mut *self.unwind.get() }) = Some(reason);
}
}
| pub struct CallThreadState { | ||
| unwind: Cell<UnwindReason>, | ||
| pub struct CallThreadState<'a> { | ||
| unwind: UnsafeCell<MaybeUninit<UnwindReason>>, |
There was a problem hiding this comment.
Maybe add an explicit "Poison" state to the UnwindReason and then initialize to that?
| pub struct CallThreadState { | ||
| unwind: Cell<UnwindReason>, | ||
| pub struct CallThreadState<'a> { | ||
| unwind: UnsafeCell<MaybeUninit<UnwindReason>>, |
There was a problem hiding this comment.
I made a pidgin copy of the problem in the rust playground, this code works:
use std::cell::UnsafeCell;
pub struct Foo {
unwind: UnsafeCell<Option<UnwindReason>>,
}
enum UnwindReason {
Foo(u8),
Bar
}
impl Foo {
fn a(mut self) {
match unsafe { &*self.unwind.get() } {
Some(UnwindReason::Foo(_unwind)) => {},
Some(UnwindReason::Bar) => {},
None => {}
}
}
fn b(&self, reason: UnwindReason) {
*(unsafe { &mut *self.unwind.get() }) = Some(reason);
}
}
|
bors try |
|
Merging manually to avoid CI since tests have already passed and there are no new changes since last tests |
Description
This PR moves the trap code forward so we can start refactoring easily in subsequent PRs:
is_wasm_pcmethod to the traps/engine (so we only trap Wasm-related signals/...)Storegeneric overTrapInfo(so users can implement their custom trap handlers)Interrupt(non-used) andVMOutOfMemory(moved intoRuntimeError) cases inTrapCode, so it's deterministicReview