wasmtime / PR #1490 Implement interrupting wasm code, rei... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #1490 Implement interrupting wasm code, rei...

Wasmtime GitHub notifications bot (Apr 09 2020 at 19:08):

alexcrichton opened PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 09 2020 at 19:29):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 09 2020 at 20:54):

bjorn3 submitted PR Review.

Wasmtime GitHub notifications bot (Apr 09 2020 at 20:54):

bjorn3 submitted PR Review.

Wasmtime GitHub notifications bot (Apr 09 2020 at 20:54):

bjorn3 created PR Review Comment:

How is this serialized to a .clif file?

Wasmtime GitHub notifications bot (Apr 09 2020 at 21:49):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Apr 09 2020 at 21:49):

alexcrichton created PR Review Comment:

It is not.

Wasmtime GitHub notifications bot (Apr 10 2020 at 08:18):

bjorn3 submitted PR Review.

Wasmtime GitHub notifications bot (Apr 10 2020 at 08:18):

bjorn3 created PR Review Comment:

Maybe use a GlobalValue here instead. That way it is serializable.

Wasmtime GitHub notifications bot (Apr 10 2020 at 08:18):

bjorn3 edited PR Review Comment.

Wasmtime GitHub notifications bot (Apr 10 2020 at 16:23):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 10 2020 at 16:24):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Apr 10 2020 at 16:24):

alexcrichton created PR Review Comment:

I'm not sure if that would work out because of how manual this is due to how late in the pipeline this executes. I'm also pretty unfamiliar with cranelift itself. If you've got an example of how to do this that would be great!

Wasmtime GitHub notifications bot (Apr 10 2020 at 17:25):

fitzgen submitted PR Review.

Wasmtime GitHub notifications bot (Apr 10 2020 at 17:25):

fitzgen submitted PR Review.

Wasmtime GitHub notifications bot (Apr 10 2020 at 17:25):

fitzgen created PR Review Comment:

FWIW, this needs to be [] not ().

Wasmtime GitHub notifications bot (Apr 10 2020 at 17:25):

fitzgen created PR Review Comment:

assert that the size is greater than zero? (or some reasonable min)

Wasmtime GitHub notifications bot (Apr 10 2020 at 17:25):

fitzgen created PR Review Comment:

What @bjorn3 was hinting at is that all input cranelift accepts (not cranelift-wasm, but cranelift itself) is currently representable in the the clif text format. This is really nice for testing, reproducibility, and test case reduction. For example, see "Don't Lose Our Copy-and-Paste-This-IR-and-it-is-a-Test Property" in https://github.com/bytecodealliance/wasmtime/issues/1146#issuecomment-566790342

By providing a closure that is called in the middle of cranelift compilation, and without a text representation, we're losing those nice testing properties.

In this specific case, I'm not sure what alternative is better here. This isn't really generic behavior that makes sense inside cranelift. And we don't have anything like "always inline functions/macros" right now (maybe https://github.com/bytecodealliance/wasmtime/pull/1267 could eventually be that, its motivation was similar, but for GC barriers).

@sunfishcode felt pretty strongly about the everything-is-in-clif-text property, so tagging him for ideas here.

Wasmtime GitHub notifications bot (Apr 10 2020 at 17:25):

fitzgen created PR Review Comment:

This should be four bools because it uses two per config (one for it the swarm is enabled, another for if it actually sets the config flag).

Also the comment above should be updated.

Wasmtime GitHub notifications bot (Apr 10 2020 at 17:25):

fitzgen created PR Review Comment:

Also add a test for this file over here: https://github.com/bytecodealliance/wasmtime/blob/master/crates/c-api/tests/wasm-c-examples.rs

Wasmtime GitHub notifications bot (Apr 10 2020 at 17:25):

fitzgen created PR Review Comment:

Would it be possible to add translate_func_prologue to cranelift_wasm::FuncEnviron, where we already have embedder hooks and can side step these issues? I think this would also make it so we wouldn't need to regalloc by hand here.

Wasmtime GitHub notifications bot (Apr 10 2020 at 17:53):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Apr 10 2020 at 17:53):

alexcrichton created PR Review Comment:

Hm I think that () is what's wanted here since that's the destination of the link. I was fixing rustdoc warnings here and this is what was necessary to fix the warning.

Wasmtime GitHub notifications bot (Apr 10 2020 at 17:54):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Apr 10 2020 at 17:54):

alexcrichton created PR Review Comment:

I don't think it'd help much to single-out zero here, 1 byte of stack allocation is just as nonsensical in a sense. I personally think it's ok to take everything here, and if someone is toying around with 0-byte wasm stacks for testing that seems like it's fine to allow.

Wasmtime GitHub notifications bot (Apr 10 2020 at 17:55):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Apr 10 2020 at 17:55):

alexcrichton created PR Review Comment:

Hm I don't think this is actually that necessary, we already run all the examples on CI with cargo run -p run-examples?

Wasmtime GitHub notifications bot (Apr 10 2020 at 17:55):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 10 2020 at 17:57):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Apr 10 2020 at 17:57):

alexcrichton created PR Review Comment:

I'm not really wed to this particular solution at all, and I agree it's a great property to have everything reflected in the text format. That being said though we need some solution here which actually works and accounts for the wasmtime-specific behavior of where the stack limit is located. I don't know of a great way to do this, but would be totally onboard with alternative ways.

I don't know how to make translate_func_prologue work since the stack limit check happens at such an odd time (machine code generation) not at cranelift IR generation time (when wasm is translated). Would this need some sort of magical sequence of instructions at the prologue which gets pattern-matched for the stack check?

Wasmtime GitHub notifications bot (Apr 10 2020 at 18:06):

fitzgen submitted PR Review.

Wasmtime GitHub notifications bot (Apr 10 2020 at 18:06):

fitzgen created PR Review Comment:

Ah ok, I guess https://github.com/bytecodealliance/wasmtime/pull/1463 was unnecessary

Wasmtime GitHub notifications bot (Apr 10 2020 at 18:07):

fitzgen submitted PR Review.

Wasmtime GitHub notifications bot (Apr 10 2020 at 18:07):

fitzgen created PR Review Comment:

I don't know how to make translate_func_prologue work since the stack limit check happens at such an odd time (machine code generation) not at cranelift IR generation time (when wasm is translated). Would this need some sort of magical sequence of instructions at the prologue which gets pattern-matched for the stack check?

I think we would need an instruction to get the current stack frame's size or something. Not sure how well this would work out.

Wasmtime GitHub notifications bot (Apr 10 2020 at 18:12):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Apr 10 2020 at 18:12):

alexcrichton created PR Review Comment:

The impression I get though is that representing this in IR is the wrong idea because if we generate IR to do the stack check, then the actual stack adjustment happens in the function's prologue, which is too late then to perform the stack check. This is very machine-specific because it's part of the prologue/epilogue generation at a very particular part of codegen, so afaik it can't be very easily represented in IR without a lot of "magic", but others may know better routes!

Wasmtime GitHub notifications bot (Apr 14 2020 at 09:53):

bnjbvr submitted PR Review.

Wasmtime GitHub notifications bot (Apr 14 2020 at 09:53):

bnjbvr created PR Review Comment:

I don't know how to make translate_func_prologue work since the stack limit check happens at such an odd time (machine code generation) not at cranelift IR generation time (when wasm is translated). Would this need some sort of magical sequence of instructions at the prologue which gets pattern-matched for the stack check?

For what it's worth, the way it's dealt with in Spidermonkey is that:

Spidermonkey has its own calling convention "baldrdash", used in Cranelift.

Cranelift won't generate the prologue and epilogue if the calling convention is baldrdash; Spidermonkey will.

Then the interrupt check can be done in Cranelift IR from the Spidermonkey side.

In addition to this, Spidermonkey has to use the fallthrough_return instead of a plain return, since it generates its own epilogue.

I imagine replicating this behavior for Wasmtime would be quite a hassle, so this might not be the best path here...

Wasmtime GitHub notifications bot (Apr 14 2020 at 14:23):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 14 2020 at 16:14):

pepyakin submitted PR Review.

Wasmtime GitHub notifications bot (Apr 14 2020 at 16:14):

pepyakin created PR Review Comment:

I think you meant max_wasm_stack here, right?

Wasmtime GitHub notifications bot (Apr 14 2020 at 16:14):

pepyakin submitted PR Review.

Wasmtime GitHub notifications bot (Apr 14 2020 at 16:14):

pepyakin created PR Review Comment:

Doesn't this warrant an unsafe marker on update_stack_limit since this only works if self is on the stack. A comment at the call site might do as well.

Wasmtime GitHub notifications bot (Apr 14 2020 at 19:56):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 14 2020 at 19:57):

alexcrichton created PR Review Comment:

Oh oops good point. I've updated the comment on the function a bit

Wasmtime GitHub notifications bot (Apr 14 2020 at 19:57):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Apr 15 2020 at 13:41):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 17 2020 at 20:35):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 17 2020 at 21:03):

abrown submitted PR Review.

Wasmtime GitHub notifications bot (Apr 17 2020 at 21:03):

abrown created PR Review Comment:

-1?

Wasmtime GitHub notifications bot (Apr 17 2020 at 21:16):

bjorn3 submitted PR Review.

Wasmtime GitHub notifications bot (Apr 17 2020 at 21:16):

bjorn3 created PR Review Comment:

If you can use a GlobalValueData::Load to load from a GlobalValueData::VMContext. Then you set this field to the GlobalValue of the GlobalValueData. You can then during the stack check codegen use global_value to get the stack limit

Wasmtime GitHub notifications bot (Apr 17 2020 at 21:16):

bjorn3 edited PR Review Comment.

Wasmtime GitHub notifications bot (Apr 17 2020 at 21:33):

sunfishcode submitted PR Review.

Wasmtime GitHub notifications bot (Apr 17 2020 at 21:33):

sunfishcode created PR Review Comment:

Keeping ir::Function serializable makes it easier for us to extract standalone Cranelift testcases that don't require linking with surrounding applications. And in the future, we also hope to use this property to enable more compilation caching and remote/ipc compilation.

Global values are an expression language which you can use to define code templates that Cranelift can expand into actual code later (https://github.com/bytecodealliance/wasmtime/pull/1267 is a further step toward generalizing the concept and actually renames it to "template"). Currently, you can only do a few simple things with them: load, iadd_imm, vmctx addresses, and symbolic addresses. That looks like it may actually be enough for what you need to do here, though we can add more operations if needed.

So the idea is, you create them, one "instruction" at a time, with declare_global_value which returns the index of the global value which you can then use however you want within the ir::Function.

Wasmtime GitHub notifications bot (Apr 17 2020 at 21:34):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Apr 17 2020 at 21:34):

alexcrichton created PR Review Comment:

I tried that but got a panic that I think means the legalizer needs to run, but this is happening after the legalizer has already run.

Wasmtime GitHub notifications bot (Apr 17 2020 at 21:41):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Apr 17 2020 at 21:41):

alexcrichton created PR Review Comment:

@sunfishcode oh I think that's what @bjorn3 recommended above a few minutes ago as well, but do you know how to work around the legalizer issue? Expanding a global value seems to require the legalizer to run, but since this is in prologue/epilogue generation that's too late for the legalizer to run it seems.

Wasmtime GitHub notifications bot (Apr 17 2020 at 21:48):

bjorn3 submitted PR Review.

Wasmtime GitHub notifications bot (Apr 17 2020 at 21:48):

bjorn3 created PR Review Comment:

It seems that EncCursor doesn't allow inserting instructions that need to be legalized.

Wasmtime GitHub notifications bot (Apr 17 2020 at 22:03):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 17 2020 at 22:03):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Apr 17 2020 at 22:03):

alexcrichton created PR Review Comment:

Oops, updated the comment!

Wasmtime GitHub notifications bot (Apr 17 2020 at 22:24):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 18 2020 at 00:51):

sunfishcode submitted PR Review.

Wasmtime GitHub notifications bot (Apr 18 2020 at 00:51):

sunfishcode submitted PR Review.

Wasmtime GitHub notifications bot (Apr 18 2020 at 00:51):

sunfishcode created PR Review Comment:

This technique is independent of x86, so we should just say "the stack pointer" rather than "%rsp".

Wasmtime GitHub notifications bot (Apr 18 2020 at 00:51):

sunfishcode created PR Review Comment:

s/wasm/Wasmtime/

Wasmtime GitHub notifications bot (Apr 18 2020 at 00:51):

sunfishcode created PR Review Comment:

"How does Wasmtime interrupt wasm code"?

Wasmtime GitHub notifications bot (Apr 18 2020 at 00:51):

sunfishcode created PR Review Comment:

This focuses on how we implement it, but for users, we should say something like "Whether or not to enable the ability to interrupt wasm code dynamically", and also mention the limitation that calls into host code can't be interrupted. And maybe also link to how to perform an interrupt.

Wasmtime GitHub notifications bot (Apr 18 2020 at 00:51):

sunfishcode created PR Review Comment:

With this change, we're now overriding Rust's SA_ONSTACK handler with our own non-SA_ONSTACK handler. This means that all Rust code in a program which uses Wasmtime will no longer get the Rust stack overflow handler on stack overflow, but a generic segfault; is that right? That seems like something we should document somewhere; Rust users may be accustomed to not seeing their programs killed by segfault, and may otherwise interpret a segfault as a sign of a more serious problem.

Wasmtime GitHub notifications bot (Apr 18 2020 at 00:51):

sunfishcode created PR Review Comment:

It's surprising that this is being represented as a signed i32, since the value becomes negative, and elsewhere it's casted to a usize and treated as unsigned.

Wasmtime GitHub notifications bot (Apr 18 2020 at 00:51):

sunfishcode created PR Review Comment:

Would it be difficult to add similar code to the wasmtime-cli as a command-line argument to let users specify --timeout=30s or so?

Wasmtime GitHub notifications bot (Apr 20 2020 at 14:38):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 20 2020 at 14:38):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Apr 20 2020 at 14:38):

alexcrichton created PR Review Comment:

Sure!

I added this as --wasm-timeout=30 so in the future we could add --timeout which accounts for both compile time and for wasm execution time.

Wasmtime GitHub notifications bot (Apr 20 2020 at 14:40):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Apr 20 2020 at 14:40):

alexcrichton created PR Review Comment:

That's true yeah, and unfortunately I'm not sure there's really anything we can do about it. The only real way to fix this would be to always run on the sigaltstack which brings up the same issues that not running on the sigaltstack is trying to fix.

I'll throw a comment here in signal registration in case anyone is poking around, I'm not really sure where else would be an effective place to comment this.

Wasmtime GitHub notifications bot (Apr 20 2020 at 14:47):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 20 2020 at 14:49):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 20 2020 at 14:49):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Apr 20 2020 at 14:49):

alexcrichton created PR Review Comment:

Er I'm basically trying to be too clever by half here, I've updated this to be usize.

Wasmtime GitHub notifications bot (Apr 20 2020 at 15:18):

sunfishcode submitted PR Review.

Wasmtime GitHub notifications bot (Apr 20 2020 at 15:18):

sunfishcode created PR Review Comment:

We already have a dependency on humantime (via env_logger), so could we make this take a string and parse it with parse_duration? That way people can specify timeouts less than a second, and large timeouts are more convenient.

Wasmtime GitHub notifications bot (Apr 20 2020 at 15:18):

sunfishcode submitted PR Review.

Wasmtime GitHub notifications bot (Apr 20 2020 at 15:18):

sunfishcode created PR Review Comment:

Typo: tha->that

Wasmtime GitHub notifications bot (Apr 20 2020 at 15:38):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 20 2020 at 15:38):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Apr 20 2020 at 15:38):

alexcrichton created PR Review Comment:

Oh nice idea!

Updated now

Wasmtime GitHub notifications bot (Apr 20 2020 at 15:48):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 20 2020 at 20:22):

sunfishcode submitted PR Review.

Wasmtime GitHub notifications bot (Apr 20 2020 at 20:52):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 20 2020 at 20:58):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 20 2020 at 21:43):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 21 2020 at 03:51):

sunfishcode submitted PR Review.

Wasmtime GitHub notifications bot (Apr 21 2020 at 03:51):

sunfishcode created PR Review Comment:

func() has been renamed to into_func(), but this can be simplified to just instance.get_func("foo").

Wasmtime GitHub notifications bot (Apr 21 2020 at 14:20):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 21 2020 at 16:58):

alexcrichton updated PR #1490 from catch-stack-overflow to master:

This commit is a relatively large change for wasmtime with two main
goals:

Primarily this enables interrupting executing wasm code with a trap,
preventing infinite loops in wasm code. Note that resumption of the
wasm code is not a goal of this commit.

Additionally this commit reimplements how we handle stack overflow to
ensure that host functions always have a reasonable amount of stack to
run on. This fixes an issue where we might longjmp out of a host
function, skipping destructors.

Lots of various odds and ends end up falling out in this commit once the
two goals above were implemented. The strategy for implementing this was
also lifted from Spidermonkey and existing functionality inside of
Cranelift. I've tried to write up thorough documentation of how this all
works in crates/environ/src/cranelift.rs where gnarly-ish bits are.

A brief summary of how this works is that each function and each loop
header now checks to see if they're interrupted. Interrupts and the
stack overflow check are actually folded into one now, where function
headers check to see if they've run out of stack and the sentinel value
used to indicate an interrupt, checked in loop headers, tricks functions
into thinking they're out of stack. An interrupt is basically just
writing a value to a location which is read by JIT code.

When interrupts are delivered and what triggers them has been left up to
embedders of the wasmtime crate. The wasmtime::Store type has a
method to acquire an InterruptHandle, where InterruptHandle is a
Send and Sync type which can travel to other threads (or perhaps
even a signal handler) to get notified from. It's intended that this
provides a good degree of flexibility when interrupting wasm code. Note
though that this does have a large caveat where interrupts don't work
when you're interrupting host code, so if you've got a host import
blocking for a long time an interrupt won't actually be received until
the wasm starts running again.

Some fallout included from this change is:

Unix signal handlers are no longer registered with SA_ONSTACK.
Instead they run on the native stack the thread was already using.
This is possible since stack overflow isn't handled by hitting the
guard page, but rather it's explicitly checked for in wasm now. Native
stack overflow will continue to abort the process as usual.

Unix sigaltstack management is now no longer necessary since we don't
use it any more.

Windows no longer has any need to reset guard pages since we no longer
try to recover from faults on guard pages.

On all targets probestack intrinsics are disabled since we use a
different mechanism for catching stack overflow.

The C API has been updated with interrupts handles. An example has
also been added which shows off how to interrupt a module.

Closes #139
Closes #860
Closes #900

Wasmtime GitHub notifications bot (Apr 21 2020 at 18:03):

sunfishcode merged PR #1490.

Last updated: Dec 23 2024 at 12:05 UTC