Skip to content

[release/10.0] [mono][wasm] Fix pinvoke with 64-bit enum argument#131040

Open
github-actions[bot] wants to merge 3 commits into
release/10.0from
backport/pr-131021-to-release/10.0
Open

[release/10.0] [mono][wasm] Fix pinvoke with 64-bit enum argument#131040
github-actions[bot] wants to merge 3 commits into
release/10.0from
backport/pr-131021-to-release/10.0

Conversation

@github-actions

@github-actions github-actions Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Backport of #131021 to release/10.0

/cc @lewing

Customer Impact

  • Customer reported
  • Found internally

[Select one or both of the boxes. Describe how this issue impacts customers, citing the expected and actual behaviors and scope of the issue. If customer-reported, provide the issue number.]

Regression

  • Yes
  • No

[If yes, specify when the regression was introduced. Provide the PR or commit if known.]

Testing

[How was the fix verified? How was the issue missed previously? What tests were added?]

Risk

[High/Medium/Low. Justify the indication by mentioning how risks were measured and addressed.]

IMPORTANT: If this backport is for a servicing release, please verify that:

  • For .NET 8 and .NET 9: The PR target branch is release/X.0-staging, not release/X.0.
  • For .NET 10+: The PR target branch is release/X.0 (no -staging suffix).

Package authoring no longer needed in .NET 9

IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version.
Keep in mind that we still need package authoring in .NET 8 and older versions.

lewing and others added 3 commits July 19, 2026 15:13
A P/Invoke whose argument is an enum with a 64-bit underlying type (e.g.
'enum : ulong') crashed on wasm with 'RuntimeError: null function or function
signature mismatch'.

interp_type_as_ptr() (used by interp_get_icall_sig to decide whether a native
call can take the fast MINT_CALLI_NAT_FAST / do_icall path, which passes every
argument as a pointer-sized gpointer) treated every enum as pointer-compatible
without looking at its underlying type. Raw long/ulong are already correctly
excluded on 32-bit targets via the SIZEOF_VOID_P == 8 guard, but a 64-bit enum
slipped through and do_icall then called the native 'void(int64_t)' as
'void(int32_t)' on wasm32, causing a call_indirect signature mismatch.

Resolve the enum's underlying type so it inherits the same (correctly guarded)
decision as the raw integer type. Behavior only changes for enums with a 64-bit
underlying type on 32-bit targets such as wasm32.

Adds enum:ulong and enum:long coverage to the WASM ABI pinvoke test.

Fixes #112262
Only enums with a 64-bit underlying type can be misclassified as pointer-sized
on a 32-bit target, so defer to the (width-guarded) underlying type only for
those and leave the classification of smaller enums unchanged. Verified that a
pinvoke with a byte/sbyte/short/ushort/int/uint/long/ulong enum argument all
round-trip correctly on wasm.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 62dcd5b3-7227-426b-a072-8812e9c4382d
Guard against mono_class_enum_basetype_internal returning NULL (SRE/broken
types) before dereferencing, and read m_type_data_get_klass_unchecked once
into a local instead of evaluating it twice.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 62dcd5b3-7227-426b-a072-8812e9c4382d
@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 added this to the 10.0.x milestone Jul 19, 2026
@lewing lewing added runtime-mono specific to the Mono runtime Servicing-consider Issue for next servicing release review area-Interop-mono and removed runtime-mono specific to the Mono runtime labels Jul 19, 2026
@github-actions

github-actions Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "6bac59a1b0d00145dda639ccbe0d898072795c6c",
  "last_dispatched_base_ref": "release/10.0",
  "last_dispatched_base_sha": "018b2627823158a827a8222f0327f99f6a970565",
  "last_reviewed_commit": "6bac59a1b0d00145dda639ccbe0d898072795c6c",
  "last_reviewed_base_ref": "release/10.0",
  "last_reviewed_base_sha": "018b2627823158a827a8222f0327f99f6a970565",
  "last_recorded_worker_run_id": "29694125095",
  "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": "6bac59a1b0d00145dda639ccbe0d898072795c6c",
      "review_id": 4731077898
    }
  ]
}

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Holistic Review

Motivation: Justified. This backports merged PR #131021 to release/10.0, fixing a real, customer-reported crash (issue #112262): a P/Invoke taking a 64-bit-underlying enum (e.g. enum : ulong) traps on wasm with null function or function signature mismatch.

Approach: Correct and minimal. interp_type_as_ptr previously classified every enum as pointer-compatible, so a 64-bit enum on a 32-bit target (wasm32) was routed through do_icall's gpointer fast path, producing a call_indirect signature mismatch. The fix defers I8/U8-underlying enums to their underlying type, which is already SIZEOF_VOID_P-guarded; smaller enums and all enums on 64-bit hosts are unchanged. The recursion into the width-guarded scalar check is sound, and the null-guard on mono_class_enum_basetype_internal is appropriate defensive coding.

Summary: ✅ LGTM. Small, well-scoped fix matching the already-merged source PR, with a targeted regression test extending the existing WASM ABI interpreter test to cover both enum:ulong and enum:long round-trips. No concerns.


Detailed Findings

✅ Correctness — Fix targets the exact misclassification

The behavior change is confined to enums with a 64-bit underlying type on 32-bit targets: for those, classification now follows the underlying I8/U8 type (which returns FALSE on wasm32 via the #if SIZEOF_VOID_P == 8 guard), so the call no longer takes the pointer-sized fast icall path. enum:int/enum:uint/smaller enums, and every enum on 64-bit hosts, keep their prior classification. The klass lookup is cached in a local and the basetype is null-guarded before dereference.

✅ Scope — Sibling signature paths verified unaffected

The fix is limited to interp_type_as_ptr, which is interp-transform-only. The AOT (type_to_c) and jiterpreter (mono_type_to_ldind) signature paths already reduce enums to their underlying type, so no parallel change is needed; consistent with the source PR's analysis.

✅ Test quality — Regression exercises the buggy path

AbiRules.cs and wasm-abi.c add enum:ulong/enum:long direct pinvokes, and the assertions in EnsureWasmAbiRulesAreFollowed verify full 64-bit round-trips (eu (eu)=18374966859414961921 = 0xFF00FF00FF00FF00 + 1, ei (ei)=-2 = -3 + 1). EnsureWasmAbiRulesAreFollowedInInterpreter covers the interpreter path where the bug lives. The native helpers accept_and_return_ulong/accept_and_return_long correctly declare unsigned long long/long long signatures.

Note

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

Generated by Holistic Review · 83.6 AIC · ⌖ 10.6 AIC · ⊞ 10K

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

Labels

area-Interop-mono Servicing-consider Issue for next servicing release review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants