CPU: Bound call stack function scan by the live call target

Fixes leaf functions failing to unwind when the preceding code is an
out-of-line block of the previous function containing a call.

src/core-tests/cpu_execution_tests.cpp

@@ -3,6 +3,7 @@
 
 #include "cpu_test_harness.h"
 
+#include "core/cpu_call_stack.h"
 #include "core/cpu_code_cache.h"
 #include "core/cpu_code_cache_private.h"
 #include "core/cpu_core.h"
@@ -303,6 +304,45 @@ TEST_P(CPUExecution, PGXPUnalignedStoreInvalidatesWrittenHalf)
   CPU::CodeCache::Reset();
 }
 
+TEST_P(CPUExecution, CallStackLeafAfterOutOfLineBlocks)
+{
+  // Compilers can place blocks of a function after its return, so the instructions preceding a leaf function are
+  // not necessarily the previous function's return. The leaf must not inherit the out-of-line block's call.
+  static constexpr u32 BASE = 0x80030000u;
+  static constexpr u32 LEAF = BASE + 0x30;
+  static constexpr u32 CALLER = BASE + 0x100;
+  static constexpr u32 SP = 0x801FF000u;
+  std::memset(&g_bus.unprotected_ram[BASE & g_bus.ram_mask], 0, 0x200);
+  std::memset(&g_bus.unprotected_ram[SP & g_bus.ram_mask], 0, 0x40);
+
+  u32 address = BASE;
+  for (const u32 bits :
+       {// Previous function, with an out-of-line block containing a call after the return.
+        addiu(Reg::SP, Reg::SP, -0x18), sw(Reg::RA, 0x14, Reg::SP), jal(CALLER), nop(), lw(Reg::RA, 0x14, Reg::SP),
+        nop(), jr(Reg::RA), addiu(Reg::SP, Reg::SP, 0x18), jal(CALLER), nop(), j(BASE + 0x10), nop(),
+        // Leaf function.
+        beq(Reg::A2, Reg::R0, 2), nop(), lbu(Reg::V1, 0, Reg::A1), jr(Reg::RA), nop()})
+  {
+    std::memcpy(&g_bus.unprotected_ram[address & g_bus.ram_mask], &bits, sizeof(bits));
+    address += sizeof(bits);
+  }
+
+  address = CALLER;
+  for (const u32 bits : {addiu(Reg::SP, Reg::SP, -0x20), sw(Reg::RA, 0x1C, Reg::SP), jal(LEAF), nop(),
+                         lw(Reg::RA, 0x1C, Reg::SP), nop(), jr(Reg::RA), addiu(Reg::SP, Reg::SP, 0x20)})
+  {
+    std::memcpy(&g_bus.unprotected_ram[address & g_bus.ram_mask], &bits, sizeof(bits));
+    address += sizeof(bits);
+  }
+
+  CPU::CallStack call_stack(LEAF + 8, SP, CALLER + 0x10);
+  call_stack.Walk();
+  const CPU::CallStack::FrameList& frames = call_stack.GetFrames();
+  ASSERT_EQ(frames.size(), 2u);
+  EXPECT_EQ(frames[1].address, CALLER + 8);
+  EXPECT_EQ(frames[1].stack_pointer, SP);
+}
+
 INSTANTIATE_TEST_SUITE_P(ExecutionModes, CPUExecution, ::testing::ValuesIn(EXECUTION_MODES), GetExecutionModeTestName);
 
 TEST_P(CPUMemoryExecution, FastmemAccessesFollowDRAMWindowsAndInstalledRAM)

src/core/cpu_call_stack.cpp

@@ -37,9 +37,11 @@ struct CallStackFrameLayout
 static bool IsStackPointerAdjustment(const Instruction instruction, s16* adjustment);
 static bool IsReturnAddressStackAccess(const Instruction instruction, InstructionOp op, s16* offset);
 static bool AddSignedOffset(VirtualMemoryAddress base, s32 offset, VirtualMemoryAddress* result);
-static bool ScanCallStackInstructions(VirtualMemoryAddress pc, std::vector<CallStackInstruction>* instructions,
+static bool ScanCallStackInstructions(VirtualMemoryAddress pc, VirtualMemoryAddress function_start,
+                                      std::vector<CallStackInstruction>* instructions,
                                       size_t* current_instruction_index, bool* found_start, bool* found_end);
-static bool AnalyzeCallStackFrame(VirtualMemoryAddress pc, CallStackFrameLayout* layout);
+static bool AnalyzeCallStackFrame(VirtualMemoryAddress pc, VirtualMemoryAddress function_start,
+                                  CallStackFrameLayout* layout);
 static bool UnwindCallStackFrame(VirtualMemoryAddress pc, VirtualMemoryAddress sp,
                                  std::optional<VirtualMemoryAddress> live_ra, VirtualMemoryAddress* caller_address,
                                  VirtualMemoryAddress* caller_sp);
@@ -76,8 +78,9 @@ bool CPU::AddSignedOffset(VirtualMemoryAddress base, s32 offset, VirtualMemoryAd
   return true;
 }
 
-bool CPU::ScanCallStackInstructions(VirtualMemoryAddress pc, std::vector<CallStackInstruction>* instructions,
-                                    size_t* current_instruction_index, bool* found_start, bool* found_end)
+bool CPU::ScanCallStackInstructions(VirtualMemoryAddress pc, VirtualMemoryAddress function_start,
+                                    std::vector<CallStackInstruction>* instructions, size_t* current_instruction_index,
+                                    bool* found_start, bool* found_end)
 {
   instructions->clear();
   *found_start = false;
@@ -93,6 +96,12 @@ bool CPU::ScanCallStackInstructions(VirtualMemoryAddress pc, std::vector<CallSta
       break;
 
     const VirtualMemoryAddress address = pc - distance;
+    if (address < function_start)
+    {
+      *found_start = true;
+      break;
+    }
+
     Instruction instruction;
     if (!SafeReadInstruction(address, &instruction.bits))
       break;
@@ -163,13 +172,15 @@ bool CPU::ScanCallStackInstructions(VirtualMemoryAddress pc, std::vector<CallSta
   return !instructions->empty();
 }
 
-bool CPU::AnalyzeCallStackFrame(VirtualMemoryAddress pc, CallStackFrameLayout* layout)
+bool CPU::AnalyzeCallStackFrame(VirtualMemoryAddress pc, VirtualMemoryAddress function_start,
+                                CallStackFrameLayout* layout)
 {
   std::vector<CallStackInstruction> instructions;
   size_t current_instruction_index;
   bool found_start;
   bool found_end;
-  if (!ScanCallStackInstructions(pc, &instructions, &current_instruction_index, &found_start, &found_end))
+  if (!ScanCallStackInstructions(pc, function_start, &instructions, &current_instruction_index, &found_start,
+                                 &found_end))
     return false;
 
   std::optional<s16> allocation_adjustment;
@@ -264,8 +275,21 @@ bool CPU::UnwindCallStackFrame(VirtualMemoryAddress pc, VirtualMemoryAddress sp,
                                std::optional<VirtualMemoryAddress> live_ra, VirtualMemoryAddress* caller_address,
                                VirtualMemoryAddress* caller_sp)
 {
+  // If the live return address came from a direct call, its target is the start of the current function. Without
+  // this, the backward scan can run into blocks the compiler placed after the previous function's return.
+  VirtualMemoryAddress function_start = 0;
+  Instruction live_call_instruction;
+  if (live_ra.has_value() && live_ra.value() >= (2 * INSTRUCTION_SIZE) &&
+      SafeReadInstruction(live_ra.value() - (2 * INSTRUCTION_SIZE), &live_call_instruction.bits) &&
+      live_call_instruction.op == InstructionOp::jal)
+  {
+    function_start = GetDirectBranchTarget(live_call_instruction, live_ra.value() - (2 * INSTRUCTION_SIZE));
+    if (function_start > pc)
+      function_start = 0;
+  }
+
   CallStackFrameLayout layout;
-  if (!AnalyzeCallStackFrame(pc, &layout))
+  if (!AnalyzeCallStackFrame(pc, function_start, &layout))
     return false;
 
   VirtualMemoryAddress return_address;