Alex Crichton (Apr 29 2020 at 15:19): Alex Crichton (Apr 29 2020 at 15:19):@Dan Gohman did you want to take a look at https://github.com/bytecodealliance/wasmtime/pull/1577 before I merged it?
Dan Gohman (Apr 29 2020 at 17:25):yeah. I'll look now.
Dan Gohman (Apr 29 2020 at 17:25):let segfault = Func::wrap(&store, || segfault()); -- you can do that? ;-)
Dan Gohman (Apr 29 2020 at 17:26):oh, nm. segfault is already a function.
Alex Crichton (Apr 29 2020 at 17:40):@Dan Gohman oh no rush, just figured it was something you'd be interested in
Dan Gohman (Apr 29 2020 at 17:41):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.
Dan Gohman (Apr 29 2020 at 17:42):We currently have logic which says if the PC is a known trap, we're good, otherwise assume it's stack overflow
Alex Crichton (Apr 29 2020 at 17:43):yeah when working on some aarch64 stuff it was actually pretty hard b/c we kept catching segfaults and illegal instructions
Alex Crichton (Apr 29 2020 at 17:44):I think we likely want to move to a more precise "this is the list of instructions that can fault" map
Alex Crichton (Apr 29 2020 at 17:44):we sort of already have that, just need to plumb it all through
Dan Gohman (Apr 29 2020 at 17:45):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
Dan Gohman (Apr 29 2020 at 17:45):ok, but this is all independent of the PR, so I'll r+ that now
Pat Hickey (Apr 29 2020 at 19:17):in lucet we ship that "list of instructions that can fault" map, we call it a trap table.
Pat Hickey (Apr 29 2020 at 19:18):its a map of offsets from func start to cranelift_codegen::TrapCode
Pat Hickey (Apr 29 2020 at 19:19):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
Pat Hickey (Apr 29 2020 at 19:20):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.
Alex Crichton (Apr 29 2020 at 19:25):@Pat Hickey do you have a link to where that table is constructed?
Alex Crichton (Apr 29 2020 at 19:25):sounds perfect for wasmtime
Pat Hickey (Apr 29 2020 at 19:26):https://github.com/bytecodealliance/lucet/blob/master/lucetc/src/compiler.rs#L557
Pat Hickey (Apr 29 2020 at 19:26):we get the traps out of cranelift-module
Pat Hickey (Apr 29 2020 at 19:27):and put them right back into cranelift-module as data sections
Pat Hickey (Apr 29 2020 at 19:27):@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
Alex Crichton (Apr 29 2020 at 19:29):awesome thanks!
Alex Crichton (Apr 29 2020 at 19:29):I'll take a look at that and see if we can do the same
Last updated: Nov 22 2024 at 17:03 UTC