Skip to content

[wasm] Fix reflection invoke of R2R-compiled methods - #129808

Closed
AndyAyersMS wants to merge 1 commit into
dotnet:mainfrom
AndyAyersMS:fix-wasm-reflection-invoke-r2r
Closed

[wasm] Fix reflection invoke of R2R-compiled methods#129808
AndyAyersMS wants to merge 1 commit into
dotnet:mainfrom
AndyAyersMS:fix-wasm-reflection-invoke-r2r

Conversation

@AndyAyersMS

Copy link
Copy Markdown
Member

On wasm, CallDescrWorkerInternal (the reflection / dynamic-invoke path) only
dispatched to interpreted targets. For an R2R (native wasm) compiled method,
GetInterpreterCode() stays null even after DoPrestub, so it called
ExecuteInterpretedMethodWithArgs with a null target: the method never executed
and the return buffer was left aliasing the incoming arguments. As a result,
reflection Invoke of any value-returning R2R method returned the first argument
(or first struct field) instead of the real return value.

Dispatch through InvokeManagedMethod (the interpreter-to-R2R thunk) when there is
no interpreter code, mirroring
ExecuteInterpretedMethodWithArgs_PortableEntryPoint_Complex.

Fixes InterlockedTest.TestCompareExchangeUnextended (which invokes the target via
reflection) and, more broadly, all value-returning reflection invocations of R2R
methods on wasm. Validated against a standalone repro and the JIT_r test group.

CallDescrWorkerInternal only dispatched to interpreted targets: for an
R2R method GetInterpreterCode() stays null after the prestub, so it ran
the interpreter with a null target, executed nothing, and left the
return buffer aliasing the arguments. Dispatch through InvokeManagedMethod
(the interpreter-to-R2R thunk) when there is no interpreter code, as
ExecuteInterpretedMethodWithArgs_PortableEntryPoint_Complex already does.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

@AndyAyersMS

Copy link
Copy Markdown
Member Author

@davidwrighton PTAL
fyi @dotnet/wasm-contrib

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes the WASM CallDescrWorkerInternal (reflection/dynamic invoke) dispatch so that methods without interpreter bytecode (e.g., R2R/native Wasm methods) are invoked via InvokeManagedMethod rather than attempting ExecuteInterpretedMethodWithArgs with a null target, which could leave the return buffer in an invalid/aliased state.

Changes:

  • Add an R2R-aware dispatch path: if GetInterpreterCode() remains null after DoPrestub, invoke via InvokeManagedMethod.
  • Preserve the existing interpreter invocation path when interpreter code is available.

Comment on lines +43 to +49
// The target has no interpreter code because it is R2R (native Wasm) compiled.
// Dispatch to the native entry point via the interpreter-to-R2R thunk, mirroring
// ExecuteInterpretedMethodWithArgs_PortableEntryPoint_Complex. Calling the
// interpreter with a NULL target would not execute the method and would leave the
// return buffer aliasing the incoming arguments.
ManagedMethodParam param = { pMethod, args, (int8_t*)retBuff, (PCODE)NULL, NULL };
InvokeManagedMethod(&param);
@AndyAyersMS

Copy link
Copy Markdown
Member Author

This is redundant with #129766.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants