_ast: address the converted tree's values by shadow slot

majit/majit-gc/src/collector.rs

@@ -3178,6 +3178,14 @@ impl MiniMarkGC {
     /// if this is the first request.
     fn find_shadow(&mut self, obj_addr: usize) -> usize {
         let hdr = unsafe { *((obj_addr - GcHeader::SIZE) as *const GcHeader) };
+        // A forwarded header is `FORWARDED_MARKER`, whose every flag bit reads
+        // set, so it would answer the test below and then die on the map lookup
+        // under the shadow message.  `_find_shadow`'s precondition is that the
+        // object has not been copied yet, so name the real fault here.
+        assert!(
+            !hdr.is_forwarded(),
+            "stale pointer into the nursery: find_shadow reached a forwarded header at {obj_addr:#x}"
+        );
         if hdr.has_flag(flags::HAS_SHADOW) {
             // incminimark.py:2855-2857 `ll_assert(shadow != NULL,
             // "GCFLAG_HAS_SHADOW but no shadow found")`.  HAS_SHADOW
@@ -4891,14 +4899,22 @@ impl MiniMarkGC {
         // block.  Upstream normally reaches the shadow through
         // GCFLAG_HAS_SHADOW during the leading minor; this is the equivalent
         // for pyre's oldgen-only non-moving major.
-        if self.is_in_nursery(obj_addr)
-            && unsafe { (*header_of(obj_addr)).has_flag(flags::HAS_SHADOW) }
-        {
-            let shadow_obj = *self
-                .nursery_objects_shadows
-                .get(&obj_addr)
-                .expect("GCFLAG_HAS_SHADOW but no shadow found");
-            unsafe { (*header_of(shadow_obj)).set_flag(flags::VISITED) };
+        if self.is_in_nursery(obj_addr) {
+            // Every flag bit of `FORWARDED_MARKER` reads set, so a worklist
+            // entry a minor collection forwarded would pass the shadow test
+            // below and then fail the map lookup under a message about the
+            // shadow map.  The fault is the stale worklist entry; say so.
+            assert!(
+                !unsafe { (*header_of(obj_addr)).is_forwarded() },
+                "stale major worklist entry: forwarded header at {obj_addr:#x}"
+            );
+            if unsafe { (*header_of(obj_addr)).has_flag(flags::HAS_SHADOW) } {
+                let shadow_obj = *self
+                    .nursery_objects_shadows
+                    .get(&obj_addr)
+                    .expect("GCFLAG_HAS_SHADOW but no shadow found");
+                unsafe { (*header_of(shadow_obj)).set_flag(flags::VISITED) };
+            }
         }
         let custom_trace;
         let (item_size, length_offset, fixed_size, items_have_gc_ptrs);

pyre/pyre-interpreter/src/builtins.rs

@@ -5770,6 +5770,13 @@ fn type_descr_new_with_metaclass(
         // typeobject.py:954 — `W_TypeObject.__init__` is the site that reads
         // the bases as `bases_w or [space.w_object]`, so the `(object,)`
         // default belongs to construction and to nothing that runs before it.
+        // On the empty-bases arm this is a `(object,)` tuple minted right here
+        // with no other referrer, and it has to survive `calculate_metaclass`,
+        // the namespace copy, `validate_c3_mro`, `create_all_slots`,
+        // `__set_name__` and `__init_subclass__` before anything else refers to
+        // it.  A tuple never moves, so the plain uses below stay valid
+        // addresses; the pin is what keeps a major cycle from sweeping it.
+        let _effective_bases_roots = pyre_object::gc_roots::push_roots();
         let w_effective_bases =
             if bases.is_null() || !unsafe { is_tuple(bases) } || unsafe { w_tuple_len(bases) } == 0
             {
@@ -5782,6 +5789,7 @@ fn type_descr_new_with_metaclass(
             } else {
                 bases
             };
+        pyre_object::gc_roots::pin_root(w_effective_bases);
         // calculate_metaclass — delegate to winner if different
         let default_meta = if w_metaclass.is_null() {
             crate::typedef::w_type()
@@ -5896,9 +5904,21 @@ fn type_descr_new_with_metaclass(
         // re-enter the type's dict.
         let dict_obj = pyre_object::gc_roots::shadow_stack_get(dict_root);
         let set_name_entries = unsafe { pyre_object::w_dict_items(dict_obj) };
-        for (key, v) in set_name_entries {
+        // The pairs sit in a native Vec the collector does not walk and every
+        // `__set_name__` runs Python, so a class body's list and dict values
+        // move out from under the entries still to come.  Pin them and read
+        // each back at the call that consumes it.
+        let _entry_roots = pyre_object::gc_roots::push_roots();
+        let flat: Vec<PyObjectRef> = set_name_entries
+            .iter()
+            .flat_map(|&(key, value)| [key, value])
+            .collect();
+        let entry_base = pyre_object::gc_roots::pin_roots(&flat);
+        for index in 0..set_name_entries.len() {
+            let key = pyre_object::gc_roots::shadow_stack_get(entry_base + index * 2);
             if unsafe { pyre_object::is_str(key) } {
-                unsafe { crate::baseobjspace::set_name(w_type, key, v) }?;
+                let value = pyre_object::gc_roots::shadow_stack_get(entry_base + index * 2 + 1);
+                unsafe { crate::baseobjspace::set_name(w_type, key, value) }?;
             }
         }
 

pyre/pyre-interpreter/src/call.rs

@@ -4029,6 +4029,13 @@ fn update_bases(
     base_args: &[PyObjectRef],
     w_orig_bases: PyObjectRef,
 ) -> Result<(Vec<PyObjectRef>, bool), crate::PyError> {
+    // The entries `__mro_entries__` contributes are reachable only from the
+    // tuple it returned and from this native vector, neither of which the
+    // collector walks, while a later iteration's `getattr_str` and
+    // `__mro_entries__` call both run Python.  Pinning each returned tuple
+    // keeps its entries traced for the rest of the walk; the caller's
+    // `w_tuple_new` re-pins them before it allocates.
+    let _entry_roots = pyre_object::gc_roots::push_roots();
     let mut new_bases: Option<Vec<PyObjectRef>> = None;
     for (i, &w_base) in base_args.iter().enumerate() {
         if unsafe { pyre_object::is_type(w_base) } {
@@ -4059,6 +4066,7 @@ fn update_bases(
                         "__mro_entries__ must return a tuple",
                     ));
                 }
+                pyre_object::gc_roots::pin_root(w_new_base);
                 if new_bases.is_none() {
                     new_bases = Some(base_args[..i].to_vec());
                 }
@@ -4166,15 +4174,29 @@ pub(crate) fn real_build_class(args: &[PyObjectRef]) -> Result<PyObjectRef, crat
     let name = unsafe { pyre_object::w_str_get_value(name_obj) };
     // compiling.py:166-167 — resolve __mro_entries__ before metaclass
     // inference; record the original bases for __orig_bases__ when changed.
-    let w_orig_bases = pyre_object::w_tuple_new(base_args.to_vec());
-    let (resolved_bases, bases_changed) = update_bases(base_args, w_orig_bases)?;
+    //
+    // Neither tuple has a referrer besides this frame's Rust locals until
+    // `w_type_new` stores the resolved one into `W_TypeObject.bases`, and
+    // `__mro_entries__`, `__prepare__`, the class body and the metaclass call
+    // all run in between.  A tuple is allocated stable, so it never moves and
+    // the raw copies stay valid addresses — but an unmarked stable block is
+    // still swept, and a class body long enough to span a whole major cycle
+    // frees it under the local.  Both slots therefore stay pinned until
+    // `build_class_inner`, the one consumer, returns.
+    let bases_roots = pyre_object::gc_roots::push_roots();
+    let orig_bases_slot = bases_roots.base();
+    bases_roots.pin_root(pyre_object::w_tuple_new(base_args.to_vec()));
+    let (resolved_bases, bases_changed) =
+        update_bases(base_args, bases_roots.get(orig_bases_slot))?;
     // Non-type bases are not rejected here: `__build_class__` hands the
     // resolved tuple to whichever metaclass was selected, and a metaclass
     // that is not a type may legitimately accept them.  `best_base` performs
     // the `bases must be types` check on the type-construction path.
-    let bases_tuple = pyre_object::w_tuple_new(resolved_bases);
+    let bases_slot = pyre_object::gc_roots::shadow_stack_len();
+    bases_roots.pin_root(pyre_object::w_tuple_new(resolved_bases));
+    let bases_tuple = bases_roots.get(bases_slot);
     let w_orig_bases = if bases_changed {
-        Some(w_orig_bases)
+        Some(bases_roots.get(orig_bases_slot))
     } else {
         None
     };
@@ -4224,6 +4246,12 @@ pub(crate) fn real_build_class(args: &[PyObjectRef]) -> Result<PyObjectRef, crat
     )
 }
 
+/// `bases` and `w_orig_bases` are borrowed, not owned: the only caller,
+/// `real_build_class`, keeps both pinned for the whole of this call.  They are
+/// tuples, so they never move and the raw copies below stay valid addresses;
+/// what the caller's scope buys is that the blocks are still allocated when
+/// the class body returns.  A second caller would have to pin them the same
+/// way before calling.
 fn build_class_inner(
     body_fn: PyObjectRef,
     name: &str,
@@ -5046,16 +5074,34 @@ pub(crate) fn call_init_subclass_on_bases(
     // proxy.  This matters for a custom metaclass mro() that omits the
     // nascent class: `super(w_type, w_type)` must reject that incomplete
     // hierarchy instead of manufacturing an invalid proxy.
+    // The keywords are a raw copy the collector cannot see, and `super_check`,
+    // the `__init_subclass__` lookup and a `__getattr__` under it all run
+    // Python.  A class keyword's value can be a list or a dict, so pin the
+    // pairs here and read them back where the call's keywords are built.
+    let _roots = pyre_object::gc_roots::push_roots();
+    let flat: Vec<PyObjectRef> = init_subclass_kwargs
+        .iter()
+        .flat_map(|&(key, value)| [key, value])
+        .collect();
+    let kwarg_base = pyre_object::gc_roots::pin_roots(&flat);
     let w_objtype = crate::builtins::super_check(w_type, w_type)?;
     let w_super = pyre_object::descriptor::w_super_new(w_type, w_objtype, w_type);
     let w_func = crate::baseobjspace::getattr_str(w_super, "__init_subclass__")?;
     // typeobject.py:1025-1026 — `args = __args__.replace_arguments([])` then
     // `space.call_args(w_func, args)`: keywords only, no positionals, and no
     // frame, because `call_args` (descroperation.py:189) never takes one.
-    let kwds: Vec<(Wtf8Buf, PyObjectRef)> = init_subclass_kwargs
-        .iter()
-        .filter(|(k, _)| unsafe { pyre_object::is_str(*k) })
-        .map(|(k, v)| (unsafe { pyre_object::w_str_get_wtf8(*k) }.to_owned(), *v))
+    let kwds: Vec<(Wtf8Buf, PyObjectRef)> = (0..init_subclass_kwargs.len())
+        .filter_map(|index| {
+            let key = pyre_object::gc_roots::shadow_stack_get(kwarg_base + index * 2);
+            if !unsafe { pyre_object::is_str(key) } {
+                return None;
+            }
+            let value = pyre_object::gc_roots::shadow_stack_get(kwarg_base + index * 2 + 1);
+            Some((
+                unsafe { pyre_object::w_str_get_wtf8(key) }.to_owned(),
+                value,
+            ))
+        })
         .collect();
     call_with_kwargs_in_ctx(take_last_exec_ctx(), w_func, &[], &kwds)?;
     Ok(())

pyre/pyre-interpreter/src/eval.rs

@@ -231,8 +231,12 @@ impl Drop for FrameAnchor {
 /// pyre's JIT-compiled code allocates W_IntObject / result boxes into the
 /// nursery (`NewWithVtable` → `gc_alloc_typed_nursery_shim`). When the
 /// nursery fills and a minor collection runs, only registered roots are
-/// forwarded — unforwarded nursery refs become stale after
-/// `Nursery::reset` zero-fills the region. The interpreter stores live
+/// forwarded — an unforwarded nursery ref is left addressing a corpse.
+/// `Nursery::reset` only rewinds the free pointer on native (it zero-fills
+/// on wasm32, and writes the 0xAA poison only when that debug mode is on),
+/// so the corpse keeps its forwarding header until something is allocated
+/// over it and the stale ref reads whichever of the two it finds. The
+/// interpreter stores live
 /// refs in `PyFrame.locals_cells_stack_w`; without this walker those
 /// slots turn into NULL-`ob_type` stale pointers on the next LOAD_FAST
 /// (reproduced by `inline_helper` n >= 10000).