Prevent root_count overflow in GcHeader

[mrhapile]

Summary

This PR prevents a potential overflow in GcHeader::inc_roots.

root_count is currently stored as Cell<u16>. The previous implementation incremented the counter using unchecked arithmetic:

self.root_count.set(self.root_count.get() + 1);

If more than u16::MAX roots were created:

• Debug builds panic due to overflow checks
• Release builds silently wrap the value to 0

Silent wraparound violates the garbage collector invariant that:

root_count == 0 → object can be collected

If the counter wraps from 65535 → 0, a live object could incorrectly appear unreferenced and become eligible for collection.

---

Changes

• Replace unchecked increment with checked_add
• Add explanatory safety comment in inc_roots
• Add a regression test to ensure overflow cannot silently occur

Updated implementation:

pub fn inc_roots(&self) {
    // Prevent silent overflow of root_count.
    // In release builds, `u16::MAX + 1` would wrap to 0 and break GC invariants.
    let current = self.root_count.get();
    let new = current
        .checked_add(1)
        .expect("GcHeader root_count overflow");

    self.root_count.set(new);
}

---

Tests

Added a regression test ensuring overflow is detected:

#[test]
#[should_panic(expected = "GcHeader root_count overflow")]
fn root_count_overflow_panics() {
    let header = GcHeader::new_white();

    for _ in 0..=u16::MAX {
        header.inc_roots();
    }
}

All existing tests pass successfully.

---

Impact

• Prevents silent integer wraparound in release builds
• Preserves existing u16 storage layout
• Maintains no_std compatibility
• Ensures GC invariants remain valid

[Copilot]

Pull request overview

This PR hardens the mark-sweep GC’s root tracking by preventing GcHeader::inc_roots from silently overflowing a u16 root counter, which could otherwise violate the “root_count == 0 implies collectible” invariant.

Changes:

• Update GcHeader::inc_roots to use checked arithmetic and panic on overflow instead of wrapping.
• Add a regression test that asserts overflow triggers a panic.
• Add a conditional extern crate std; to the mark-sweep test module.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
oscars/src/collectors/mark_sweep/internals/gc_header.rs	Switches inc_roots to checked increment and adds a panic regression test for overflow.
oscars/src/collectors/mark_sweep/tests.rs	Adds conditional extern crate std; at the top of the mark-sweep tests module.

Comments suppressed due to low confidence (1)

oscars/src/collectors/mark_sweep/internals/gc_header.rs:115

• inc_roots now panics on overflow, but dec_roots still uses saturating_sub(1), which can silently mask an underflow (extra decrements) and also break the root_count == 0 => collectible invariant. Consider switching dec_roots to checked_sub(1) (or an assert!/expect) and adding a small regression test for underflow, so root accounting bugs fail loudly in both directions.

        self.root_count.set(new);
    }

    pub fn dec_roots(&self) {
        // NOTE: if we are underflowing on subtraction, something is seriously wrong

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

extern crate std; is gated on feature = "std", but this test module doesn’t use std anywhere and the crate already conditionally links std in src/lib.rs. Consider removing this line, or (if it’s needed to make cargo test work for a #![no_std] crate) gating it with cfg(test) instead so tests don’t depend on enabling the std feature explicitly.

Suggested change

	#[cfg(feature = "std")]
	extern crate std;