[js-api] Reject a mutable global import before allocating

[chicoxyzzy]

Fixes #2253.

read the imports ignored mut and always called global_alloc with const valtype when the value was not a Global. A var import then failed in module_instantiate, and the const global stayed in the store.

Throw a LinkError in that case before allocating. Allocate a fresh global with the import's mut. After that rejection, mut is const.

A Global is passed through. Mutability mismatches stay in module_instantiate, including a const import of a var Global and a var import of a const Global.

The new test covers a const import of a mutable Global for i32, i64, f32, and f64. Instantiation still throws LinkError.

[guybedford]

Why not also include the case of instantiating a mutable global with an immutable one here? Is there a reason to only support the one direction?

[guybedford]

Hmm, actually of course most type validations are left for later, but then the converse point would be -

Rather, why do we need to verify that we are not instantiating an immutable global with a mutable global object here - what makes that case special that it can't also be a deferred validation?

If we removed that, the diff here would drop all the inner |mut| checking for implements Global.

[chicoxyzzy]

A var import of a const Global is the same kind of mismatch, and read the imports already leaves it to module_instantiate. Linking rejects it, and instantiate the core of a WebAssembly module throws LinkError. The existing mutable-import test covers that.

The global_type check does not avoid an allocation. The Global is reused, and linking fails the same way. I put it there because the old ESM ExecuteModule path threw before reading the rest of the imports. On this algorithm it is one of the type checks that are supposed to be deferred, same as a later import that allocates only because linking has not failed yet.

The allocation is the other branch: the value does not implement Global and mut is var. global_alloc runs inside read the imports, and that global stays in the store after linking fails. That LinkError stays.

I'll remove the global_type check. global_alloc then only runs for a const import, still with that import's mut. The new test still expects LinkError, from instantiation.

[guybedford]

While not a bug or major correctness issue, the risk of this is relatively low, and it ensures better hygiene by avoiding an unnecessary global store.

Formally, making the LinkError earlier for this case is an error ordering change, but we don't see strict error ordering in implementations currently anyway - for example, V8 doesn't respect error ordering per the spec (Get's enumerate first in a batch instead of interleaving validation).

LGTM

[chicoxyzzy]

rebased onto main