Update arm32 managed code unwinder

[janvorli]

This change brings the arm32 managed code unwinder to the most up to
date state from Windows source.

Close #128590

[Copilot]

Copilot wasn't able to review any files in this pull request.

[dotnet-policy-service]

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

[janvorli]

/azp run runtime-coreclr libraries-jitstress

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[janvorli]

cc @JulieLeeMSFT

[jkotas]

We have managed clone of the unwinder at https://github.com/dotnet/runtime/blob/main/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/ARM/ARMUnwinder.cs that should be updated to match. It can be separate PR as long as there is tracking issue. cc @max-charlamb

[jkotas]

The tests on linux arm32 are still failing with the assert from #128590. It does not appear fixed.

ASSERT FAILED
	Expression: ((FARPROC) (TADDR)m_pvHJRetAddr) != NULL
	Location:   /__w/1/s/src/coreclr/vm/threadsuspend.cpp:4552
	Function:   HijackThread
	Process:    20

[jkotas]

Suggested change

	#define MSFT_OP_TRAP_FRAME_OLD 0
	#define MSFT_OP_MACHINE_FRAME 1
	#define MSFT_OP_CONTEXT 2
	#define MSFT_OP_TRAP_FRAME 3
	#define MSFT_OP_REDZONE_FRAME 4

We should never encounter these opcodes and some of the code to handle them is ifdefed out below. Delete them?

[jkotas]

I think it would look better to delete the whole body of this method in our copy. It is going to help with cdac clone

[github-actions]

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "fb75c1ad76dc956970aae040034c01ed90aa5e7b",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "8a395fa3d63a95b22b1e60dfeb3263668ef9b086",
  "last_reviewed_commit": "fb75c1ad76dc956970aae040034c01ed90aa5e7b",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "8a395fa3d63a95b22b1e60dfeb3263668ef9b086",
  "last_recorded_worker_run_id": "29677688780",
  "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": "fb75c1ad76dc956970aae040034c01ed90aa5e7b",
      "review_id": 4730528324
    }
  ]
}

[github-actions]

Holistic Review

Motivation: The arm32 managed-code unwinder in src/coreclr/unwinder/arm/unwinder.cpp was an older, divergent copy of the Windows kernel ARM unwinder. This PR re-syncs it to the current upstream Windows source (closing #128590), so future updates stay a near-verbatim drop and structural bugs fixed upstream flow into CoreCLR.

Approach: The file is largely replaced with the upstream layout: added Windows-compat shims (crosscomp.h, CONTEXT/RUNTIME_FUNCTION aliases, __in/__out, FIELD_OFFSET, NT_ASSERT, STATUS_BAD_FUNCTION_TABLE, MSFT opcodes), reordered helpers, and rewrote RtlpUnwindCustom to use FIELD_OFFSET(ARM_KTRAP_FRAME, ...)/MSFT_OP_* cases (with the kernel-only trap-frame cases #if 0-commented). The public entry points are restructured to the upstream RtlVirtualUnwind / RtlVirtualUnwind2 / RtlpxVirtualUnwind trio, and the previous #if defined(HOST_UNIX) RtlVirtualUnwind shim is removed. OOPStackUnwinderArm::Unwind and DacUnwindStackFrame are retained as the CoreCLR-specific consumers. The VALIDATE_STACK_ADDRESS* and UNWIND_PARAMS_SET_TRAP_FRAME macros are no-ops in this build, and DEBUGGER_UNWIND is not defined, so the active code path is the !defined(DEBUGGER_UNWIND) variant.

Summary: The port is a faithful, well-scoped resync and mostly mechanical, which is the right strategy for this shared-with-Windows file. However, I found one likely functional regression: the resynced UPDATE_CONTEXT_POINTERS / UPDATE_FP_CONTEXT_POINTERS macros drop the outer if (ARGUMENT_PRESENT(Params)) guard that the previous CoreCLR copy carried, yet OOPStackUnwinderArm::Unwind still passes NULL as UnwindParams to RtlpUnwindFunctionCompact/RtlpUnwindFunctionFull. On ARM32 that is a NULL dereference whenever a nonvolatile register is restored during unwind (the common case), unlike the arm64 sibling where RtlpxVirtualUnwind supplies a stack-allocated params struct. This should be fixed before merge (details inline). Because this worker cannot build or run ARM32 tests, please confirm via CI (arm/Linux DAC + runtime) that stack unwinding still works. Aside from that issue, the change looks correct and consistent with the arm64 port.

Detailed Findings

• NULL UnwindParams dereference on ARM32 (inline on the macro definition): the outer ARGUMENT_PRESENT(Params) guard was removed while the sole in-tree caller still passes NULL. See the inline comment for the two suggested fixes (restore the guard, or pass a local ARM_UNWIND_PARAMS).

• Doc-comment typos carried from upstream (non-blocking): the ported comments reintroduce IamgeBase (should be ImageBase) and a doubled the the language handler data in the RtlpUnwindFunctionCompact / RtlLookupExceptionHandler headers. These are cosmetic and match upstream, so they can be left as-is to minimize drift, but noting them for awareness.

Note

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

Generated by Holistic Review · 241.9 AIC · ⌖ 10.9 AIC · ⊞ 10K

[github-actions]

Possible NULL dereference regression. The base version of these macros wrapped the body in an outer if (ARGUMENT_PRESENT(Params)) guard, so a NULL UnwindParams was safe. This ported version drops that guard and dereferences (Params)->ContextPointers unconditionally (both here and in UPDATE_FP_CONTEXT_POINTERS just below).

However, OOPStackUnwinderArm::Unwind still calls RtlpUnwindFunctionCompact/RtlpUnwindFunctionFull with NULL as the UnwindParams argument (see the two NULL calls near the bottom of the file). On ARM32, any function whose unwind restores a nonvolatile integer (r4-r11/lr) or VFP (d8-d15) register will hit UPDATE_CONTEXT_POINTERS/UPDATE_FP_CONTEXT_POINTERS and dereference the NULL Params, crashing the unwinder/DAC.

Unlike the arm64 port, where RtlpxVirtualUnwind allocates a local ARM64_UNWIND_PARAMS and passes its address, the arm32 OOPStackUnwinderArm::Unwind still calls the low-level helpers directly with NULL. Either restore the outer ARGUMENT_PRESENT(Params) guard in these macros, or have OOPStackUnwinderArm::Unwind allocate a local ARM_UNWIND_PARAMS (with ContextPointers = NULL) and pass its address instead of NULL.