Skip to content

Commit ff6e1bf

Browse files
committed
feat: implement better way to handle root overload
1 parent b3d8203 commit ff6e1bf

1 file changed

Lines changed: 114 additions & 9 deletions

File tree

core/gc/src/internals/gc_header.rs

Lines changed: 114 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,27 @@ impl GcHeader {
3838

3939
/// Increments [`GcHeader`]'s non-roots count.
4040
pub(crate) fn inc_non_root_count(&self) {
41-
let non_root_count = self.non_root_count.get();
42-
43-
if (non_root_count & NON_ROOTS_MASK) < NON_ROOTS_MAX {
44-
self.non_root_count.set(non_root_count.wrapping_add(1));
41+
let non_root_count = self.non_root_count.get() & NON_ROOTS_MASK;
42+
43+
// `non_root_count` must not exceed `ref_count`.
44+
// This prevents `is_rooted()` from returning false on live objects,
45+
// which would cause a UAF.
46+
// `inc_ref_count` caps `ref_count` at `NON_ROOTS_MAX`, ensuring
47+
// `ref_count` is always reachable.
48+
if non_root_count < self.ref_count.get() {
49+
self.non_root_count
50+
.set(self.non_root_count.get().wrapping_add(1));
4551
} else {
46-
// TODO: implement a better way to handle root overload.
47-
panic!("non-roots counter overflow");
52+
// Saturated: `non_root_count` has reached `ref_count`.
53+
// The debug assertion below catches corrupted state (non_root_count > ref_count),
54+
// which is unreachable through this function but can occur via direct field writes
55+
// in unsafe code or tests.
56+
debug_assert_eq!(
57+
non_root_count,
58+
self.ref_count.get(),
59+
"non_root_count exceeded ref_count: state corruption detected \
60+
(only reachable via direct field writes that bypass the saturation cap)"
61+
);
4862
}
4963
}
5064

@@ -70,11 +84,15 @@ impl GcHeader {
7084

7185
let count = self.ref_count.get().wrapping_add(1);
7286

73-
self.ref_count.set(count);
74-
75-
if count == 0 {
87+
// `non_root_count` shares storage with the mark bit (using 31 bits).
88+
// A `ref_count` > `NON_ROOTS_MAX` would make `is_rooted()` always true,
89+
// leaking memory. Treat this as a hard error identically to `u32` wrap.
90+
// Check before writing to maintain a clean `ref_count` on `catch_unwind`.
91+
if count == 0 || count > NON_ROOTS_MAX {
7692
overflow_panic();
7793
}
94+
95+
self.ref_count.set(count);
7896
}
7997

8098
pub(crate) fn dec_ref_count(&self) {
@@ -104,6 +122,93 @@ impl GcHeader {
104122
}
105123
}
106124

125+
#[cfg(test)]
126+
mod tests {
127+
use super::*;
128+
129+
#[test]
130+
fn mark_bit_preserved() {
131+
let header = GcHeader::new();
132+
header.mark();
133+
assert!(header.is_marked());
134+
135+
header.inc_non_root_count();
136+
assert!(header.is_marked());
137+
assert_eq!(header.non_root_count(), 1);
138+
139+
header.inc_non_root_count();
140+
assert!(header.is_marked());
141+
assert_eq!(header.non_root_count(), 1);
142+
}
143+
144+
#[test]
145+
fn reset_preserves_mark() {
146+
let header = GcHeader::new();
147+
header.inc_non_root_count();
148+
header.mark();
149+
150+
header.reset_non_root_count();
151+
assert_eq!(header.non_root_count(), 0);
152+
assert!(header.is_marked());
153+
}
154+
155+
#[test]
156+
#[should_panic(expected = "too many references to a gc allocation")]
157+
fn inc_ref_panics() {
158+
let header = GcHeader::new();
159+
header.ref_count.set(NON_ROOTS_MAX);
160+
161+
header.inc_ref_count();
162+
}
163+
164+
#[test]
165+
fn is_rooted_before_saturation() {
166+
let header = GcHeader::new();
167+
header.inc_ref_count();
168+
169+
header.inc_non_root_count();
170+
assert!(header.is_rooted());
171+
172+
header.inc_non_root_count();
173+
assert!(!header.is_rooted());
174+
}
175+
176+
#[test]
177+
fn saturation_at_higher_ref_count() {
178+
let header = GcHeader::new();
179+
header.inc_ref_count();
180+
header.inc_ref_count();
181+
182+
header.inc_non_root_count();
183+
header.inc_non_root_count();
184+
header.inc_non_root_count(); // saturates at ref_count
185+
header.inc_non_root_count(); // no-op
186+
assert_eq!(header.non_root_count(), 3);
187+
assert!(!header.is_rooted());
188+
}
189+
190+
#[test]
191+
fn unmark_preserves_non_root_count() {
192+
let header = GcHeader::new();
193+
header.inc_ref_count();
194+
header.inc_non_root_count();
195+
header.mark();
196+
header.unmark();
197+
assert_eq!(header.non_root_count(), 1);
198+
}
199+
200+
/// Verifies `debug_assert!` panics if `inc_non_root_count` exceeds `ref_count`.
201+
#[test]
202+
#[cfg(debug_assertions)]
203+
#[should_panic(expected = "non_root_count exceeded ref_count: state corruption detected")]
204+
fn debug_assert_fires_when_non_root_exceeds_ref_count() {
205+
let header = GcHeader::new();
206+
// Corrupt the state to bypass the cap.
207+
header.non_root_count.set(2);
208+
header.inc_non_root_count(); // triggers debug_assert_eq!
209+
}
210+
}
211+
107212
impl fmt::Debug for GcHeader {
108213
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
109214
f.debug_struct("GcHeader")

0 commit comments

Comments
 (0)