Do not halt emulator on branches in MIPS (#2921)

pwndbg/aglib/disasm/disassembly.py

@@ -414,10 +414,11 @@ def near(
             # If this instruction has a delay slot, disassemble the delay slot instruction
             # And append it to the list
             if insn.causes_branch_delay:
-                # The Unicorn emulator forgets branch decisions when stopped inside of a
-                # delay slot. We disable emulation in this case
-                if emu:
-                    emu.valid = False
+                # Delay slots are instructions after branches that always execute.
+                # Unicorn cannot be paused in a delay slot instruction.
+                # Single stepping on a branch will cause Unicorn to execute the delay slot instruction and take the branch action.
+                # This means the emulator's program counter will take on the value that the branch action dictates, and we would normally continue disassembling there.
+                # We disassemble the delay slot instructions here as the normal codeflow will not reach them.
 
                 split_insn = one(insn.address + insn.size, None, put_cache=True)
 

pwndbg/aglib/disasm/mips.py

@@ -1,15 +1,3 @@
-# When single stepping in Unicorn with MIPS, the address it arrives at in Unicorn
-# is often incorrect with branches.
-# This is due to "Delay slots" - the instruction AFTER a branch is always executed
-# before the jump, and the Unicorn emulator respects this behavior.
-# This causes single stepping branches to not arrive at the correct instruction -
-# it will simply go to the next location in memory, not respecting the branch. It doesn't appear to be extremely consistent.
-# Unicorn doesn't have a workaround for this single stepping issue:
-# https://github.com/unicorn-engine/unicorn/issues/332
-#
-# The way to fix the issue this causes (incorrect instruction.next) is by implementing the
-# condition function to manually specify when a jump is taken. Our manual decision will override the emulator.
-
 from __future__ import annotations
 
 from typing import TYPE_CHECKING

tests/qemu-tests/tests/user/test_mips.py

@@ -43,10 +43,10 @@ def test_mips32_delay_slot(qemu_assembly_run):
         " ► 0x10000000 <_start>     ✔ beq    $t1, $t0, _target           <_target>\n"
         "   0x10000004 <_start+4>     nop    \n"
         "    ↓\n"
-        "   0x10000008 <_target>      addu   $gp, $gp, $ra         GP => 0 + 0\n"
+        "   0x10000008 <_target>      addu   $gp, $gp, $ra         GP => 0 (0 + 0)\n"
         "   0x1000000c <_target+4>    nop    \n"
-        "   0x10000010 <end>          addiu  $v0, $zero, 0xfa1\n"
-        "   0x10000014 <end+4>        addiu  $a0, $zero, 0\n"
+        "   0x10000010 <end>          addiu  $v0, $zero, 0xfa1     V0 => 0xfa1 (0x0 + 0xfa1)\n"
+        "   0x10000014 <end+4>        addiu  $a0, $zero, 0         A0 => 0 (0 + 0)\n"
         "   0x10000018 <end+8>        syscall \n"
         "\n"
         "\n"
@@ -110,8 +110,8 @@ def test_mips32_bnez_instruction(qemu_assembly_run):
         "   0x10000004 <_start+4>  ✔ bnez   $t0, end                    <end>\n"
         "   0x10000008 <_start+8>    nop    \n"
         "    ↓\n"
-        "   0x10000014 <end>         addiu  $v0, $zero, 0xfa1     V0 => 0x0 + 0xfa1\n"
-        "   0x10000018 <end+4>       addiu  $a0, $zero, 0\n"
+        "   0x10000014 <end>         addiu  $v0, $zero, 0xfa1     V0 => 0xfa1 (0x0 + 0xfa1)\n"
+        "   0x10000018 <end+4>       addiu  $a0, $zero, 0         A0 => 0 (0 + 0)\n"
         "   0x1000001c <end+8>       syscall \n"
         "\n"
         "\n"
@@ -402,3 +402,57 @@ def test_mips32_binary_operations(qemu_assembly_run):
     )
 
     assert dis == expected
+
+
+MIPS_JUMPS = f"""
+nop
+beq $t1, $t0, first
+nop
+nop
+
+first:
+    li $t0, 10
+    bnez $t0, second
+    nop
+    nop
+
+second:
+    b end
+    nop
+    nop
+
+end:
+{MIPS_GRACEFUL_EXIT}
+"""
+
+
+def test_mips32_multiple_branches_followed(qemu_assembly_run):
+    """
+    Ensure that emulation is setup correctly so as to follow multiple branches - bugs in how we handle delay slots and disable the emulator might break this.
+    """
+    qemu_assembly_run(MIPS_JUMPS, "mips")
+
+    dis = gdb.execute("context disasm", to_string=True)
+    dis = pwndbg.color.strip(dis)
+
+    expected = (
+        "LEGEND: STACK | HEAP | CODE | DATA | WX | RODATA\n"
+        "───────────────────────[ DISASM / mips / set emulate on ]───────────────────────\n"
+        " ► 0x10000000 <_start>      nop    \n"
+        "   0x10000004 <_start+4>  ✔ beq    $t1, $t0, first             <first>\n"
+        "   0x10000008 <_start+8>    nop    \n"
+        "    ↓\n"
+        "   0x10000010 <first>       addiu  $t0, $zero, 0xa     T0 => 10 (0x0 + 0xa)\n"
+        "   0x10000014 <first+4>   ✔ bnez   $t0, second                 <second>\n"
+        "   0x10000018 <first+8>     nop    \n"
+        "    ↓\n"
+        "   0x10000020 <second>      b      end                         <end>\n"
+        "   0x10000024 <second+4>    nop    \n"
+        "    ↓\n"
+        "   0x1000002c <end>         addiu  $v0, $zero, 0xfa1     V0 => 0xfa1 (0x0 + 0xfa1)\n"
+        "   0x10000030 <end+4>       addiu  $a0, $zero, 0         A0 => 0 (0 + 0)\n"
+        "   0x10000034 <end+8>       syscall \n"
+        "────────────────────────────────────────────────────────────────────────────────\n"
+    )
+
+    assert dis == expected