mirror/psxrecomp
1
0
Fork 0
mirror of https://github.com/RetroPortingToolKit/psxrecomp synced 2026-10-25 18:05:12 -04:00
Code Issues Packages Projects Releases Wiki Activity

recompiler: fix chained-branch delay-slot predicate-init UB

Browse Source 
When a `beq+delay-slot` terminator's delay-slot address is also a basic-block
label (target of an external `b BFCxxxx` from a sibling branch), the previously
emitted C placed the variable declaration BEFORE the next block label and the
dangling if-goto AFTER it:

  label_BFC2185C:
      capture brA, brB
      int psx_taken_BFC2185C = (brA == brB);     // declaration
  label_BFC21860:                                 // goto target
      cpu->gpr[1] = ...;                          // delay-slot c_code
      if (psx_taken_BFC2185C) goto label_BFC21800; // ← dangling

When control reaches `label_BFC21860` via a goto from elsewhere (e.g., the
COPY branch ends with `b 0xbfc21860`), C semantics make `psx_taken_BFC2185C`
indeterminate because its declaration with initializer was skipped. On MinGW,
stack garbage is non-zero → the if-goto fires unconditionally → spurious
cross-branch jump → infinite loop in DELETE branch (BFC21800).

Concretely this caused the memcard COPY confirm-modal freeze: pressing CROSS
on a save-block target should push widget 0xA modal, but the COPY branch
would push 0xA correctly, then on `b 0xbfc21860` cleanup the dangling
if-goto would fire spuriously and jump into the DELETE branch, which keeps
re-pushing widget 0xB and reverting state.

Fix: in the first pass, collect every branch terminator address and emit
`int psx_taken_BFCxxxx = 0;` at function entry. Change the terminator-emit
from declaration to assignment. Now external gotos that bypass the
assignment see the zero default and the dangling if-goto correctly does
not fire.

Regenerated `generated/SCPH1001_full.c`: 6083 zero-init declarations,
all `int psx_taken_*` at terminator sites converted to assignments.

User-verified: COPY confirm-modal now appears. (YES/NO button/close
behavior still broken — likely additional bugs in the same class or
adjacent codegen edge cases; tracked separately.)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Matthew Stanley
2026-05-04 22:28:26 -07:00
parent bf60f821b1
commit e54e63131c
2 changed files with 12183 additions and 6084 deletions
Download Patch File Download Diff File Expand all files Collapse all files
generated/SCPH1001_full.c
+12166 -6083
View File
File diff suppressed because it is too large Load Diff
recompiler/src/full_function_emitter.cpp
+17 -1
View File
@@ -286,6 +286,19 @@ bool FullFunctionEmitter::emit_function(
block_cycles[leader] = count;
}
/* Emit branch-predicate variable declarations at function entry,
* each initialized to 0. Subsequent terminator-emit assigns instead of
* declares, so a `goto label_X` that lands on the delay slot of a
* preceding beq does NOT execute the dangling `if (psx_taken_X) goto Y`
* with an indeterminate value (would be UB per C; on MinGW the stack
* garbage was non-zero, causing a spurious branch — root cause of the
* memcard COPY confirm-modal freeze in widget 0xB → BFC21800 loop). */
for (const auto& [ds_addr, pb] : pending_at) {
if (is_branch_kind(pb.kind.c_str())) {
out += fmt::format(" int psx_taken_{:08X} = 0;\n", pb.terminator_addr);
}
}
for (auto it = addr_to_raw.begin(); it != addr_to_raw.end(); ++it) {
uint32_t addr = it->first;
uint32_t raw = it->second;
@@ -331,7 +344,10 @@ bool FullFunctionEmitter::emit_function(
out += fmt::format(" {}\n", tr.pre_delay_code);
}
std::string cond = branch_condition(tr.terminator_kind, addr);
out += fmt::format(" int psx_taken_{:08X} = ({});\n", addr, cond);
/* Assignment, not declaration — variable is declared at
* function entry initialized to 0 (see fix for chained
* branch + delay-slot label-placement bug). */
out += fmt::format(" psx_taken_{:08X} = ({});\n", addr, cond);
} else if (kind == "rfe") {
// RFE: emit cop0 stack pop immediately (no delay slot).
//