From 6c55d698bb3776a75e83981e63fb708c613c4866 Mon Sep 17 00:00:00 2001 From: Matthew Stanley <1379tech@gmail.com> Date: Sat, 12 Sep 2026 23:06:06 -0700 Subject: [PATCH] fix(recompiler): rebuild CFG metadata and prove natural back edges --- recompiler/CMakeLists.txt | 7 + recompiler/include/control_flow.h | 22 ++- recompiler/src/code_generator.cpp | 11 +- recompiler/src/control_flow.cpp | 153 ++++++++++++++---- .../tests/control_flow_metadata_test.cpp | 135 ++++++++++++++++ 5 files changed, 285 insertions(+), 43 deletions(-) create mode 100644 recompiler/tests/control_flow_metadata_test.cpp diff --git a/recompiler/CMakeLists.txt b/recompiler/CMakeLists.txt index e2fc759b..fa35779f 100644 --- a/recompiler/CMakeLists.txt +++ b/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 diff --git a/recompiler/include/control_flow.h b/recompiler/include/control_flow.h index b29d8845..4f91f6f2 100644 --- a/recompiler/include/control_flow.h +++ b/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 successors; // Next basic blocks (targets + fall-through) - std::vector predecessors; // Blocks that jump to this one + std::vector 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 blocks; // Map: block start address -> block std::vector block_order; // Blocks in address order - // Loop information - std::vector> 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> 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& 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& blocks); - // Detect loops (back edges in CFG) - std::vector> detect_loops( - const std::map& blocks); }; } // namespace PSXRecomp diff --git a/recompiler/src/code_generator.cpp b/recompiler/src/code_generator.cpp index e2c0adae..ef96f8d8 100644 --- a/recompiler/src/code_generator.cpp +++ b/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", diff --git a/recompiler/src/control_flow.cpp b/recompiler/src/control_flow.cpp index 833db11e..664f334a 100644 --- a/recompiler/src/control_flow.cpp +++ b/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& bloc } } -std::vector> ControlFlowAnalyzer::detect_loops( - const std::map& blocks) { +void rebuild_control_flow_metadata( + ControlFlowGraph& cfg, const std::vector& extra_entries) { + cfg.loops.clear(); + cfg.loop_count = 0; + std::map index; + std::vector 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> 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 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> loops; + // Iterative DFS avoids host stack exhaustion on large capture CFGs. + std::vector seen(root + 1, false); + std::vector postorder; + std::vector> 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 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(blocks.at(successor)).is_loop_header = true; - } + // Dominator-tree intervals make each back-edge dominance query O(1). + std::vector> 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 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(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(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); diff --git a/recompiler/tests/control_flow_metadata_test.cpp b/recompiler/tests/control_flow_metadata_test.cpp new file mode 100644 index 00000000..13a00828 --- /dev/null +++ b/recompiler/tests/control_flow_metadata_test.cpp @@ -0,0 +1,135 @@ +#include "control_flow.h" +#include +#include +#include +#include +#include + +using namespace PSXRecomp; +using Edges = std::vector>; +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 reach(const ControlFlowGraph& cfg, + std::vector roots, uint32_t omit = UINT32_MAX) { + std::set 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 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> 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(expected.size()), "duplicate loop edges"); + for (auto& [a, b] : cfg.blocks) { + std::set 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{base}, "live path omitted predecessor"); + check(cfg.blocks.at(base+0x10).predecessors == std::vector{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{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{unsigned(rng()%n)} : std::vector{}); + } + 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; + } +}