reviewing stack overflow changes · wasmtime

Stream: wasmtime

Topic: reviewing stack overflow changes

Alex Crichton (Apr 17 2020 at 20:32):

Would someone be willing to volunteer to review https://github.com/bytecodealliance/wasmtime/pull/1490?

Implement interrupting wasm code, reimplement stack overflow by alexcrichton · Pull Request #1490 · bytecodealliance/wasmtime

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 r...

Dan Gohman (Apr 17 2020 at 21:11):

I'm reviewing it now

Alex Crichton (Apr 17 2020 at 21:12):

Thanks!

Dan Gohman (Apr 17 2020 at 21:41):

oh, arg, you're right, this needs to get expanded after the legalizer. let me think about that a bit

Alex Crichton (Apr 17 2020 at 21:41):

oh lol just wrote the same thing down

Alex Crichton (Apr 17 2020 at 21:41):

to be clear I'm fine with w/e works here

Alex Crichton (Apr 17 2020 at 21:42):

could maybe get by with a "mini interpreter" of global values

Alex Crichton (Apr 17 2020 at 21:42):

and only a small whitelist would be allowed in the prologue

Dan Gohman (Apr 17 2020 at 21:49):

Yeah, that's what I'm thinking too.

Dan Gohman (Apr 17 2020 at 21:50):

Use the GlobalValue to describe where the offset is, and have the backend interpret just enough to emit the code you need.

Dan Gohman (Apr 17 2020 at 21:50):

Does that sound workable?

Alex Crichton (Apr 17 2020 at 21:50):

mk, I'll work on implementing that

Alex Crichton (Apr 17 2020 at 21:50):

shouldn't be too too hard

Andrew Brown (Apr 18 2020 at 14:43):

It took me a while to connect the dots but I already have a Cranelift interpreter in progress over at https://github.com/bytecodealliance/cranelift/pull/1351 that could be used for this. @Alex Crichton, I need to rebase that on some more recent work and address some of Benjamin's comments but I would be interested in figuring out what exactly you need in GlobalValue expressions to make that interpreter work.

Add initial Cranelift interpreter functionality by abrown · Pull Request #1351 · bytecodealliance/cranelift

This has been discussed in meetings and chat messages. A short description of what this does, why it is needed: this implements a rudimentary interpreter for Cranelift IR; only a subset of instruc...

Andrew Brown (Apr 18 2020 at 14:48):

Uh, that's the old cranelift PR, now the code is at https://github.com/bytecodealliance/wasmtime/pull/1223

Add initial Cranelift interpreter by abrown · Pull Request #1223 · bytecodealliance/wasmtime

See bytecodealliance/cranelift#1351.

Dan Gohman (Apr 18 2020 at 14:49):

It turns out that we're using "interpret" here in the sense of "recognize". See interpret_gv in the PR; it's translating to new instructions rather than executing them.

Alex Crichton (Apr 18 2020 at 17:10):

Oh sorry yeah interpret is probably the wrong term

Andrew Brown (Apr 18 2020 at 17:53):

:+1:

Last updated: Dec 23 2024 at 13:07 UTC