[wasi] Stand up CoreCLR-WASI library-test leg (per-app corerun relink)

[lewing]

Draft. Stands up a CoreCLR-WASI library-test leg (smoke scope: System.Runtime.Tests), addressing part of #130129.

Builds on the CoreCLR-WASI wasihost corehost (#130816, merged), which ships libWasiHost.a in the runtime pack. This PR generates each test's own reverse P/Invoke thunks and links them into that host per-app.

What this does

Adds a per-app native link of the shipping wasihost corehost (libWasiHost.a) so a test's own [UnmanagedCallersOnly] reverse P/Invoke thunks are covered — mirroring the browser-CoreCLR relink. The baked callhelpers table (libcoreclr_gen_static.a) only covers framework top-level UCO callbacks; library/test callbacks (e.g. a nested WindowsUILanguageHelper.EnumUiLanguagesCallback) are unknown to it and trap at first use without the per-app link.

• Host: consumes libWasiHost.a from the wasihost corehost ([wasi] Add CoreCLR-WASI corehost (wasihost) #130816) — the leg does not manufacture its own host.
• Per-app link (src/mono/wasi/build/WasiApp.CoreCLR.targets): ManagedToNativeGenerator (TargetOS=wasi) → compile the generated callhelpers with the wasi-sdk clang → link libWasiHost.a from the runtime-pack static archives + the app callhelper .o (replacing libcoreclr_gen_static.a) via wasm-component-ld. The wasi:http import that System.Net.* pulls in is declared with -Wl,--component-type (WasiHttpWorld_component_type.wit, as Mono does); wasmtime is given -S http by the test targets.
• Generator (ManagedToNativeGenerator.cs / PInvokeTableGenerator.cs): a WarnOnUnresolvedPInvokeModules flag downgrades WASM0066 to a message for untrimmed library-test closures (whose foreign-platform P/Invokes are unresolved on wasm and never called), so they don't fail the build under warn-as-error.
• Test/CI wiring: tests.wasi.targets, sendtohelix-wasi.targets, tests.proj smoke set, and a wasi-wasm-coreclr-library-tests CI template (non-gating, rolling).

The leg runs with full ICU globalization (non-invariant): the wasihost corehost preloads icudt.dat from CORE_ROOT.

Validation

Locally (macOS arm64, wasi-sdk + wasmtime): the baseline (clr+libs+host+packs -os wasi -c Release) builds green and stages libWasiHost.a from the wasihost corehost into the runtime pack. The System.Runtime.Tests bundle links the host per-app and discovers + runs the full suite:

Discovered: managed/System.Runtime.Tests.dll (found 9966 of 9966 test cases)
Starting:   managed/System.Runtime.Tests.dll

Managed exception dispatch through the interpreter EH + reverse UCO thunks works with no precode assert. A System.Tests.VersionTests subset ran 251/251 passing (WASM EXIT 0).

Relationship to other work

• [wasi] Add CoreCLR-WASI corehost (wasihost) #130816 (merged) — the wasihost corehost this leg links.
• [wasm] Fix reverse P/Invoke thunk key for nested [UnmanagedCallersOnly] types (CoreCLR + Mono) #130740 (merged) — the nested-UCO thunk-key generator fix the interpreter needs to resolve nested reverse thunks.
• [wasm][R2R] function signature mismatch on the first (cold) R2R call to a method whose native code is not yet on its portable entry point #130634 stays scoped to the R2R call_indirect path (not this pure-interpreter leg).

Tracked follow-ups (not in this PR)

• [wasi] Build-switchable wasi:http capability for the CoreCLR-WASI test host (corerun) #130742 — build-switchable wasi:http capability (no-http vs http-capable host), and the longer-term "statify the app-declared imports" direction.
• [wasm] reverse P/Invoke thunk key drops enclosing type for nested [UnmanagedCallersOnly] methods #130739 — token-based reverse-thunk key robustness.
• [browser][CoreCLR] Clean up Wasm.Build.Tests relink dependecies once R2R is available #128362 — relocate coreclr_compat.h to a neutral shared location + stage the vm/minipal headers and the wit for out-of-tree/Helix runs (the per-app link currently uses in-repo fallbacks in-tree).
• Polish: -Wl,-u,__main_void in place of --whole-archive, and incremental Inputs/Outputs on the link target.

Note

This pull request was authored with GitHub Copilot.

[azure-pipelines]

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

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR wires up an initial CoreCLR-on-WASI library tests lane (smoke scope: System.Runtime.Tests) by adding a per-app relink of the corerun host so app/test [UnmanagedCallersOnly] reverse-thunk callhelpers are generated and linked into the host, and by hooking that into Helix + CI.

Changes:

• Fix reverse P/Invoke thunk key generation for nested [UnmanagedCallersOnly] callbacks so runtime lookup keys match.
• Add a CoreCLR-specific WASI app-builder target set that generates callhelper sources and relinks corerun per app using wasi-sdk + wasm-component-ld.
• Add smoke test selection + Helix/CI plumbing for a non-gating CoreCLR-WASI library-tests leg.

Reviewed changes

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

Show a summary per file

File	Description
src/tasks/WasmAppBuilder/coreclr/PInvokeCollector.cs	Align nested-type namespace handling for reverse-thunk key generation.
src/mono/wasi/build/WasiApp.InTree.targets	Select CoreCLR vs Mono WASI app-builder targets based on RuntimeFlavor.
src/mono/wasi/build/WasiApp.InTree.props	Configure WASI in-tree defaults for RuntimeFlavor (e.g., invariant globalization for CoreCLR bring-up).
src/mono/wasi/build/WasiApp.CoreCLR.targets	New: CoreCLR-WASI app-builder that copies framework bits and performs per-app corerun relink with generated callhelpers.
src/libraries/tests.proj	Add CoreCLR-WASI smoke project set (initially System.Runtime.Tests) and keep Mono-WASI smoke set intact.
src/libraries/sendtohelix-wasi.targets	Distinguish CoreCLR work items on the shared WASI queue and adjust payload staging for CoreCLR vs Mono.
src/installer/pkg/sfx/Microsoft.NETCore.App/Directory.Build.props	Add libWasiHost.a to the platform manifest so it ships in the runtime pack.
src/coreclr/hosts/corerun/CMakeLists.txt	Build/install a WASI host static archive (libWasiHost.a) for relink scenarios.
eng/testing/tests.wasi.targets	Teach WASI test runner args/env for CoreCLR (corerun, CORE_ROOT, EH proposal flag).
eng/pipelines/runtime.yml	Add a rolling/non-gating CoreCLR-WASI library-tests smoke leg template invocation.
eng/pipelines/common/templates/wasi-wasm-coreclr-library-tests.yml	New: pipeline template for CoreCLR-WASI Helix library tests (smoke-only bring-up leg).

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[azure-pipelines]

Azure Pipelines:
Successfully started running 5 pipeline(s).
10 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

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

[Copilot]

Pull request overview

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

[github-actions]

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "400271c150b54d691b57f581553b13f185419ffc",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "f079ec9d15385ccc314129e62249d0838daced6b",
  "last_reviewed_commit": "67c7ec50329e720407ce2342d799f113525ebf3f",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "f079ec9d15385ccc314129e62249d0838daced6b",
  "last_recorded_worker_run_id": "29687155833",
  "review_attempt_commit": "400271c150b54d691b57f581553b13f185419ffc",
  "review_attempt_base_ref": "main",
  "review_attempt_count": 1,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "67c7ec50329e720407ce2342d799f113525ebf3f",
      "review_id": 4730771238
    }
  ]
}

[github-actions]

Holistic Review

Motivation: Justified. The PR stands up a CoreCLR-WASI library-test leg (smoke scope: System.Runtime.Tests) toward #130129, mirroring the existing browser-CoreCLR relink pattern. The core problem is real: the baked libcoreclr_gen_static.a only covers framework top-level [UnmanagedCallersOnly] callbacks, so app/test reverse P/Invoke thunks (e.g. nested UCO callbacks) trap at first use without a per-app link.

Approach: Reasonable and consistent with the codebase. A new WasiApp.CoreCLR.targets performs a per-app ManagedToNativeGenerator → wasi-sdk clang compile → wasm-component-ld link of libWasiHost.a, closely paralleling BrowserWasmApp.CoreCLR.targets. The RuntimeFlavor-conditioned splits in the shared Mono WASI targets/props and the new non-gating rolling CI leg are the established way to fork Mono vs CoreCLR wiring. The generator change is a small, backward-compatible opt-in flag.

Summary: ⚠️ Needs Human Review. No blocking correctness defects found in the changed code, and the C#/test changes are low-risk. However, this is a draft with a large, hand-authored native link line (library set/order, EH flags, memory sizes, component-type WIT) validated only locally on macOS arm64; a human with WASI toolchain expertise should confirm the link recipe and that the new rolling CI leg goes green on Helix before this is considered done. Key items to focus on below.

---

Detailed Findings

✅ Generator flag — backward-compatible and correctly scoped

WarnOnUnresolvedPInvokeModules defaults to true, so existing browser/Mono callers are unaffected; only the new WASI library-test leg opts out (WasiApp.CoreCLR.targets line 170). Downgrading WASM0066 to a low-importance message for untrimmed cross-platform closures (whose foreign P/Invokes are genuinely unreachable on wasm) is a sensible way to avoid failing under warn-as-error, and the ignoredModules.Add(...) guard dedups the message per module. One behavioral nuance worth noting (not a bug): in warn mode WASM0066 is emitted per-method, while in message mode it is deduped per-module — intended, but asymmetric.

✅ Test disabling — correct and minimal

The two [ActiveIssue(...IsWasi, ...IsCoreCLR)] annotations in ExceptionTests.cs reference a tracked issue (#130796) and a valid PlatformDetection.IsWasi predicate, scoping the skip to exactly the CoreCLR-WASI leg. This is the right mechanism rather than deleting/rewriting the tests.

⚠️ Native link recipe — broad, hand-maintained, locally-validated only

WasiApp.CoreCLR.targets encodes a large explicit static-lib set with order-sensitive resolution (duplicated libminipal.a, mid-list -lstdc++), a full VM -D define set that must stay in lockstep with the runtime callhelper compile to avoid ABI/EH/interp thunk skew, fixed memory/stack sizes, and -wasm-use-legacy-eh=false. The comments acknowledge this mirrors the corerun CMake link line, but there is no build-time guard that keeps the define set or lib list in sync with the runtime's own link — future drift would surface only as runtime traps, not build errors. A reviewer familiar with the wasihost link should confirm the recipe and consider whether any of it can be sourced from the pack rather than duplicated (the PR already tracks header relocation via #128362).

⚠️ Verification is local-only; CI leg is new and non-gating

Validation is a single local macOS arm64 run (9966 cases discovered, a 251-case subset passing). The full suite pass/fail on the actual Helix wasi_wasm queue is unproven in-PR, and the leg is deliberately smoke-only, rolling, and alwaysRun-on-rolling (non-PR-gating). That is appropriate for a bring-up leg, but means regressions here won't block PRs — human sign-off should include confirming the rolling leg is green after merge.

💡 Cross-cutting parity note

The generator comment and WasiApp.CoreCLR.targets (lines 150-151) note that BrowserWasmApp.CoreCLR.targets should set the same WarnOnUnresolvedPInvokeModules=false flag when its library-test leg runs untrimmed closures. Consider filing/linking a follow-up so the browser sibling doesn't silently diverge (fail under warn-as-error) when it reaches the same scope.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 149 AIC · ⌖ 10.4 AIC · ⊞ 10K

[Copilot]

Pull request overview

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

[github-actions]

Holistic Review

Motivation: This PR stands up a non-gating CoreCLR-WASI library-test leg (smoke scope: System.Runtime.Tests), addressing part of #130129. It links the shipping wasihost corehost (libWasiHost.a, from merged #130816) per-app so each test's own reverse P/Invoke thunks are covered, and adds generator, test, and CI wiring. The single new commit since the prior review (400271c1) removes a hardcoded WasmMainAssemblyFileName=WasmTestRunner.dll default from the general-purpose WasiApp.CoreCLR.targets.

Approach: The incremental change deletes one conditional property assignment from src/mono/wasi/build/WasiApp.CoreCLR.targets. A test-runner default (WasmTestRunner.dll) does not belong in a general app-builder targets file; it is wrong for a normal non-test app. The test path continues to supply the value via eng/testing/tests.wasi.targets (line 9, Condition="'$(WasmMainAssemblyFileName)' == ''"), and the browser sibling BrowserWasmApp.CoreCLR.targets likewise does not set it. Both the removed line and the test-targets line were conditional defaults, so import ordering does not change the effective value for the test leg.

Summary: The incremental change is a clean, correctly-scoped cleanup that removes a test-specific default from a shared targets file while preserving the value for the test path. No functional regression for the library-test leg is expected, and it brings the WASI targets in line with the browser sibling. No new actionable findings. Verdict: LGTM for the incremental change.

Assessment History

• review 4730771238 reviewed commit 67c7ec50329e720407ce2342d799f113525ebf3f. Its verdict and the current verdict are both LGTM; the assessment is unchanged. The only new commit (400271c1) removes a misplaced test-runner default from the app-builder targets, which does not alter the PR's motivation, approach, or risk profile.

Detailed Findings

No actionable findings in the incremental change. The removal is verified safe: eng/testing/tests.wasi.targets still defines WasmMainAssemblyFileName conditionally for the test path, and the browser CoreCLR sibling target follows the same convention of not defaulting it in the shared app-builder targets.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 49.5 AIC · ⌖ 10.4 AIC · ⊞ 10K

[radekdoulik]

Should this be false only for library test projects?

[lewing]

it should probably be false by default since pinvoke problems are runtime failures

[radekdoulik]

Besides the WarnOnUnresolvedPInvokeModules question it LGTM.