On 2/1/22 15:49, Rémi Bernon wrote:
On 2/1/22 13:25, Paul Gofman wrote:
On 2/1/22 14:54, Bernhard Übelacker wrote:
Am 31.01.22 um 16:24 schrieb Rémi Bernon:
MK11 creates an alternate stack and sometimes throws an exception which gets incorrectly handled by a Wine exception handler, causing the game to crash.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com
dlls/ntdll/signal_x86_64.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c index 7e77329363c..36985832e4a 100644 --- a/dlls/ntdll/signal_x86_64.c +++ b/dlls/ntdll/signal_x86_64.c @@ -463,7 +463,9 @@ static NTSTATUS call_stack_handlers( EXCEPTION_RECORD *rec, CONTEXT *orig_contex } } /* hack: call wine handlers registered in the tib list */ - else while ((ULONG64)teb_frame < context.Rsp) + else while ((ULONG64)teb_frame < context.Rsp && + (ULONG64)teb_frame >= (ULONG64)NtCurrentTeb()->Tib.StackLimit && + (ULONG64)teb_frame <= (ULONG64)NtCurrentTeb()->Tib.StackBase) { TRACE_(seh)( "found wine frame %p rsp %p handler %p\n", teb_frame, (void *)context.Rsp, teb_frame->Handler );
I am not sure but this seems kind of similar to what I think I found in this bug: https://bugs.winehq.org/show_bug.cgi?id=52159
Yes, that looks quite similar. All these fix attempts workaround the problem where it is first visibly hit (calling wrong handler) but ignore what may happen to these handlers after. What if the app, e. g., switches stack back? And RtlUnwindEx()?
Actually the majority of Wine __TRY blocks should have rather low chances to hit app's stack switch in between the handler is installed and removed (unless the app is doing something really fun with hotpatching). Those IsBad..Ptr(), a bunch of cases when the parameter access is wrapped into __TRY, OutputDebugString() are not much likely to hit such an issue. There are relatively few __TRY blocks which embrace application functions: calling thread func, calling DLL entry points, exception handlers calls in signal_x86_64.c (and maybe I missed some). Since you probably hit the issue with Proton that is not the nested exception handlers as those in Proton already have manual asm wrappers to use straightforward x64 .seh handling and don't install Teb handlers. If maybe there are just one or two of other "global" handlers which are problematic maybe it worth solving the issue with some sort of manual .seh specifically for those ones?
In the case of MK11, the wine handler is the one installed by RtlUserThreadStart.
Yes, RtlUserThreadStart() is the one which will always be there. So if we replace that single one with an asm wrapper (and there is already a wrapper for i386) we will likely solve the big share of practical issues in a straightforward way. I can make such a patch (unless you want to do it?). Not sure though if that is good enough though, the prior understanding was that these sort of fixes are waiting for mingw .seh support.
I suppose MK11 will work this way (e. g., works with just removing __TRY in RtlUserThreadStart())?
I don't see anywhere any clue about how the thread is switching stacks, it just happens to use alternate stacks after a while, and has actually quite a few different stacks, probably re-implementing its own fibers (it doesn't call SwitchToFiber).
When they switch stacks they don't change the ExceptionList pointer though (probably as I understand it because it's not supposed to be used on Win64), so it's always ultimately pointing to the initial handler on the initial stack.
Yes, sure, they should not, ExceptionList on x64 is a Wine thing.