Skip to content

[cDAC][wasm] Fix stack walks and MethodDesc naming on WebAssembly - #135044

Merged
lewing merged 8 commits into
mainfrom
lewing-cdac-wasm-stackwalk-fixes
Oct 2, 2026
Merged

lewing merged 8 commits into
mainfrom
lewing-cdac-wasm-stackwalk-fixes

Conversation

@lewing

@lewing lewing commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

Fixes four cDAC failures found by walking stacks on a live CoreCLR browser-wasm target (nightly 12.0.0-alpha.1.26480.103) through IStackWalk. On main, the walk fails on its first step, or never ends, and no MethodDesc can be named.

Changes

MethodDesc validation rejects every MethodDesc (#135035). With FEATURE_PORTABLE_ENTRYPOINTS, the runtime has no precode stubs and doesn't describe PrecodeMachineDescriptor, so constructing PrecodeStubs threw. MethodValidation swallowed the exception and reported every MethodDesc as invalid.

  • New PrecodeStubs version c2 (PrecodeStubs_2) for portable entry points. GetMethodDescFromStubAddress reads PortableEntryPoint.MethodDesc, matching native MethodDesc::GetMethodDescFromPrecode. The runtime advertises c2 under FEATURE_PORTABLE_ENTRYPOINTS, and PrecodeStubs_1 is unchanged.

Stack walks require the Debugger contract (#135034). WASM didn't advertise Debugger, but StackWalk_1 calls it for every native-context frame.

  • WASM now advertises the existing Debugger c1 contract. The in-process debugger isn't built there, so g_pDebugger stays null and CLRJitAttachState stays 0. Debugger_1 already reports that as "not initialized": no debugger data, no hijacks.
  • The mistyped int g_pDebugger linker stub is replaced with correctly typed definitions.

Walk never ends on an R2R InlinedCallFrame (#135036). WasmFrameHandler now handles the INLINED_PINVOKE_FROM_R2R marker as native InlinedCallFrame::UpdateRegDisplay_Impl does. SP comes from CallSiteSP, IP is the R2R virtual IP of the shadow frame there, and FP is that frame's base. The frame pointer doesn't account for funclets yet; that comes with #133890. If an active InlinedCallFrame's context still isn't managed code (no virtual IP could be recovered), the walk fails with StackWalkState.Error, as native NextRaw returns SWA_FAILED, rather than repeating.

Interpreter-only walk repeats forever (#135037). This now matches native StackFrameIterator:

  • An active InlinedCallFrame for an interpreted P/Invoke (InlinedCallFrame::IsInInterpreter) moves straight to the InterpreterFrame that owns it, without touching the context.
  • A walk that starts in interpreted code moves the Frame cursor to the Next of the owning InterpreterFrame named in the first-argument register, as native Init/ResetRegDisp do. If the register is null or doesn't name an InterpreterFrame, the walk throws, where native asserts.

The StackWalk.md, PrecodeStubs.md (new Version 2 section) and Debugger.md specs are updated, and the generated usage tables are regenerated.

WASM isn't a shipping cDAC scenario for .NET 11, so readers built from this PR don't support WASM runtimes built before it: those don't advertise Debugger and still advertise PrecodeStubs c1 with portable entry points.

Follow-up: moving the ExecutionManager portable-entry-point special cases (NonVirtualEntry2MethodDesc, GetDiagnosticCodeStartFromEntryPoint) next to PrecodeStubs_2. I've left that out of this PR to avoid colliding with #133890's ExecutionManager changes.

Validation

  • ./dotnet.sh test src/native/managed/cdac/tests/UnitTests/Microsoft.Diagnostics.DataContractReader.Tests.csproj: passed, 3245/3245.

  • pwsh src/native/managed/cdac/tools/CdacUsageGraph/generate-docs.ps1 -Check: passed, docs up to date.

  • ./build.sh -os browser -c Debug -subset clr.runtime: passed. The generated WASM contract descriptor advertises Debugger c1 with its globals and PrecodeStubs c2.

  • ./build.sh -os browser -subset clr+libs+packs -c Release /p:BuildCrossgen2HostPackForWorkloadTesting=true: passed. These are the packs used for the live run below.

  • New tests:

    • portable entry point lookup through c2;
    • the R2R InlinedCallFrame virtual IP;
    • a WASM walk ending, with the Debugger contract advertised and a null g_pDebugger;
    • an interpreted P/Invoke chain walked once from both kinds of starting point.

    The tests from the original fixes each failed with the corresponding fix removed. I didn't repeat that check after the review changes. The throw for a missing owning InterpreterFrame has no dedicated test.

  • Live browser-wasm runs at 289085e0e06 (before the last review round removed the older-runtime fallbacks) through Blazor-Playground/nesm, with nesm's workarounds and its frame guard turned off. Five configs, 5 walks each: R2R paused at a breakpoint (seeded from the frame chain and from a leaf $sp), R2R paused in a [JSImport] call, and interpreter-only paused in a tight loop and in a [JSImport] call.

    • Against nightly 12.0.0-alpha.1.26480.103, which advertises no Debugger contract and PrecodeStubs c1, so the since-removed fallback paths ran: all pass.
    • Against Release packs built from this branch, which advertise Debugger c1 and PrecodeStubs c2: all pass. Frame names, states and kinds match the nightly run exactly, and nesm's own Debugger and PrecodeStubs workarounds are never called.
    • In every run, walks complete with no errors and every frame is named. The R2R names match a separate static ReadyToRun lookup.
  • Live re-run at the current head b996b7187da with the same branch packs (the runtime code is unchanged since 289085e0e06), nesm workarounds and frame guard off: R2R paused at a breakpoint (all seeds) and interpreter-only paused in a [JSImport] call pass, including the interpreted-code seed that now requires the owning InterpreterFrame. Against the nightly, the reader fails with ContractMissingException (Debugger) unless nesm's workarounds are on, as expected now that older WASM runtimes aren't supported.

TestPlaceholderTarget.TryGetThreadContext now returns false (no OS context, as on WASM) instead of throwing, so tests can exercise the walk's fallback to the Frame chain.

Resolves #135034
Resolves #135035
Resolves #135036
Resolves #135037

Note

This PR description was generated with assistance from GitHub Copilot.

lewing and others added 3 commits October 1, 2026 13:07
With FEATURE_PORTABLE_ENTRYPOINTS (WASM) the runtime has no precode stubs and
does not describe PrecodeMachineDescriptor, so constructing PrecodeStubs threw
and MethodDesc validation rejected every MethodDesc with a temporary entry
point. Mirror MethodDesc::GetMethodDescFromPrecode and read the owning
MethodDesc from the PortableEntryPoint instead.

Fixes #135035

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

- Treat hijack kind as None on WASM, which has no hijack stubs and does not
  advertise the Debugger contract; skip validating it there.
- Handle the INLINED_PINVOKE_FROM_R2R marker in WasmFrameHandler by deriving
  SP/IP/FP from the R2R shadow frame at CallSiteSP, like native
  InlinedCallFrame::UpdateRegDisplay_Impl.
- Advance past an active InlinedCallFrame whose context is not managed code so
  the walk always makes progress instead of yielding the same Frame forever.

Fixes #135034
Fixes #135036

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
An active InlinedCallFrame pushed by the interpreter for a P/Invoke is
followed by its owning InterpreterFrame. The walker applied the ICF context
(an interpreter IP, so managed) and never advanced past the ICF, re-walking
the outer interpreted chain forever. Mirror native StackFrameIterator:

- In the Frame state, an ICF that IsInInterpreter moves straight to the
  owning InterpreterFrame without updating the context.
- When starting in interpreted code, skip past the owning InterpreterFrame
  recorded in the first-argument register (native Init), falling back to
  skipping a head InterpreterFrame.

Fixes #135037

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

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 5 pipeline(s).
11 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 requested a review from radekdoulik October 1, 2026 18:57
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

@rcj1

rcj1 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Shouldn't we expose the debugger in WASM instead of working around it? Even without hijacks, the synchronization methods found on the debugger contract are probably useful.

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

lewing commented Oct 1, 2026

Copy link
Copy Markdown
Member Author

@rcj1 Every synchronization member writes state that only the in-process debugger consumes. RequestSyncAtEvent writes Debugger::m_RSRequestedSync, SetSendExceptionsOutsideOfJMC and EnableGCNotificationEvents write Debugger or RC-thread fields, and GetAttachStateFlags reads CLRJitAttachState, which is defined in debug/ee/debugger.cpp. On WASM, src/coreclr/CMakeLists.txt doesn't build debug/ at all, and g_pDebugger is an int stub in vm/wasm/helpers.cpp that exists only to satisfy the linker. There's no Debugger object to describe, and no RC thread to act on a sync request. The only underlying global that exists is g_CORDebuggerControlFlags, and nothing on WASM reads it.

So exposing the contract would mean porting the left side (debug/ee, the RC thread and an IPC transport) to the browser. That's worth discussing, but it's a separate design question from this PR, and I'm happy to open an issue for it. If we do it, the WASM check in StackWalk_1.GetHijackKind and ValidateForDataAccess can go away, since hijacks would still never be found. I added a note to Debugger.md (b9df321) documenting the current absence.

Note

This comment was generated with assistance from GitHub Copilot.

lewing and others added 3 commits October 1, 2026 17:18
…ling

- Set the Frame cursor to the owning InterpreterFrame's Next, as native
  StackFrameIterator::Init/ResetRegDisp do, instead of scanning the chain.
- Fail the walk when an active InlinedCallFrame's context is not managed code,
  matching native NextRaw (SWA_FAILED), instead of skipping the Frame.
- Combine the duplicate first-argument register readers.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The in-process debugger is not built for wasm, but Debugger_1 already
describes a target whose debugger is not initialized (null g_pDebugger: no
debugger data, no hijacks). Advertise c1 on wasm with g_pDebugger null and
CLRJitAttachState 0, replacing the mistyped g_pDebugger linker stub with
correctly typed definitions.

The stack walker and ValidateForDataAccess now use the contract, and only on
wasm treat a missing contract (runtimes built before this change) as no
hijacks instead of an error.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Runtimes with FEATURE_PORTABLE_ENTRYPOINTS have no executable precode stubs:
every entry point is a PortableEntryPoint. Rather than gating PrecodeStubs_1 on
the feature flag, add PrecodeStubs_2, which reads the owning MethodDesc from
the PortableEntryPoint, and advertise c2 from those runtimes. PrecodeStubs_1
goes back to precode-only logic.

Runtimes with portable entry points built before c2 advertise c1; the c1
registration serves them with PrecodeStubs_2 when FeatureFlags reports
PortableEntrypoints.

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

lewing commented Oct 1, 2026

Copy link
Copy Markdown
Member Author

@rcj1 Following up on my earlier reply: after Max's review, this PR now advertises the Debugger contract on WASM (c7eaee7). It's the existing c1, with g_pDebugger always null because the in-process debugger isn't built there, so it reports no debugger data and no hijacks. If the left side is ported later, the contract will pick it up with no reader changes. The walker keeps a fallback only for WASM runtimes built before this change.

Note

This comment was generated with assistance from GitHub Copilot.

lewing added a commit that referenced this pull request Oct 2, 2026
… section (#135059)

Native `ReadyToRunInfo::GetDebugInfo`
(`src/coreclr/vm/readytoruninfo.cpp`) returns `NULL` immediately when
`m_pSectionDebugInfo == NULL`. The cDAC mirror,
`ReadyToRunJitManager.GetDebugInfo`, dereferenced
`ReadyToRunInfo.DebugInfoSection` without that check and built a
`NativeArray` from whatever was at address 0.

This now matters because #134690 makes Browser/WASI ReadyToRun publishes
pass `--strip-debug-info` by default, including CoreLib and the
framework. iOS, tvOS, and MacCatalyst already default to it. On wasm,
linear address 0 is readable, so cDAC decoded garbage. A live run
against a stripped browser image threw `BadImageFormatException: offset
out of bounds` (`NativeReader.DecodeUnsigned` ← `NativeArray..ctor` ←
`ReadyToRunJitManager.GetDebugInfo` ← `DebugInfo_1.GetMethodVarInfo`)
instead of reporting that there was no debug info. On native targets,
the same path fails with a read exception.

## Changes

- `ReadyToRunJitManager.GetDebugInfo` returns `TargetPointer.Null` (with
`hasFlagByte = false`) when `DebugInfoSection` is null, matching native.
- `docs/design/datacontracts/ExecutionManager.md`: the R2R
`GetDebugInfo` description now includes the null-section early return.
- No caller changes were needed. `DebugInfo_1.HasDebugInfo`,
`GetMethodNativeMap`, `GetMethodVarInfo`, and `GetAsyncSuspensionPoints`
already treat a null debug-info pointer as "no debug info". The two
map/var methods still compute `codeOffset`.

## Tests

New test
`ExecutionManagerTests.GetDebugInfo_R2R_NoDebugInfoSection_ReturnsNull`
runs as a `[Theory]` over `StdArchAllVersions` (4 arch cases). It builds
an R2R module whose `DebugInfoSection` is null and makes the low 4 KB of
the address space readable as zeros, the way wasm linear memory is, so
the unfixed code decodes instead of hitting a read fault. It asserts
that:
- `IExecutionManager.GetDebugInfo` returns `TargetPointer.Null` and
`hasFlagByte == false`
- `IDebugInfo.HasDebugInfo` is `false`
- `GetMethodVarInfo` and `GetMethodNativeMap` return empty sequences
with `codeOffset == 4`

Results:
- `./build.sh -s tools.cdactests -test`: passed. UnitTests 3236/3236,
DataGeneratorTests 46/46, UsageTests 4/4 (no stale generated docs).
- `./.dotnet/dotnet test src/native/managed/cdac/tests/UnitTests
--filter "FullyQualifiedName~GetDebugInfo_R2R_NoDebugInfoSection"`:
passed, 4/4.
- Mutation check: I removed the null check and reran the same filtered
command. It failed 4/4 with `System.BadImageFormatException : offset out
of bounds` at `NativeReader.DecodeUnsigned` ← `NativeArray..ctor` ←
`ReadyToRunJitManager.GetDebugInfo`, the same failure as the live wasm
run. With the fix restored, it passes 4/4.

This PR has a single concern. It leaves wasm stack-walk and
variable-location work to #133890, #135044, and #135055.

> [!NOTE]
> This PR description was generated with GitHub Copilot.

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

WASM is not a shipping cDAC scenario for .NET 11, so readers need not support
WASM runtimes built before they advertised Debugger c1 and PrecodeStubs c2:
- Require the Debugger contract everywhere again, in the walker and in
  ValidateForDataAccess.
- Register PrecodeStubs c1 as precode-only; c2 serves portable entry points.

When a walk starts in interpreted code, require the first-argument register to
name the owning InterpreterFrame, as native StackFrameIterator::Init and
ResetRegDisp assert, instead of falling back to skipping a head InterpreterFrame.

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

@max-charlamb max-charlamb left a comment

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.

Thanks. lgtm.

I may take a look at simplifying more of the PortableEntryPoint feature flag switching in ExecutionionManager by centralizing it into IPrecodeStubs.

@lewing
lewing enabled auto-merge (squash) October 2, 2026 20:28
@lewing

lewing commented Oct 2, 2026

Copy link
Copy Markdown
Member Author

/ba-g failures are known and tracked

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