Skip to content

Four more dangling path issues #85

Description

@adamv-symbolica

Common helpers for the snippets:

use pathmap::PathMap;
use pathmap::zipper::*;

fn mk(keys: &[&[u8]]) -> PathMap<()> {
    let mut m = PathMap::new();
    for k in keys { m.set_val_at(k, ()); }
    m
}
fn keys(m: &PathMap<()>) -> Vec<String> {
    m.iter().map(|(k, _)| String::from_utf8_lossy(&k).into_owned()).collect()
}
/// {ca, cb, d} with "c" turned into a dangling branch
fn dangling_c() -> PathMap<()> {
    let mut m = mk(&[b"ca", b"cb", b"d"]);
    { let mut wz = m.write_zipper(); wz.descend_to(b"c"); wz.remove_branches(false); }
    assert_eq!(keys(&m), ["d"]);
    m
}

Mutating below a dangling branch of a DenseByteNode asserts "Attempted to make_unique on an empty sentinel node"

Summary. Any write-zipper operation whose path continues below a dangling byte of a dense node panics in
make_unique. The same operation on a pair-node (LineListNode) parent works, and mutating exactly at the
dangling byte works, so this is specific to the dense node's child lookup.

Reproduction.

#[test]
fn set_val_below_dangling_byte_of_dense_node() {
    let mut m = dangling_c();                 // {d} plus dangling "c", root is a pair node
    m.set_val_at(b"e", ());
    m.set_val_at(b"f", ());                   // root now has c, d, e, f: upgraded to a dense node
    m.set_val_at(b"cx", ());                  // panics
    assert_eq!(keys(&m), ["cx", "d", "e", "f"]);
}

Same panic with a graft below the dangling byte:

let mut wz = m.write_zipper(); wz.descend_to(b"cxy"); wz.graft_map(mk(&[b"z"]));

Controls that pass: dangling_c() then set_val_at(b"cx") directly (pair-node parent), and
set_val_at(b"c") after the dense upgrade (mutation at the dangling byte itself).

Expected. {cx, d, e, f}.

Actual.

panicked at src/trie_node.rs:3063:13:
Attempted to make_unique on an empty sentinel node

Root cause. ByteNode::node_get_child_mut (dense_byte_node.rs:690) returns the child rec for any byte in
the mask, including a rec holding the empty sentinel (the dangling slot is copied over when the pair node is
upgraded to a dense node). WriteZipperCore::descend_step_internal (write_zipper.rs:2549) then does
Some(next_node.make_mut()) on it (line 2556). The assert is the only thing between this path and a refcount
access on a node that has no refcount word.

Suggested fix. Treat an empty rec as "no child" during descent: either node_get_child_mut returns None
for an empty rec, or descend_step_internal stops at the parent when next_node.is_empty(). The dense-node
mutators that then run on the parent (node_set_val, node_set_branch, graft) must replace the empty rec
rather than make_mut it. This is the same normalization the join/drop_head paths need for the
dangling-sentinel join bugs reported separately.

How found. Randomized edit program (every ZipperWriting op at random foci, node invariants checked after
each edit); surfaced as set_val, restrict, and remove_unmasked_branches panics.


join_into_take at a dangling destination focus asserts "Attempted to make_unique on an empty sentinel node"

Summary. WriteZipper::join_into_take panics when the destination focus is a dangling branch. join_map_into
at the same focus works, so the semantics are clear and only this entry point is broken.

Reproduction.

#[test]
fn join_into_take_at_dangling_focus() {
    let mut m = dangling_c();                 // {d} plus dangling "c"
    let mut o = mk(&[b"x"]);
    {
        let mut wz = m.write_zipper();
        wz.descend_to(b"c");
        let mut src = o.write_zipper();
        wz.join_into_take(&mut src, false);   // panics
    }
    assert_eq!(keys(&m), ["cx", "d"]);
    assert_eq!(keys(&o), Vec::<String>::new());
}

Control that passes: same setup with wz.join_map_into(mk(&[b"x"])) instead, giving {cx, d}.

Expected. m == {cx, d}, o emptied.

Actual.

panicked at src/trie_node.rs:3063:13:
Attempted to make_unique on an empty sentinel node

Root cause. In join_into_take (write_zipper.rs:1798), self.take_focus(false) hands back the empty
sentinel for a dangling focus, and line 1810 does self_node.make_mut().join_into_dyn(src) on it.

Notes. A second site in the same function was seen in randomized runs but not isolated: when the taken
source is the sentinel (dangling source focus), it is passed to graft_internal(Some(..)), whose
debug_assert!(!src.as_tagged().node_is_empty()) (write_zipper.rs:2313) fires. A fix that treats an empty
node as absent on both sides covers it.

Suggested fix. In join_into_take, treat an empty self_node as absent (graft src directly) and an
empty src as absent (leave the destination alone, return Identity), instead of calling make_mut or
graft_internal(Some(..)) with a sentinel. Alternatively route both through TrieNodeODRc::join_into once
that is dangling-safe.


graft_masked_branches with three or more mask bits at a dangling focus panics on unwrap() of None

Summary. graft_masked_branches takes a different code path when the mask has three or more bits. At a
dangling focus that path unwraps a missing focus node. The one- and two-bit paths work at the same focus, and
the three-bit path works at a focus that does not exist at all.

Reproduction.

use pathmap::utils::{ByteMask, BitMask};
fn mask(bytes: &[u8]) -> ByteMask { let mut m = ByteMask::EMPTY; for b in bytes { m.set_bit(*b); } m }

#[test]
fn graft_masked_branches_three_bits_at_dangling_focus() {
    let mut m = dangling_c();                 // {d} plus dangling "c"
    let o = mk(&[b"ax", b"bx", b"dx"]);
    {
        let mut wz = m.write_zipper();
        wz.descend_to(b"c");
        wz.graft_masked_branches(&o.read_zipper(), mask(b"abd"), false);   // panics
    }
    assert_eq!(keys(&m), ["cax", "cbx", "cdx", "d"]);
}

Controls that pass: mask(b"ab") at the same dangling focus gives {cax, cbx, d}; mask(b"abd") at focus
"c" of a plain {d} (no dangling branch) gives {cax, cbx, cdx, d}.

Expected. {cax, cbx, cdx, d}.

Actual.

panicked at src/write_zipper.rs:1573:61:
called `Option::unwrap()` on a `None` value

Root cause. In the ≥3-bit arm of graft_masked_branches (write_zipper.rs:1533), line 1572 calls
self.split_at_focus() and line 1573 does self.try_borrow_focus_mut().unwrap(). At a dangling focus the
split produces no node to borrow.

Suggested fix. After split_at_focus(), materialize a focus node when none exists (the same preparation
the non-existent-focus case already gets before it reaches this arm), or fall back to the per-byte path the
one- and two-bit arms use.


restrict panics with "explicit panic" in AbstractNodeRef::as_tagged when the other operand has a dangling branch at a child-link key

Summary. PathMap::restrict and WriteZipper::restrict panic when a child-link slot of a pair node on the
self side is followed into other and lands on a dangling branch there. meet and restricting on the same
operands work.

Reproduction.

/// dense `other` whose byte `a` is a dangling branch: set in the child mask, no value, no child node
fn other_dangling_a() -> PathMap<()> {
    let mut o = mk(&[b"a", b"b", b"c", b"e"]);
    o.remove_val_at(b"a", false);
    assert_eq!(keys(&o), ["b", "c", "e"]);
    o
}

#[test]
fn restrict_against_dangling_branch() {
    let m = mk(&[b"ab", b"ac"]);              // root pair node: child link with key "a"
    let r = m.restrict(&other_dangling_a());  // panics
    assert_eq!(keys(&r), Vec::<String>::new());
}

Same panic through the zipper, at the root or at a mid-key focus:

let mut m = mk(&[b"ab", b"ac"]);   let o = other_dangling_a();
m.write_zipper().restrict(&o.read_zipper());                                            // panics
let mut m = mk(&[b"dab", b"dac"]); let mut wz = m.write_zipper(); wz.descend_to(b"d"); wz.restrict(&o.read_zipper()); // panics

Controls that pass: m.meet(&o) and wz.restricting(&o.read_zipper()) on the same operands give {};
mk(&[b"ab"]).restrict(&o) (a value slot instead of a child link) gives {}.

Expected. {}: other has no value at or below a, so nothing under a survives the restriction.

Actual.

panicked at src/trie_node.rs:784:38:
explicit panic

Root cause. LineListNode::restrict_slot_contents (line_list_node.rs:1175): for a child-link slot it
follows the slot key into other, then at line 1188 calls onward_node.get_node_at_key(onward_key) and at
line 1189 .as_tagged() on the result unconditionally. A dangling byte is in other's child mask but has no
node, so the lookup returns AbstractNodeRef::None, whose as_tagged is panic!() (trie_node.rs:784). The
sibling subtract_slot_contents (line 1154) already handles this correctly with .into_option() and a
None arm.

Suggested fix. Mirror the subtract path:

match onward_node.get_node_at_key(onward_key).into_option() {
    Some(other_onward) => self_onward_link.as_tagged().prestrict_dyn(other_onward.as_tagged()),
    None => AlgebraicResult::None,
}

How found. Randomized edit program; deterministic with seed 511198 in the all_dense_nodes run, where the
program's own remove_branches(prune = false) tweak had left the dangling byte in other. The three-line
repro above was isolated from the physical node dump at that step.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions