macaw-x86: Fix call semantics when call target involves the stack pointer

x86/src/Data/Macaw/X86/Semantics.hs

@@ -1376,11 +1376,16 @@ def_set_list =
 
 def_call :: InstructionDef
 def_call = defUnary "call" $ \_ v -> do
+  -- Get the address to branch to. Make sure to do this *before* pushing the
+  -- value of the next instruction. If the call target is the stack pointer,
+  -- then pushing a value will change the underlying value of the stack, so we
+  -- want to ensure that we get the call target before the stack is modified.
+  -- (See the T420 test case in macaw-x86-symbolic's test suite.)
+  tgt <- getCallTarget v
   -- Push value of next instruction
   old_pc <- getReg R.X86_IP
   push addrRepr old_pc
   -- Set IP
-  tgt <- getCallTarget v
   rip .= tgt
 
 -- | Conditional jumps

x86_symbolic/tests/pass/T420.c

@@ -0,0 +1,42 @@
+/**
+ * A regression test for #420 which ensures that macaw-x86's semantics for the
+ * `call` instruction are correct when the call target involves the stack
+ * pointer. If we get it wrong, then the `call *0x8(%rsp)` instruction below
+ * will call a nonsensical address.
+ *
+ * It is surprisingly difficult to get `gcc` to generate code with a
+ * `call *0x8(%rsp)` instruction, so we resort to inline assembly to guarantee
+ * that this happens.
+ */
+
+int one(void) {
+  return 1;
+}
+
+int __attribute__((noinline)) test_callit(void) {
+  int (*fn)(void) = &one;
+  int ret = 0;
+  __asm__(
+    // 1. Decrement the stack pointer in preparation for storing `fn` on the
+    //    stack.
+    "sub $0x10,%%rsp;"
+    // 2. Store the `fn` function pointer (passed as an input) on the stack.
+    "mov  %1,0x8(%%rsp);"
+    // 3. Load `fn` from the stack and call it. This is the key part of the
+    //    test, as we want to ensure that macaw-x86's semantics for `call` will
+    //    retrieve `fn` *before* pushing the next instruction's address onto the
+    //    stack.
+    "call *0x8(%%rsp);"
+    // 4. Save the return value of `fn` as output.
+    "mov %%eax, %0;"
+    // 5. Increment the stack pointer back to its original position.
+    "add $0x10,%%rsp;"
+    : "=r"(ret) /* Outputs */
+    : "r"(fn) /* Inputs */
+    );
+  return ret;
+}
+
+void _start() {
+  test_callit();
+}