Skip to content

Fix the GC freeing objects rooted only by an interior pointer - #3261

Merged
valentinbreiz merged 3 commits into
gen3from
fix/gc-segment-walk
Sep 26, 2026
Merged

valentinbreiz merged 3 commits into
gen3from
fix/gc-segment-walk

Conversation

@valentinbreiz

Copy link
Copy Markdown
Member

An object whose only root is an interior pointer (a byref or a span) could be freed while still in use. GC_InteriorPointerRoot (#384) passes on gen3 only because of where boot allocations happen to land: the driver-kit stack (#3260) shifts them and the cell fails on arm64 every time, with [Test] interior byref after collect: 5A5A0008, after churn: 0. This PR fixes the lookup so the result no longer depends on heap layout.

Problem

PreciseRootTrampoline resolves a GC_CALL_INTERIOR slot to its object through GetParentObject. That lookup had four defects:

  1. It started from the brick table (GCSegment.FindClosestObjectBelow). Only bump carves write the table: objects allocated inside a TLAB, and TLABs refilled from the free list, are never recorded. Entries are never cleared either, so one can point inside a live object. In the failing run the lookup returned the segment's first object, not the int[2100] the byref pointed into.
  2. The walk stopped after one object. GCSegment.Enumerator.MoveNext checked Current + size >= (GCObject*)_end on a GCObject*, which scales the byte size by sizeof(GCObject) (16). From a 242-byte string at 0x59ac6038 the check reads 0x59ac6f58, already past the byref at 0x59ac61d8.
  3. Fixing the arithmetic alone would have crashed. The walk stepped by the unaligned ComputeSize (the allocator rounds sizes up to 8 bytes), and it read a MethodTable from free blocks and zeroed filler. On the failing layout an aligned walk reaches a zero word before the array.
  4. A miss returned the raw pointer. TryMarkRoot then read array data as a MethodTable and rejected it, so the array was swept, and the next TLAB refill zeroed it. Had that data looked like a kernel pointer, the mark would have set its low bit and corrupted it.

It also picked the segment list from GC_CALL_PINNED. That flag describes the stack slot (a fixed local), not the heap the object lives on, so a byref into a pinned-heap array searched the wrong list.

Fix

GarbageCollector.PreciseStack.cs:

  • GetParentObject finds the segment by address in the SOH list, then in the pinned list (GCSegmentManager.GetSegmentContaining), and ignores GC_CALL_PINNED.
  • FindObjectContaining walks that segment from Start up to Bump with SweepSegment's rules:
    • a FreeBlock advances by its Size;
    • a word that cannot be a MethodTable (zeroed gap, reserved header slot) advances by one pointer;
    • an object advances by Align(ComputeSize()).
      It returns the object whose extent covers the pointer, or null when it lies in a free block, filler or unallocated space.
  • A miss marks nothing.
  • ComputeSize is unchanged: EnumerateReferences needs the unaligned size for its GCDesc series math.

GCSegment.cs: the defective Enumerator and FindClosestObjectBelow are removed; this lookup was their only caller. The brick table is still written but no longer read (see known gaps).

Cost. One walk of one segment per precise interior root. Segments are sized for one TLAB refill or one large allocation, about 12 KiB for TLAB segments.

Docs. garbage-collector.md and garbage-collector-gcinfo.md describe the new lookup and the state of the brick table.

Verification

Build. make setup: 0 errors, no new warnings.

New cells (GarbageCollector suite, 47 to 50 tests; expectedTests was 46 and now matches the registered count):

Cell Without the fix, x64 Without the fix, arm64 With the fix
GC_InteriorPointerRootMidTlab: a byref into an array placed behind another object in its TLAB fails fails passes on both
GC_InteriorPointerRootPinnedHeap: a byref into GC.AllocateArray<int>(8, pinned: true) fails fails passes on both
GC_InteriorPointerRootFixed: a fixed byref into a regular-heap array fails fails passes on both

Each cell observes a weak reference, so its result does not depend on whether a later allocation reuses the freed block. The "without the fix" runs used this branch's tests with the two GC sources reverted to gen3.

The original failure. On #3260's tree (the driver-kit engine; it does not touch the GC):

  • without the fix: GC_InteriorPointerRoot fails on arm64 (after churn: 0);
  • with the fix: GarbageCollector 50/50 on x64 and arm64 (after churn: 5A5A0008).

Test suites on this branch, QEMU. Every suite matches gen3 CI run 35973349716 case by case, apart from the three new cells:

Suite x64 arm64
GarbageCollector 50/50 50/50
Memory 71/71 71/71
Threading 76/76 76/76
Runtime 103/103 103/103
HelloWorld, TypeCasting, Math all pass all pass
Timer 26/26 20/20
Interrupts 12/12 25 pass / 11 skipped (3 cells)
Storage (all profiles) 426 pass / 42 skipped 668 pass / 736 skipped
Network (2 profiles) 48/48 48/48
Fat, File 42/42, 37/37 42/42, 37/37

Real hardware. The runs above are QEMU only.

Known gaps.

  • The brick table is now written by MarkObject but never read. Removing it moves each segment's Start and so changes heap layout; making it sound means recording every TLAB start and rebuilding entries in both sweeps. Either is a follow-up.
  • Conservative scanning (other threads' stacks, register dumps, the precise walker's conservative fallbacks) still does not resolve interior pointers.
  • The walker copies SweepSegment's stepping rules rather than sharing a helper with it, so the sweep code is untouched here.

Found here but not changed:

  • ReorderPinnedSegmentsAndFreeEmpty never updates s_pinnedSegmentManager.TailSegment (the SOH version does, Sweep.cs:270). The next pinned AllocateSegment can then append through a freed or mid-list tail.
  • SweepPinnedSegment has no FreeBlock branch. It reads a free block as a 24-byte object and then walks the block's stale bytes, which miscounts freed objects and can skip a live one.
  • StampUnusedTlab's small-gap branch zeroes up to AllocLimit, including the header slot of the object that starts there.
  • Threading registers 76 cells against ExpectedTestCount = 74.

An object whose only root was a byref or span could be freed. The
lookup started from the brick table, which never records objects inside
a TLAB and is never cleared; the segment enumerator's end check did
GCObject* arithmetic, scaling the size by 16, so it stopped after one
object; and a miss handed the raw pointer to TryMarkRoot, which dropped
it.

GetParentObject now finds the segment by address in both the SOH and
pinned lists, since GC_CALL_PINNED describes the stack slot and not the
heap, and walks it from Start with the sweep's rules: a FreeBlock
advances by its Size, a non-MethodTable word by one pointer, an object
by its aligned size. A miss marks nothing. The enumerator and
FindClosestObjectBelow, whose only caller this was, are removed.
Three cells that fail without the fix on x64 and arm64 whatever the
boot heap layout: a byref into an array behind another object in its
TLAB, a byref into a pinned-heap array, and a fixed byref into a
regular-heap array. Each observes a weak reference, so the result does
not depend on the freed block being reused. expectedTests now matches
the 50 registered cells.
@valentinbreiz
valentinbreiz merged commit 5170608 into gen3 Sep 26, 2026
14 checks passed
@valentinbreiz
valentinbreiz deleted the fix/gc-segment-walk branch September 26, 2026 07:54
@github-actions

Copy link
Copy Markdown

🧪 Fat Tests

Cell x64 arm64
all ✅ 42/42 ✅ 42/42

📎 Artifacts

Architecture Test Results UART Log Kernel ISO
x64 XML Log ISO
arm64 XML Log ISO

📋 View full test summary | 📄 Kernel.cs

@github-actions

Copy link
Copy Markdown

🧪 File Tests

Cell x64 arm64
all ✅ 37/37 ✅ 37/37

📎 Artifacts

Architecture Test Results UART Log Kernel ISO
x64 XML Log ISO
arm64 XML Log ISO

📋 View full test summary | 📄 Kernel.cs

@github-actions

Copy link
Copy Markdown

🧪 GarbageCollector Tests

Cell x64 arm64
all ✅ 50/50 ✅ 50/50

📎 Artifacts

Architecture Test Results UART Log Kernel ISO
x64 XML Log ISO
arm64 XML Log ISO

📋 View full test summary | 📄 Kernel.cs

@github-actions

Copy link
Copy Markdown

🧪 Graphic Tests

Cell x64 arm64
all ✅ 49/75, 26 skip ⚠️ 24/50, 26 skip
bare ⚠️ 12/25, 13 skip ⚠️ 12/25, 13 skip
virtio-gpu ⚠️ 12/25, 13 skip ⚠️ 12/25, 13 skip
vmware-svga ✅ 25/25 n/a

Skipped

  • x64: 26 skipped: VMware SVGA II adapter not present, needs the vmware-svga profile (26)
  • arm64: 26 skipped: VMware SVGA II adapter not present, needs the vmware-svga profile (26)

📎 Artifacts

Architecture Test Results UART Log Kernel ISO
x64 XML Log ISO
arm64 XML Log ISO

📋 View full test summary | 📄 Kernel.cs

@github-actions

Copy link
Copy Markdown

🧪 HelloWorld Tests

Cell x64 arm64
all ✅ 3/3 ✅ 3/3

📎 Artifacts

Architecture Test Results UART Log Kernel ISO
x64 XML Log ISO
arm64 XML Log ISO

📋 View full test summary | 📄 Kernel.cs

@github-actions

Copy link
Copy Markdown

🧪 Interrupts Tests

Cell x64 arm64
all ✅ 12/12 ✅ 25/36, 11 skip
bare ✅ 12/12 ✅ 7/12, 5 skip
bare+gicv2 n/a ✅ 9/12, 3 skip
bare+gicv3 n/a ✅ 9/12, 3 skip

Skipped

  • arm64: 11 skipped: self-IPI harness is x64-only (3), LAPIC MSI address contract is x64-only (3), LAPIC timer vector is x64-only (3), gic-version not pinned by this cell (2)

📎 Artifacts

Architecture Test Results UART Log Kernel ISO
x64 XML Log ISO
arm64 XML Log ISO

📋 View full test summary | 📄 Kernel.cs

@github-actions

Copy link
Copy Markdown

🧪 Memory Tests

Cell x64 arm64
all ✅ 71/71 ✅ 71/71

📎 Artifacts

Architecture Test Results UART Log Kernel ISO
x64 XML Log ISO
arm64 XML Log ISO

📋 View full test summary | 📄 Kernel.cs

@github-actions

Copy link
Copy Markdown

🧪 Network Tests

Cell x64 arm64
all ✅ 48/48 ✅ 48/48
e1000e ✅ 24/24 n/a
virtio-net-mmio n/a ✅ 24/24
virtio-net-pci ✅ 24/24 ✅ 24/24

📎 Artifacts

Architecture Test Results UART Log Kernel ISO
x64 XML Log ISO
arm64 XML Log ISO

📋 View full test summary | 📄 Kernel.cs

@github-actions

Copy link
Copy Markdown

🧪 Pci Tests

Cell x64 arm64
all ✅ 6/6 ✅ 6/6

📎 Artifacts

Architecture Test Results UART Log Kernel ISO
x64 XML Log ISO
arm64 XML Log ISO

📋 View full test summary | 📄 Kernel.cs

@github-actions

Copy link
Copy Markdown

🧪 Power Tests

Cell x64 arm64
all ✅ 4/4 ✅ 4/4

📎 Artifacts

Architecture Test Results UART Log Kernel ISO
x64 XML Log ISO
arm64 XML Log ISO

📋 View full test summary | 📄 Kernel.cs

@github-actions

Copy link
Copy Markdown

🧪 Runtime Tests

Cell x64 arm64
all ✅ 103/103 ✅ 103/103

📎 Artifacts

Architecture Test Results UART Log Kernel ISO
x64 XML Log ISO
arm64 XML Log ISO

📋 View full test summary | 📄 Kernel.cs

@github-actions

Copy link
Copy Markdown

🧪 Storage Tests

Cell x64 arm64
all ✅ 426/468, 42 skip ❌ 667/1404, 1 fail, 736 skip
ahci ✅ 69/78, 9 skip ✅ 69/78, 9 skip
ahci+acpi-off ✅ 69/78, 9 skip ⚠️ 3/78, 75 skip
ahci+gicv2 n/a ✅ 69/78, 9 skip
ahci+gicv2+acpi-off n/a ⚠️ 3/78, 75 skip
ahci+gicv3 n/a ✅ 69/78, 9 skip
ahci+gicv3+acpi-off n/a ⚠️ 3/78, 75 skip
nvme ✅ 73/78, 5 skip ✅ 70/78, 8 skip
nvme+acpi-off ✅ 72/78, 6 skip ⚠️ 3/78, 75 skip
nvme+gicv2 n/a ✅ 71/78, 7 skip
nvme+gicv2+acpi-off n/a ⚠️ 3/78, 75 skip
nvme+gicv3 n/a ✅ 71/78, 7 skip
nvme+gicv3+acpi-off n/a ⚠️ 3/78, 75 skip
usb ✅ 74/78, 4 skip ✅ 74/78, 4 skip
usb+acpi-off ✅ 69/78, 9 skip ⚠️ 3/78, 75 skip
usb+gicv2 n/a ✅ 74/78, 4 skip
usb+gicv2+acpi-off n/a ⚠️ 3/78, 75 skip
usb+gicv3 n/a ❌ 73/78, 1 fail, 4 skip
usb+gicv3+acpi-off n/a ⚠️ 3/78, 75 skip

Skipped

  • x64: 42 skipped: not a USB profile (20), 64-bit BAR relocation probe is nvme-profile only (8), USB hot-plug thread not running (scheduler timer not ticking) (5), not an NVMe profile (4), NVMe controller API is nvme-profile only (4), acpi-off has no MSI routing to pin (1)
  • arm64: 736 skipped: no block device bound for partition-table tests (441), no block device bound for this profile (171), not a USB profile (60), x64 mapper cell; arm64 installs Device mappings via DeviceMapper (18), BAR relocation probe is x64-only (same harness as the mapper cell) (18), NVMe controller API is nvme-profile only (15), not an NVMe profile (12), interrupt mode not pinned by this cell (1)

Failures

  • arm64 usb+gicv3 / UsbHotPlug_UnplugDetachesMount: the mount outlived its stick

📎 Artifacts

Architecture Test Results UART Log Kernel ISO
x64 XML Log ISO
arm64 XML Log ISO

📋 View full test summary | 📄 Kernel.cs

@github-actions

Copy link
Copy Markdown

🧪 Threading Tests

Cell x64 arm64
all ✅ 76/76 ✅ 76/76

📎 Artifacts

Architecture Test Results UART Log Kernel ISO
x64 XML Log ISO
arm64 XML Log ISO

📋 View full test summary | 📄 Kernel.cs

@github-actions

Copy link
Copy Markdown

🧪 Timer Tests

Cell x64 arm64
all ✅ 26/26 ✅ 20/20

📎 Artifacts

Architecture Test Results UART Log Kernel ISO
x64 XML Log ISO
arm64 XML Log ISO

📋 View full test summary | 📄 Kernel.cs

@github-actions

Copy link
Copy Markdown

🧪 TypeCasting Tests

Cell x64 arm64
all ✅ 17/17 ✅ 17/17

📎 Artifacts

Architecture Test Results UART Log Kernel ISO
x64 XML Log ISO
arm64 XML Log ISO

📋 View full test summary | 📄 Kernel.cs

@github-actions

Copy link
Copy Markdown

🧪 Virtio Tests

Cell x64 arm64
all ✅ 10/10 ✅ 17/20, 3 skip
virtio-mmio n/a ✅ 7/10, 3 skip
virtio-pci ✅ 10/10 ✅ 10/10

Skipped

  • arm64: 3 skipped: this cell presents virtio over MMIO (3)

📎 Artifacts

Architecture Test Results UART Log Kernel ISO
x64 XML Log ISO
arm64 XML Log ISO

📋 View full test summary | 📄 Kernel.cs

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant