Skip to content

Implement struct return thunks in CoreCLR WASM InterpToNativeGenerator#130379

Merged
lewing merged 3 commits into
dotnet:mainfrom
lewing:lewing/struct-return-thunks
Jul 14, 2026
Merged

Implement struct return thunks in CoreCLR WASM InterpToNativeGenerator#130379
lewing merged 3 commits into
dotnet:mainfrom
lewing:lewing/struct-return-thunks

Conversation

@lewing

@lewing lewing commented Jul 8, 2026

Copy link
Copy Markdown
Member

Summary

Implements struct return support in the CoreCLR WASM interp-to-native thunk generator, fixing #120897.

Problem

InterpToNativeGenerator.cs emitted a PORTABILITY_ASSERT("Indirect struct return is not yet implemented.") for any PInvoke signature with a struct return type. This caused a runtime crash when CoreCLR WASM apps called native functions returning structs.

Fix

  • Collect unique struct return sizes from all signatures during code generation
  • Emit C typedefs (typedef struct { char d[N]; } wasm_ret_SN;) for each unique size
  • Use those typedefs as the return type in generated CallFunc_* thunks instead of asserting
  • emcc automatically lowers struct returns to the sret pointer convention — no special ABI handling needed

The fix works for both browser-wasm and wasi-wasm (same generator, same wasm32 ABI).

Test Changes

  • Removed native-mono test category from EnsureWasmAbiRulesAreFollowedInInterpreter — this test now passes on CoreCLR too

Note

This PR was generated with the assistance of GitHub Copilot.

Remove the PORTABILITY_ASSERT for indirect struct returns in the
CoreCLR WASM interp-to-native thunk generator. For struct return
types (S<N> tokens), emit C typedefs of the correct size so emcc
generates the proper sret ABI calling convention. This fixes the
runtime crash when CoreCLR WASM code calls native functions that
return structs.

Also remove the native-mono test category filter from
EnsureWasmAbiRulesAreFollowedInInterpreter since it now works
on CoreCLR.

Fixes dotnet#120897

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

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

This pull request updates the CoreCLR WASM interp-to-native thunk generator to support P/Invokes that return structs, removing the prior assert-only behavior and enabling CoreCLR interpreter scenarios that involve indirect struct returns. It also updates a WASM build test to run on CoreCLR now that this ABI shape is supported.

Changes:

  • Collect unique S<N> struct return sizes up-front and emit corresponding typedef struct { char d[N]; } wasm_ret_S<N>; definitions in generated code.
  • Generate call-thunks that use the emitted typedef as the native function’s return type (instead of asserting), allowing the toolchain to apply the correct hidden-return-buffer calling convention.
  • Remove the native-mono test category restriction from EnsureWasmAbiRulesAreFollowedInInterpreter, allowing the interpreter ABI test to run on CoreCLR.

Reviewed changes

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

File Description
src/tasks/WasmAppBuilder/coreclr/InterpToNativeGenerator.cs Adds struct-return typedef emission and uses those types for S<N> return thunks to avoid the previous portability assert.
src/mono/wasm/Wasm.Build.Tests/PInvokeTableGeneratorTests.cs Enables the interpreter ABI validation test to run on CoreCLR by removing the mono-only category gate.

@lewing lewing marked this pull request as ready for review July 8, 2026 22:20
@lewing lewing added the arch-wasm WebAssembly architecture label Jul 8, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

The CoreCLR WASM pipeline generates callhelpers-pinvoke.cpp, not
pinvoke-table.cpp. This was already fixed in the workload PR but
needs to be included here since the test now runs on CoreCLR after
removing the native-mono category.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 9, 2026 03:57

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

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

@pavelsavara

Copy link
Copy Markdown
Member

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara

Copy link
Copy Markdown
Member

Log

System.DllNotFoundException: Unable to load shared library 'wasm-abi' or one of its dependencies.

The EnsureWasmAbiRulesAreFollowedInInterpreter test requires the native
build pipeline (NativeFileReference for wasm-abi.c) which is only
available in the workload enablement PR. Re-add native-mono category
so this test is skipped on CoreCLR until the pipeline is available.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 9, 2026 13:08
@lewing

lewing commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

Log

System.DllNotFoundException: Unable to load shared library 'wasm-abi' or one of its dependencies.

The change is correct, it needs the native build changes in the workload pr to go green #130380 I've added back the active issue for now

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread src/tasks/WasmAppBuilder/coreclr/InterpToNativeGenerator.cs
Comment thread src/mono/wasm/Wasm.Build.Tests/PInvokeTableGeneratorTests.cs
@pavelsavara

Copy link
Copy Markdown
Member

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@lewing lewing enabled auto-merge (squash) July 13, 2026 20:53
@lewing

lewing commented Jul 13, 2026

Copy link
Copy Markdown
Member Author

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@lewing lewing merged commit c0a703f into dotnet:main Jul 14, 2026
56 of 61 checks passed
[Theory]
[BuildAndRun(aot: false)]
[TestCategory("native-mono")] // coreclr ActiveIssue: https://github.com/dotnet/runtime/issues/120897
[TestCategory("native-mono")]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The native-mono runs only on mono. The native runs on both.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot create new PR and make this change

@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 11.0-preview7 milestone Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-wasm WebAssembly architecture area-Infrastructure-coreclr Only use for closed issues

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants