mirror of
https://github.com/hedge-dev/XenonRecomp.git
synced 2026-04-28 12:42:02 +00:00
XenonAnalyse: block walker uses switchMap to push successors at known bctrs
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.
This commit is contained in:
parent
e6c9124119
commit
861e04db76
1 changed files with 90 additions and 17 deletions
|
|
@ -41,11 +41,6 @@ size_t Function::SearchBlock(size_t address) const
|
||||||
Function Function::Analyze(const void* code, size_t size, size_t base,
|
Function Function::Analyze(const void* code, size_t size, size_t base,
|
||||||
const AnalyzerSwitchTableMap* switchMap)
|
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 };
|
Function fn{ base, 0 };
|
||||||
|
|
||||||
if (*((uint32_t*)code + 1) == 0x04000048) // shifted ptr tail call
|
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.reserve(32);
|
||||||
blockStack.emplace_back();
|
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
|
#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
|
// 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
|
// 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);
|
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)
|
if (conditional)
|
||||||
{
|
{
|
||||||
// right block's just going to return
|
// 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)
|
if (blocks.size() > 1)
|
||||||
{
|
{
|
||||||
std::sort(blocks.begin(), blocks.end(), [](const Block& a, const Block& b)
|
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;
|
return a.base < b.base;
|
||||||
});
|
});
|
||||||
|
|
||||||
size_t discontinuity = -1;
|
if (!switchAwareTermination)
|
||||||
for (size_t i = 0; i < blocks.size() - 1; i++)
|
|
||||||
{
|
{
|
||||||
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;
|
if (discontinuity != -1)
|
||||||
break;
|
{
|
||||||
}
|
blocks.erase(blocks.begin() + discontinuity, blocks.end());
|
||||||
|
}
|
||||||
if (discontinuity != -1)
|
|
||||||
{
|
|
||||||
blocks.erase(blocks.begin() + discontinuity, blocks.end());
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue