[wasm] Fix JIT helper calls modeled with the wrong return arity

[lewing]

Fixes #131377
Fixes #131378

Problem

On wasm the emitted call signature is derived from the JIT-modeled return type of the call node — CodeGen::genCallInstruction in codegenwasm.cpp pushes CORINFO_WASM_TYPE_VOID when genActualType(call) is TYP_VOID and the callee's type otherwise. Wasm requires exact return-arity match at the call site, so the common "model a value-returning helper as TYP_VOID because we discard the result" shortcut — harmless on register machines, where the value simply sits unread in the return register — becomes a function signature mismatch engine trap.

#130813 fixed this for the class-init helpers. Three more instances remained.

1. The inline unbox slow path. The importer lowers unbox to (*clone == typeToken) ? nop : CORINFO_HELP_UNBOX(clone, typeToken) and created the helper call as TYP_VOID, since the unboxed byref is formed separately from clone + TARGET_POINTER_SIZE. But CastHelpers.Unbox returns ref byte. Symptom: a trap on the first cold unbox in System.Enum.ToObject during reflection-heavy startup.

2. ObjectAllocator::RewriteUses. When the unboxed object is a stack-allocated box, this retargets the CORINFO_HELP_UNBOX call node to CORINFO_HELP_UNBOX_TYPETEST but never retyped it. Unbox returns ref byte; Unbox_TypeTest returns void. The node was left claiming a byref return for a void helper.

3. CORINFO_HELP_CHECK_OBJ in fgMorphInit. JIT_CheckObj returns Object* and the importer already models it TYP_REF; only fgMorphInit used TYP_VOID.

Fix

Model each call with the return type its callee actually has, discarding unused results via gtUnusedValNode.

For unbox the value-returning model is applied on wasm only, via a HelperUnboxDiscardedRetType constant alongside #130813's. Modeling TYP_BYREF unconditionally is equally correct — CastHelpers.Unbox returns a byref everywhere, and the non-inline path already models it that way — and was tried, but SPMI showed it costs code size on every native target for no benefit, since the result is discarded and only wasm derives the call signature from the modeled type. See Validation.

isForEffect in objectalloc.cpp is deliberately still computed before the retype — computing it after would make it unconditionally true and drop the COMMA(call, ADD(obj, TARGET_POINTER_SIZE)) the consumer needs.

Fixes 1 and 2 must ship together: before fix 1 the inline-unbox call reached ObjectAllocator as TYP_VOID, so isForEffect was true and the void model matched Unbox_TypeTest by accident. Fix 1 alone regresses that path, confirmed experimentally below.

Validation

Measured on a browser R2R rig. Static disassembly at the UNBOX_TYPETEST site:

	emitted
unfixed	call_indirect (type 5) + drop — i32-returning signature for a void helper
fixed	call_indirect (type 4) — void, no drop

The images differ only at that site. Runtime differential across three JIT states, driven through the helper:

JIT state	result
fix 1 only	function signature mismatch trap, ec=1, 3/3
fixes 1+2	helper runs, type-tests, throws InvalidCastException (caught), ec=100, 3/3
neither	ec=100, no trap

Regression sweep with both fixes: System.Text.Json discovery 12166/12166, sig-mismatch=0, nre=0, oob=0 across seeds 1/42/99; Unsafe 128/128 and Bcl.Memory 550/550 with R2R on and off, trap=0.

Native impact. Fix 2 is not wasm-guarded, since the node was mistyped on every target and is simply benign off wasm. Its defect reproduces on arm64: on the hot path the node already arrives TYP_VOID so the retype is a no-op, but when the unbox lands in a rarely-run block the importer takes the non-inline path modeling TYP_BYREF, and the unfixed JIT emits CALL help byref CORINFO_HELP_UNBOX_TYPETEST. Diffing full arm64 dumps with ASLR noise filtered, the only deltas are the call's type and register def (REG x0 → REG NA) and two removed Byref regs: ... {x0} tracking lines; Total bytes of code 236, instruction count 59 is identical. So the unfixed JIT transiently reported x0 as a live byref across a call to a helper that returns nothing — dead immediately and not practically observable, but bogus GC info rather than a cosmetic modeling wart.

runtime-coreclr superpmi-diffs on this commit reports "No diffs found" on every native platform — linux arm64 (3,040,474 contexts), linux x64 (2,794,079), osx arm64 (478,226), windows arm64 (2,803,887), windows x64 (2,625,062).

The only collection with diffs is browser wasm, which is the fix landing: 208 contexts, 0 improved / 208 regressed, +279 bytes total across 80 files, concentrated at inline unbox sites — EventProvider.EncodeObject +19, RuntimeType.IsEnumDefined +13, Enum.ToObject +12, CustomAttributeBuilder.EmitValue +12. That is the cost of emitting a correctly-typed call_indirect where the JIT previously emitted one the engine rejects.

An intermediate revision made fix 1 unconditional, and SPMI is what ruled that out: it produced code size regressions on every native target — linux arm64, linux x64, osx arm64, windows arm64 and windows x64 — concentrated exactly where you would expect, at inline unbox sites (Enum.ToObject +12 bytes, RuntimeType.IsEnumDefined +13, EventProvider.EncodeObject +19, and EnumConverter[int].ConvertToUInt64 +124, 21.8% of that method). Small in absolute terms and 0.00% of base overall, but pure cost off wasm. That revision was reverted; an arm64 spot check had suggested it was instruction-neutral, but it had simply landed on a method that did not change.

Fix 3 is not runtime-testable by construction. CORINFO_HELP_CHECK_OBJ is absent from the helper switch in CorInfoImpl.ReadyToRun.cs and falls to default: throw new NotImplementedException — it is not even in the graceful RequiresRuntimeJitException list. Any method that would emit it fails R2R codegen outright, on every target, before return-arity modeling is reached:

ILCompiler.CodeGenerationFailedException: Code generation failed for method 'GcCheck.Foo(object)'
 ---> System.NotImplementedException: CORINFO_HELP_CHECK_OBJ
   at CorInfoImpl.GetHelperFtnUncached(...) CorInfoImpl.ReadyToRun.cs:1342

Confirmed by building a Checked wasm JIT and attempting to crossgen a GC-ref-param method with --codegenopt:JitGCChecks=1. So no R2R image can contain a CHECK_OBJ call, and wasm has no runtime JIT. The change is kept as alignment with the helper's real signature and with the importer, and would become live if CHECK_OBJ gains R2R support. #131378 as originally filed overstated this and has been corrected.

Regression test

Runtime_131377 drives a non-matching type through an unbox site whose box is stack allocated. That detail is load-bearing: the mismatched call_indirect sits behind an inline method table check that short-circuits the helper whenever the unbox type matches, so a same-type unbox compiles the bad instruction but never executes it.

This is why the existing JIT/opt/ObjectStackAllocation Case3 — written to trigger CORINFO_HELP_UNBOX_TYPETEST — would not catch the trap: it is only ever called with null, which stack-allocates a Guid and takes the matching path. It also cannot be extended in place, because ZeroAllocTest captures GC.GetAllocatedBytesForCurrentThread() once and asserts zero allocation after each case, so allocating a non-matching operand there breaks every subsequent case. That project additionally sets <CrossGenTest>false</CrossGenTest> (#109314), so it has no R2R coverage today.

AlwaysUseCrossGen2 is set for TargetOS=browser so the browser leg actually compiles to wasm R2R; without it the test runs as ordinary JIT and silently passes. Verified on the rig, 3x each: fixed gives Tests run: 1, Passed: 1, Failed: 0; unfixed traps and the module aborts before Assert.Throws runs, so it fails as a crash.

Follow-up

#131379 tracks the systemic gap — nothing validates modeled helper return arity against the real helper signature, which is why this class of bug has now been found three times by runtime traps rather than at JIT time.

Contributes to #129622.

Note

This change was authored with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 5 pipeline(s).
11 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

This PR corrects several CoreCLR JIT helper-call nodes whose modeled return type didn’t match the actual helper signature, which is especially problematic for wasm where the emitted call signature is derived from the modeled return type.

Changes:

• Retypes an UNBOX → UNBOX_TYPETEST helper retarget in ObjectAllocator::RewriteUses to ensure the call node becomes TYP_VOID.
• Updates fgMorphInit GC-check insertion to model CORINFO_HELP_CHECK_OBJ as value-returning and explicitly discard the result.
• Updates the inline unbox slow-path helper call to use a wasm-appropriate modeled return type and discard the result via gtUnusedValNode.
• Introduces a wasm-specific modeled return type constant for the unbox helper to centralize this choice.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/jit/objectalloc.cpp	Retypes retargeted UNBOX_TYPETEST helper calls to TYP_VOID to match helper signature.
src/coreclr/jit/morph.cpp	Models CHECK_OBJ as TYP_REF and discards the unused result to keep statements effectively void.
src/coreclr/jit/importer.cpp	Models inline-unbox slow-path helper calls with a wasm-correct return type and discards the value.
src/coreclr/jit/compiler.h	Adds a wasm-specific constant to drive modeled helper return type for discarded unbox results.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/tests/JIT/Regression/JitBlue/Runtime_131377/Runtime_131377.cs:30

• The test currently doesn’t appear to exercise the intended stack-allocated-box + UNBOX_TYPETEST slow path: Unbox(null) creates a Guid box and then unboxes as Guid (same-type, so the helper path is typically skipped), while the mismatch case ("not a guid") isn’t a box and therefore won’t be rewritten to UNBOX_TYPETEST. Consider making the boxed value type definite and unboxing it as a different value type so the slow path is guaranteed to execute when the box is stack-allocated.

    [Fact]
    public static void TestEntryPoint()
    {
        Unbox(null);
        Assert.Throws<InvalidCastException>(() => Unbox("not a guid"));
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static void Unbox(object o)
    {
        if (o is null)
        {
            o = new Guid();
        }

        Consume((Guid)o);
    }

[lewing]

Sorry — that was a transient artifact of my rebase, not a bad merge. I force-pushed a rebase onto current main while you were looking, and GitHub rendered the diff against the stale merge base for a while, so everything that had landed on main in between (System.Text.Json, StreamReader/StreamWriter, XmlSerializer, process.cpp, copilot-instructions.md, ...) showed up as if it were mine.

It has since recomputed. gh pr view --json files now lists exactly six:

src/coreclr/jit/compiler.h
src/coreclr/jit/importer.cpp
src/coreclr/jit/morph.cpp
src/coreclr/jit/objectalloc.cpp
src/tests/JIT/Regression/JitBlue/Runtime_131377/Runtime_131377.cs
src/tests/JIT/Regression/JitBlue/Runtime_131377/Runtime_131377.csproj

Nothing outside the JIT and the new test was ever in the branch.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/coreclr/jit/compiler.h:3366

• The comment above HelperInitClassRetType says these helpers return void* only "On wasm", but InitHelpers.InitClass/InitInstantiatedClass are declared as void* in CoreLib unconditionally. Since this PR also adds a similar helper-typing constant right next to it, it would be good to keep this comment accurate so future readers don’t infer the managed signature is wasm-conditional.

Evidence: src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/InitHelpers.cs defines private static void* InitClass(...) / InitInstantiatedClass(...) without target conditionals.

    // On wasm these helpers return void* (InitHelpers.InitClass/InitInstantiatedClass). Model them as
    // value-returning so the call_indirect signature matches the compiled managed helper; the value is unused.
    static constexpr var_types HelperInitClassRetType = TYP_I_IMPL;
    // Likewise for CastHelpers.Unbox at sites that discard the result. Modeling it TYP_BYREF off
    // wasm is equally correct but costs code size for no benefit.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[lewing]

The arg change broke nativeaot, fixing that too.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

[AndyAyersMS]

The arg change broke nativeaot, fixing that too.

Maybe we should fix this by leaving everything as it was and reversing the args in the call to ThrowInvalidCastException in Unbox_TypeTest_Helper ?

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/coreclr/jit/compiler.h:3367

• The comment above HelperInitClassRetType implies the class-init helpers return void* only on wasm, but InitHelpers.InitClass/InitInstantiatedClass return void* unconditionally. The intent here is that only wasm requires accurate return-arity modeling for call_indirect; the comment should reflect that to avoid future misuse.

    // On wasm these helpers return void* (InitHelpers.InitClass/InitInstantiatedClass). Model them as
    // value-returning so the call_indirect signature matches the compiled managed helper; the value is unused.
    static constexpr var_types HelperInitClassRetType = TYP_I_IMPL;
    // Likewise for CastHelpers.Unbox at sites that discard the result. Modeling it TYP_BYREF off
    // wasm is equally correct but costs code size for no benefit.
    static constexpr var_types HelperUnboxDiscardedRetType = TYP_BYREF;

[AndyAyersMS]

Thanks, this looks good.

[dotnet-policy-service]

Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara
See info in area-owners.md if you want to be subscribed.