fix(recompiler): rebuild CFG metadata and prove natural back edges

recompiler/CMakeLists.txt

@@ -346,6 +346,13 @@ target_include_directories(emitter_directive_line_test PRIVATE
 target_link_libraries(emitter_directive_line_test PRIVATE fmt rabbitizer)
 
 if(BUILD_TESTING)
+    add_executable(control_flow_metadata_test
+        tests/control_flow_metadata_test.cpp
+        src/control_flow.cpp)
+    target_include_directories(control_flow_metadata_test PRIVATE include)
+    target_link_libraries(control_flow_metadata_test PRIVATE fmt)
+    add_test(NAME control_flow_metadata_test COMMAND control_flow_metadata_test)
+
     add_test(NAME emitter_directive_line_test
              COMMAND emitter_directive_line_test)
     add_executable(debug_trace_ranges_test

recompiler/include/control_flow.h

@@ -41,12 +41,13 @@ struct BasicBlock {
     // Control flow information
     ControlFlowInstr exit_instr; // How this block exits (branch/jump/return)
     std::vector<uint32_t> successors; // Next basic blocks (targets + fall-through)
-    std::vector<uint32_t> predecessors; // Blocks that jump to this one
+    std::vector<uint32_t> predecessors; // Unique in-graph sources, including unreachable ones
 
     // Block properties
     bool is_entry;              // Function entry point
     bool is_exit;               // Contains return instruction
-    bool is_loop_header;        // Target of back edge (loop entry)
+    bool is_loop_header;        // Header dominates a reachable back-edge source
+    bool is_reachable = false;  // Reachable through known edges from a declared entry
 };
 
 // Control flow graph for a function
@@ -58,11 +59,19 @@ struct ControlFlowGraph {
     std::map<uint32_t, BasicBlock> blocks; // Map: block start address -> block
     std::vector<uint32_t> block_order;     // Blocks in address order
 
-    // Loop information
-    std::vector<std::pair<uint32_t, uint32_t>> loops; // (header, back_edge_source)
-    int loop_count;
+    // Natural back edges in the KNOWN static graph, not backward addresses.
+    // Missing indirect edges / runtime CPS entries mean this is NOT proof that
+    // a block is private or that IRQ, I-cache or device checks may be removed.
+    std::vector<std::pair<uint32_t, uint32_t>> loops; // (dominating header, source)
+    int loop_count = 0; // Number of back edges, not distinct/nested loop bodies
 };
 
+// Rebuild reverse edges, static reachability and dominance-proven back edges.
+// function_start, is_entry blocks and extra_entries are independent roots.
+// Does not change block ownership, instruction ranges or successor ordering.
+void rebuild_control_flow_metadata(
+    ControlFlowGraph& cfg, const std::vector<uint32_t>& extra_entries = {});
+
 class ControlFlowAnalyzer {
 public:
     explicit ControlFlowAnalyzer(const PS1Executable& exe);
@@ -108,9 +117,6 @@ private:
     // Link basic blocks (build CFG edges)
     void link_basic_blocks(std::map<uint32_t, BasicBlock>& blocks);
 
-    // Detect loops (back edges in CFG)
-    std::vector<std::pair<uint32_t, uint32_t>> detect_loops(
-        const std::map<uint32_t, BasicBlock>& blocks);
 };
 
 } // namespace PSXRecomp

recompiler/src/code_generator.cpp

@@ -2837,11 +2837,12 @@ GeneratedFunction CodeGenerator::generate_function(
         }
 
         if (needs_fallthrough) {
-            // Emit fallthrough if the last block is reachable (has predecessors
-            // or is the entry block). Dead code after a return (e.g., padding
-            // nops) has no predecessors and should NOT get a fallthrough call.
+            // Incoming edges alone do not prove reachability: unreachable
+            // padding/loops can have predecessors too. This final safety net
+            // uses declared-entry reachability; block-level CPS/indirect entry
+            // handling above remains independent of this static metadata.
             const BasicBlock& last = cfg.blocks.at(cfg.block_order.back());
-            bool is_reachable = last.is_entry || !last.predecessors.empty();
+            bool is_reachable = last.is_entry || last.is_reachable;
             if (is_reachable) {
                 body_ss << emit_stale_static_guard_named(fallthrough_name, "    ");
                 body_ss << fmt::format("    {}(cpu);  /* fallthrough to next function */\n",
@@ -2860,7 +2861,7 @@ GeneratedFunction CodeGenerator::generate_function(
             ((last_block.exit_instr.type == ControlFlowType::Branch ||
               last_block.exit_instr.type == ControlFlowType::Jump) &&
              last_block.successors.empty());
-        bool is_reachable = last_block.is_entry || !last_block.predecessors.empty();
+        bool is_reachable = last_block.is_entry || last_block.is_reachable;
         if (runs_off_end && is_reachable) {
             body_ss << fmt::format(
                 "    cpu->pc = 0x{:08X}u; return;  /* image-edge fallthrough: tail-transfer */\n",

recompiler/src/control_flow.cpp

@@ -187,8 +187,8 @@ uint32_t ControlFlowAnalyzer::analysis_walk_hi(const Function& func) const {
     return func.end_addr;
 }
 
-// NOTE: find_block_boundaries / build_basic_blocks / link_basic_blocks /
-// detect_loops below are NOT on the live path — analyze_function does its own
+// NOTE: find_block_boundaries / build_basic_blocks / link_basic_blocks
+// below are NOT on the live path — analyze_function does its own
 // single-pass walk (see the comment there). They are kept in sync with the
 // analysis bound anyway so wiring them up can never reintroduce the
 // guard-word-as-block-leader defect.
@@ -346,27 +346,129 @@ void ControlFlowAnalyzer::link_basic_blocks(std::map<uint32_t, BasicBlock>& bloc
     }
 }
 
-std::vector<std::pair<uint32_t, uint32_t>> ControlFlowAnalyzer::detect_loops(
-    const std::map<uint32_t, BasicBlock>& blocks) {
+void rebuild_control_flow_metadata(
+    ControlFlowGraph& cfg, const std::vector<uint32_t>& extra_entries) {
+    cfg.loops.clear();
+    cfg.loop_count = 0;
+    std::map<uint32_t, size_t> index;
+    std::vector<uint32_t> addresses;
+    for (auto& [addr, block] : cfg.blocks) {
+        index.emplace(addr, addresses.size());
+        addresses.push_back(addr);
+        block.predecessors.clear();
+        block.is_reachable = block.is_loop_header = false;
+    }
+    // A synthetic root preserves dominance with multiple alias entries.
+    const size_t root = addresses.size(), none = root + 1;
+    std::vector<std::vector<size_t>> succ(root + 1), pred(root + 1);
+    for (const auto& [addr, block] : cfg.blocks) {
+        const size_t source = index.at(addr);
+        for (uint32_t target : block.successors) {
+            auto it = index.find(target);
+            if (it == index.end()) continue; // External edges do not create blocks.
+            succ[source].push_back(it->second);
+            pred[it->second].push_back(source);
+        }
+    }
+    for (size_t i = 0; i < root; ++i) {
+        auto& incoming = pred[i];
+        std::sort(incoming.begin(), incoming.end());
+        incoming.erase(std::unique(incoming.begin(), incoming.end()), incoming.end());
+        for (size_t source : incoming)
+            cfg.blocks.at(addresses[i]).predecessors.push_back(addresses[source]);
+    }
+    std::set<size_t> entries;
+    auto add_entry = [&](uint32_t addr) {
+        auto it = index.find(addr);
+        if (it != index.end()) entries.insert(it->second);
+    };
+    add_entry(cfg.function_start);
+    for (const auto& [addr, block] : cfg.blocks) if (block.is_entry) add_entry(addr);
+    for (uint32_t addr : extra_entries) add_entry(addr);
+    for (size_t entry : entries) {
+        succ[root].push_back(entry);
+        pred[entry].push_back(root); // Synthetic edges are NOT public predecessors.
+    }
 
-    std::vector<std::pair<uint32_t, uint32_t>> loops;
+    // Iterative DFS avoids host stack exhaustion on large capture CFGs.
+    std::vector<bool> seen(root + 1, false);
+    std::vector<size_t> postorder;
+    std::vector<std::pair<size_t, size_t>> stack{{root, 0}};
+    seen[root] = true;
+    while (!stack.empty()) {
+        auto& [node, next] = stack.back();
+        if (next < succ[node].size()) {
+            const size_t child = succ[node][next++];
+            if (!seen[child]) {
+                seen[child] = true;
+                stack.emplace_back(child, 0);
+            }
+        } else {
+            postorder.push_back(node);
+            stack.pop_back();
+        }
+    }
+    std::reverse(postorder.begin(), postorder.end());
+    std::vector<size_t> rank(root + 1, none), idom(root + 1, none);
+    for (size_t i = 0; i < postorder.size(); ++i) rank[postorder[i]] = i;
+    idom[root] = root;
+    // Reverse-postorder immediate-dominator fixed point. Storage is O(V+E),
+    // unlike a set of all dominators for every block. Intersections walk up
+    // strictly decreasing RPO ranks, never address order.
+    auto intersect = [&](size_t a, size_t b) {
+        while (a != b) {
+            while (rank[a] > rank[b]) a = idom[a];
+            while (rank[b] > rank[a]) b = idom[b];
+        }
+        return a;
+    };
+    bool changed;
+    do {
+        changed = false;
+        for (size_t i = 1; i < postorder.size(); ++i) {
+            const size_t node = postorder[i];
+            size_t parent = none;
+            for (size_t p : pred[node]) {
+                if (idom[p] == none) continue;
+                parent = parent == none ? p : intersect(parent, p);
+            }
+            if (idom[node] != parent) { idom[node] = parent; changed = true; }
+        }
+    } while (changed);
 
-    for (const auto& [addr, block] : blocks) {
-        // Check each successor
-        for (uint32_t successor : block.successors) {
-            // Back edge: successor address < current block address
-            if (successor <= addr) {
-                loops.push_back({successor, addr}); // (header, back_edge_source)
-
-                // Mark loop header
-                if (blocks.count(successor)) {
-                    const_cast<BasicBlock&>(blocks.at(successor)).is_loop_header = true;
-                }
+    // Dominator-tree intervals make each back-edge dominance query O(1).
+    std::vector<std::vector<size_t>> children(root + 1);
+    for (size_t node = 0; node < root; ++node) {
+        cfg.blocks.at(addresses[node]).is_reachable = seen[node];
+        if (seen[node]) children[idom[node]].push_back(node);
+    }
+    std::vector<size_t> enter(root + 1), leave(root + 1);
+    size_t clock = 0;
+    stack.emplace_back(root, 0);
+    enter[root] = clock++;
+    while (!stack.empty()) {
+        auto& [node, next] = stack.back();
+        if (next < children[node].size()) {
+            const size_t child = children[node][next++];
+            enter[child] = clock++;
+            stack.emplace_back(child, 0);
+        } else {
+            leave[node] = clock++;
+            stack.pop_back();
+        }
+    }
+    for (size_t source = 0; source < root; ++source) {
+        if (!seen[source]) continue;
+        for (size_t target : succ[source]) {
+            if (enter[target] <= enter[source] && leave[source] <= leave[target]) {
+                cfg.loops.emplace_back(addresses[target], addresses[source]);
+                cfg.blocks.at(addresses[target]).is_loop_header = true;
             }
         }
     }
-
-    return loops;
+    std::sort(cfg.loops.begin(), cfg.loops.end());
+    cfg.loops.erase(std::unique(cfg.loops.begin(), cfg.loops.end()), cfg.loops.end());
+    cfg.loop_count = static_cast<int>(cfg.loops.size());
 }
 
 ControlFlowGraph ControlFlowAnalyzer::analyze_function(const Function& func) {
@@ -493,18 +595,9 @@ ControlFlowGraph ControlFlowAnalyzer::analyze_function(const Function& func) {
 
         cfg.blocks[block.start_addr] = block;
     }
-    // Detect back-edges (loops)
-    for (const auto& [addr, block] : cfg.blocks) {
-        for (uint32_t succ : block.successors) {
-            if (succ <= addr) {
-                cfg.loops.push_back({succ, addr});
-                if (cfg.blocks.count(succ)) {
-                    cfg.blocks[succ].is_loop_header = true;
-                }
-            }
-        }
-    }
-    cfg.loop_count = static_cast<int>(cfg.loops.size());
+    auto entries = func.alias_group_entries;
+    entries.push_back(func.start_addr);
+    rebuild_control_flow_metadata(cfg, entries);
 
     for (const auto& [addr, block] : cfg.blocks) {
         cfg.block_order.push_back(addr);

recompiler/tests/control_flow_metadata_test.cpp

@@ -0,0 +1,135 @@
+#include "control_flow.h"
+#include <algorithm>
+#include <iostream>
+#include <random>
+#include <set>
+#include <stdexcept>
+
+using namespace PSXRecomp;
+using Edges = std::vector<std::pair<uint32_t, uint32_t>>;
+static void check(bool ok, const char* why) {
+    if (!ok) throw std::runtime_error(why);
+}
+static ControlFlowGraph graph(uint32_t count, const Edges& edges) {
+    ControlFlowGraph cfg{};
+    cfg.function_start = 0;
+    for (uint32_t i = 0; i < count; ++i) {
+        BasicBlock b{};
+        b.start_addr = i; b.is_entry = i == 0;
+        cfg.blocks.emplace(i, b);
+    }
+    for (auto [a, b] : edges) cfg.blocks.at(a).successors.push_back(b);
+    return cfg;
+}
+static std::set<uint32_t> reach(const ControlFlowGraph& cfg,
+                               std::vector<uint32_t> roots, uint32_t omit = UINT32_MAX) {
+    std::set<uint32_t> seen;
+    while (!roots.empty()) {
+        auto node = roots.back(); roots.pop_back();
+        if (node == omit || !cfg.blocks.count(node) || !seen.insert(node).second) continue;
+        const auto& next = cfg.blocks.at(node).successors;
+        roots.insert(roots.end(), next.begin(), next.end());
+    }
+    return seen;
+}
+// Independent oracle: deleting a dominator must disconnect the source from
+// ALL entries. No RPO/immediate-dominator algorithm is shared with production.
+static void verify(ControlFlowGraph cfg, std::vector<uint32_t> extra = {}) {
+    auto original = cfg.blocks;
+    auto roots = extra;
+    roots.push_back(cfg.function_start);
+    for (auto& [a, b] : cfg.blocks) if (b.is_entry) roots.push_back(a);
+    auto reachable = reach(cfg, roots);
+    std::set<std::pair<uint32_t, uint32_t>> expected;
+    for (auto& [target, block] : cfg.blocks) {
+        auto without = reach(cfg, roots, target);
+        for (auto& [source, b] : cfg.blocks)
+            if (reachable.count(source) && !without.count(source) &&
+                std::find(b.successors.begin(), b.successors.end(), target) != b.successors.end())
+                expected.emplace(target, source);
+    }
+    rebuild_control_flow_metadata(cfg, extra);
+    check(std::set(cfg.loops.begin(), cfg.loops.end()) == expected, "back edges disagree with vertex-removal oracle");
+    check(cfg.loop_count == static_cast<int>(expected.size()), "duplicate loop edges");
+    for (auto& [a, b] : cfg.blocks) {
+        std::set<uint32_t> incoming;
+        for (auto& [p, pb] : original)
+            if (std::find(pb.successors.begin(), pb.successors.end(), a) != pb.successors.end()) incoming.insert(p);
+        check(b.predecessors == std::vector(incoming.begin(), incoming.end()), "reverse edges incomplete/duplicated");
+        check(b.successors == original.at(a).successors, "successor order was changed");
+        check(b.is_reachable == bool(reachable.count(a)), "reachability incorrect");
+        bool header = false;
+        for (auto edge : expected) header |= edge.first == a;
+        check(b.is_loop_header == header, "loop header flag incorrect");
+    }
+    auto loops = cfg.loops;
+    rebuild_control_flow_metadata(cfg, extra);
+    check(loops == cfg.loops, "metadata rebuild not idempotent");
+    // Change edges after the first pass: no predecessor/loop state may linger.
+    for (auto& [a, b] : cfg.blocks) b.successors.clear();
+    rebuild_control_flow_metadata(cfg, extra);
+    check(cfg.loops.empty() && cfg.loop_count == 0, "stale loop metadata");
+    for (auto& [a, b] : cfg.blocks) check(b.predecessors.empty(), "stale predecessor metadata");
+}
+static void live_analyzer() {
+    constexpr uint32_t base = 0x80010000;
+    PS1Executable exe{};
+    exe.header.load_address = exe.header.initial_pc = base;
+    exe.header.file_size = 0x28; exe.code_data.resize(0x28);
+    auto word = [&](uint32_t off, uint32_t value) {
+        for (unsigned i = 0; i < 4; ++i) exe.code_data[off+i] = value >> (i*8);
+    };
+    auto jump = [&](uint32_t off) { return 0x08000000u | ((base+off)>>2 & 0x03ffffffu); };
+    word(0x00, jump(0x20)); word(0x08, 0x03e00008);
+    word(0x10, 0x03e00008); word(0x18, 0x03e00008); word(0x20, jump(0x10));
+    Function f{}; f.start_addr = base; f.end_addr = base+0x28; f.size = 0x28;
+    auto cfg = ControlFlowAnalyzer(exe).analyze_function(f);
+    check(cfg.blocks.at(base+0x20).predecessors == std::vector<uint32_t>{base}, "live path omitted predecessor");
+    check(cfg.blocks.at(base+0x10).predecessors == std::vector<uint32_t>{base+0x20}, "live backward edge omitted");
+    check(cfg.loops.empty(), "acyclic backward jump was called a loop");
+    check(!cfg.blocks.at(base+0x18).is_reachable, "padding is not entry-reachable");
+    word(0x10, jump(0x20));
+    cfg = ControlFlowAnalyzer(exe).analyze_function(f);
+    check(cfg.loops == Edges{{base+0x20, base+0x10}}, "forward-address natural back edge missed");
+    f.alias_walk_lo = base; f.start_addr = base+0x10;
+    f.alias_group_entries = {base+0x10, base+0x18};
+    cfg = ControlFlowAnalyzer(exe).analyze_function(f);
+    check(cfg.loops.empty(), "independent alias entry was ignored by dominance");
+    check(cfg.blocks.at(base+0x18).is_reachable, "alias root not reachable");
+    word(0x00, 0x10000001); // beq zero,zero,+1: taken and fallthrough both +8
+    f.alias_walk_lo = 0; f.start_addr = base; f.alias_group_entries.clear();
+    cfg = ControlFlowAnalyzer(exe).analyze_function(f);
+    check(cfg.blocks.at(base+8).predecessors == std::vector<uint32_t>{base}, "duplicate branch edges duplicated predecessor");
+}
+int main() {
+    try {
+        live_analyzer();
+        verify(graph(0, {}));
+        verify(graph(1, {{0,0}, {0,0}}));
+        verify(graph(4, {{0,3}, {3,1}, {1,2}})); // backward, acyclic
+        verify(graph(5, {{0,1}, {1,2}, {2,3}, {3,2}, {2,4}, {4,1}})); // nested
+        verify(graph(4, {{0,1}, {1,2}, {1,3}, {2,1}, {3,1}})); // two latches
+        verify(graph(4, {{0,1}, {0,2}, {1,2}, {2,1}, {2,3}})); // irreducible
+        verify(graph(5, {{0,1}, {2,3}, {3,2}, {3,4}})); // unreachable cycle/tail
+        verify(graph(4, {{0,1}, {1,2}, {2,1}, {2,3}}), {2,2,999}); // aliases
+        verify(graph(3, {{0,1}, {0,1}, {1,999}, {1,2}})); // duplicate/external
+        std::mt19937 rng(0x434647u);
+        for (unsigned i = 0; i < 1000; ++i) {
+            unsigned n = 2 + rng()%11;
+            Edges edges;
+            for (unsigned a = 0; a < n; ++a)
+                for (unsigned b = 0; b < n; ++b) if (rng()%9 == 0) edges.emplace_back(a,b);
+            verify(graph(n, edges), i%2 ? std::vector<uint32_t>{unsigned(rng()%n)} : std::vector<uint32_t>{});
+        }
+        Edges chain;
+        for (unsigned i = 1; i < 12000; ++i) chain.emplace_back(i-1, i);
+        auto deep = graph(12000, chain);
+        rebuild_control_flow_metadata(deep);
+        check(deep.blocks.at(11999).is_reachable && deep.loops.empty(), "deep CFG traversal failed");
+        std::cout << "PASS: live CFG, aliases, loops, 1000 oracle graphs, 12000-block chain\n";
+        return 0;
+    } catch (const std::exception& e) {
+        std::cout << "FAIL: " << e.what() << '\n';
+        return 1;
+    }
+}