Skip to content

Commit e12839c

Browse files
committed
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.
1 parent 6eaca7a commit e12839c

1 file changed

Lines changed: 90 additions & 17 deletions

File tree

XenonAnalyse/function.cpp

Lines changed: 90 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,6 @@ size_t Function::SearchBlock(size_t address) const
4141
Function Function::Analyze(const void* code, size_t size, size_t base,
4242
const AnalyzerSwitchTableMap* switchMap)
4343
{
44-
(void)switchMap; // consumed in a follow-up commit; receiving now so
45-
// that callers can adopt the signature and pass a
46-
// map immediately without waiting for the walker
47-
// change to land.
48-
4944
Function fn{ base, 0 };
5045

5146
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,
6560
blockStack.reserve(32);
6661
blockStack.emplace_back();
6762

63+
// Set when the walker pushes switch-label successor blocks via the
64+
// switchMap. When set, the end-of-Analyze discontinuity erase is
65+
// skipped — see the comment on the discontinuity pass below for why.
66+
bool switchAwareTermination = false;
67+
6868
#define RESTORE_DATA() if (!blockStack.empty()) data = (dataStart + ((blocks[blockStack.back()].base + blocks[blockStack.back()].size) / sizeof(*data))) - 1; // continue adds one
6969

7070
// TODO: Branch fallthrough
@@ -188,6 +188,66 @@ Function Function::Analyze(const void* code, size_t size, size_t base,
188188
{
189189
// 5th bit of BO tells cpu to ignore the counter, which is a blr/bctr otherwise it's conditional
190190
const bool conditional = !(PPC_BO(instruction) & 0x10);
191+
192+
// Switch-aware branch: when this is an unconditional
193+
// bctr (xop 528) at a known-switch site, push every
194+
// label (plus the default) as a successor block
195+
// instead of terminating without successors.
196+
//
197+
// Without this branch, an unconditional bctr pops
198+
// the current block and adds no new blocks, so the
199+
// walker never reaches the switch-dispatched code.
200+
// The discontinuity pass at the end of this function
201+
// then erases every block past the first address
202+
// gap — which for a switch-dispatched function is
203+
// immediately after bctr+4, sweeping all label
204+
// blocks away. Downstream consumers (e.g., a
205+
// recompiler's "label in fn.base..fn.base+fn.size"
206+
// boundary check) then mis-flag the labels as out
207+
// of function range.
208+
//
209+
// Two guards mirror the existing pre-base-branch
210+
// handling for unconditional `b` (see the "Branches
211+
// before base are just tail calls" note above):
212+
//
213+
// label < base: skip. Labels that point
214+
// before the function base are either malformed
215+
// TOML entries or tail-call-style jumps; they
216+
// do not extend `fn.size`.
217+
//
218+
// label >= base + size: skip. Labels outside the
219+
// caller's analysis window cannot be walked
220+
// safely; emplacing them here would compute an
221+
// out-of-buffer `data` pointer at the next
222+
// RESTORE_DATA.
223+
if (!conditional && xop == 528 && switchMap)
224+
{
225+
auto it = switchMap->find(addr);
226+
if (it != switchMap->end())
227+
{
228+
switchAwareTermination = true;
229+
auto pushLabel = [&](uint32_t label)
230+
{
231+
if (label < base) return;
232+
if (label >= base + size) return;
233+
if (fn.SearchBlock(label) == -1)
234+
{
235+
const size_t lBase = label - base;
236+
blocks.emplace_back(lBase, 0);
237+
DEBUG(blocks.back().parent = blockBase);
238+
blockStack.emplace_back(blocks.size() - 1);
239+
}
240+
};
241+
for (uint32_t label : it->second.labels)
242+
{
243+
pushLabel(label);
244+
}
245+
pushLabel(it->second.defaultLabel);
246+
RESTORE_DATA();
247+
continue;
248+
}
249+
}
250+
191251
if (conditional)
192252
{
193253
// right block's just going to return
@@ -216,29 +276,42 @@ Function Function::Analyze(const void* code, size_t size, size_t base,
216276
}
217277
}
218278

219-
// Sort and invalidate discontinuous blocks
279+
// Sort and invalidate discontinuous blocks.
280+
//
281+
// When switchAwareTermination is set, the walker pushed successor
282+
// blocks for every label of at least one switch dispatch. Those
283+
// label blocks are separated from the pre-bctr block by the jump-
284+
// table bytes themselves (data region living in the code section),
285+
// which creates a legitimate address gap. The generic discontinuity
286+
// heuristic would erase every block past that gap, which is exactly
287+
// the set of blocks we just worked to make reachable. Skip the
288+
// erase in that case; sort still runs because fn.size below picks
289+
// max(block.base + block.size) and the sort is cheap.
220290
if (blocks.size() > 1)
221291
{
222292
std::sort(blocks.begin(), blocks.end(), [](const Block& a, const Block& b)
223293
{
224294
return a.base < b.base;
225295
});
226296

227-
size_t discontinuity = -1;
228-
for (size_t i = 0; i < blocks.size() - 1; i++)
297+
if (!switchAwareTermination)
229298
{
230-
if (blocks[i].base + blocks[i].size >= blocks[i + 1].base)
299+
size_t discontinuity = -1;
300+
for (size_t i = 0; i < blocks.size() - 1; i++)
231301
{
232-
continue;
233-
}
302+
if (blocks[i].base + blocks[i].size >= blocks[i + 1].base)
303+
{
304+
continue;
305+
}
234306

235-
discontinuity = i + 1;
236-
break;
237-
}
307+
discontinuity = i + 1;
308+
break;
309+
}
238310

239-
if (discontinuity != -1)
240-
{
241-
blocks.erase(blocks.begin() + discontinuity, blocks.end());
311+
if (discontinuity != -1)
312+
{
313+
blocks.erase(blocks.begin() + discontinuity, blocks.end());
314+
}
242315
}
243316
}
244317

0 commit comments

Comments
 (0)