Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 26 additions & 5 deletions majit/majit-gc/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2127,6 +2127,8 @@ impl MiniMarkGC {
&mut self,
obj_addr: usize,
site: &str,
// The child site names the slot kind; the root path identifies the producer.
parent_site: &'static str,
holder_addr: usize,
slot_addr: usize,
) -> GcRef {
Expand Down Expand Up @@ -2164,13 +2166,15 @@ impl MiniMarkGC {
panic!(
"GC BUG: invalid type_id={} at obj_addr={:#x} \
(header_addr={:#x}, nursery_start={:#x}, site={}, \
parent_site={}, \
nursery_free={:#x}, nursery_top={:#x}, holder_addr={:#x}, \
holder_type_id={:?}, holder_offset={:?}, holder_words={:#x?})",
type_id,
obj_addr,
obj_addr - GcHeader::SIZE,
self.nursery.start_ptr() as usize,
site,
parent_site,
self.nursery.free_ptr() as usize,
self.nursery.top_ptr() as usize,
holder_addr,
Expand Down Expand Up @@ -2281,6 +2285,8 @@ impl MiniMarkGC {
slot_addr: usize,
holder_addr: usize,
site: &str,
// The child site names the slot kind; the root path identifies the producer.
parent_site: &'static str,
) {
const NURSERY_POISON_WORD: usize = (usize::MAX / 0xff) * 0xaa;
if self.nursery.poison_enabled() && field_ref.0 == NURSERY_POISON_WORD {
Expand All @@ -2291,8 +2297,8 @@ impl MiniMarkGC {
};
let holder_offset = slot_addr.checked_sub(holder_addr);
panic!(
"GC BUG: traced slot contains nursery poison at slot_addr={:#x} holder_addr={:#x} holder_type_id={:?} holder_offset={:?} site={}",
slot_addr, holder_addr, holder_type_id, holder_offset, site,
"GC BUG: traced slot contains nursery poison at slot_addr={:#x} holder_addr={:#x} holder_type_id={:?} holder_offset={:?} site={} parent_site={}",
slot_addr, holder_addr, holder_type_id, holder_offset, site, parent_site,
);
}
}
Expand All @@ -2311,11 +2317,18 @@ impl MiniMarkGC {
/// white old objects to `more_objects_to_trace`.
#[inline]
fn drag_out_root(&mut self, gcref: &mut GcRef) {
self.assert_traced_slot_initialized(*gcref, gcref as *mut GcRef as usize, 0, "minor_root");
self.assert_traced_slot_initialized(
*gcref,
gcref as *mut GcRef as usize,
0,
"minor_root",
"minor_root",
);
let pinned = self.pinned_objects.contains(&gcref.0);
if self.is_nursery_object_start(gcref.0) && !pinned {
let slot_addr = gcref as *mut GcRef as usize;
*gcref = self.copy_nursery_object(gcref.0, "minor_root_target", 0, slot_addr);
*gcref =
self.copy_nursery_object(gcref.0, "minor_root_target", "minor_root", 0, slot_addr);
}
// incminimark.py:2140-2143: append iff (VISITED | PINNED) == 0. pyre's
// marking convention sets VISITED at push time (see `seed_major_root`
Expand All @@ -2332,7 +2345,7 @@ impl MiniMarkGC {

/// Trace an object's GC pointer fields and update any that point
/// into the nursery by copying the target.
fn trace_and_update_object(&mut self, obj_addr: usize, site: &str) {
fn trace_and_update_object(&mut self, obj_addr: usize, site: &'static str) {
let type_id = unsafe { (*header_of(obj_addr)).type_id() };
self.validate_type_id(type_id, obj_addr, site);
let custom_trace = self.types.get(type_id).custom_trace;
Expand All @@ -2347,11 +2360,13 @@ impl MiniMarkGC {
slot_ptr as usize,
obj_addr,
"minor_custom_trace",
site,
);
if self.is_nursery_object_start(field_ref.0) {
let new_ref = self.copy_nursery_object(
field_ref.0,
"minor_custom_trace_target",
site,
obj_addr,
slot_ptr as usize,
);
Expand All @@ -2378,11 +2393,13 @@ impl MiniMarkGC {
slot as usize,
obj_addr,
"minor_fixed_field",
site,
);
if self.is_nursery_object_start(field_ref.0) {
let new_ref = self.copy_nursery_object(
field_ref.0,
"minor_fixed_field_target",
site,
obj_addr,
slot as usize,
);
Expand All @@ -2404,11 +2421,13 @@ impl MiniMarkGC {
slot as usize,
obj_addr,
"minor_varsize_item",
site,
);
if self.is_nursery_object_start(field_ref.0) {
let new_ref = self.copy_nursery_object(
field_ref.0,
"minor_varsize_item_target",
site,
obj_addr,
slot as usize,
);
Expand Down Expand Up @@ -4257,11 +4276,13 @@ impl MiniMarkGC {
slot as usize,
obj,
"minor_dirty_card_item",
"minor_dirty_card",
);
if self.is_nursery_object_start(field_ref.0) {
let new_ref = self.copy_nursery_object(
field_ref.0,
"minor_dirty_card_item_target",
"minor_dirty_card",
obj,
slot as usize,
);
Expand Down
12 changes: 11 additions & 1 deletion majit/majit-translate/src/codewriter/jitcode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1084,8 +1084,18 @@ impl BhFieldSpec {
/// `BhDescr::Size.all_fielddescrs` matching `descr.py:188
/// init_size_descr` parity.
pub fn from_field_descr(fd: &dyn majit_ir::descr::FieldDescr) -> Self {
// descr.py:241-254 `get_type_flag`: a `Ptr` to a GC struct is
// FLAG_POINTER, and only a `Ptr` to a raw struct degrades to
// FLAG_UNSIGNED. pyre models the raw case as `Type::Int`, so a
// pointer field always round-trips as `Pointer` — the same mapping the
// codewriter's own `value_type_to_field_flag` and
// `bh_field_flag_from_descr` already use. Emitting `Unsigned` here
// made the round trip lossy: `SimpleFieldDescr::is_pointer_field()` is
// `flag == Pointer` (descr.py:173), so the rebuilt descr denied being
// a pointer field and `handle_write_barrier_setfield` dropped the
// store's write barrier.
let field_flag = if fd.is_pointer_field() {
majit_ir::descr::ArrayFlag::Unsigned
majit_ir::descr::ArrayFlag::Pointer
} else if fd.is_float_field() {
majit_ir::descr::ArrayFlag::Float
} else if fd.field_type() == majit_ir::value::Type::Void {
Expand Down
86 changes: 86 additions & 0 deletions pyre/bench/synth/_pending/exception_tb_f_locals_vref_root_walk.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# pyre-check: skip-backends=wasm
#
# PARKED: its guard_failures is not the same on every host. macos-latest and
# ubuntu-24.04 both report 5980 on cranelift; windows-latest reported 5979 in
# the run that promoted this file. That is the band #1043 removed the
# closure_per_call overlay over — two counters there disagreed with themselves
# across jobs, one toward its shared value and one away from it — so a
# `.cranelift.win32.jitstats` overlay cannot hold this either, and a missing
# baseline is a hard fail rather than an opt-out. The walker guard therefore has
# no suite gate; this file is the reproduction, and it is correct on all three
# native backends and PYRE_NO_JIT=1 at every size of a 512K-32M PYPY_GC_NURSERY
# sweep.
#
# wasm is exempted above. It reads the catching frame mid-`except` as if the
# implicit `del e` had already run, so `f_locals` loses `e` on part of the loop
# and a second tuple reaches `seen`:
# 2 [(('drive', ('e', 'k', 'seen')), 'mid'), (('drive', ('k', 'seen')), 'mid')]
# Compiled-code only — clean at N=4000, wrong from N=8000 — and measured
# identical with the whole source tree reset to the merge base, so it predates
# the walker guard this file gates. Same stale-`f_locals`-from-compiled-code
# class as getframe_caller_locals_nested_compiled_callee, exempted the same way:
# not because wasm is right here.
#
# Reading `f_locals` off the OUTERMOST traceback node — the catching frame —
# leaves a JIT virtual ref in that frame's `locals_cells_stack_w`. A minor
# collection triggered by a nursery allocation from compiled code then walks the
# slot as a GC root. A vref's leading word is the `JIT_VIRTUAL_REF_VTABLE` magic
# rather than a PyObject `ob_type`, so a root walker that hands the slot to the
# raw exception walk dereferences the magic as a type pointer and takes a
# SIGSEGV. Reading the innermost node instead is clean.
#
# The collection has to land while the vref is on the stack, which is why the
# loop is long: on cranelift the crash is deterministic from roughly the 6000th
# iteration (2000 and 4000 stay clean). dynasm and the plain interpreter never
# reach that GC point on this shape, so this costs all three backends a run to
# gate one of them.
#
# The argument form is for narrowing by hand — `<pyre> thisfile.py 4000 tail`
# and the rest. The suite runs it with none, which takes the defaults below.
#
# Expected output: 1 [(('drive', ('e', 'k', 'seen')), 'mid')]

import sys

N = int(sys.argv[1]) if len(sys.argv) > 1 else 15000
WHICH = sys.argv[2] if len(sys.argv) > 2 else "head"


def mid(i):
raise ValueError("boom")


def locs(tb):
out = []
idx = 0
while tb is not None:
f = tb.tb_frame
want = (
WHICH == "all"
or (WHICH == "head" and idx == 0)
or (WHICH == "tail" and tb.tb_next is None)
)
if want:
out.append((f.f_code.co_name, tuple(sorted(f.f_locals))))
else:
out.append(f.f_code.co_name)
tb = tb.tb_next
idx += 1
return tuple(out)


def drive():
seen = set()
k = 0
while k < N:
try:
mid(k)
except ValueError as e:
seen.add(locs(e.__traceback__))
e.__traceback__ = None
k += 1
return sorted(seen)


r = drive()
print(len(r), r)
78 changes: 78 additions & 0 deletions pyre/bench/synth/_pending/gc_varsize_item_const_shape_witness.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
"""Deterministic witness for `GC BUG ... site=minor_varsize_item_target`.

Run with no arguments. cranelift aborts 3/3; dynasm, `PYRE_NO_JIT=1`, pypy and
cpython all print `1 [(('drive', ('e', 'k', 'seen')), 'mid')]`.

OPEN, and independent of the traceback-journal and virtual-ref root-walk fixes:
it reproduces identically with either of those reverted.

GC BUG: invalid type_id=<varies per run> at obj_addr=0x...fc28
(header_addr=0x...fc20, nursery_start=0x...b0000,
site=minor_varsize_item_target, nursery_free=0x...fee0,
nursery_top=0x...b0000, holder_addr=0x...,
holder_type_id=Some(9), holder_offset=Some(8),
holder_words=[0xd, 0x...fc28, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0])

Reached from `gc_alloc_nursery_shim` -> `alloc_with_type_slow` ->
`do_collect_nursery` -> `trace_and_update_object` -> `copy_nursery_object`, so
it is a minor collection triggered by a nursery allocation out of compiled code.

What the message says: the holder is a varsize object of type_id 9 carrying its
length (13) at offset 0 and items from offset 8, and only `items[0]` is set. The
holder address is above `nursery_top`, so the holder is old-gen and `items[0]`
points into the nursery. The reported `type_id` differs every run and is
pointer-shaped, so the target's header is recycled memory rather than a live
object — the shape of an unbarriered old-to-young store whose target an earlier
minor collection already moved.

The trigger is allocation layout, not the loop bounds. The same body reading
`N`/`WHICH` from `sys.argv` is clean when `15000 head` is passed, and aborts when
it takes the identical values from `else` defaults; wrapping those defaults to
defeat constant folding (`int("15000")`, `len(sys.argv)` arithmetic) does not
change it, which is what rules the folding explanation out.
"""

N = 15000
WHICH = "head"

ERR = ValueError("boom")


def mid(i):
raise ValueError("boom")


def locs(tb):
out = []
idx = 0
while tb is not None:
f = tb.tb_frame
want = (
WHICH == "all"
or (WHICH == "head" and idx == 0)
or (WHICH == "tail" and tb.tb_next is None)
)
if want:
out.append((f.f_code.co_name, tuple(sorted(f.f_locals))))
else:
out.append(f.f_code.co_name)
tb = tb.tb_next
idx += 1
return tuple(out)


def drive():
seen = set()
k = 0
while k < N:
try:
mid(k)
except ValueError as e:
seen.add(locs(e.__traceback__))
e.__traceback__ = None
k += 1
return sorted(seen)


r = drive()
print(len(r), r)
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
bridges_compiled=0
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
guard_failures=1
internal_compile_panics=0
loops_aborted=0
loops_compiled=1
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
bridges_compiled=0
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
guard_failures=1
internal_compile_panics=0
loops_aborted=0
loops_compiled=1
44 changes: 44 additions & 0 deletions pyre/bench/synth/exception_inline_callee_tb_frame_locals.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# pyre-check: max-pypy-ratio=24
# Regression guard: the frame an inlined callee's traceback node names must
# report the callee's NON-PARAMETER locals, not just the arguments it was
# entered with.
#
# A `STORE_FAST` on the callee's own fresh frame is folded to an SSA register
# and emits no store into `locals_cells_stack_w`, so the array holds only the
# parameters seeded at the inline. Storing that frame into `PyTraceback.frame`
# lets it outlive the trace, and without replaying the fold first every
# `tb_frame.f_locals` consumer — `traceback` formatting and debuggers among
# them — silently loses `marker`.
#
# The loop has to pass the compile threshold: the interpreted iterations write
# the live frame directly and read correctly either way, so a short run cannot
# see this. Before the fix, dynasm and cranelift both lost `marker` from the
# moment the loop compiled — 1242 of 4000 iterations still answered
# `('i', 'marker')`, the rest `('i',)`.
#
# Expected output: 4000 ('i', 'marker')

N = 4000


def mid(i):
marker = i * 2
raise ValueError(marker)


def drive():
kinds = {}
k = 0
while k < N:
try:
mid(k)
except ValueError as e:
tb = e.__traceback__.tb_next
names = tuple(sorted(tb.tb_frame.f_locals)) if tb is not None else None
kinds[names] = kinds.get(names, 0) + 1
k += 1
return kinds


for names, count in sorted(drive().items(), key=lambda kv: -kv[1]):
print(count, names)
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
bridges_compiled=0
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
guard_failures=1
internal_compile_panics=0
loops_aborted=0
loops_compiled=1
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ bridges_compiled=4
descr_set_absent=0
descr_set_ambiguous=0
descr_set_stale_absent=0
guard_failures=975
guard_failures=1562
internal_compile_panics=0
loops_aborted=0
loops_compiled=6
Loading
Loading