Values: a merge reads as a variable only when every forward edge in renders its copy

`Values::assigned` (a bool set by any copy or write spelled so far) is `owed`, a count by value
index. A parameter owes 0, an operation's output 1, and a merge one per distinct placed forward
edge in (`forward_in`, from the CFG's predecessors and `DomTree::is_back_edge`, O(preds) once per
block that holds a named merge). A write sets it to 0 (`defines`); a copy on a forward edge pays
one (`account_copy`); a copy on a back edge, a self copy and a gap (`unrendered_copy`) pay none. A
read of a name still owed is the NeverAssigned residual, as before. So a latch copy, which is
spelled after every header-side read in the same order the facts give, can never justify one, and
one gapped forward copy keeps the merge residual: a gap traps where it stands, but the certificate
(`GapKind::ValuesNotRendered.ends_control()` is false) lets control go on past it, and a verdict
must not lean on what the certificate denies (`a_gap_is_written_as_the_residual_call_whose_helper_traps`
pins the premise). A read turns to a residual where only a back edge or a gapped copy justified it, and to a
variable where the old write order read a name before its forward copy was spelled.

Tests, through the real pipeline from byte literals, each under the four write orders with a fresh
program per order (`crates/r2engine/tests/engine/structure.rs`): a loop-carried merge whose entry
copy is a frame-address gap (43 and 48 bytes) has the same verdicts under every order and every read
of it is `r2sleigh_residual_u64(`; a merge with one gapped and one rendered forward copy reads
residual (24 bytes, which pins ALL against ANY); a merge whose copies all render reads as a variable
in every order (16 bytes). Fail first: at the mutants-only head the first fails on O1 against
natural (`return rax_2;` after the loop against `return r2sleigh_residual_u64(2);`, ledger 22
rendered and 6 residual against 20 and 8); at the spelling-seam head the verdicts agree under every
order and both residual tests still fail on the plain read the latch copy justified; all three pass
here.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01456qmqCGpCWJkrg6JUZNNM

crates/r2dec/src/codegen.rs

@@ -1727,6 +1727,23 @@ mod tests {
         );
     }
 
+    /// A gap runs as the void residual call, whose helper traps: what a gap stands for is not
+    /// computed, so control cannot go on past it as if it were (ADR D58, Merges).
+    #[test]
+    fn a_gap_is_written_as_the_residual_call_whose_helper_traps() {
+        let gap = CStmt::Gap(crate::ast::GapMarker {
+            kind: crate::ast::GapKind::ValuesNotRendered,
+            origin: "render::control".to_owned(),
+            block_addr: 0x1000,
+            op_idx: 0,
+            ops: 1,
+        });
+        let text = CodeGenerator::new(CodeGenConfig::default()).generate_stmt(&gap);
+        let residual = crate::prelude::Helper::Residual(crate::prelude::ResidualType::Void);
+        assert!(text.contains(&format!("{}(", residual.name())), "{text}");
+        assert!(residual.definition().contains("__builtin_trap();"));
+    }
+
     /// A declared address defines nothing, and its declaration stands alone.
     ///
     /// The storage class is spelled because there is no definition beside it,

crates/r2dec/src/render/values.rs

@@ -41,8 +41,12 @@ pub(super) struct Values<'a> {
     /// The structure member each cell term is, by term index: the accesses through a parameter
     /// that points to its inferred structure.
     members: Vec<Option<CExpr>>,
-    /// The named values a statement written so far assigns: a read of any other is a residual.
-    assigned: RefCell<Vec<bool>>,
+    /// By value index: the definitions a named value owes before a read of it is a variable; a merge
+    /// owes one per forward edge in, and a back edge pays none (doc/adr-decompiler-rewrite.md, Merges).
+    owed: RefCell<Vec<u32>>,
+    /// The dominator tree and root, which tell a forward edge from a back edge.
+    dom: &'a r2ssa::domtree::DomTree,
+    root: u64,
     /// By instruction index: whether a statement assigns the instruction's output where it stands.
     bound: Vec<bool>,
     /// The frame array each stack object a spelled term names is declared as, on first use.
@@ -813,7 +817,9 @@ impl<'a> Values<'a> {
             names: vec![None; graph.values.len()],
             declared: vec![false; graph.values.len()],
             members: Vec::new(),
-            assigned: RefCell::new(vec![false; graph.values.len()]),
+            owed: RefCell::new(vec![1; graph.values.len()]),
+            dom: input.function().domtree(),
+            root: input.function().root(),
             bound,
             frame: super::frame::Frame::of(artifact),
             frame_array: RefCell::new(None),
@@ -947,7 +953,7 @@ impl<'a> Values<'a> {
                 SymbolRole::Parameter(index),
             );
             self.names[value.0 as usize] = Some((name, ty));
-            self.assigned.get_mut()[value.0 as usize] = true;
+            self.owed.get_mut()[value.0 as usize] = 0;
             if let CType::Pointer(pointee) = &c
                 && let CType::Struct(tag) = pointee.as_ref()
             {
@@ -956,6 +962,7 @@ impl<'a> Values<'a> {
             self.params.push(CParam { ty: c, name });
         }
         self.members = self.members_of(input, pointing);
+        let mut forward = vec![None; self.graph.blocks.len()];
         for inst in &self.graph.insts {
             let Some(output) = inst.output.filter(|_| self.bound[inst.id.0 as usize]) else {
                 continue;
@@ -972,6 +979,11 @@ impl<'a> Values<'a> {
                 .borrow_mut()
                 .declare(name, c.clone(), SymbolRole::Carrier);
             self.names[output.0 as usize] = Some((id, ty));
+            self.owed.get_mut()[output.0 as usize] = match inst.payload {
+                InstPayload::Phi { .. } => *forward[inst.block.0 as usize]
+                    .get_or_insert_with(|| self.forward_in(input, inst.block)),
+                InstPayload::Op(_) => 1,
+            };
             self.locals.get_mut().push(CLocal {
                 ty: c,
                 name: id,
@@ -981,6 +993,23 @@ impl<'a> Values<'a> {
         }
     }
 
+    /// The forward edges into `block`, each a copy its merges owe: the placed predecessors it does not
+    /// dominate, at least one. `O(preds)`.
+    fn forward_in(&self, input: &RenderInput<'_>, block: r2ssa::BlockId) -> u32 {
+        let Some(block) = self.graph.block(block) else {
+            return 1;
+        };
+        let (dom, root) = (self.dom, self.root);
+        let mut forward = (input.function().cfg().predecessors(block.addr).into_iter())
+            .filter(|pred| {
+                crate::structure::place::is_placed(dom, root, *pred)
+                    && !dom.is_back_edge(*pred, block.addr)
+            })
+            .collect::<Vec<_>>();
+        forward.dedup();
+        u32::try_from(forward.len()).map_or(u32::MAX, |n| n.max(1))
+    }
+
     /// A frame object's address in the function's one frame array (D4), and its extent: the
     /// array is declared on first use, aligned as the machine's frame is.
     fn object(&self, object: ObjectId) -> Option<terms::Placed> {
@@ -1106,16 +1135,10 @@ impl<'a> Values<'a> {
         }
     }
 
-    /// A name as its class reads it: a parameter declared a pointer is read as its word, and a
-    /// name no statement written before assigns (its definition a gap) is never assigned.
+    /// A name as its class reads it: a parameter declared a pointer is read as its word, and a name
+    /// still owed a definition (a gapped copy, or a write not yet spelled) is never assigned.
     fn read_name(&self, value: ValueId, name: SymbolId, held: &MachineType) -> Option<CExpr> {
-        if !self
-            .assigned
-            .borrow()
-            .get(value.0 as usize)
-            .copied()
-            .unwrap_or(false)
-        {
+        if self.owed.borrow().get(value.0 as usize).copied() != Some(0) {
             let read = crate::prelude::residual(
                 &terms::c_type(held)?,
                 crate::prelude::ResidualCause::NeverAssigned,
@@ -1475,13 +1498,14 @@ impl<'a> Values<'a> {
         self.mark(inst.id);
         self.residual_in(inst.id, stmt);
         if let Some(output) = inst.output.filter(|value| self.is_named(*value)) {
-            self.assigns(output);
+            self.defines(output);
         }
     }
 
-    fn assigns(&self, value: ValueId) {
-        if let Some(slot) = self.assigned.borrow_mut().get_mut(value.0 as usize) {
-            *slot = true;
+    /// A statement written assigns `value` itself, which owes nothing more.
+    fn defines(&self, value: ValueId) {
+        if let Some(slot) = self.owed.borrow_mut().get_mut(value.0 as usize) {
+            *slot = 0;
         }
     }
 
@@ -1857,7 +1881,7 @@ impl<'a> Values<'a> {
         if let Some(def) = self.graph.def_inst(*result) {
             self.mark(def);
         }
-        self.assigns(*result);
+        self.defines(*result);
         Some(vec![assign(*name, value)])
     }
 
@@ -1914,7 +1938,7 @@ impl<'a> Values<'a> {
             .iter()
             .filter_map(|(named, _)| named.map(|(value, ..)| value))
         {
-            self.assigns(value);
+            self.defines(value);
         }
         for value in [halves.0, halves.1].into_iter().flatten() {
             if let Some(def) = self.graph.def_inst(value) {
@@ -1960,7 +1984,7 @@ impl<'a> Values<'a> {
         if let Some(def) = self.graph.def_inst(result) {
             self.mark(def);
         }
-        self.assigns(result);
+        self.defines(result);
         Some(assign(*name, value))
     }
 
@@ -2330,6 +2354,7 @@ impl<'a> Values<'a> {
         ) else {
             return Vec::new();
         };
+        let forward = !self.dom.is_back_edge(from, to);
         let mut copies = Vec::new();
         for inst in to_block.insts.iter().filter_map(|id| self.graph.inst(*id)) {
             let InstPayload::Phi { predecessors } = &inst.payload else {
@@ -2354,7 +2379,7 @@ impl<'a> Values<'a> {
             if input == output
                 && let Some((spelled, attempt)) = source
             {
-                self.account_copy(inst.id, output, attempt, &CStmt::Expr(spelled));
+                self.account_copy(inst.id, output, attempt, (&CStmt::Expr(spelled), false));
                 continue;
             }
             copies.push((inst.id, output, source, reads));
@@ -2380,7 +2405,8 @@ impl<'a> Values<'a> {
                 out.push(unrendered_copy(to));
                 continue;
             };
-            self.account_copy(inst, output, attempt, &CStmt::Expr(source.clone()));
+            let copy = CStmt::Expr(source.clone());
+            self.account_copy(inst, output, attempt, (&copy, forward));
             match clobbers {
                 false => out.push(self.assign_value(output, *name, source)),
                 true => {
@@ -2394,13 +2420,22 @@ impl<'a> Values<'a> {
         out
     }
 
-    /// The merge `inst` has a copy on this edge: its instruction and what the source absorbed are
-    /// rendered, whether or not `stmt` is written.
-    fn account_copy(&self, inst: InstId, output: ValueId, attempt: Attempt, stmt: &CStmt) {
+    /// The merge `inst` has a copy on this edge, rendered whether or not `stmt` is written. Only a copy
+    /// on a forward edge pays what the merge owes, once per edge; a self copy rides a back edge.
+    fn account_copy(
+        &self,
+        inst: InstId,
+        output: ValueId,
+        attempt: Attempt,
+        (stmt, forward): (&CStmt, bool),
+    ) {
         *self.pending.borrow_mut() = attempt;
         self.mark(inst);
         self.residual_in(inst, stmt);
-        self.assigns(output);
+        if let (true, Some(owed)) = (forward, self.owed.borrow_mut().get_mut(output.0 as usize)) {
+            debug_assert!(*owed > 0, "a forward edge pays a merge once");
+            *owed = owed.saturating_sub(1);
+        }
     }
 
     /// A local `<name>_next` of the merge's type, where a cycle of copies goes through.

crates/r2dec/src/render/values/transfer.rs

@@ -255,7 +255,7 @@ impl Values<'_> {
         }
         for (read, output) in answered {
             self.mark(read);
-            self.assigns(output);
+            self.defines(output);
         }
     }
 

crates/r2engine/tests/engine/structure.rs

@@ -2,7 +2,10 @@
 
 use crate::common;
 
+use std::collections::BTreeSet;
+
 use common::{BASE, Literal};
+use r2dec::placement_mutants::{WriteOrder, with_write_order};
 use r2engine::program::OpenProgram;
 
 fn c_of(code: &'static [u8]) -> String {
@@ -281,3 +284,174 @@ fn a_loop_that_leaves_on_a_chain_stays_on_both_tests_negated() {
         "{c}"
     );
 }
+
+/// The four write orders a placement can be asked for: the natural one and the three mutants (ROADMAP D58).
+const ORDERS: [WriteOrder; 4] = [
+    WriteOrder::Natural,
+    WriteOrder::KeepExits,
+    WriteOrder::ExitsReversed,
+    WriteOrder::MergesReversed,
+];
+
+/// What no block write order may change: the ledger's columns, the never-assigned names and the set of
+/// statements apart from control and site numbers (a set: a tail is written once per jump to it).
+#[derive(Debug, PartialEq, Eq)]
+struct Verdicts {
+    ledger: String,
+    never_assigned: String,
+    statements: BTreeSet<String>,
+}
+
+fn verdicts_of(c: &str) -> Verdicts {
+    let proof = c
+        .lines()
+        .find(|line| line.contains("r2dec proof:"))
+        .unwrap_or_else(|| panic!("a proof line: {c}"));
+    let between = |from: &str, to: char| {
+        proof
+            .split_once(from)
+            .map(|(_, rest)| rest.split(to).next().unwrap_or_default().to_owned())
+            .unwrap_or_default()
+    };
+    let site = regex_free_site_numbers;
+    Verdicts {
+        ledger: between("source obligations:", ';'),
+        never_assigned: between("never assigned, read as residuals (", ')'),
+        statements: c
+            .lines()
+            .map(str::trim)
+            .filter(|line| line.ends_with(';') && !line.starts_with("goto "))
+            .filter(|line| !matches!(*line, "break;" | "continue;") && !line.ends_with(": ;"))
+            .map(site)
+            .collect(),
+    }
+}
+
+/// `line` with each residual site's number replaced by `N`: sites are numbered in text order.
+fn regex_free_site_numbers(line: &str) -> String {
+    let mut out = String::new();
+    let mut rest = line;
+    while let Some(at) = rest.find("r2sleigh_residual_") {
+        let (head, tail) = rest.split_at(at);
+        out.push_str(head);
+        let open = tail.find('(').map_or(tail.len(), |open| open + 1);
+        out.push_str(&tail[..open]);
+        let digits = tail[open..]
+            .chars()
+            .take_while(char::is_ascii_digit)
+            .count();
+        out.push('N');
+        rest = &tail[open + digits..];
+    }
+    out.push_str(rest);
+    out
+}
+
+/// A rendering of `code` with blocks written in `order`; each is a fresh program, so no memo is shared.
+fn c_under(order: WriteOrder, code: &'static [u8]) -> String {
+    with_write_order(order, || c_of(code))
+}
+
+/// The lines that name the variable `name` other than as the target of an assignment to it.
+fn reads_of<'c>(c: &'c str, name: &str) -> Vec<&'c str> {
+    c.lines()
+        .map(str::trim)
+        .filter(|line| line.contains(name) && !line.starts_with(&format!("{name} = ")))
+        .filter(|line| !line.ends_with(&format!(" {name};")))
+        .collect()
+}
+
+/// `sub rsp, 0x18; lea rax, [rsp + 8]; loop: cmp byte [rdi], 0; je out; cmp sil, [rdi]; je found;
+/// add rax, 1; add rdi, 1; jmp loop; out: add rsp, 0x18; ret; found: add rax, 5; add rsp, 0x18; ret`
+const FRAME_ADDRESS_LOOP: &[u8] = &[
+    0x48, 0x83, 0xec, 0x18, 0x48, 0x8d, 0x44, 0x24, 0x08, 0x80, 0x3f, 0x00, 0x74, 0x0f, 0x40, 0x3a,
+    0x37, 0x74, 0x0f, 0x48, 0x83, 0xc0, 0x01, 0x48, 0x83, 0xc7, 0x01, 0xeb, 0xec, 0x48, 0x83, 0xc4,
+    0x18, 0xc3, 0x48, 0x83, 0xc0, 0x05, 0x48, 0x83, 0xc4, 0x18, 0xc3,
+];
+
+/// The same, with `test rdx, rdx; je found2` before the loop: a second way to the exit that adds 5,
+/// which makes that exit a merge written after the loop whichever order the exits are written in.
+const FRAME_ADDRESS_LOOP_TWO_WAYS: &[u8] = &[
+    0x48, 0x83, 0xec, 0x18, 0x48, 0x8d, 0x44, 0x24, 0x08, 0x48, 0x85, 0xd2, 0x74, 0x19, 0x80, 0x3f,
+    0x00, 0x74, 0x0f, 0x40, 0x3a, 0x37, 0x74, 0x0f, 0x48, 0x83, 0xc0, 0x01, 0x48, 0x83, 0xc7, 0x01,
+    0xeb, 0xec, 0x48, 0x83, 0xc4, 0x18, 0xc3, 0x48, 0x83, 0xc0, 0x05, 0x48, 0x83, 0xc4, 0x18, 0xc3,
+];
+
+#[test]
+fn a_merge_whose_entry_copy_is_a_gap_reads_as_a_residual_under_every_write_order() {
+    // The loop's merge enters with a frame address (a gap); its only other copy is the latch's.
+    let mut texts = BTreeSet::new();
+    for code in [FRAME_ADDRESS_LOOP, FRAME_ADDRESS_LOOP_TWO_WAYS] {
+        let natural = c_under(WriteOrder::Natural, code);
+        assert!(natural.contains("ValuesNotRendered"), "{natural}");
+        let verdicts = verdicts_of(&natural);
+        for order in ORDERS {
+            let c = c_under(order, code);
+            assert_eq!(verdicts_of(&c), verdicts, "{order:?}\n{c}");
+            texts.insert(c);
+        }
+        // The loop's merge has no value on its way in, so no write order reads it as a variable.
+        assert!(verdicts.never_assigned.contains("RAX_2"), "{natural}");
+        for order in ORDERS {
+            let c = c_under(order, code);
+            let reads = reads_of(&c, "rax_2");
+            assert!(
+                reads
+                    .iter()
+                    .all(|line| line.contains("r2sleigh_residual_u64(")),
+                "{order:?}: {reads:?}\n{c}"
+            );
+        }
+    }
+    // The orders do move the text (the knob is live), or equal verdicts would prove nothing.
+    assert!(
+        texts.len() > 2,
+        "the orders wrote {} distinct texts",
+        texts.len()
+    );
+}
+
+/// `sub rsp, 0x18; test rdi, rdi; je other; lea rax, [rsp + 8]; jmp join; other: mov rax, rsi;
+/// join: add rsp, 0x18; ret`: one way into the join carries a frame address, which is a gap.
+const ONE_GAPPED_COPY: &[u8] = &[
+    0x48, 0x83, 0xec, 0x18, 0x48, 0x85, 0xff, 0x74, 0x07, 0x48, 0x8d, 0x44, 0x24, 0x08, 0xeb, 0x03,
+    0x48, 0x89, 0xf0, 0x48, 0x83, 0xc4, 0x18, 0xc3,
+];
+
+#[test]
+fn a_merge_with_one_gapped_forward_copy_reads_as_a_residual() {
+    // A gap traps where it stands, but the certificate lets control go on past it, so the value the
+    // merge holds on the other way in is no proof that a read of it is defined.
+    for order in ORDERS {
+        let c = c_under(order, ONE_GAPPED_COPY);
+        assert!(c.contains("ValuesNotRendered"), "{order:?}\n{c}");
+        assert!(
+            verdicts_of(&c).never_assigned.contains("RAX_3"),
+            "{order:?}\n{c}"
+        );
+        let reads = reads_of(&c, "rax_3");
+        assert!(
+            reads
+                .iter()
+                .all(|line| line.contains("r2sleigh_residual_u64(")),
+            "{order:?}: {reads:?}\n{c}"
+        );
+    }
+}
+
+/// `test rdi, rdi; je other; mov rax, rsi; jmp join; other: mov rax, rdx; join: ret`
+const BOTH_COPIES_RENDER: &[u8] = &[
+    0x48, 0x85, 0xff, 0x74, 0x05, 0x48, 0x89, 0xf0, 0xeb, 0x03, 0x48, 0x89, 0xd0, 0xc3, 0x90, 0x90,
+];
+
+#[test]
+fn a_merge_whose_forward_copies_all_render_reads_as_a_variable_in_every_order() {
+    let natural = verdicts_of(&c_under(WriteOrder::Natural, BOTH_COPIES_RENDER));
+    assert_eq!(natural.never_assigned, "");
+    for order in ORDERS {
+        let c = c_under(order, BOTH_COPIES_RENDER);
+        assert_eq!(verdicts_of(&c), natural, "{order:?}\n{c}");
+        assert!(!c.contains("r2sleigh_residual_"), "{order:?}\n{c}");
+        assert!(c.contains("return (uint64_t)rax_3;"), "{order:?}\n{c}");
+    }
+}