Fix MOVEM boundary check

3 files changed, 46 insertions, 13 deletions

diff --git a/disasm.cpp b/disasm.cpp
index 7dac5ff..38ad8d1 100644
--- a/disasm.cpp
+++ b/disasm.cpp
@@ -385,14 +385,18 @@ static void disasm_jsr_jmp(
 static void disasm_movem(
         DisasmNode& node, uint16_t instr, const DataBuffer &code, const Settings &s)
 {
+    if (node.offset + kInstructionSizeStepBytes >= code.occupied_size) {
+        // Not enough space for regmask
+        return disasm_verbatim(node, instr, code, s);
+    }
+    const unsigned regmask = GetU16BE(code.buffer + node.offset + kInstructionSizeStepBytes);
+    if (regmask == 0) {
+        // This is just not representable: at least one register must be specified
+        return disasm_verbatim(node, instr, code, s);
+    }
     const auto dir = static_cast<MoveDirection>((instr >> 10) & 1);
     const auto opsize = static_cast<OpSize>(((instr >> 6) & 1) + 1);
     const char suffix = suffix_from_opsize(opsize);
-    // Although it would be much mode logical to fetch register mask first,
-    // since it goes right next after the instruction, but fetching addressing
-    // mode register first provides us the ultimate boundary check with early
-    // return, so we don't have to check for node.occupied_size when fetching
-    // regmask after this.
     const auto a = AddrModeArg::Fetch(
             node.offset + kInstructionSizeStepBytes * 2, code, instr, suffix);
     switch (a.mode) {
@@ -426,13 +430,6 @@ static void disasm_movem(
     case AddrMode::kImmediate: // 4ebc / 4efc
         return disasm_verbatim(node, instr, code, s);
     }
-    // Make sure that regmask is fetched after AddrModeArg has done boundary
-    // check.
-    const unsigned regmask = GetU16BE(code.buffer + node.offset + kInstructionSizeStepBytes);
-    if (regmask == 0) {
-        // This is just not representable: at least one register must be specified
-        return disasm_verbatim(node, instr, code, s);
-    }
     node.size = kInstructionSizeStepBytes * 2 + a.Size();
     snprintf(node.mnemonic, kMnemonicBufferSize, "movem%c", suffix);
     char regmask_str[48]{};
diff --git a/test.bash b/test.bash
index 1704c44..c6c26ca 100644
--- a/test.bash
+++ b/test.bash
@@ -49,6 +49,37 @@ run_test_simple() {
   fi
 }
 
+run_test_expect_short() {
+  local test_name=$1
+  local test_name_sanitized=${test_name//[^a-zA-Z0-9_\-]/-}
+  local data=$2
+  local file_orig_bin=${TEST_DIR}/${test_name_sanitized}.orig.bin
+  local file_asm=${TEST_DIR}/${test_name_sanitized}.S
+  local file_as_o=${TEST_DIR}/${test_name_sanitized}.as.o
+  local file_as_elf=${TEST_DIR}/${test_name_sanitized}.as.elf
+  local file_as_bin=${TEST_DIR}/${test_name_sanitized}.as.bin
+  echo -ne "Test expect .short \"${test_name}\"... "
+  echo -ne "${data}" >${file_orig_bin}
+  ${DISASM} -o ${file_asm} ${file_orig_bin}
+  ${AS} -m68000 -o ${file_as_o} ${file_asm}
+  ${LD} -o ${file_as_elf} ${file_as_o}
+  ${OBJCOPY} ${file_as_elf} -O binary ${file_as_bin}
+  if ! grep ".short" ${file_asm} >/dev/null 2>&1; then
+    echo -e "${CRED}FAIL${CRST}: NOT .short emitted, but .short EXPECTED"
+    cat ${file_asm}
+  elif ! cmp ${file_orig_bin} ${file_as_bin} >/dev/null 2>&1; then
+    echo -e "${CRED}FAIL${CRST}: output and input binaries do not match"
+    cat ${file_asm}
+    echo ${file_orig_bin}
+    hexdump -Cv ${file_orig_bin} | head -n1
+    echo ${file_as_bin}
+    hexdump -Cv ${file_as_bin} | head -n1
+  else
+    echo -e "${CGREEN}OK${CRST}"
+    #cat ${file_asm}
+  fi
+}
+
 run_test_iterative() {
   local test_name=$1
   local prefix=$2
@@ -142,6 +173,11 @@ run_test_simple "moveml (d8,An,Xi) to all registers" "\x4c\xf7\xff\xff\x48\x0a"
 run_test_simple "movemw (xxx).W to all registers" "\x4c\xb8\xff\xff\x80\x10"
 run_test_simple "moveml (xxx).L to all registers" "\x4c\xf9\xff\xff\x00\x00\x7f\xf0"
 
+# From random tests
+#
+run_test_expect_short "movem truncated" "\x48\x92"
+
+
 # 5x38 / 5x78 / 5xb8 (xxx).W
 #
 run_test_simple "addqb #8,offset:w" "\x50\x38\x00\x73"
diff --git a/test_random.bash b/test_random.bash
index 20de9cd..a6e6d9d 100644
--- a/test_random.bash
+++ b/test_random.bash
@@ -8,7 +8,7 @@ AS=m68k-none-elf-as
 OBJCOPY=m68k-none-elf-objcopy
 LD="m68k-none-elf-ld -Ttest.ld"
 DISASM="./cmake-build/m68k-disasm -fabs-marks -frel-marks -fmarks"
-TEST_DIR=/tmp/m68k-disasm-tests
+TEST_DIR=/tmp/m68k-disasm-random-tests
 
 set -e
 CRED="\033[31m"