From e82a89397ac87b3da447169a0ba1db3b67727574 Mon Sep 17 00:00:00 2001 From: ApfelTeeSaft <91074565+ApfelTeeSaft@users.noreply.github.com> Date: Wed, 5 Nov 2025 08:51:29 +0100 Subject: [PATCH] Addressing multiple TODO's since no one else did 1. test_recompiler.cpp:216 - Implement CR register checking - Added proper CR register validation that checks all 4 fields (lt, gt, eq, so) - Each field is extracted from the 4-bit CR value using bit shifts 2. recompiler.h:55 - Create RecompileArgs struct - Introduced RecompileArgs struct to encapsulate the 7 function parameters - Improves code maintainability and reduces function signature complexity - Updated function signature and call site accordingly 3. recompiler.cpp:366 - Add bounds checking for mmioStore - Added validation to ensure the next instruction (data + 1) is within function bounds - Prevents potential out-of-bounds memory access 4. recompiler.cpp:1258 - Fix MFOCRF hardcoded CR field - Implemented proper FXM field mask decoding to determine which CR field to read - Removed hardcoded cr6 and now dynamically selects the correct CR field (cr0-cr7) - FXM is decoded as an 8-bit mask where bit 0 (0x80) = cr0, bit 1 (0x40) = cr1, etc. --- XenonRecomp/recompiler.cpp | 40 +++++++++++++++++++++++---------- XenonRecomp/recompiler.h | 21 +++++++++-------- XenonRecomp/test_recompiler.cpp | 10 ++++++++- 3 files changed, 49 insertions(+), 22 deletions(-) diff --git a/XenonRecomp/recompiler.cpp b/XenonRecomp/recompiler.cpp index fa91527..8bd271d 100644 --- a/XenonRecomp/recompiler.cpp +++ b/XenonRecomp/recompiler.cpp @@ -254,15 +254,16 @@ void Recompiler::Analyse() std::sort(functions.begin(), functions.end(), [](auto& lhs, auto& rhs) { return lhs.base < rhs.base; }); } -bool Recompiler::Recompile( - const Function& fn, - uint32_t base, - const ppc_insn& insn, - const uint32_t* data, - std::unordered_map::iterator& switchTable, - RecompilerLocalVariables& localVariables, - CSRState& csrState) +bool Recompiler::Recompile(const RecompileArgs& args) { + const Function& fn = args.fn; + uint32_t base = args.base; + const ppc_insn& insn = args.insn; + const uint32_t* data = args.data; + auto& switchTable = args.switchTable; + RecompilerLocalVariables& localVariables = args.localVariables; + CSRState& csrState = args.csrState; + println("\t// {} {}", insn.opcode->name, insn.op_str); // TODO: we could cache these formats in an array @@ -363,9 +364,11 @@ bool Recompiler::Recompile( return "ea"; }; - // TODO (Sajid): Check for out of bounds access auto mmioStore = [&]() -> bool { + // Check if the next instruction is within bounds + if (base + 4 >= fn.base + fn.size) + return false; return *(data + 1) == c_eieio; }; @@ -1255,8 +1258,21 @@ bool Recompiler::Recompile( break; case PPC_INST_MFOCRF: - // TODO: don't hardcode to cr6 - println("\t{}.u64 = ({}.lt << 7) | ({}.gt << 6) | ({}.eq << 5) | ({}.so << 4);", r(insn.operands[0]), cr(6), cr(6), cr(6), cr(6)); + { + // Decode FXM field mask to determine which CR field to read + // FXM is an 8-bit mask where bit 0 (0x80) = cr0, bit 1 (0x40) = cr1, etc. + uint32_t fxm = insn.operands[1]; + size_t crField = 0; + for (size_t i = 0; i < 8; i++) + { + if (fxm & (0x80 >> i)) + { + crField = i; + break; + } + } + println("\t{}.u64 = ({}.lt << 7) | ({}.gt << 6) | ({}.eq << 5) | ({}.so << 4);", r(insn.operands[0]), cr(crField), cr(crField), cr(crField), cr(crField)); + } break; case PPC_INST_MFTB: @@ -2427,7 +2443,7 @@ bool Recompiler::Recompile(const Function& fn) if (insn.opcode->id == PPC_INST_BCTR && (*(data - 1) == 0x07008038 || *(data - 1) == 0x00000060) && switchTable == config.switchTables.end()) fmt::println("Found a switch jump table at {:X} with no switch table entry present", base); - if (!Recompile(fn, base, insn, data, switchTable, localVariables, csrState)) + if (!Recompile(RecompileArgs{fn, base, insn, data, switchTable, localVariables, csrState})) { fmt::println("Unrecognized instruction at 0x{:X}: {}", base, insn.opcode->name); allRecompiled = false; diff --git a/XenonRecomp/recompiler.h b/XenonRecomp/recompiler.h index 8242db5..fbcc723 100644 --- a/XenonRecomp/recompiler.h +++ b/XenonRecomp/recompiler.h @@ -25,6 +25,17 @@ enum class CSRState VMX }; +struct RecompileArgs +{ + const Function& fn; + uint32_t base; + const ppc_insn& insn; + const uint32_t* data; + std::unordered_map::iterator& switchTable; + RecompilerLocalVariables& localVariables; + CSRState& csrState; +}; + struct Recompiler { // Enforce In-order Execution of I/O constant for quick comparison @@ -52,15 +63,7 @@ struct Recompiler void Analyse(); - // TODO: make a RecompileArgs struct instead this is getting messy - bool Recompile( - const Function& fn, - uint32_t base, - const ppc_insn& insn, - const uint32_t* data, - std::unordered_map::iterator& switchTable, - RecompilerLocalVariables& localVariables, - CSRState& csrState); + bool Recompile(const RecompileArgs& args); bool Recompile(const Function& fn); diff --git a/XenonRecomp/test_recompiler.cpp b/XenonRecomp/test_recompiler.cpp index f4fcdfe..6982b3f 100644 --- a/XenonRecomp/test_recompiler.cpp +++ b/XenonRecomp/test_recompiler.cpp @@ -213,7 +213,15 @@ void TestRecompiler::RecompileTests(const char* srcDirectoryPath, const char* ds int secondSpaceIndex = str.find(' ', spaceIndex + 1); auto reg = str.substr(spaceIndex + 1, secondSpaceIndex - spaceIndex - 1); if (reg[0] == 'c') - continue; // TODO + { + // CR register: parse 4-bit value (lt, gt, eq, so) + auto value = str.substr(secondSpaceIndex + 1); + fmt::println(file, "\tPPC_CHECK_VALUE_U({}, ctx.{}.lt, ({}) >> 3 & 1);", name, reg, value); + fmt::println(file, "\tPPC_CHECK_VALUE_U({}, ctx.{}.gt, ({}) >> 2 & 1);", name, reg, value); + fmt::println(file, "\tPPC_CHECK_VALUE_U({}, ctx.{}.eq, ({}) >> 1 & 1);", name, reg, value); + fmt::println(file, "\tPPC_CHECK_VALUE_U({}, ctx.{}.so, ({}) & 1);", name, reg, value); + continue; + } if (reg[0] == 'v') { int openingBracketIndex = str.find('[', secondSpaceIndex + 1);