Wasm: stop R2R stack walk at reverse-pinvoke frames

[AndyAyersMS]

Fixes a failure seen in #131493 (enable SPC R2R). We were walking off the end of the managed part of the shadow stack at a reverse pinvoke boundary.

[AndyAyersMS]

@dotnet/wasm-contrib PTAL

Not sure who best to tag as a reviewer given that David is away for a few days. @janvorli ?

[azure-pipelines]

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.

[Copilot]

Pull request overview

Updates the WASM RtlVirtualUnwind implementation to stop R2R shadow-stack-based unwinding when the current frame is a reverse-P/Invoke boundary (i.e., the managed frame’s caller is native), preventing the unwind from treating a native caller SP as a managed shadow frame.

Changes:

• Extends WasmUnwindStackFrameCore to optionally leave the caller IP unset when the caller is native.
• Detects reverse-P/Invoke frames via GC info (DECODE_REVERSE_PINVOKE_VAR) and uses that to terminate the R2R walk at the native boundary.

[dotnet-policy-service]

Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara
See info in area-owners.md if you want to be subscribed.

[lewing]

Holistic Review

Motivation: Real and well-founded. WasmUnwindStackFrameCore unconditionally interprets the shadow-stack slot at the unwound-to SP as an R2R frame (GetWasmVirtualIPFromStackPointer). For a reverse-pinvoke method the caller is arbitrary native code whose shadow frame holds no function-table index, so the walk reads garbage and can wander off the end of the managed shadow stack. Reverse pinvoke is the one native→managed entry path in this port that doesn't already plant a TERMINATE_R2R_STACK_WALK marker.

Approach: Directionally right and consistent with the existing precedent in exceptionhandling.cpp and UnixNativeCodeManager.cpp. The SP != 0 / IP == 0 state it produces is already an established, handled shape (helpers.cpp:1851-1858, from #131243). Two questions below before this goes in.

Summary: ⚠️ Needs Human Review — one likely correctness gap around funclets, plus a per-frame stack-walk cost worth a second opinion.

---

Detailed Findings

⚠️ Should the reverse-pinvoke probe skip funclets?

The new check at helpers.cpp:1852-1860 probes DECODE_REVERSE_PINVOKE_VAR on ControlPc with no funclet guard. Funclets share their parent method's GC info, so a catch/finally funclet of an [UnmanagedCallersOnly] method will report callerIsNative = true — but a funclet is never entered from native code.

When the funclet is dispatched by the VM via CallFunclet this looks benign (the TERMINATE_R2R_STACK_WALK marker already yields IP 0). But in the case the code right above explicitly documents at helpers.cpp:1811-1817 — "The funclet was called by its containing method or funclet… the non-exceptional finally case" — the caller is managed R2R code, and forcing IP to 0 would terminate the R2R walk at the funclet, dropping the parent method and everything below it. During a GC stack walk that would mean unreported roots.

exceptionhandling.cpp:4128-4135 guards the identical probe for exactly this reason:

"If we are unwinding from a funclet, we don't care about the reverse pinvoke frame of the parent method that we would get from the GC info. It applies to the parent method only. The funclet itself cannot be invoked via a reverse pinvoke."

Is there something about the wasm shadow-stack shape that makes this unreachable here, or should the check mirror that guard?

bool callerIsNative = false;
{
EECodeInfo codeInfo;
codeInfo.Init((PCODE)ControlPc);
// Funclets share their parent method's GC info, but a funclet is never entered from native// code, so only honor the reverse-pinvoke frame when unwinding the method body itself.if (codeInfo.IsValid() && !codeInfo.IsFunclet())
{
GcInfoDecoder gcInfoDecoder(codeInfo.GetGCInfoToken(), DECODE_REVERSE_PINVOKE_VAR);
callerIsNative = gcInfoDecoder.GetReversePInvokeFrameStackSlot() != NO_REVERSE_PINVOKE_FRAME;
}
}

ExecutionManager::IsFuncletFunctionIndex (codeman.h:2547) is the cheaper wasm-native alternative to EECodeInfo::IsFunclet() if the R2R funclet-start-address comparison isn't wanted on this path.

⚠️ Is the per-frame EECodeInfo::Init + gcinfo decode a concern for GC pause time?

RtlVirtualUnwind runs for every managed frame of every GC suspension and every EH pass. EECodeInfo::Init does a range-section lookup and the decoder parses the gcinfo header — now paid unconditionally, per frame, to answer a question that is false for nearly all of them. The caller in stackwalk.cpp already has an EECodeInfo in hand, but the fixed RtlVirtualUnwind signature can't take it.

Has this been measured on a GC-heavy wasm scenario? And is the longer-term fix to make this a constant-time check — either by planting a TERMINATE_R2R_STACK_WALK shadow frame in the reverse-pinvoke prolog (which would unify reverse pinvoke with every other native→managed entry path in the port), or by setting a bit in the R2R unwind data already reachable via FunctionEntry->UnwindData? Happy to see that as a follow-up if so.

💡 Silent fallback on invalid EECodeInfo

RtlVirtualUnwind already asserts ExecutionManager::IsVirtualIP(ControlPc) at line 1840, so codeInfo.IsValid() should never be false here. Silently falling back to the old behavior hides a broken invariant — consider _ASSERTE(codeInfo.IsValid()).

✅ Verified as correct

• SP != 0 / IP == 0 is a pre-existing, handled state (helpers.cpp:1851-1858); the walker falls back to the explicit Frame chain, and InterpreterFP = 0 is right.
• Wasm codegen does emit the reverse-pinvoke slot (codegenwasm.cpp:4401-4406), so the gcinfo probe is meaningful on this target.
• The GetWasmFramePointerFromStackPointer recursion at line 1801 correctly retains the old behavior — a funclet's FP computation must not be terminated.
• Contracts are satisfied: EECodeInfo::Init and GcInfoDecoder are NOTHROW / GC_NOTRIGGER / SUPPORTS_DAC.

Note

This review was generated by GitHub Copilot (reviewed with three models; the funclet question was raised independently by all three).

[pavelsavara]

During a GC stack walk that would mean unreported roots.

@janvorli is copilot right here ?

[janvorli]

LGTM, thank you!

[janvorli]

Hmm, actually, I think copilot is right w.r.t. non-exceptionally called finallys inside [UnmanagedCallersOnly] marked method. This code would detect them as being called from native code.

[janvorli]

@AndyAyersMS I wonder how we can recognize this case (finally called non-exceptionally).

[AndyAyersMS]

@AndyAyersMS I wonder how we can recognize this case (finally called non-exceptionally).

We can tell if the virtual IP is in a funclet (we report all funclet IP ranges to the runtime in the unwind info), so we should be able to add this condition as a guard. Let me find the right logic.

[janvorli]

It is not just about whether it is in a funclet. For exceptionally called R2R funclet, the caller is native. For non-exceptionally called ones, it is R2R.

[AndyAyersMS]

It is not just about whether it is in a funclet. For exceptionally called R2R funclet, the caller is native. For non-exceptionally called ones, it is R2R.

Maybe I'm missing the point of what you say, but it seems like the suggested fix above is sufficient?

R2R funclets are either called by the runtime (with a special frame marker setup) or by R2R managed code (with normal R2R frame setup), so it should be safe to assume that if we are in an R2R funclet the caller has made proper arrangements and we can probe the caller's frame value to see which is which.

So it's only for non-funclet UCO cases where we can't make any assumptions about the caller and need to be careful. And IsFunclet() exists on the code info and is properly implemented... so adding && !codeInfo.IsFunclet() to the special case logic for UCO as suggested above should work.

[Copilot]

Pull request overview

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

[janvorli]

I didn't know we call finally funclets in the exceptional case through a special helper written in WASM "asm" that takes care of this transition.