UB when copying strings to WASM.

[khoover]

Describe the Bug

When converting a JS string into a Rust string via JsString::into, dropping the produced String generates a layout that violates the dealloc contract.

Steps to Reproduce

1. Checkout https://github.com/khoover/base32768-rs/tree/talc-error
2. From base32768-rs/examples/js-bench, run

RUSTFLAGS="-C target-cpu=mvp" wasm-pack build --debug --target nodejs
node bench-runner.js

3. See error

Expected Behavior

Some benchmark timings being emitted to console.

Actual Behavior

A RuntimeError crashes node while the benchmarks are running.

khoover@DESKTOP-CUT25MA:~/base32768-rs/examples/js-bench$ node pkg/bench-runner.js 
Took 7 ms to load and init bench wasm
wasm://wasm/00189ade:1


RuntimeError: unreachable
    at __rust_start_panic (wasm://wasm/00189ade:wasm-function[1201]:0x449d6)
    at rust_panic (wasm://wasm/00189ade:wasm-function[1173]:0x44891)
    at std::panicking::rust_panic_with_hook::hc20eadded6bfe687 (wasm://wasm/00189ade:wasm-function[363]:0x32839)
    at std::panicking::begin_panic_handler::{{closure}}::h82415fe35b0e2001 (wasm://wasm/00189ade:wasm-function[443]:0x36681)
    at std::sys_common::backtrace::__rust_end_short_backtrace::h71f504d46a203d88 (wasm://wasm/00189ade:wasm-function[1196]:0x449b3)
    at rust_begin_unwind (wasm://wasm/00189ade:wasm-function[708]:0x3e2fa)
    at core::panicking::panic_fmt::h7a368385936888dc (wasm://wasm/00189ade:wasm-function[947]:0x425b9)
    at core::panicking::panic::h7bbea3773b752235 (wasm://wasm/00189ade:wasm-function[859]:0x41116)
    at talc::Talc<O>::free::h14aa50d45611ce00 (wasm://wasm/00189ade:wasm-function[81]:0x1a142)
    at <talc::talck::Talck<R,O> as core::alloc::global::GlobalAlloc>::dealloc::hcf08400e756d2221 (wasm://wasm/00189ade:wasm-function[496]:0x386e4)

Additional Context

Initially reported as an allocator bug in SFBdragon/talc#24. After investigation, seems like the allocator is being passed a different layout than the most recent alloc for the given pointer; std doesn't catch because it ignores layout information for wasm32-unknown-unknown.

Also of note: the Box<[u16]> produced by the encode function can contain unpaired surrogates and other potentially invalid BMP codepoints. If we comment out the function that tries to do a round trip on these unpaired surrogate-containing sequences, the program stops crashing even with Talck as the allocator.

This triggers UB for when using, e.g., let s: String = js_string.into(); on any JS string that contains non-ASCII codepoints, where at least one requires 2 or 4 UTF-8 bytes to encode.

[khoover]

I believe the issue comes from VectorFromWasmAbi::vector_from_abi, where it assumes that the passed-in slice has a backing allocation that is equal in size to the slice length. But the generated JS glue for passStringToWasm0 does a conservative realloc to the worst-case string length, and then only returns the number of bytes that were actually written to. So then when Vec goes to drop that allocation, it has the wrong capacity and passes a too-small size for the Layout.

Quick fix to stop the UB would be adding a 2nd realloc to reduce the allocation size down to the exact length. Actual fix would be making passStringToWasm0 have both a WASM_VECTOR_CAPACITY alongside WASM_VECTOR_LEN, so the backing Vec can be constructed with the right values. (I think this is the only place where len != capacity when passing a Vec from JS to WASM.)

[daxpedda]

Cc @Liamolucko.

[Liamolucko]

I've written a fix in #3808.