Many Go programs crash at request time under p1-to-p2 adaptation

[erikrose]

We have observed this about Big Go services in production tests. Not all (Big) Go services crash, but most yield 500s.

Here are a few truths we know:

• Big Go doesn't export a malloc() we can straightforwardly abuse as we do with TinyGo.
• Big Go will not, in the forseeable future, support wasip2 directly, because of Google's lack of interest. Somebody may make a MediumGo that does and that doesn't make such space-constrained tradeoffs (lack of RTTI, etc.) as TinyGo does.
• Big Go implements its own linker and does not use wasm-ld.
• Big Go’s GC gets regularly rewritten, so poking its internals is frought. Similarly, glomming 2 Go programs together as in Remove restriction on .wasm extension #2 in Empty TinyGo project crashes under component adapter #491 (comment) is frought.
• It looks like Big Go claims the whole heap, like TinyGo does. Not totally proven yet.

Ideas:

• Maybe we can lie about the start of the heap, though, again, that's in a private Go-invented linker struct. Maybe it's stabler than GC free-ness data?
• Maybe we can use multiple memories somehow. Look into why adapters currently have no memories of their own and understand if that's vital.

[erikrose]

@sunfishcode had a brilliant idea just now: what if we set aside RAM for the adapter not on the heap but by having a great big stack variable, over in the adapter? Then we don't have to talk to the GC at all!

• Make sure you don't let Rust create a static copy of the State struct while initing (You'd get err something like: "can't reduce such and such" at adaptation time.)
• May need MaybeUninit: let state = MaybeUninit::<State>::uninit();

(Relevant mystery: why does the adaptation code allocate its own stack explicitly? I'd love to understand that in case it bears on this.)

Failing that, we could rewrite the customer module to add 64K-ish to each load and store instruction using an offset. But then we'd have to make up for that when we pass a LM ptr to a host call. Maybe we could do that in cabi_realloc(), which folks call to get buffers?

[erikrose]

Also got the skinny from Dan on why we can't easily use a second linear memory for the adapter state: Rust assumes memory 0, so you wouldn't be able to just write the adapter in Rust code as we currently do unless we do a lot of toolchain work to get it to tag each memory access with which memory it's talking about.

[chenyan2002]

Rust assumes memory 0, so you wouldn't be able to just write the adapter in Rust code as we currently do unless we do a lot of toolchain work to get it to tag each memory access with which memory it's talking about.

I think it's easier than that. We can let Rust generate the adapter which always access memory 0, then we do a wasm-to-wasm transformation to rewrite all memory 0 to memory 1. Would that work?

[erikrose]

That did come up! I forget if it would be that straightforward or if there were cases where we still need to hit memory 0.

[erikrose]

Having a swing at the on-stack strategy while you finish up what you're working on, Yan. If this works, it'll solve both TinyGo and Go.

[erikrose]

To be super-duper clear (and to remind myself of this, because I just had a scary moment when I forgot it), if we get the stack and State out of heap space, then we can continue to use the memory.grow-based method to provide cabi_realloc().

[erikrose]

Here are some of my "lab notes" thus far.

Though this work has global applicability, I’ve been doing my development in Viceroy, in its Fastly-specific fork of the p1-to-p2 adapter, because it is easy/possible to reproduce the production crashes here—at least the TinyGo ones; I haven’t tried the Big Go ones yet. See the list of attempted rollouts of Big Go services and the results to find a good testing candidate.

The current state of affairs is on the adapter-state-on-stack branch.

ERROR failed to decode world from module

Here’s something you may want to know how to fix. Encountered this during an early attempt to keep a State ptr in a Rust global, in the adapter’s lib.rs. Here’s how I provoked it:

make adapter && cargo build && RUST_BACKTRACE=1 target/debug/viceroy --adapt /home/some/folder/windash/bin/main\ original\ crashes.wasm

Hypothesis: "failed to decode world from module" is talking about the customer module with the adapter applied. We're screwing up the component somehow.

Simpler-to-run and more-informative reproduction:

% make adapter && wasm-tools component new /home/some/folder/windash/bin/main\ original\ crashes.wasm  --adapt wasi_snapshot_preview1=/home/some/folder/Viceroy/lib/data/viceroy-component-adapter.wasm  > adapted_with_2f74a.wasm
error: failed to encode a component from module

Caused by:
    0: failed to decode world from module
    1: failed to reduce input adapter module to its minimal size
    2: unsupported section `11` in adapter

That means there's a bogus data section in the adapter. See the output from wasm-tools dump:

0x27166 | 0b 0a     | data section
0x27168 | 01        | 1 count
0x27169 | 00        | data memory[0]
0x2716a | 41 00     | i32_const value:0
0x2716c | 0b        | end
0x2716d |-------------| ... 4 bytes of data (which are 04 00 00 00)

The data section is due to the global STATE_PTR in lib.rs. When I comment it out, it goes away. (When I wrap it in MaybeUninit, it stays.)

Moved it back to a wasm global. It was a nice attempt at simplification, but it didn’t work out.

Now that I have the State on the stack and the wasm-global state ptr pointing at it, why does it still crash with a magic2 assertion?

Maybe it will help to understand the nature of the stack gc.rs (in wasm-tools) allocates. What's it for?

It is the "unmanaged" or "linear" stack. The motivation behind it is that you can obtain ptrs to values on it, unlike on the wasm managed stack. Rust functions use the unmanaged stack by incr'ing and decr'ing the $__stack_pointer in function prologues/epilogues. It's an LLVM convention. https://github.com/WebAssembly/tool-conventions/blob/main/BasicCABI.md#the-linear-stack.

This stack is allocated only by allocate_stack_via_realloc(), which ALWAYS calls cabi_realloc(). So it always ends up on the heap, where Go can clobber it. I suspect my State is ending up on that stack, where it gets promptly overwritten.

allocate_stack_via_realloc() gets called during Viceroy componentization (even with my latest State-on-stack changes). You can see the start section call to it ((start $allocate_stack)) and its impl in the component. It delegates to realloc_via_memory_grow(), which is bad news, as Go claims that space. So we've either got to make changes to gc.rs or signal it not to allocate a stack on the heap like this and instead use the existing $__stack_pointer if there is one. Go and most/all other toolchains do define such a $__stack_pointer, and the stack grows downward. Do we have enough space to place a 64K State with room left over for typical stack activities? Perhaps not; TinyGo allocates only a 64K stack total:

(global $__stack_pointer (;0;) (mut i32) i32.const 65536)

It may be possible to increment that during adaptation. But does TinyGo’s memory-layout-holding struct, made by its linker (along with any similar struct used by Big Go), need to be updated as well? That’s a very private data structure.

[erikrose]

Further realization: if Go uses $__stack_pointer for any of its own stacks and we have Rust blithely do the same, Go and Rust frames will end up interlaced, and any kind of stack-walking (panicking, tracebacks, etc.) will crash. If they can be made to use separate stack pointer globals, we could point the Rust one to a 64K array of bytes carved out of the initial stack frame, and the interlacing goes away.

Yan and I agree that, if the multi-memory approach can be made to work without too much of a mess, it's preferable, as it reduces the number of assumptions made (who pays attention to which globals?) and the cases to handle (which globals are there?). It should even simplify some parts of the adaptation code, it no longer needing to memory.grow somebody else's memory or cabi_realloc() space for a linear stack.

[erikrose]

Perhaps negating the above realization: stack frames don't point to each other. "One consequence of of [sic] the lack of return addresses and frame pointer chains is that there is no way to traverse the linear stack. ... This functionality is not needed for backtracing or unwinding (since the wasm VM must do this in any case)". So perhaps interlacing stack frames could work after all.