Use user-arg accessors when expanding runtime lookups

[tannergooding]

fgExpandRuntimeLookupsForCall indexed the runtime-lookup helper call's arguments with the raw CountArgs/GetArgByIndex accessors, assuming exactly its two user args (genericCtx, signatureCns).

On wasm, AddFinalArgsAndDetermineABIInfo prepends a non-user WasmShadowStackPointer arg to the front of every managed call during global morph, which runs before fgExpandRuntimeLookups. That leading arg shifts the raw indices, so:

• assert(call->gtArgs.CountArgs() == 2) fires (there are three args), and
• with asserts off, GetArgByIndex(0)/GetArgByIndex(1) read the shadow-stack pointer and the context instead of the context and the signature.

Switch to CountUserArgs/GetUserArgByIndex, which skip non-user args (r2r cell, wasm sp) the same way the rest of the JIT already reads helper-call args. This is behavior-identical on targets that don't prepend such args, and fixes shared-generic runtime lookups on wasm.

This shows up across shared-generic ([System.__Canon]) methods; it accounts for ~10k of the assert-failing methods in a wasm SuperPMI altjit replay over the libraries/coreclr test collections.

Note

This PR was authored with GitHub Copilot.

[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.

[dotnet-policy-service]

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

[tannergooding]

CC. @dotnet/jit-contrib, @adamperlin, @lewing this fixes around 10k helperexpansion asserts from SPMI, is product code impacting

[Copilot]

Pull request overview

This PR updates fgExpandRuntimeLookupsForCall to read runtime-lookup helper-call arguments via the “user arg” APIs (CountUserArgs / GetUserArgByIndex) instead of raw indices, so non-user arguments prepended during morphing (e.g., on TARGET_WASM) don’t shift the expected (genericCtx, signatureCns) positions.

Changes:

• Switch runtime-lookup helper-call arity assertion from CountArgs() to CountUserArgs().
• Switch argument reads from GetArgByIndex(0/1) to GetUserArgByIndex(0/1) when extracting genericCtx and signatureCns.

Show a summary per file

File	Description
src/coreclr/jit/helperexpansion.cpp	Uses user-arg accessors for runtime lookup helper-call argument indexing to avoid non-user arg prepends shifting indices (notably on wasm).

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 1

[Copilot]

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 0 new

[tannergooding]

CC. @dotnet/wasm-contrib

[github-actions]

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "ad3c1aab6c47032f4a4fe543dcec36b8aab1dc19",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "874b9e9eea4a88e3a1709e8d95b15368dbb05888",
  "last_reviewed_commit": "ad3c1aab6c47032f4a4fe543dcec36b8aab1dc19",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "874b9e9eea4a88e3a1709e8d95b15368dbb05888",
  "last_recorded_worker_run_id": "29688134815",
  "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": "ad3c1aab6c47032f4a4fe543dcec36b8aab1dc19",
      "review_id": 4730808347
    }
  ]
}

[github-actions]

Holistic Review

Motivation: fgExpandRuntimeLookupsForCall, fgExpandThreadLocalAccessForCall, and fgExpandStackArrayAllocation indexed helper-call arguments by raw position (CountArgs/GetArgByIndex), assuming the argument list contains only the expected user args. On wasm, AddFinalArgsAndDetermineABIInfo prepends a non-user WasmShadowStackPointer arg to every managed call during global morph, which runs before these expansion phases. That leading arg shifts the raw indices, so assert(CountArgs() == N) fires and, with asserts off, the wrong nodes (shadow-stack pointer / context / signature) are read. The description reports this accounts for ~10k assert-failing methods in a wasm SuperPMI altjit replay.

Approach: Switch the three expansion sites to the user-arg accessors CountUserArgs/GetUserArgByIndex, which skip non-user args (r2r cell, wasm sp) the same way the rest of the JIT already reads helper-call args. GetUserArgByIndex iterates the arg list advancing its logical index only on IsUserArg() entries, so on targets that don't prepend non-user args the Nth user arg is the Nth raw arg and behavior is identical. This is a minimal, mechanical 7-line change confined to one file.

Summary: The fix is correct, well-scoped, and consistent with existing JIT conventions for reading helper-call arguments. The user-arg accessors are the right tool here: they are behavior-identical on non-wasm targets and correctly skip the prepended shadow-stack pointer on wasm. The runtime-lookup signature node remains a user arg marked DONT_CSE, and the thread-local and stack-array index constants (typeArgIndex, lengthArgIndex) refer to user (IL) argument positions, so translating them through GetUserArgByIndex is consistent. No correctness, performance, or behavioral concerns for existing targets; being a JIT-internal change, no test additions are expected and CI/SuperPMI replay provides the validation. 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 · 47.5 AIC · ⌖ 16.1 AIC · ⊞ 10K