[PATCH] Fix thiscall trampoline for x86

[Peter Rosin <peda at lysator dot liu dot se>]

---
 ChangeLog     |    5 ++++-
 src/x86/ffi.c |    2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Chanosin skrev 2012-03-21 17:55:
*snip*
> Anyway, thiscall fails like this (5 times, slightly different first value):
> 
> FAIL: libffi.call/closure_thiscall.c output pattern test, is 4199769 0 1 2: 4199775
> res: 4199775
> stack pointer match
> ? should match 0 1 2 3: 9
> ?es: 9
> stack pointer match
> 
> And, when looking at src/x86/win32.S, thiscall seems to be handled much
> like cdecl.  But I was under the impression that on Windows thiscall was
> like stdcall with the twist that ecx is 'this'.  It appears as if the
> libffi code assumes that thiscall is like cdecl with 'this' pushed last.
> Question is if MinGW (and Cygwin) implements thiscall à la Microsoft
> or à la Linux?

Studying some C++ disassembly reveals that Cygwin/MinGW has thiscall as
cdecl (args on stack, caller cleans up stack) with 'this' pushed last (after
the first arg) and that MSVC has thiscall as stdcall (args on stack, callee
cleans up stack) with 'this' in ecx.

Diving into the disassembly of closure_thiscall.c for MinGW reveals that
the closure_test_type0 type is called as MSVC implements thiscall, i.e.
as stdcall with 'this' in ecx.

(modulo how the return value is handled, *not* looking into the dark
corners of that maze yet...)

So, MinGW gcc does things differently for __thiscall compared to how
MinGW g++ calls through this pointers.  Beautiful.  But that's all
irrelevant because it can't explain why closure_thiscall.c fails on
MinGW as ffi expects the gcc thiscall (stdcall/ecx).

*using gdb for a bit*

Aha, I think I nailed it.  src/x86/ffi.c:FFI_INIT_TRAMPOLINE_THISCALL
makes a relative "call __fun" near the end, but it miscalculates the
relative distance (__dis) between the call site and the destination.

CC:ing Kai as this seems to be "his" code and I have no idea how it
will perform on other OSes.  It looks sane though, and it does fix
the last testsuite failure on MinGW (if you sneak in a -posix option
to gcc behind the back of the testsuite).

Cheers,
Peter

index 32577f1..449ec5c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,7 @@
-2012-03-21  Peter Rosin	 <peda@lysator.liu.se>
+2012-03-21  Peter Rosin  <peda@lysator.liu.se>
+
+	* src/x86/ffi.c (FFI_INIT_TRAMPOLINE_THISCALL): Use the correct
+	distance (__dis) between the call site and the destination.
 
 	* testsuite/libffi.call/float_va.c (float_va_fn): Use %f when
 	printing doubles (%lf is for long doubles).
diff --git a/src/x86/ffi.c b/src/x86/ffi.c
index 9343c26..ea5a83c 100644
--- a/src/x86/ffi.c
+++ b/src/x86/ffi.c
@@ -593,7 +593,7 @@ ffi_prep_incoming_args_SYSV(char *stack, void **rvalue, void **avalue,
 { unsigned char *__tramp = (unsigned char*)(TRAMP); \
    unsigned int  __fun = (unsigned int)(FUN); \
    unsigned int  __ctx = (unsigned int)(CTX); \
-   unsigned int  __dis = __fun - (__ctx + 22);  \
+   unsigned int  __dis = __fun - (__ctx + 49);  \
    unsigned short __size = (unsigned short)(SIZE); \
    *(unsigned int *) &__tramp[0] = 0x8324048b;	/* mov (%esp), %eax */ \
    *(unsigned int *) &__tramp[4] = 0x4c890cec;	/* sub $12, %esp */ \
-- 
1.7.9