[PATCH 2/4] ntdll: Keep track of syscall frame unix return addresses.

[Jinoh Kang]

On 2/10/22 03:13, Rémi Bernon wrote:
> On 2/9/22 17:34, Jinoh Kang wrote:
>> On 2/8/22 04:05, Rémi Bernon wrote:
>>> For pthread_exit unwinding purposes, as libunwind needs to retrieve %rip
>>> and we don't want to let it unwind the PE frames, we need to keep track
>>> of a unix-only return address chain.
>>>
>>> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52213
>>> Signed-off-by: Rémi Bernon <rbernon at codeweavers.com>
>>> ---
>>>   dlls/ntdll/unix/signal_i386.c   | 7 +++++--
>>>   dlls/ntdll/unix/signal_x86_64.c | 5 ++++-
>>>   2 files changed, 9 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c
>>> index 0e3b1daf51a..d98a3b1d4bb 100644
>>> --- a/dlls/ntdll/unix/signal_i386.c
>>> +++ b/dlls/ntdll/unix/signal_i386.c
>>> @@ -463,9 +463,10 @@ struct syscall_frame
>>>        * 32-bit mode, but some processors fault if they're not in writable memory.
>>>        */
>>>       DECLSPEC_ALIGN(64) XSTATE xstate;     /* 240 */
>>> +    DWORD              unwind_eip;        /* 380 */
>>>   };
>>>   -C_ASSERT( sizeof(struct syscall_frame) == 0x380 );
>>> +C_ASSERT( sizeof(struct syscall_frame) == 0x3c0 );
>>>     struct x86_thread_data
>>>   {
>>> @@ -2432,7 +2433,7 @@ __ASM_GLOBAL_FUNC( signal_start_thread,
>>>                      "movl 0x1f8(%ecx),%eax\n\t"  /* x86_thread_data()->syscall_frame */
>>>                      "orl %eax,%eax\n\t"
>>>                      "jnz 1f\n\t"
>>> -                   "leal -0x380(%esp),%eax\n\t" /* sizeof(struct syscall_frame) */
>>> +                   "leal -0x3c0(%esp),%eax\n\t" /* sizeof(struct syscall_frame) */
>>>                      "andl $~63,%eax\n\t"
>>>                      "movl %eax,0x1f8(%ecx)\n"    /* x86_thread_data()->syscall_frame */
>>>                      "1:\tmovl %eax,%esp\n\t"
>>> @@ -2591,6 +2592,8 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher,
>>>                      "6:\tmovl $0xc000000d,%eax\n\t" /* STATUS_INVALID_PARAMETER */
>>>                      "jmp 5b\n"
>>>                      __ASM_NAME("__wine_syscall_dispatcher_return") ":\n\t"
>>> +                   "movl 0(%esp),%eax\n\t"
>>> +                   "movl %eax,0x380(%esp)\n\t"     /* frame->unwind_eip */
>>
>> We can't unwind to the caller without also saving the caller's nonvolatile registers.
>> Also, KeUserModeCallback is one of the callers of __wine_syscall_dispatcher_return, and returning to it would not be useful since it is usually called from PE modules anyway.
>> Maybe just use prev_frame directly, and set unwind_eip to an hardcoded address?
>> (Speaking of which, we could use .cfi_val_encoded_addr here, but it's a GNU-only extension AFAIK.)
>>
> 
> Hmm, I'm pretty sure that KeUserModeCallback is meant to be called from the unix side (although it's a Windows kernel API, it is meant to be called from kernel mode, which is our unix world), and would switch back to the PE stack, pushing a new syscall frame on the unix stack.

Yeah, that sounds about right.  Sorry for the noise.

My point of not using the return address and using the prev_frame/exit_frame chain still stands.  No functions expect __wine_syscall_dispatcher_return to return.

> 
> What I meant to do here, is to keep track of the unix caller (so either call_init_thunk or KeUserModeCallback), so that we will later link to it when a new syscall will be done, to the exit frame, as if we were returning to call_init_thunk and wherever the thread was started from, when the thread exits.
> 
> Regarding the registers, I think it's what __wine_setjmpex does, no?

Yes, but simply unwinding to the caller does not mean we're using the jmp_buf.

> Otherwise it would have trouble already.

Maybe it does not immediately result in crash because CFI instructions gradually recover nonvolatile registers along the frames and mask the clobbering.  We should not rely on this behavior though.

> Maybe call_init_thunk should do it too, although it's never supposed to returning to it normally (only through unwinding in pthread_exit).

-- 
Sincerely,
Jinoh Kang