On 01/22/2016 01:32 AM, Vincent Povirk wrote:
In Wine with your patch, everything works except that the result is incorrect the first time add() is called. I suspect ReallyFixupVTable is changing some registers, and the thunk will have to preserve them.
You are right, I've made a dumb mistake forgetting that arguments regs are actually volatile regs and ReallyFixupVTable would not preserve it. I've updated the patch. I want to make a few notes on this: 1. XMM/YMM (0-5) registers save is required to work correctly if MS vectorcall convention is used for method. Though wine & mono itself should not touch SSE regs, app module init is finally called which can do whatever. 2. The modern convention is to align stack on 16 bytes boundary on ms_abi calls, ReallyFixupVTable originally relied on that. I could potentially do a stack alignment in the thunk but I suppose it is not desirable to complicate it further, and so added __attribute__((force_align_arg_pointer)) to declaration (it is not included in CDECL, and probably should not). I realize that some older versions of gcc may somehow ignore this attr (I've read some bug reports on that), but the problem may come only in case our thunk is called unaligned (which potentially could be for some old code, but is not common). If thunk is called correctly it passes the stack pointer aligned. 3. It looks like ReallyFixupVTableEntry has potential thread safety issue. I mean the case when multiple vtable entries are present within one fixup. I could possibly fix that but I think should be an extremely rare case to occur, as application should start calling methods in parallel right from start of module usage at the first place. So I am not sure if it worth doing it.