[cDAC] Fix bugs in x86 unwinder - #132169

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/X86/X86Unwinder.cs:597

• These are subtle behavioral fixes in the x86 unwinder (e.g., EBP-frame prolog ESP computation, signed-imm8 compression, and instruction decoding asserts), but there doesn’t appear to be any unit test coverage for x86 unwinding. Wasm has targeted unwinder unit tests (WasmUnwinderTests) using TestPlaceholderTarget; adding a small x86 counterpart that exercises at least one EBP-frame prolog and one SKIP_ARITH_REG(-8) path would help prevent regressions.

        /* The caller's saved EBP is pointed to by our EBP */
        context.Ebp = (uint)_target.ReadPointer(curEBP);
        context.Esp = curEBP + _pointerSize;

        /* Stack pointer points to return address */
        context.Eip = (uint)_target.ReadPointer(context.Esp);
    }

src/native/managed/cdac/tests/UnitTests/StackWalkTests.cs:22

• The PR changes multiple x86 unwinder code paths (ESP-frame prolog reg tracking, double-align saved-reg pointer adjustment, imm8/imm32 decoding via CAN_COMPRESS, and mov-reg-reg decoding), but the only new unit test exercises the EBP-frame prolog case. I couldn’t find any other tests referencing X86Unwinder, so regressions in the other fixed paths would go untested. Consider adding targeted regression tests for (1) an ESP-frame prolog with partial callee-saved pushes, and (2) a double-aligned EBP frame where (curEBP & 0x04) != 0, plus at least one prolog containing a stack adjustment that requires the imm32 form (so CAN_COMPRESS returns false).

    [Fact]
    public void X86Unwind_EbpProlog_RestoresCallerContext()
    {
        MockTarget.Architecture arch = new() { IsLittleEndian = true, Is64Bit = false };
        TestPlaceholderTarget.Builder targetBuilder = new(arch);

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/Context/X86/X86Unwinder.cs:374

• This change fixes the order used to walk already-executed push instructions in an ESP-frame prolog, which is subtle and easy to regress. There isn’t currently a unit test covering the ESP-frame + in-prolog unwind path; adding one would help ensure this stays correct (similar to the new EBP-prolog test, but with an ESP-frame header and no push ebp; mov ebp, esp).

        TargetPointer savedRegPtr = esp;

        // Find out how many callee-saved regs have already been pushed
        foreach (RegMask regMask in registerOrder.Reverse())
        {
            if (!gcInfo.SavedRegsMask.HasFlag(regMask))

src/native/managed/cdac/tests/UnitTests/StackWalkTests.cs:50

• The test writes a value into the would-be saved-EBX stack slot even though, at RelativeOffset=5 (EIP at the push ebx), that push has not executed yet. Using the same value as CurrentEbx can mask bugs where the unwinder incorrectly restores EBX from the stack at this point. Consider writing a distinct sentinel value so the test fails if EBX is restored prematurely.

        byte[] stack = new byte[0x20];
        helpers.Write(stack.AsSpan(0x04, sizeof(uint)), CurrentEbx);
        helpers.Write(stack.AsSpan(0x08, sizeof(uint)), SavedEsi);