[PATCH] ntdll: Only call wine exception handlers on the current stack.
Rémi Bernon
rbernon at codeweavers.com
Tue Feb 1 07:09:43 CST 2022
On 2/1/22 13:57, Paul Gofman wrote:
> 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 at 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.
>
Well if it's better than checking the handler location go for it, I'm
not sure to see what that wrapper would need to be doing.
> I suppose MK11 will work this way (e. g., works with just removing __TRY
> in RtlUserThreadStart())?
>
Yes.
--
Rémi Bernon <rbernon at codeweavers.com>
More information about the wine-devel
mailing list