https://bugs.winehq.org/show_bug.cgi?id=53682
--- Comment #14 from Martin Storsjö martin@martin.st --- (In reply to Kevin Puetz from comment #13)
The attached patch, with the separate "DECLSPEC_NORETURN __attribute__((noinline)) call_user_mode_callback", there's a much greater chance of this still holding up (with a noreturn function, there's little point for the compiler to store anything else on the stack.
No it's stronger than that. It's noinline that's controlling the compiler, noreturn is just descriptive. In that patch, the setjmp(&jmpbuf) is in KeUserModeCallback. But the callback_frame is in call_user_exception_dispatcher, which, always gets its *own* stack frame (because of noinline). We still smash up the stack of call_user_exception_dispatcher, just like it used to for KeUserModeCallback. But since the longjmp points to KeUserModeCallback, the epilogue of call_user_exception_dispatcher will never get a chance to care.
Oh, right, that's true. Indeed, that variant should be fairly safe. It's mildly ugly that we're expecting to maybe clobber this stack frame a bit, but it shouldn't have any practical effect.
When setting up a new syscall_frame on the current stack, and calling __wine_syscall_dispatcher_return, we'd need to make sure that the new syscall_frame is at the very exact bottom of the stack.
Yes (given the current __wine_syscall_dispatcher_return). zf and I weren't sure whether that is just convenience, or if there was an actual reason why the stack and syscall_frame needed to be contiguous. If you think it's OK to just add an arbitrary gap, then it would seem more robust for asm code in __wine_syscall_dispatcher_return to simply preserve its own $sp value (which was properly set up by its caller), rather than overwriting it with $sp = syscall_frame. Then it's just going to naturally be the correct stack, rather than an arbitrary 0x100 offset.
(maybe this would mean we need a __wine_syscall_dispatcher_return_callback that differs from __wine_syscall_dispatcher_return, if the usage in handle_syscall_fault and call_init_thunk need to do this).
Oh, yes, that would work too. I tried making such a patch too, and that seems to work for me. I changed the unused align field in syscall_frame into syscall_stack. I made __wine_syscall_dispatcher_callback set this up, and likewise have signal_init_process set it up. But despite this, __wine_syscall_dispatcher does run into a case where this field is zero, early on in the startup. I fixed this by just using the syscall_frame itself as stack pointer, if it was null, but I wonder where I'm missing an initialization of it.
The patch for doing this is fairly small too, but the end of __wine_syscall_dispatcher_callback which restores the original user mode registers still uses sp pointing at syscall_frame. I can rewrite it not to do that (there's a slight risk of clobbering the stack below sp at that point I believe), but I'd leave that to a second separate patch for clarity.