diff --git a/x86/src/Data/Macaw/X86/Semantics.hs b/x86/src/Data/Macaw/X86/Semantics.hs index 370567a3..6aafcc83 100644 --- a/x86/src/Data/Macaw/X86/Semantics.hs +++ b/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 diff --git a/x86_symbolic/tests/pass/T420.c b/x86_symbolic/tests/pass/T420.c new file mode 100644 index 00000000..d0c94919 --- /dev/null +++ b/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(); +} diff --git a/x86_symbolic/tests/pass/T420.opt.exe b/x86_symbolic/tests/pass/T420.opt.exe new file mode 100755 index 00000000..062abd86 Binary files /dev/null and b/x86_symbolic/tests/pass/T420.opt.exe differ diff --git a/x86_symbolic/tests/pass/T420.unopt.exe b/x86_symbolic/tests/pass/T420.unopt.exe new file mode 100755 index 00000000..57da9251 Binary files /dev/null and b/x86_symbolic/tests/pass/T420.unopt.exe differ