Skip to content

[mono][wasm] Respect [Un]SupportedOSPlatform in the Mono pinvoke generator#131022

Merged
lewing merged 1 commit into
mainfrom
lewing-wasm-mono-osplatform-pinvoke
Jul 19, 2026
Merged

[mono][wasm] Respect [Un]SupportedOSPlatform in the Mono pinvoke generator#131022
lewing merged 1 commit into
mainfrom
lewing-wasm-mono-osplatform-pinvoke

Conversation

@lewing

@lewing lewing commented Jul 19, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #110870. Building a browser-wasm app that references a Windows-only P/Invoke with non-blittable parameters (e.g. the gdi32 GetCharABCWidthsFloat, which takes HandleRef / object) emits spurious diagnostics on the Mono wasm backend:

WASM0060 : Type System.Object is not blittable: Not a ValueType
WASM0062 : Type System.Runtime.InteropServices.HandleRef is not blittable: Field _wrapper is not blittable
WASM0001 : Could not get pinvoke, or callbacks for method '...GetCharABCWidthsFloat' because 'System.NotSupportedException: Unsupported parameter type ...'

Root cause

The Mono pinvoke collector (src/tasks/WasmAppBuilder/mono/PInvokeCollector.cs) analyzed every P/Invoke and callback in every referenced assembly, without consulting [SupportedOSPlatform] / [UnsupportedOSPlatform]. So a method that is only usable on Windows still had its (non-blittable) signature analyzed when targeting the browser, producing the warnings.

The CoreCLR collector already handles this — it skips a method up front via IsUnsupportedOnPlatform / EvaluatePlatformAttributes, driven by a TargetOS. That check was never ported to the Mono collector, so #110870 still reproduces on Mono while it's fixed on CoreCLR.

Fix

Mirror the CoreCLR platform check into the Mono generator:

  • Thread TargetOS through ManagedToNativeGeneratorPInvokeTableGeneratorPInvokeCollector (the shared <ManagedToNativeGenerator> invocation now passes TargetOS="$(TargetOS)"; the Mono task gets a TargetOS property defaulting to browser, mirroring CoreCLR).
  • Skip pinvokes and callbacks whose method, declaring type (including nesting), or assembly is unsupported on the target OS — i.e. [UnsupportedOSPlatform(target)], or a [SupportedOSPlatform(...)] list that doesn't include the target.

Only affects the Mono wasm build task; CoreCLR is unchanged.

Test

Adds PInvokeTableGeneratorTests.UnsupportedOSPlatformPInvokeIsSkipped: builds a browser app containing a [SupportedOSPlatform("windows")] non-blittable pinvoke and asserts none of WASM0001/WASM0060/WASM0062 are emitted.

Verification

Built the Mono wasm build task and ran a repro (browser app with a [SupportedOSPlatform("windows")] gdi32-style pinvoke taking HandleRef/object), plus the new WBT test:

  • Without the fix: WASM0060 + WASM0062 + WASM0001 (matching the report).
  • With the fix: 0 warnings; the new test passes (Debug + Release).

Note

This pull request was authored with GitHub Copilot.

The Mono wasm pinvoke collector analyzed every P/Invoke and callback in every
referenced assembly without consulting platform attributes. A Windows-only
method with non-blittable parameters (e.g. a gdi32 P/Invoke taking HandleRef /
object) therefore produced spurious WASM0060/WASM0062/WASM0001 diagnostics when
building for the browser, even though the method is never usable there.

The CoreCLR pinvoke collector already skips such methods via an
IsUnsupportedOnPlatform / EvaluatePlatformAttributes check driven by TargetOS.
Mirror that into the Mono collector: thread TargetOS through
ManagedToNativeGenerator -> PInvokeTableGenerator -> PInvokeCollector and skip
pinvokes/callbacks whose method, declaring type, or assembly is unsupported on
the target OS (UnsupportedOSPlatform matching the target, or a SupportedOSPlatform
list that does not include it).

Adds a Wasm.Build.Tests regression that builds a browser app containing a
[SupportedOSPlatform("windows")] non-blittable pinvoke and asserts none of
WASM0001/WASM0060/WASM0062 are emitted.

Fixes #110870

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 62dcd5b3-7227-426b-a072-8812e9c4382d
Copilot AI review requested due to automatic review settings July 19, 2026 00:15
@lewing
lewing temporarily deployed to copilot-pat-pool July 19, 2026 00:15 — with GitHub Actions Inactive
@lewing
lewing temporarily deployed to copilot-pat-pool July 19, 2026 00:16 — with GitHub Actions Inactive
@azure-pipelines

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

@lewing
lewing had a problem deploying to copilot-pat-pool July 19, 2026 00:16 — with GitHub Actions Failure
@lewing
lewing temporarily deployed to copilot-pat-pool July 19, 2026 00:17 — with GitHub Actions Inactive
@lewing
lewing temporarily deployed to copilot-pat-pool July 19, 2026 00:18 — with GitHub Actions Inactive
@lewing lewing added the arch-wasm WebAssembly architecture label Jul 19, 2026
@lewing
lewing temporarily deployed to copilot-pat-pool July 19, 2026 00:18 — with GitHub Actions Inactive
@lewing lewing added this to the 10.0.x milestone Jul 19, 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.

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 PR updates the Mono WASM P/Invoke collection/generation step to respect [SupportedOSPlatform] / [UnsupportedOSPlatform] attributes, so platform-specific (e.g., Windows-only) P/Invokes and callbacks are skipped when building for browser/WASI targets and don’t emit irrelevant non-blittable/signature diagnostics.

Changes:

  • Thread TargetOS through the Mono ManagedToNativeGeneratorPInvokeTableGeneratorPInvokeCollector pipeline and validate supported target values (browser, wasi).
  • Add platform-attribute evaluation to the Mono PInvokeCollector and skip unsupported P/Invokes/callbacks (mirroring the CoreCLR collector behavior).
  • Add a WASM build test asset + regression test asserting Windows-only non-blittable P/Invokes don’t produce WASM0001/WASM0060/WASM0062 when targeting browser.

Reviewed changes

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

Show a summary per file
File Description
src/tasks/WasmAppBuilder/mono/PInvokeTableGenerator.cs Passes targetOS into the Mono PInvokeCollector so platform filtering can occur during scan.
src/tasks/WasmAppBuilder/mono/PInvokeCollector.cs Implements platform attribute evaluation/caching and skips unsupported P/Invokes and callbacks.
src/tasks/WasmAppBuilder/mono/ManagedToNativeGenerator.cs Introduces/validates TargetOS and forwards it into PInvokeTableGenerator.
src/mono/wasm/Wasm.Build.Tests/PInvokeTableGeneratorTests.cs Adds a regression test asserting the diagnostics are not emitted for a Windows-only P/Invoke.
src/mono/wasm/testassets/EntryPoints/PInvoke/UnsupportedOSPlatform.cs Adds a Windows-only, intentionally non-blittable P/Invoke sample used by the new test.
src/mono/wasm/build/WasmApp.Common.targets Passes $(TargetOS) into ManagedToNativeGenerator so the task can apply platform filtering.

@lewing lewing changed the title [wasm] Respect [Un]SupportedOSPlatform in the Mono pinvoke generator [mono][wasm] Respect [Un]SupportedOSPlatform in the Mono pinvoke generator Jul 19, 2026
@github-actions

github-actions Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "22b820636a25ef95f0c7eb94f127c69c25dd0e45",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "13c03a4a9a854291f77636b1e15038e65919e68b",
  "last_reviewed_commit": "22b820636a25ef95f0c7eb94f127c69c25dd0e45",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "13c03a4a9a854291f77636b1e15038e65919e68b",
  "last_recorded_worker_run_id": "29689450331",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "22b820636a25ef95f0c7eb94f127c69c25dd0e45",
      "review_id": 4730871164
    }
  ]
}

@github-actions github-actions Bot 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.

Holistic Review

Motivation: When building a browser/wasm app, the wasm P/Invoke collector analyzes every DllImport method regardless of platform. A Windows-only P/Invoke (e.g. GDI/USER32 with non-blittable parameters) that can never be reached on the browser was still being analyzed, producing spurious WASM0060/WASM0062/WASM0001 diagnostics and build failures (#110870).

Approach: The ManagedToNativeGenerator MSBuild task now accepts a TargetOS property (defaulting to browser, validated against browser/wasi), threaded through PInvokeTableGenerator into PInvokeCollector. The collector skips P/Invoke methods and callbacks that are unsupported on the target OS, determined by walking SupportedOSPlatform/UnsupportedOSPlatform attributes at method → declaring-type (recursively through nesting) → assembly scope, with per-type and per-assembly caches. WasmApp.Common.targets passes TargetOS to the task. A regression test asset and a [Theory] test assert the diagnostics are no longer emitted.

Summary: This is a clean, well-targeted fix. The platform-attribute resolution correctly mirrors runtime precedence (explicit method result wins over type; SupportedOSPlatform present-but-not-listing-target implies unsupported), caching is keyed appropriately, and the new TargetOS plumbing matches the already-existing pattern in the coreclr sibling (src/tasks/WasmAppBuilder/coreclr/ManagedToNativeGenerator.cs), keeping the two variants consistent. The try/catch around attribute inspection preserves the collector's prior tolerance of unresolvable attribute assemblies. The regression test uses the established CopyTestAsset/ReplaceFile/BuildProject helpers and the correct native-mono category. I found no correctness or design concerns; behavior for the default browser target is unchanged for platform-agnostic P/Invokes. Non-blocking observation: EvaluatePlatformAttributes compares the platform name by exact string equality, so a version-qualified attribute value would not match browser/wasi—harmless in practice since those targets are not version-qualified.

Detailed Findings

No actionable issues. LGTM.

Note

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

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

@lewing
lewing merged commit ce2ddcc into main Jul 19, 2026
63 of 64 checks passed
@lewing
lewing deleted the lewing-wasm-mono-osplatform-pinvoke branch July 19, 2026 15:07
@lewing

lewing commented Jul 19, 2026

Copy link
Copy Markdown
Member Author

/backport to release/10.0

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/10.0 (link to workflow run)

@github-actions

Copy link
Copy Markdown
Contributor

@lewing backporting to release/10.0 failed, the patch most likely resulted in conflicts. Please backport manually!

git am output
$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: [wasm] Respect [Un]SupportedOSPlatform in the Mono pinvoke generator
Using index info to reconstruct a base tree...
M	src/mono/wasm/Wasm.Build.Tests/PInvokeTableGeneratorTests.cs
M	src/mono/wasm/build/WasmApp.Common.targets
A	src/tasks/WasmAppBuilder/mono/ManagedToNativeGenerator.cs
A	src/tasks/WasmAppBuilder/mono/PInvokeCollector.cs
A	src/tasks/WasmAppBuilder/mono/PInvokeTableGenerator.cs
Falling back to patching base and 3-way merge...
Auto-merging src/mono/wasm/Wasm.Build.Tests/PInvokeTableGeneratorTests.cs
Auto-merging src/mono/wasm/build/WasmApp.Common.targets
Auto-merging src/tasks/WasmAppBuilder/ManagedToNativeGenerator.cs
CONFLICT (content): Merge conflict in src/tasks/WasmAppBuilder/ManagedToNativeGenerator.cs
Auto-merging src/tasks/WasmAppBuilder/PInvokeCollector.cs
Auto-merging src/tasks/WasmAppBuilder/PInvokeTableGenerator.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 [wasm] Respect [Un]SupportedOSPlatform in the Mono pinvoke generator
Error: The process '/usr/bin/git' failed with exit code 128

Link to workflow output

@dotnet-milestone-bot dotnet-milestone-bot Bot modified the milestones: 10.0.x, 11.0-preview7 Jul 20, 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-Build-mono

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NET9 - warnings on building Blazor Wasm project (WASM0060, WASM0062, WASM0001)

3 participants