@Dan Gohman did you want to take a look at https://github.com/bytecodealliance/wasmtime/pull/1577 before I merged it?
yeah. I'll look now.
let segfault = Func::wrap(&store, || segfault());
-- you can do that? ;-)
oh, nm. segfault
is already a function.
@Dan Gohman oh no rush, just figured it was something you'd be interested in
I've read it and it looks good. I'm just on a bit of a tangent thinking about unexpected segfaults in JIT code, which could indicate JIT bugs.
We currently have logic which says if the PC is a known trap, we're good, otherwise assume it's stack overflow
yeah when working on some aarch64 stuff it was actually pretty hard b/c we kept catching segfaults and illegal instructions
I think we likely want to move to a more precise "this is the list of instructions that can fault" map
we sort of already have that, just need to plumb it all through
yeah. And a while ago I put some effort into trying to get Cranelift to annotate all its stack references as possible StackOverflow trap sites, but what I don't remember offhand is whether that's complete
ok, but this is all independent of the PR, so I'll r+ that now
in lucet we ship that "list of instructions that can fault" map, we call it a trap table.
its a map of offsets from func start to cranelift_codegen::TrapCode
if we ever get a fault that doesn't map to a TrapCode, we terminate the entire process, on the grounds that its a possible cranelift codegen bug, or somehow the code was overwritten
in the early days of that, we found some missing cases for StackOverflow trap sites, but I believe they are now all in there. there's always a possibility that we have over-approximated the trap sites, but currently I believe they are not under-approximated.
@Pat Hickey do you have a link to where that table is constructed?
sounds perfect for wasmtime
https://github.com/bytecodealliance/lucet/blob/master/lucetc/src/compiler.rs#L557
we get the traps out of cranelift-module
and put them right back into cranelift-module as data sections
@Nathan Froyd was the last person to work on this code, he did a great job refactoring it to use cranelift-module directly, previously not everything was exposed properly so we had to inject it straight into faerie
awesome thanks!
I'll take a look at that and see if we can do the same
Last updated: Dec 23 2024 at 13:07 UTC