Skip to content

Commit

Permalink
Dedicated trap code for suspending with a unhandled tag (bytecodealli…
Browse files Browse the repository at this point in the history
…ance#35)

In the generated code for `resume`, we have a block that is branched to when none of the tags handled by the resume instruction under consideration handle the tag that we encountered. This block then contains the necessary logic to forward to the parent continuation.

However, if no such parent exists, the current behaviour means that we re-suspend the unhandled tag, but now there is no handler at all. This means that the error for the case where we have active handlers, but none of them handle the `suspend`-ed tag is equivalent to the error shown when `suspend`-ing when there is no handler at all.

This PR addresses an existing TODO: In the forwarding block, we check if there actually is a parent to forward to. If not, we now trap with a new, dedicated trap code to indicate that there is no matching handler.

---------

Co-authored-by: Daniel Hillerström <daniel.hillerstrom@ed.ac.uk>
  • Loading branch information
frank-emrich and dhil authored Nov 2, 2023
1 parent 65f7629 commit 82539bf
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 3 deletions.
6 changes: 6 additions & 0 deletions cranelift/codegen/src/ir/trapcode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ pub enum TrapCode {

/// A null reference was encountered which was required to be non-null.
NullReference,

/// We are suspending to a tag for which there is no active handler.
UnhandledTag,
}

impl TrapCode {
Expand All @@ -72,6 +75,7 @@ impl TrapCode {
TrapCode::UnreachableCodeReached,
TrapCode::Interrupt,
TrapCode::NullReference,
TrapCode::UnhandledTag,
]
}
}
Expand All @@ -93,6 +97,7 @@ impl Display for TrapCode {
Interrupt => "interrupt",
User(x) => return write!(f, "user{}", x),
NullReference => "null_reference",
UnhandledTag => "unhandled_tag",
};
f.write_str(identifier)
}
Expand All @@ -116,6 +121,7 @@ impl FromStr for TrapCode {
"unreachable" => Ok(UnreachableCodeReached),
"interrupt" => Ok(Interrupt),
"null_reference" => Ok(NullReference),
"unhandled_tag" => Ok(UnhandledTag),
_ if s.starts_with("user") => s[4..].parse().map(User).map_err(|_| ()),
_ => Err(()),
}
Expand Down
1 change: 1 addition & 0 deletions crates/cranelift-shared/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ pub fn mach_trap_to_trap(trap: &MachTrap) -> Option<TrapInformation> {
ir::TrapCode::User(ALWAYS_TRAP_CODE) => Trap::AlwaysTrapAdapter,
ir::TrapCode::User(CANNOT_ENTER_CODE) => Trap::CannotEnterComponent,
ir::TrapCode::NullReference => Trap::NullReference,
ir::TrapCode::UnhandledTag => Trap::UnhandledTag,

// These do not get converted to wasmtime traps, since they
// shouldn't ever be hit in theory. Instead of catching and handling
Expand Down
5 changes: 3 additions & 2 deletions crates/cranelift/src/func_environ.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3013,8 +3013,9 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m

let parent_contobj = self.typed_continuations_load_parent(builder, resumed_contobj);

// TODO(frank-emrich): Check if parent is null. If so, we are at
// the toplevel and simply have no handler for the given tag and must trap.
builder
.ins()
.trapz(parent_contobj, ir::TrapCode::UnhandledTag);

// We suspend, thus deferring handling to the parent.
// We do nothing about tag *parameters*, these remain unchanged within the
Expand Down
5 changes: 5 additions & 0 deletions crates/environ/src/trap_encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ pub enum Trap {
/// would have violated the reentrance rules of the component model,
/// triggering a trap instead.
CannotEnterComponent,

/// We are suspending to a tag for which there is no active handler.
UnhandledTag,
// if adding a variant here be sure to update the `check!` macro below
}

Expand All @@ -121,6 +124,7 @@ impl fmt::Display for Trap {
AtomicWaitNonSharedMemory => "atomic wait on non-shared memory",
NullReference => "null reference",
CannotEnterComponent => "cannot enter component instance",
UnhandledTag => "unhandled tag",
};
write!(f, "wasm trap: {desc}")
}
Expand Down Expand Up @@ -237,6 +241,7 @@ pub fn lookup_trap_code(section: &[u8], offset: usize) -> Option<Trap> {
AtomicWaitNonSharedMemory
NullReference
CannotEnterComponent
UnhandledTag
}

if cfg!(debug_assertions) {
Expand Down
5 changes: 4 additions & 1 deletion crates/runtime/src/traphandlers/backtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ impl Backtrace {

/// Walk through a contiguous sequence of Wasm frames starting with the
/// frame at the given PC and FP and ending at `trampoline_sp`.
// TODO(frank-emrich) Implement tracing across continuations.
unsafe fn trace_through_wasm(
mut pc: usize,
mut fp: usize,
Expand Down Expand Up @@ -211,7 +212,9 @@ impl Backtrace {
// The stack grows down, and therefore any frame pointer we are
// dealing with should be less than the stack pointer on entry
// to Wasm.
assert!(trampoline_sp >= fp, "{trampoline_sp:#x} >= {fp:#x}");
// TODO(frank-emrich) Disabled for now, as it does not hold in
// presence of continuations.
//assert!(trampoline_sp >= fp, "{trampoline_sp:#x} >= {fp:#x}");

arch::assert_fp_is_aligned(fp);

Expand Down

0 comments on commit 82539bf

Please sign in to comment.