From d5ab74220da3d8468d6873ed29afbcad03b890f5 Mon Sep 17 00:00:00 2001 From: Wiseguy <68165316+Mr-Wiseguy@users.noreply.github.com> Date: Thu, 12 Sep 2024 18:54:08 -0400 Subject: [PATCH] Various mod fixes (#95) * Terminate offline mod recompilation if any functions fail to recompile * Fixed edge case with switch case jump table detection when lo16 immediate is exactly 0 * Prevent emitting duplicate reference symbol defines in offline mod recompilation * Fix function calls and add missing runtime function pointers in offline mod recompiler --- OfflineModRecomp/main.cpp | 47 +++++++++++++++++++++++++++++++++++---- src/analysis.cpp | 6 +++-- src/mod_symbols.cpp | 2 ++ 3 files changed, 49 insertions(+), 6 deletions(-) diff --git a/OfflineModRecomp/main.cpp b/OfflineModRecomp/main.cpp index d128c8f..aa25dc8 100644 --- a/OfflineModRecomp/main.cpp +++ b/OfflineModRecomp/main.cpp @@ -118,7 +118,8 @@ int main(int argc, const char** argv) { std::vector> static_funcs_by_section{}; static_funcs_by_section.resize(mod_context.sections.size()); - std::ofstream output_file { argv[4] }; + const char* output_file_path = argv[4]; + std::ofstream output_file { output_file_path }; RabbitizerConfig_Cfg.pseudos.pseudoMove = false; RabbitizerConfig_Cfg.pseudos.pseudoBeqz = false; @@ -146,12 +147,19 @@ int main(int argc, const char** argv) { // Use reloc list to write reference symbol function pointer array and defines (i.e. `#define func_80102468 reference_symbol_funcs[0]`) output_file << "// Array of pointers to functions from the original ROM with defines to alias their names.\n"; + std::unordered_set written_reference_symbols{}; size_t num_reference_symbols = 0; for (const auto& section : mod_context.sections) { for (const auto& reloc : section.relocs) { if (reloc.type == N64Recomp::RelocType::R_MIPS_26 && reloc.reference_symbol && mod_context.is_regular_reference_section(reloc.target_section)) { const auto& sym = mod_context.get_reference_symbol(reloc.target_section, reloc.symbol_index); - output_file << "#define " << sym.name << " reference_symbol_funcs[" << num_reference_symbols << "]\n"; + + // Prevent writing multiple of the same define. This means there are duplicate symbols in the array if a function is called more than once, + // but only the first of each set of duplicates is referenced. This is acceptable, since offline mod recompilation is mainly meant for debug purposes. + if (!written_reference_symbols.contains(sym.name)) { + output_file << "#define " << sym.name << " reference_symbol_funcs[" << num_reference_symbols << "]\n"; + written_reference_symbols.emplace(sym.name); + } num_reference_symbols++; } } @@ -166,11 +174,27 @@ int main(int argc, const char** argv) { // Write the event trigger function pointer. output_file << "// Pointer to the runtime function for triggering events.\n"; output_file << "RECOMP_EXPORT void (*recomp_trigger_event)(uint8_t* rdram, recomp_context* ctx, uint32_t) = NULL;\n\n"; - // Write the get_function pointer. + // Write the get_function pointer. output_file << "// Pointer to the runtime function for looking up functions from vram address.\n"; output_file << "RECOMP_EXPORT recomp_func_t* (*get_function)(int32_t vram) = NULL;\n\n"; + // Write the cop0_status_write pointer. + output_file << "// Pointer to the runtime function for performing a cop0 status register write.\n"; + output_file << "RECOMP_EXPORT void (*cop0_status_write)(recomp_context* ctx, gpr value) = NULL;\n\n"; + + // Write the cop0_status_read pointer. + output_file << "// Pointer to the runtime function for performing a cop0 status register read.\n"; + output_file << "RECOMP_EXPORT gpr (*cop0_status_read)(recomp_context* ctx) = NULL;\n\n"; + + // Write the switch_error pointer. + output_file << "// Pointer to the runtime function for reporting switch case errors.\n"; + output_file << "RECOMP_EXPORT void (*switch_error)(const char* func, uint32_t vram, uint32_t jtbl) = NULL;\n\n"; + + // Write the do_break pointer. + output_file << "// Pointer to the runtime function for handling the break instruction.\n"; + output_file << "RECOMP_EXPORT void (*do_break)(uint32_t vram) = NULL;\n\n"; + // Write the section_addresses pointer. output_file << "// Pointer to the runtime's array of loaded section addresses for the base ROM.\n"; output_file << "RECOMP_EXPORT int32_t* reference_section_addresses = NULL;\n\n"; @@ -183,12 +207,27 @@ int main(int argc, const char** argv) { // Create a set of the export indices to avoid renaming them. std::unordered_set export_indices{mod_context.exported_funcs.begin(), mod_context.exported_funcs.end()}; + // Name all the functions in a first pass so function calls emitted in the second are correct. Also emit function prototypes. + output_file << "// Function prototypes.\n"; for (size_t func_index = 0; func_index < mod_context.functions.size(); func_index++) { auto& func = mod_context.functions[func_index]; + // Don't rename exports since they already have a name from the mod symbol file. if (!export_indices.contains(func_index)) { func.name = "mod_func_" + std::to_string(func_index); } - N64Recomp::recompile_function(mod_context, func, output_file, static_funcs_by_section, true); + output_file << "RECOMP_FUNC void " << func.name << "(uint8_t* rdram, recomp_context* ctx);\n"; + } + output_file << "\n"; + + // Perform a second pass for recompiling all the functions. + for (size_t func_index = 0; func_index < mod_context.functions.size(); func_index++) { + auto& func = mod_context.functions[func_index]; + if (!N64Recomp::recompile_function(mod_context, func, output_file, static_funcs_by_section, true)) { + output_file.close(); + std::error_code ec; + std::filesystem::remove(output_file_path, ec); + return EXIT_FAILURE; + } } return EXIT_SUCCESS; diff --git a/src/analysis.cpp b/src/analysis.cpp index 0310875..5dfd955 100644 --- a/src/analysis.cpp +++ b/src/analysis.cpp @@ -158,9 +158,11 @@ bool analyze_instruction(const rabbitizer::InstructionCpu& instr, const N64Recom } // If the base register has a valid lui state and a valid addend before this, then this may be a load from a jump table else if (reg_states[base].valid_lui && reg_states[base].valid_addend) { - // Exactly one of the lw and the base reg should have a valid lo16 value + // Exactly one of the lw and the base reg should have a valid lo16 value. However, the lo16 may end up just being zero by pure luck, + // so allow the case where the lo16 immediate is zero and the register state doesn't have a valid addiu immediate. + // This means the only invalid case is where they're both true. bool nonzero_immediate = imm != 0; - if (nonzero_immediate != reg_states[base].valid_addiu) { + if (!(nonzero_immediate && reg_states[base].valid_addiu)) { uint32_t lo16; if (nonzero_immediate) { lo16 = (int16_t)imm; diff --git a/src/mod_symbols.cpp b/src/mod_symbols.cpp index cd1faa3..24675fe 100644 --- a/src/mod_symbols.cpp +++ b/src/mod_symbols.cpp @@ -199,6 +199,8 @@ bool parse_v1(std::span data, const std::unordered_map