Implement wasm JumpStubNode with signature-matched argument forwarding by anurag6569201 · Pull Request #7 · ShipWright-Review/runtime

anurag6569201 · 2026-09-12T21:25:58Z

Implements JumpStubNode for Wasm so address-taken method stubs no longer throw NotImplementedException during emission. The stub now preserves the target method’s lowered Wasm signature, forwards all arguments, and transfers control to the target.

Wasm JumpStub emission
- Replaces the Wasm JumpStubNode.EmitCode stub implementation with real body generation.
- Computes the lowered Wasm function type from INodeWithTypeSignature.
- Emits local.get for each parameter and calls the target symbol.
Signature propagation for address-taken methods
- Updates AddressTakenMethodNode to implement IMethodCodeNodeWithTypeSignature.
- Ensures Wasm method declaration/signature recording uses the target method signature for emitted jump stubs.

for (int i = 0; i < signature.Params.Types.Length; i++)
{
    expressions.Add(Local.Get(i));
}
expressions.Add(ControlFlow.Call(_target));
encoder.FunctionBody = new WasmFunctionBody(signature, expressions.ToArray());

Fixes Implement JumpStubNode for WASM dotnet/runtime#133247

Source merge-base: a5b8d110306eff632cced48a4d108d619393a6a4
Source head: 1449c726c67aefe4fce8aa68e3fb5a98fa879308

Source PR: dotnet#133248 Source head: 1449c72

shipwright-agent · 2026-09-12T21:26:21Z

⛔ Shipwright · Blocked

Recommendation: do not merge PR #7 · Tier T1
Checks: 0 total · 0 needing attention

Next step: resolve the blocking findings before merge.

Findings (7)

CRITICAL WasmJumpStubNode.EmitCode now emits a ReturnCall instruction for every jump stub, but ReturnCall (0x12) is a tail-call that transfers control and returns from the caller's frame. · src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_Wasm/WasmJumpStubNode.cs:14
- Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
CRITICAL The stub emits Local.Get(i) for i in [0, parameterCount) to forward arguments, but this assumes the stub's parameters are laid out as locals 0..N-1 in the same order as the target' · src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_Wasm/WasmJumpStubNode.cs:14
- Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
CRITICAL AddressTakenMethodNode now implements IMethodCodeNodeWithTypeSignature and overrides Signature/IsUnmanagedCallersOnly/IsAsyncCall/HasGenericContextArg, but JumpStubNode declares th · src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/JumpStubNode.cs:25
- Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
HIGH The new GetLoweringFlags(CORINFO_SIG_INFO*) overload uses '(callSig->callConv & CorInfoCallConv.CORINFO_CALLCONV_MASK) != CorInfoCallConv.CORINFO_CALLCONV_DEFAULT' to set IsUnmanag · src/coreclr/tools/Common/JitInterface/WasmLowering.cs:630
- Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
HIGH The refactor moves flag computation from WasmObjectWriter.RecordMethodDeclaration into WasmLowering.GetLoweringFlags(INodeWithTypeSignature), but the new code path is used by Write · src/coreclr/tools/Common/Compiler/ObjectWriter/WasmObjectWriter.cs:191
- Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
HIGH The new GetLoweringFlags(CORINFO_SIG_INFO*) overload dereferences callSig after a Debug.Assert(callSig != null). · src/coreclr/tools/Common/JitInterface/WasmLowering.cs:620
- Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
HIGH The ReturnCall instruction (0x12) is added to the WASM instruction set and used in jump stubs, but there is no validation that the target function's signature is compatible with th · src/coreclr/tools/Common/Compiler/ObjectWriter/WasmInstructions.cs:125
- Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

Fireworks usage: 11,477 input · 1,193 output · 12,670 total tokens · $0.0033 · 17s · 0 fix iteration(s)

Open the Shipwright check for full evidence and the audit bundle. Use /shipwright rerun to verify again.

shipwright-agent · 2026-09-12T21:26:22Z

        protected override void EmitCode(NodeFactory factory, ref WasmEmitter encoder, bool relocsOnly)
        {
-            throw new NotImplementedException();
+            WasmFuncType signature = WasmLowering.GetSignature(this).FuncType;


Shipwright · CRITICAL

WasmJumpStubNode.EmitCode now emits a ReturnCall instruction for every jump stub, but ReturnCall (0x12) is a tail-call that transfers control and returns from the caller's frame.

Impact: WasmJumpStubNode.EmitCode now emits a ReturnCall instruction for every jump stub, but ReturnCall (0x12) is a tail-call that transfers control and returns from the caller's frame. For address-taken method stubs, this changes semantics: any caller that invokes the stub expecting a normal call/return will instead have its own frame returned from. This will break all address-taken method calls on WASM, not just optimi…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

shipwright-agent · 2026-09-12T21:26:23Z

        protected override void EmitCode(NodeFactory factory, ref WasmEmitter encoder, bool relocsOnly)
        {
-            throw new NotImplementedException();
+            WasmFuncType signature = WasmLowering.GetSignature(this).FuncType;


Shipwright · CRITICAL

The stub emits Local.Get(i) for i in [0, parameterCount) to forward arguments, but this assumes the stub's parameters are laid out as locals 0..N-1 in the same order as the target'

Impact: The stub emits Local.Get(i) for i in [0, parameterCount) to forward arguments, but this assumes the stub's parameters are laid out as locals 0..N-1 in the same order as the target's signature. If the target signature has a generic context arg, async state, or unmanaged calling convention (all of which are now surfaced via INodeWithTypeSignature), the parameter mapping may be offset or reordered. The code does not ac…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

shipwright-agent · 2026-09-12T21:26:24Z

            _target = target;
        }

+        public abstract MethodSignature Signature { get; }


Shipwright · CRITICAL

AddressTakenMethodNode now implements IMethodCodeNodeWithTypeSignature and overrides Signature/IsUnmanagedCallersOnly/IsAsyncCall/HasGenericContextArg, but JumpStubNode declares th

Impact: AddressTakenMethodNode now implements IMethodCodeNodeWithTypeSignature and overrides Signature/IsUnmanagedCallersOnly/IsAsyncCall/HasGenericContextArg, but JumpStubNode declares these as abstract. Any other existing JumpStubNode subclass in the codebase that was not updated will now fail to compile. The diff only shows AddressTakenMethodNode being updated; if there are other subclasses (e.g., for delegate thunks or…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

shipwright-agent · 2026-09-12T21:26:25Z

+            Debug.Assert(callSig != null);
+
+            LoweringFlags flags = 0;
+            if (callSig->hasTypeArg())


Shipwright · HIGH

The new GetLoweringFlags(CORINFO_SIG_INFO*) overload uses '(callSig->callConv & CorInfoCallConv.CORINFO_CALLCONV_MASK) != CorInfoCallConv.CORINFO_CALLCONV_DEFAULT' to set IsUnmanag

Impact: The new GetLoweringFlags(CORINFO_SIG_INFO*) overload uses '(callSig->callConv & CorInfoCallConv.CORINFO_CALLCONV_MASK) != CorInfoCallConv.CORINFO_CALLCONV_DEFAULT' to set IsUnmanagedCallersOnly, but the old code used '((int)callSig->getCallConv() & 0xF) != 0'. These are not obviously equivalent: the old code checked the low 4 bits of the full callConv, while the new code checks a named mask. If CORINFO_CALLCON…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

shipwright-agent · 2026-09-12T21:26:26Z

-            MethodSignature managedSignature,
-            WasmLowering.LoweringFlags flags,
-            ISymbolNode node)
+        private void WriteSignatureIndexForFunction(INodeWithTypeSignature node)


Shipwright · HIGH

The refactor moves flag computation from WasmObjectWriter.RecordMethodDeclaration into WasmLowering.GetLoweringFlags(INodeWithTypeSignature), but the new code path is used by Write

Impact: The refactor moves flag computation from WasmObjectWriter.RecordMethodDeclaration into WasmLowering.GetLoweringFlags(INodeWithTypeSignature), but the new code path is used by WriteSignatureIndexForFunction(node) which now calls WasmLowering.GetSignature(node). If any INodeWithTypeSignature implementation returns a Signature that differs from the MethodDesc.Signature used previously (e.g., a stub or wrapper node), th…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

shipwright-agent · 2026-09-12T21:26:27Z

+            }
+            if (node.IsUnmanagedCallersOnly)
+            {
+                flags |= LoweringFlags.IsUnmanagedCallersOnly;


Shipwright · HIGH

The new GetLoweringFlags(CORINFO_SIG_INFO*) overload dereferences callSig after a Debug.Assert(callSig != null).

Impact: The new GetLoweringFlags(CORINFO_SIG_INFO*) overload dereferences callSig after a Debug.Assert(callSig != null). In release builds, Debug.Assert is compiled out, so a null callSig will cause a null pointer dereference. The old code did not have this assert and presumably handled null or was never called with null. This is a potential crash/DoS vector if any code path passes a null callSig.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

shipwright-agent · 2026-09-12T21:26:27Z

        End = 0x0B,
        Call = 0x10,
        CallIndirect = 0x11,
+        ReturnCall = 0x12,


Shipwright · HIGH

The ReturnCall instruction (0x12) is added to the WASM instruction set and used in jump stubs, but there is no validation that the target function's signature is compatible with th

Impact: The ReturnCall instruction (0x12) is added to the WASM instruction set and used in jump stubs, but there is no validation that the target function's signature is compatible with the stub's signature. WASM tail calls require exact signature match; if the stub's signature (derived from INodeWithTypeSignature) differs from the target's actual signature, this produces invalid WASM that may be rejected by the runtime or,…

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

Implement wasm JumpStubNode with signature-matched argument forwarding

96e53fa

Source PR: dotnet#133248 Source head: 1449c72

shipwright-agent Bot reviewed Sep 12, 2026

View reviewed changes

Navigation Menu

Uh oh!

Implement wasm JumpStubNode with signature-matched argument forwarding - #7

Implement wasm JumpStubNode with signature-matched argument forwarding#7
anurag6569201 wants to merge 1 commit into
qa/agent-dotnet-runtime/pr-07-133248/basefrom
qa/agent-dotnet-runtime/pr-07-133248/head

anurag6569201 commented Sep 12, 2026

Uh oh!

shipwright-agent Bot commented Sep 12, 2026

Uh oh!

shipwright-agent Bot Sep 12, 2026

Uh oh!

shipwright-agent Bot Sep 12, 2026

Uh oh!

shipwright-agent Bot Sep 12, 2026

Uh oh!

shipwright-agent Bot Sep 12, 2026

Uh oh!

shipwright-agent Bot Sep 12, 2026

Uh oh!

shipwright-agent Bot Sep 12, 2026

Uh oh!

shipwright-agent Bot Sep 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

anurag6569201 commented Sep 12, 2026

Uh oh!

shipwright-agent Bot commented Sep 12, 2026

⛔ Shipwright · Blocked

Findings (7)

Uh oh!

shipwright-agent Bot Sep 12, 2026

Choose a reason for hiding this comment

Uh oh!

shipwright-agent Bot Sep 12, 2026

Choose a reason for hiding this comment

Uh oh!

shipwright-agent Bot Sep 12, 2026

Choose a reason for hiding this comment

Uh oh!

shipwright-agent Bot Sep 12, 2026

Choose a reason for hiding this comment

Uh oh!

shipwright-agent Bot Sep 12, 2026

Choose a reason for hiding this comment

Uh oh!

shipwright-agent Bot Sep 12, 2026

Choose a reason for hiding this comment

Uh oh!

shipwright-agent Bot Sep 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant