Stream: cranelift

Topic: cranelift and stack limit checks


view this post on Zulip Alex Crichton (Apr 08 2020 at 19:29):

@Dan Gohman hey so I'm poking around (after yesterday) at more of the stack limit/interrupt stuff, and I was looking to tackle #900 while I was at it. I'm thinking of mirroring spidermonkey where each relevant function has a prologue which checks the stack limit and traps if we ran out. It looks like there's an ArgumentPurpose::StackLimit parameter which is already in cranelift which is roughly analagous to this, but it seems like it may not be quite right? Does that require that every caller of the function pass in the stack limit? I'd ideally prefer to avoid reserving another argument for our purposes (we already take 2 with the two vmctx arguments)

Currently wasmtime handles stack overflow in wasm code through the use of the segfault signal handler, relying on wasm code to hit the guard page to trigger a segfault which we the longjmp to recov...

view this post on Zulip Alex Crichton (Apr 08 2020 at 19:30):

and fwiw you seemed active on both https://github.com/bytecodealliance/cranelift/pull/372 and https://github.com/bytecodealliance/cranelift/issues/349

Another approach to close #349
Cretonne is currently capable of detecting stack overflow when the stack is allocated with a guard page at the end, and it performs stack probes as needed when allocating large frames. Most popular...

view this post on Zulip Alex Crichton (Apr 08 2020 at 19:34):

or is there a way we could like synthesize an "argument" inside of each function?

view this post on Zulip Alex Crichton (Apr 08 2020 at 19:34):

so it's calculated from other arguments

view this post on Zulip Dan Gohman (Apr 08 2020 at 19:36):

right now, you're right, StackLimit is an argument that needs to be passed in

view this post on Zulip Alex Crichton (Apr 08 2020 at 19:36):

bah ok, so this isn't quite what I want then

view this post on Zulip Dan Gohman (Apr 08 2020 at 19:38):

take a look at insert_common_prologue in cranelift/codegen/src/isa/x86/abi.rs

view this post on Zulip Dan Gohman (Apr 08 2020 at 19:38):

You probably want to reuse insert_stack_check, but you just need another way of telling it where to find the limit

view this post on Zulip Alex Crichton (Apr 08 2020 at 19:38):

kk, I'll see if I can whip up something

view this post on Zulip Alex Crichton (Apr 08 2020 at 20:06):

error[E0061]: this function takes 12 parameters but 11 parameters were supplied

view this post on Zulip Alex Crichton (Apr 08 2020 at 20:06):

cry

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:35):

@Dan Gohman so I'm getting can't encode v17 = ifcmp_sp.i32 v16 as a panic message which is called from here and panics here

Standalone JIT-style runtime for WebAssembly, using Cranelift - bytecodealliance/wasmtime
Standalone JIT-style runtime for WebAssembly, using Cranelift - bytecodealliance/wasmtime

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:35):

it seems like this previous StackLimit stuff just wasn't ever fully tested before? do you know if that's the case?

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:36):

I see a text test or two but I don't know why that doesn't panic but what I'm doing panics (unless something is specific to actually encoding instructions)

view this post on Zulip Dan Gohman (Apr 08 2020 at 21:37):

cranelift/filetests/filetests/isa/x86/prologue-epilogue.clif is a test which exercises a stack_limit argument

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:37):

right yeah

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:37):

is that testing the actual x86 encoding/emission

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:37):

which means that I'm just doing something wrong?

view this post on Zulip Dan Gohman (Apr 08 2020 at 21:37):

but, I wouldn't be too surprised if it doesn't work if you use it differently

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:37):

hm ok, I know very little about cranelift and all this...

view this post on Zulip Dan Gohman (Apr 08 2020 at 21:38):

oh, you probably want to do ifcmp_sp.i64, not i32, on x86-64

view this post on Zulip Dan Gohman (Apr 08 2020 at 21:38):

It probably doesn't know how to encode a 32-bit version of that

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:38):

is there a separate method for that?

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:39):

oh wait I think I had a type error in the IR

view this post on Zulip Dan Gohman (Apr 08 2020 at 21:39):

It's inferred from the operand.

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:39):

yeah that made me realize I was doing a 32-bit load when I wanted a 64-bit one

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:39):

which may be why the encoding failed

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:39):

oh hey!

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:39):

now it works!

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:39):

sorta at least

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:40):

ok cool thanks

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:40):

for being a rubber duck :)

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:40):

and pointing out bit-nes

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:41):

@Dan Gohman is there a way to dump generated code to stderr?

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:41):

or otherwise debug what's happening?

view this post on Zulip Dan Gohman (Apr 08 2020 at 21:43):

Not from wasmtime right now.

view this post on Zulip Dan Gohman (Apr 08 2020 at 21:43):

Cranelift has some utilities in cranelift/src/disasm.rs which can use capstone to disassemble the output, but they're not yet wired up in convenient ways

view this post on Zulip Dan Gohman (Apr 08 2020 at 21:43):

outside of clif-util, at least

view this post on Zulip Yury Delendik (Apr 08 2020 at 21:44):

or use lldb :P

view this post on Zulip Dan Gohman (Apr 08 2020 at 21:46):

that too :smile:

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:46):

ok no worries, I've now got every single wasm function to immediately trap

view this post on Zulip Alex Crichton (Apr 08 2020 at 21:46):

which is good, and it seems to be passing the verifier too

view this post on Zulip Benjamin Bouvier (Apr 09 2020 at 10:55):

Fwiw, Spidermonkey doesn't pass the stack check limit as an argument; stack checks are generated in Cranelift IR directly.


Last updated: Dec 23 2024 at 12:05 UTC