DSPDisassembler: Fix out-of-bounds read when the last word is an instruction with a large immediate

For instance, ending with 0x009e (which you can do with CW 0x009e) indicates a LRI $ac0.m instruction, but there is no immediate value to load, so before whatever garbage in memory existed after the end of the file was used.

The bounds-checking also previously assumed that IRAM or IROM was being used, both of which were exactly 0x1000 long.

Source/Core/Core/DSP/DSPCodeUtil.cpp

@@ -78,9 +78,9 @@ bool Compare(const std::vector<u16>& code1, const std::vector<u16>& code2)
     {
       std::string line1, line2;
       u16 pc = i;
-      disassembler.DisassembleOpcode(&code1[0], &pc, line1);
+      disassembler.DisassembleOpcode(code1, &pc, line1);
       pc = i;
-      disassembler.DisassembleOpcode(&code2[0], &pc, line2);
+      disassembler.DisassembleOpcode(code2, &pc, line2);
       fmt::print("!! {:04x} : {:04x} vs {:04x} - {}  vs  {}\n", i, code1[i], code2[i], line1,
                  line2);
     }
@@ -93,7 +93,7 @@ bool Compare(const std::vector<u16>& code1, const std::vector<u16>& code2)
     {
       u16 pc = i;
       std::string line;
-      disassembler.DisassembleOpcode(&longest[0], &pc, line);
+      disassembler.DisassembleOpcode(longest, &pc, line);
       fmt::print("!! {}\n", line);
     }
   }

Source/Core/Core/DSP/DSPDisassembler.cpp

@@ -33,9 +33,10 @@ bool DSPDisassembler::Disassemble(const std::vector<u16>& code, std::string& tex
 
   for (u16 pc = 0; pc < code.size();)
   {
-    if (!DisassembleOpcode(code.data(), &pc, text))
+    bool failed = !DisassembleOpcode(code, &pc, text);
-      return false;
     text.append("\n");
+    if (failed)
+      return false;
   }
   return true;
 }
@@ -139,16 +140,23 @@ std::string DSPDisassembler::DisassembleParameters(const DSPOPCTemplate& opc, u1
   return buf;
 }
 
-bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, u16* pc, std::string& dest)
+bool DSPDisassembler::DisassembleOpcode(const std::vector<u16>& code, u16* pc, std::string& dest)
 {
-  if ((*pc & 0x7fff) >= 0x1000)
+  return DisassembleOpcode(code.data(), code.size(), pc, dest);
+}
+
+bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, size_t binbuf_size, u16* pc,
+                                        std::string& dest)
+{
+  const u16 wrapped_pc = (*pc & 0x7fff);
+  if (wrapped_pc >= binbuf_size)
   {
     ++pc;
     dest.append("; outside memory");
     return false;
   }
 
-  const u16 op1 = binbuf[*pc & 0x0fff];
+  const u16 op1 = binbuf[wrapped_pc];
 
   // Find main opcode
   const DSPOPCTemplate* opc = FindOpInfoByOpcode(op1);
@@ -179,14 +187,23 @@ bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, u16* pc, std::string&
   // printing
 
   if (settings_.show_pc)
-    dest += fmt::format("{:04x} ", *pc);
+    dest += fmt::format("{:04x} ", wrapped_pc);
 
   u16 op2;
 
   // Size 2 - the op has a large immediate.
   if (opc->size == 2)
   {
-    op2 = binbuf[(*pc + 1) & 0x0fff];
+    if (wrapped_pc + 1 >= binbuf_size)
+    {
+      if (settings_.show_hex)
+        dest += fmt::format("{:04x} ???? ", op1);
+      dest += fmt::format("; Insufficient data for large immediate");
+      *pc += opc->size;
+      return false;
+    }
+
+    op2 = binbuf[wrapped_pc + 1];
     if (settings_.show_hex)
       dest += fmt::format("{:04x} {:04x} ", op1, op2);
   }

Source/Core/Core/DSP/DSPDisassembler.h

@@ -34,8 +34,10 @@ public:
 
   bool Disassemble(const std::vector<u16>& code, std::string& text);
 
-  // Warning - this one is trickier to use right.
+  // Disassembles the given opcode at pc and increases pc by the opcode's size.
-  bool DisassembleOpcode(const u16* binbuf, u16* pc, std::string& dest);
+  // The PC is wrapped such that 0x0000 and 0x8000 both point to the start of the buffer.
+  bool DisassembleOpcode(const std::vector<u16>& code, u16* pc, std::string& dest);
+  bool DisassembleOpcode(const u16* binbuf, size_t binbuf_size, u16* pc, std::string& dest);
 
 private:
   std::string DisassembleParameters(const DSPOPCTemplate& opc, u16 op1, u16 op2);

Source/Core/Core/HW/DSPLLE/DSPSymbols.cpp

@@ -77,13 +77,15 @@ void AutoDisassembly(const SDSP& dsp, u16 start_addr, u16 end_addr)
 
   u16 addr = start_addr;
   const u16* ptr = (start_addr >> 15) != 0 ? dsp.irom : dsp.iram;
+  constexpr size_t size = DSP_IROM_SIZE;
+  static_assert(size == DSP_IRAM_SIZE);
   while (addr < end_addr)
   {
     line_to_addr[line_counter] = addr;
     addr_to_line[addr] = line_counter;
 
     std::string buf;
-    if (!disasm.DisassembleOpcode(ptr, &addr, buf))
+    if (!disasm.DisassembleOpcode(ptr, size, &addr, buf))
     {
       ERROR_LOG_FMT(DSPLLE, "disasm failed at {:04x}", addr);
       break;