[cdac] x86: implement IGCInfoDecoder.EnumerateLiveSlots; unblock GCRoots stackref tests

docs/design/datacontracts/GCInfo.md

@@ -40,12 +40,9 @@ uint GetSizeOfStackParameterArea(IGCInfoHandle handle);
 uint GetCalleePoppedArgumentsSize(IGCInfoHandle handle);
 
 // Returns the list of interruptible code offset ranges from the GCInfo
-// (not implemented for x86 — x86 encodes per-offset transitions rather than explicit ranges).
 IReadOnlyList<InterruptibleRange> GetInterruptibleRanges(IGCInfoHandle handle);
 
 // Returns all live GC slots at the given instruction offset
-// (not implemented for x86 — see X86GCInfo for the underlying transition data; the cDAC
-// adapter is future work).
 IReadOnlyList<LiveSlot> EnumerateLiveSlots(IGCInfoHandle handle, uint instructionOffset, GcSlotEnumerationOptions options);
 ```
 
@@ -603,3 +600,51 @@ IReadOnlyList<LiveSlot> EnumerateLiveSlots(IGCInfoHandle handle,
     // Collect each live slot into a list and return it.
 }
 ```
+
+
+## x86 specifics
+
+x86 uses the legacy bit-packed `InfoHdr` byte-stream encoding (`src/coreclr/vm/gc_unwind_x86.inl`, `src/coreclr/inc/gcdecoder.cpp`) rather than the modern `GcInfoDecoder` shared by all other architectures. The cDAC decoder lives at `src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/X86/` and is shared with the x86 stack walker.
+
+A few API behaviors are worth calling out:
+
+- `GetSizeOfStackParameterArea` always returns 0 -- x86 has no separate outgoing-argument scratch area; pushed args are reported directly through the per-offset transition stream.
+- `GetInterruptibleRanges` reports one range covering the post-prolog body for fully-interruptible methods, or a single-byte range per call site for partially-interruptible methods. Consumed by `StackWalk_1.WalkStackReferences` for the catch-handler PC override (x86 uses the funclet EH model, see PR [#115957](https://github.com/dotnet/runtime/pull/115957)).
+- `EnumerateLiveSlots` mirrors `EnumGcRefsX86`; see below.
+
+### `EnumerateLiveSlots` behavior
+
+Mirrors `EnumGcRefsX86` (gc_unwind_x86.inl), `scanArgRegTableI` (fully-interruptible) and `scanArgRegTable` (partially-interruptible).
+
+Early-return gates (no slots reported):
+- `IsParentOfFuncletStackFrame` — the funclet sharing this parent's locals will report them itself.
+- Code offset is in the prolog or any epilog (only reachable via `IsExecutionAborted`; GC info does not describe these regions).
+- `IsExecutionAborted` and the method is not fully interruptible.
+
+Filter-funclet `SuppressUntrackedSlots` is honored within the untracked-locals step (the parent frame already reported them) but does not gate the rest of the walk.
+
+Sources of live slots:
+- **Untracked frame locals** — always live for the entire method body. Encoded as signed delta-from-previous offsets in the untracked table. On EBP frames the encoded value resolves directly to `EBP + stkOffs` (`FRAMEREG_REL`). On ESP frames the encoded value is `argBase`-relative where `argBase = ESP + pushedSize` (mirrors `EnumGcRefsX86`); the decoder rebases to a true `SP_REL` offset by adding the pushed-arg size computed at the queried instruction offset.
+- **VarPtr-tracked locals** — live when the lifetime-check offset is within `[BeginOffset, EndOffset)`. EBP-frame offsets are stored as their negated form (mirrors `if (info.ebpFrame) stkOffs = -stkOffs`); ESP-frame offsets receive the same `pushedSize` bias as untracked locals. Non-active frames evaluate the lifetime at `instructionOffset - 1` because a variable can be dead at the return address (e.g. when the call is the last instruction of a try and the return is the catch-block jump target).
+- **Live registers** — accumulated from the LIVE/DEAD transition stream up to the queried offset. Callee-saved registers (EBX/EBP/ESI/EDI) are reported when execution will continue; callee-trashed scratch (EAX/ECX/EDX) is reported only on the active leaf frame. On non-leaf frames register liveness is evaluated at the instruction *before* the call (`regOffset = instructionOffset - 1`) since liveness can change across calls.
+- **Pushed pointer args** — for fully-interruptible code, accumulated from the PUSH/POP transition stream. Non-pointer pushes (`IsPtr = false`) still bump the stack depth (so subsequent pushed-ptr indices stay aligned) but do not contribute a slot. At emit time, once `finalDepth` is known, each tracked push is reported as a positive SP-relative offset: `addr = ESP_call + (finalDepth - 1 - pushIndex) * sizeof(DWORD)` (mirrors `pPendingArgFirst - i * sizeof(DWORD)` in `EnumGcRefsX86`). The translation must be deferred because subsequent pushes/pops change `finalDepth`. For partially-interruptible call sites, slots come from the matching `GcTransitionCall` instead: explicit per-pointer offsets in the huge (0xFB) encoding, or a uint32 `ArgMask` / `IArgs` bitmap walked low-to-high with `addr = ESP + i * sizeof(DWORD)` for the tiny / small / medium / large encodings.
+- **`IsParentOfFuncletStackFrame`** suppresses all reporting from the parent: the funclet itself reports the shared locals via `EnumerateLiveSlots` on the funclet frame.
+
+When `ReportFPBasedSlotsOnly` is set, the result list is filtered to drop register slots and any stack slot whose base is not the frame register (matching `GCInfoDecoder.ReportSlot`).
+
+### Encoding correctness notes (x86)
+
+A few subtleties of the legacy byte-stream encoding caught during stress validation, mirrored from native and worth remembering when modifying the decoder:
+
+- **Huge call-site (`0xFB`) code-delta is cumulative.** The uint32 code delta is `curOffs += delta`, not `curOffs = delta`. Assigning loses all preceding offset accumulation and corrupts every subsequent call site.
+- **Partial-EBP this-pointer tag (`val & 0x80 == 0 && val & 0x0F == 0`).** This encodes the callee-saved register holding `this` at the next call site; native (`gc_unwind_x86.inl` ~line 970) sets `thisPtrReg` only and does *not* record a call entry. Emitting a `GcTransitionCall` at the current offset would overwrite the real call site's `CallRegisters` when both fall at the same `curOffs`.
+- **Partial-interrupt EBP-less call sites emit a negative stack-depth delta** (callee-popped args reverse the prior pushes); the transition stream is generated accordingly.
+- **`0xC0..0xCF` partial-interruptible byte range** in the huge encoding is a call entry that names only callee-saved registers (no pointer args), distinct from `0xFD..0xFF`.
+
+### Deferred edges
+
+These do not affect the GC root scan / `WalkStackReferences` path validated by the cDAC stress suite, but are noted for future work:
+
+- `info.thisPtrResult` reporting for synchronized methods on the `!willContinueExecution` path (the regular live-register report covers `willContinueExecution`, which is what stress exercises).
+- VarPtr `0x2` legacy-encoder "this" bit (the modern x86 JIT uses `0x2` only for pinned, which we already pass through; the legacy "this" interpretation never appears in code from the current JIT).
+- `IPtrMask` (`0xF0`) interior-pointer bitmaps for pushed args — accepted by the decoder as informational, but the bitmap is not yet applied to pushed-arg slots. Only relevant on the partial-interruptible ESP-frame path; in practice the current x86 JIT rarely emits these.

eng/pipelines/runtime-diagnostics.yml

@@ -70,6 +70,7 @@ parameters:
   type: object
   default:
     - windows_x64
+    - windows_x86
     - linux_x64
     - windows_arm64
     - linux_arm64

src/coreclr/vm/datadescriptor/datadescriptor.inc

@@ -1195,6 +1195,11 @@ CDAC_TYPE_SIZE(sizeof(ExternalMethodFrame))
 CDAC_TYPE_FIELD(ExternalMethodFrame, T_POINTER, Indirection, cdac_data<ExternalMethodFrame>::Indirection)
 CDAC_TYPE_END(ExternalMethodFrame)
 
+CDAC_TYPE_BEGIN(PInvokeCalliFrame)
+CDAC_TYPE_SIZE(sizeof(PInvokeCalliFrame))
+CDAC_TYPE_FIELD(PInvokeCalliFrame, T_POINTER, VASigCookiePtr, cdac_data<PInvokeCalliFrame>::VASigCookiePtr)
+CDAC_TYPE_END(PInvokeCalliFrame)
+
 CDAC_TYPE_BEGIN(DynamicHelperFrame)
 CDAC_TYPE_SIZE(sizeof(DynamicHelperFrame))
 CDAC_TYPE_FIELD(DynamicHelperFrame, T_INT32, DynamicHelperFrameFlags, cdac_data<DynamicHelperFrame>::DynamicHelperFrameFlags)

src/coreclr/vm/frames.h

@@ -1358,6 +1358,14 @@ class PInvokeCalliFrame : public FramedMethodFrame
         trace->InitForUnmanaged(GetPInvokeCalliTarget());
         return TRUE;
     }
+
+    friend struct ::cdac_data<PInvokeCalliFrame>;
+};
+
+template <>
+struct cdac_data<PInvokeCalliFrame>
+{
+    static constexpr size_t VASigCookiePtr = offsetof(PInvokeCalliFrame, m_pVASigCookie);
 };
 
 // Some context-related forwards.

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/GCInfo/X86/GCArgTable.cs

@@ -197,6 +197,7 @@ private void GetTransitionsFullyInterruptible(ref TargetPointer offset)
     /// </summary>
     private void GetTransitionsEbpFrame(ref TargetPointer offset)
     {
+        uint curOffs = 0;
         while (true)
         {
             uint argMask = 0, byrefArgMask = 0;
@@ -207,7 +208,6 @@ private void GetTransitionsEbpFrame(ref TargetPointer offset)
             uint argTabSize;
 
             uint val, nxt;
-            uint curOffs = 0;
 
             // Get the next byte and check for a 'special' entry
             uint encType = _target.Read<byte>(offset++);
@@ -231,19 +231,14 @@ private void GetTransitionsEbpFrame(ref TargetPointer offset)
                         }
                         else
                         {
-                            RegMask reg;
-                            if ((val & 0x10) != 0)
-                                reg = RegMask.EDI;
-                            else if ((val & 0x20) != 0)
-                                reg = RegMask.ESI;
-                            else if ((val & 0x40) != 0)
-                                reg = RegMask.EBX;
-                            else
-                                throw new BadImageFormatException("Invalid register");
-                            transition = new GcTransitionCall((int)curOffs);
-                            transition.CallRegisters.Add(new GcTransitionCall.CallRegister(reg, false));
-                            AddNewTransition(transition);
-
+                            // "This pointer liveness encoding" (val & 0x80 == 0 && val & 0x0F == 0):
+                            // metadata for which callee-saved register holds the 'this' pointer
+                            // at the next call site. Native (gc_unwind_x86.inl ~line 970) does NOT
+                            // record a call entry here -- it only sets thisPtrReg. Adding a spurious
+                            // GcTransitionCall at the current curOffs would overwrite the real
+                            // call site's CallRegisters during EnumerateLiveSlots (since the
+                            // partial-EBP decoder may emit the this-ptr tag at the same curOffs
+                            // as a real call site), so we just consume the byte and continue.
                             continue;
                         }
                     }
@@ -296,7 +291,12 @@ private void GetTransitionsEbpFrame(ref TargetPointer offset)
                     val = _target.Read<byte>(offset++);
                     regMask = val & 0x7;
                     byrefRegMask = val >> 4;
-                    curOffs = _target.Read<uint>(offset);
+                    // Code delta is 32-bit and is added to curOffs (mirrors `scanOffs +=` in
+                    // native gc_unwind_x86.inl scanArgRegTable case 0xFB). The pre-PR cDAC port
+                    // assigned `curOffs = ...` here, which silently truncated method-relative
+                    // offsets for the first 0xFB call site and corrupted all subsequent calls
+                    // in long methods (e.g. EventSource cctors).
+                    curOffs += _target.Read<uint>(offset);
                     offset += 4;
                     argCnt = _target.Read<uint>(offset);
                     offset += 4;
@@ -345,7 +345,7 @@ argMask      ...    bitmask of pushed pointer arguments
     /// <summary>
     /// based on <a href="https://github.com/dotnet/runtime/blob/main/src/coreclr/gcdump/i386/gcdumpx86.cpp">GCDump::DumpGCTable</a>
     /// </summary>
-    private void SaveCallTransition(ref TargetPointer offset, uint val, uint curOffs, uint callRegMask, bool callPndTab, uint callPndTabCnt, uint callPndMask, uint lastSkip, ref uint imask)
+    private void SaveCallTransition(ref TargetPointer offset, uint curOffs, uint callRegMask, bool callPndTab, uint callPndTabCnt, uint callPndMask, ref uint imask)
     {
         uint iregMask, iargMask;
         iregMask = imask & 0xF;
@@ -359,11 +359,6 @@ private void SaveCallTransition(ref TargetPointer offset, uint val, uint curOffs
             for (int i = 0; i < callPndTabCnt; i++)
             {
                 uint pndOffs = _target.GCDecodeUnsigned(ref offset);
-
-                uint stkOffs = val & ~byref_OFFSET_FLAG;
-                uint lowBit = val & byref_OFFSET_FLAG;
-                Console.WriteLine($"stkOffs: {stkOffs}, lowBit: {lowBit}");
-
                 transition.PtrArgs.Add(new GcTransitionCall.PtrArg(pndOffs, 0));
             }
         }
@@ -375,15 +370,14 @@ private void SaveCallTransition(ref TargetPointer offset, uint val, uint curOffs
                 transition.IArgs = iargMask;
         }
 
-        Console.WriteLine($"lastSkip: {lastSkip}");
         imask /* = lastSkip  */ = 0;
     }
 
     private void GetTransitionsNoEbp(ref TargetPointer offset)
     {
         uint curOffs = 0;
-        uint lastSkip = 0;
         uint imask = 0;
+        uint lastSkip;
 
         for (; ; )
         {
@@ -418,7 +412,6 @@ private void GetTransitionsNoEbp(ref TargetPointer offset)
                         //
                         skip = _target.GCDecodeUnsigned(ref offset);
                         curOffs += skip;
-                        lastSkip = skip;
                     }
                     else
                     {
@@ -431,18 +424,16 @@ private void GetTransitionsNoEbp(ref TargetPointer offset)
                         {
                             AddNewTransition(new GcTransitionRegister((int)curOffs, RegMask.ESP, Action.POP, false, false, (int)popSize));
                         }
-                        else
-                            lastSkip = skip;
                     }
                 }
             }
             else
             {
-                uint callArgCnt = 0;
+                uint callArgCnt;
                 uint callRegMask;
                 bool callPndTab = false;
                 uint callPndMask = 0;
-                uint callPndTabCnt = 0, callPndTabSize = 0;
+                uint callPndTabCnt = 0;
 
                 switch ((val & 0x70) >> 4)
                 {
@@ -452,8 +443,8 @@ private void GetTransitionsNoEbp(ref TargetPointer offset)
                         //
                         CallPattern.DecodeCallPattern((val & 0x7f), out callArgCnt, out callRegMask, out callPndMask, out lastSkip);
                         curOffs += lastSkip;
-                        SaveCallTransition(ref offset, val, curOffs, callRegMask, callPndTab, callPndTabCnt, callPndMask, lastSkip, ref imask);
-                        AddNewTransition(new StackDepthTransition((int)curOffs, (int)callArgCnt));
+                        SaveCallTransition(ref offset, curOffs, callRegMask, callPndTab, callPndTabCnt, callPndMask, ref imask);
+                        AddNewTransition(new StackDepthTransition((int)curOffs, -(int)callArgCnt));
                         break;
 
                     case 5:
@@ -467,8 +458,8 @@ private void GetTransitionsNoEbp(ref TargetPointer offset)
                         callArgCnt = (val >> 3) & 0x7;
                         lastSkip = CallPattern.CallCommonDelta[(int)(val >> 6)];
                         curOffs += lastSkip;
-                        SaveCallTransition(ref offset, val, curOffs, callRegMask, callPndTab, callPndTabCnt, callPndMask, lastSkip, ref imask);
-                        AddNewTransition(new StackDepthTransition((int)curOffs, (int)callArgCnt));
+                        SaveCallTransition(ref offset, curOffs, callRegMask, callPndTab, callPndTabCnt, callPndMask, ref imask);
+                        AddNewTransition(new StackDepthTransition((int)curOffs, -(int)callArgCnt));
                         break;
                     case 6:
                         //
@@ -478,8 +469,8 @@ private void GetTransitionsNoEbp(ref TargetPointer offset)
                         callRegMask = val & 0xf;    // EBP,EBX,ESI,EDI
                         callArgCnt = _target.GCDecodeUnsigned(ref offset);
                         callPndMask = _target.GCDecodeUnsigned(ref offset);
-                        SaveCallTransition(ref offset, val, curOffs, callRegMask, callPndTab, callPndTabCnt, callPndMask, lastSkip, ref imask);
-                        AddNewTransition(new StackDepthTransition((int)curOffs, (int)callArgCnt));
+                        SaveCallTransition(ref offset, curOffs, callRegMask, callPndTab, callPndTabCnt, callPndMask, ref imask);
+                        AddNewTransition(new StackDepthTransition((int)curOffs, -(int)callArgCnt));
                         break;
                     case 7:
                         switch (val & 0x0C)
@@ -505,11 +496,11 @@ private void GetTransitionsNoEbp(ref TargetPointer offset)
                                 offset += 4;
                                 callPndTabCnt = _target.Read<uint>(offset);
                                 offset += 4;
-                                callPndTabSize = _target.Read<uint>(offset);
+                                // Skip callPndTabSize - present in encoding but unused by the decoder.
                                 offset += 4;
                                 callPndTab = true;
-                                SaveCallTransition(ref offset, val, curOffs, callRegMask, callPndTab, callPndTabCnt, callPndMask, lastSkip, ref imask);
-                                AddNewTransition(new StackDepthTransition((int)curOffs, (int)callArgCnt));
+                                SaveCallTransition(ref offset, curOffs, callRegMask, callPndTab, callPndTabCnt, callPndMask, ref imask);
+                                AddNewTransition(new StackDepthTransition((int)curOffs, -(int)callArgCnt));
                                 break;
                             case 0x0C:
                                 return;
@@ -518,8 +509,6 @@ private void GetTransitionsNoEbp(ref TargetPointer offset)
                         }
                         break;
                 }
-                Console.WriteLine($"CallArgCount: {callArgCnt}");
-                Console.WriteLine($"CallPndTabCnt: {callPndTabSize}");
             }
         }
     }