Trap on argument count mismatch in DefinedFunc::DoCall

[aizu-m]

ASan, running spectest-interp on a hand-written spec JSON whose invoke passes one argument to a two-parameter export:

==ERROR: AddressSanitizer: heap-buffer-overflow READ of size 16
    #1 wabt::interp::Thread::PushValues(...) interp.cc:1162
    #2 wabt::interp::DefinedFunc::DoCall(...) interp.cc:541
    #3 wabt::interp::Func::Call(...) interp.cc:512
    #4 spectest::CommandRunner::RunAction(...) spectest-interp.cc:1468

DoCall pushes the caller's arguments by walking the callee's parameter types and indexing the params vector. The only guard was assert(params.size() == type_.params.size()), which disappears under NDEBUG. Func::Call is a public entry point and RunAction feeds it the argument list straight from the untrusted JSON without checking its length, so a short invoke reads past the end of params. wasm-interp already validates arity before calling; the library did not.

The mirror over-read lives in the WriteValues formatting helper: it walks values.size() while indexing the types vector, so an action command carrying more arguments than the function has parameters reads past func_type.params (heap-buffer-overflow at interp-util.cc:88).

DoCall now traps on the mismatch and WriteValues walks the shorter of the two vectors. Behaviour for valid calls is unchanged. Added a gtest that calls a one-parameter export with no arguments and expects a trap rather than an over-read.

[sbc100]

Can this test not be written as a normal text-based test?

Binary encoded tests like these should be avoided where possible I think.

[aizu-m]

Tried that first, but a text spec test can't reach this path. wast2json validates invoke arity, so (invoke "f") against a one-param export is rejected at parse time (too few parameters to function. got 0, expected 1) and never reaches spectest-interp. The too-many-args case that hits WriteValues is rejected the same way. The over-read only shows up once that parser check is bypassed, i.e. through the Func::Call C-API directly or a hand-written JSON, so the gtest is the only thing that exercises it. It also follows the existing binary-module style in test-interp.cc. Happy to drop the test and land just the fix if you'd rather avoid the binary blob.

[aizu-m]

Dropped the new blob. The test now reuses the existing s_fac_module, which is already a one-param (param i32) (result i32) export, so there's no new binary encoding in the diff.

Re-checked the text route before doing that. wast2json still rejects (invoke "f") against that export at parse time (too few parameters to function. got 0, expected 1), and run-tests.py only reaches spectest-interp by way of wast2json, so there's no checked-in-JSON path to drive a short call through. The gtest stays the only place the over-read is reachable.

[zherczeg]

Is this is the hot path? I mean every call goes through this path? If this affect user code only, maybe adding something higher level would be better. I know this interpreter is testing only, but still.

[aizu-m]

Not the hot path. Traced it before adding the check:

• in-wasm call, call_indirect, call_ref and return_call* all route through Thread::DoCall (interp.cc:2198). For a DefinedFunc that only does PushCall, so it never enters DefinedFunc::DoCall.
• DefinedFunc::DoCall is reached solely from the two Func::Call overloads, i.e. the embedder boundary. In tree that's spectest-interp's RunAction, wasm-interp, the C API and the wasi start call.

So it costs one size comparison per external call, next to a Thread construction and a params copy, and nothing per in-wasm call. Behaviour inside the interpreter loop is untouched.

On moving it higher: DefinedFunc::DoCall is already the single funnel for both Func::Call overloads. Putting the check in Func::Call would mean duplicating it across both (the Thread& overload is the one re-entrant host callbacks use) or leaving one unguarded. Happy to move it up if you'd prefer that shape.