From 861e04db767e3266d579f2aea35d554b8c69d5b2 Mon Sep 17 00:00:00 2001 From: Uproared Date: Fri, 24 Apr 2026 05:18:14 -0400 Subject: [PATCH] XenonAnalyse: block walker uses switchMap to push successors at known bctrs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Function::Analyze's block walker reaches an unconditional bctr (PPC_OP_CTR, xop 528, BO bit 5 set) and switchMap is non-null and contains an entry for the bctr's guest VA, the walker pushes every label (and the default) as a successor block via the existing SearchBlock-guarded emplace path. The walker then reaches those blocks, their reachability propagates, and `fn.size` correctly includes the switch-dispatched code. Two guards match the existing `branchDest < base` guard for unconditional `b` (tail-call pattern): label < base → skip. Labels before the function base are malformed or tail-call-style jumps; they should not extend fn.size. label >= base + size → skip. Labels outside the caller's window cannot be walked safely; emplacing them would compute an out-of-buffer data pointer during RESTORE_DATA. Discontinuity-pass suppression: when the walker pushes switch-label successors (tracked via local `switchAwareTermination`), the end-of- Analyze sort-and-erase pass skips the erase. Switch dispatch leaves a legitimate address gap between bctr+4 (end of pre-switch block) and the first case label — the jump-table bytes live in that gap, and the generic discontinuity heuristic would erase every block past it, including exactly the label blocks we just pushed. The sort itself still runs, which is fine: fn.size is computed as max(block.base + block.size) across all blocks afterward, so order does not change the result. Null-map safety: when switchMap is nullptr, the switch-aware branch is skipped entirely; the walker's unconditional-bctr path falls through to the legacy "pop without successors" behavior. Output is byte-identical to pre-patch behavior for all callers that pass nullptr (the default) at the Analyze call site. A companion test commit adds synthetic hand-crafted PPC bytecode fixtures covering the happy path + null-map safety + wrong-map miss. --- XenonAnalyse/function.cpp | 107 ++++++++++++++++++++++++++++++++------ 1 file changed, 90 insertions(+), 17 deletions(-) diff --git a/XenonAnalyse/function.cpp b/XenonAnalyse/function.cpp index bc9392a..5c299f9 100644 --- a/XenonAnalyse/function.cpp +++ b/XenonAnalyse/function.cpp @@ -41,11 +41,6 @@ size_t Function::SearchBlock(size_t address) const Function Function::Analyze(const void* code, size_t size, size_t base, const AnalyzerSwitchTableMap* switchMap) { - (void)switchMap; // consumed in a follow-up commit; receiving now so - // that callers can adopt the signature and pass a - // map immediately without waiting for the walker - // change to land. - Function fn{ base, 0 }; if (*((uint32_t*)code + 1) == 0x04000048) // shifted ptr tail call @@ -65,6 +60,11 @@ Function Function::Analyze(const void* code, size_t size, size_t base, blockStack.reserve(32); blockStack.emplace_back(); + // Set when the walker pushes switch-label successor blocks via the + // switchMap. When set, the end-of-Analyze discontinuity erase is + // skipped — see the comment on the discontinuity pass below for why. + bool switchAwareTermination = false; + #define RESTORE_DATA() if (!blockStack.empty()) data = (dataStart + ((blocks[blockStack.back()].base + blocks[blockStack.back()].size) / sizeof(*data))) - 1; // continue adds one // TODO: Branch fallthrough @@ -188,6 +188,66 @@ Function Function::Analyze(const void* code, size_t size, size_t base, { // 5th bit of BO tells cpu to ignore the counter, which is a blr/bctr otherwise it's conditional const bool conditional = !(PPC_BO(instruction) & 0x10); + + // Switch-aware branch: when this is an unconditional + // bctr (xop 528) at a known-switch site, push every + // label (plus the default) as a successor block + // instead of terminating without successors. + // + // Without this branch, an unconditional bctr pops + // the current block and adds no new blocks, so the + // walker never reaches the switch-dispatched code. + // The discontinuity pass at the end of this function + // then erases every block past the first address + // gap — which for a switch-dispatched function is + // immediately after bctr+4, sweeping all label + // blocks away. Downstream consumers (e.g., a + // recompiler's "label in fn.base..fn.base+fn.size" + // boundary check) then mis-flag the labels as out + // of function range. + // + // Two guards mirror the existing pre-base-branch + // handling for unconditional `b` (see the "Branches + // before base are just tail calls" note above): + // + // label < base: skip. Labels that point + // before the function base are either malformed + // TOML entries or tail-call-style jumps; they + // do not extend `fn.size`. + // + // label >= base + size: skip. Labels outside the + // caller's analysis window cannot be walked + // safely; emplacing them here would compute an + // out-of-buffer `data` pointer at the next + // RESTORE_DATA. + if (!conditional && xop == 528 && switchMap) + { + auto it = switchMap->find(addr); + if (it != switchMap->end()) + { + switchAwareTermination = true; + auto pushLabel = [&](uint32_t label) + { + if (label < base) return; + if (label >= base + size) return; + if (fn.SearchBlock(label) == -1) + { + const size_t lBase = label - base; + blocks.emplace_back(lBase, 0); + DEBUG(blocks.back().parent = blockBase); + blockStack.emplace_back(blocks.size() - 1); + } + }; + for (uint32_t label : it->second.labels) + { + pushLabel(label); + } + pushLabel(it->second.defaultLabel); + RESTORE_DATA(); + continue; + } + } + if (conditional) { // right block's just going to return @@ -216,7 +276,17 @@ Function Function::Analyze(const void* code, size_t size, size_t base, } } - // Sort and invalidate discontinuous blocks + // Sort and invalidate discontinuous blocks. + // + // When switchAwareTermination is set, the walker pushed successor + // blocks for every label of at least one switch dispatch. Those + // label blocks are separated from the pre-bctr block by the jump- + // table bytes themselves (data region living in the code section), + // which creates a legitimate address gap. The generic discontinuity + // heuristic would erase every block past that gap, which is exactly + // the set of blocks we just worked to make reachable. Skip the + // erase in that case; sort still runs because fn.size below picks + // max(block.base + block.size) and the sort is cheap. if (blocks.size() > 1) { std::sort(blocks.begin(), blocks.end(), [](const Block& a, const Block& b) @@ -224,21 +294,24 @@ Function Function::Analyze(const void* code, size_t size, size_t base, return a.base < b.base; }); - size_t discontinuity = -1; - for (size_t i = 0; i < blocks.size() - 1; i++) + if (!switchAwareTermination) { - if (blocks[i].base + blocks[i].size >= blocks[i + 1].base) + size_t discontinuity = -1; + for (size_t i = 0; i < blocks.size() - 1; i++) { - continue; + if (blocks[i].base + blocks[i].size >= blocks[i + 1].base) + { + continue; + } + + discontinuity = i + 1; + break; } - discontinuity = i + 1; - break; - } - - if (discontinuity != -1) - { - blocks.erase(blocks.begin() + discontinuity, blocks.end()); + if (discontinuity != -1) + { + blocks.erase(blocks.begin() + discontinuity, blocks.end()); + } } }