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

@@ -118,7 +118,8 @@ int main(int argc, const char** argv) {
     std::vector<std::vector<uint32_t>> 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<std::string> 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);
+
+                // 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<size_t> 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;

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;

src/mod_symbols.cpp

@@ -199,6 +199,8 @@ bool parse_v1(std::span<const char> data, const std::unordered_map<uint32_t, uin
             cur_func.rom = cur_section.rom_addr + funcs[func_index].section_offset;
             cur_func.words.resize(funcs[func_index].size / sizeof(uint32_t)); // Filled in later
             cur_func.section_index = section_index;
+
+            mod_context.functions_by_vram[cur_func.vram].emplace_back(start_func_index + func_index);
         }
 
         for (size_t reloc_index = 0; reloc_index < num_relocs; reloc_index++) {