[PATCH 14/18] ntdll: Don't use NtSetContextThread in NtContinue.

Zebediah Figura (she/her) zfigura at codeweavers.com
Fri Feb 5 10:25:20 CST 2021


On 2/5/21 9:18 AM, Jacek Caban wrote:
> On 05.02.2021 03:43, Zebediah Figura (she/her) wrote:
>> On 1/22/21 9:51 AM, Jacek Caban wrote:
>>>
>>> NtSetContextThread, unlike NtContinue, should not set nonvolatile
>>> registers.
>>>
>>> Signed-off-by: Jacek Caban <jacek at codeweavers.com>
>>> ---
>>>     dlls/ntdll/unix/server.c        |  2 +-
>>>     dlls/ntdll/unix/signal_arm.c    | 18 +++++++++++++-----
>>>     dlls/ntdll/unix/signal_arm64.c  | 20 ++++++++++++++------
>>>     dlls/ntdll/unix/signal_i386.c   | 14 +++++++++++++-
>>>     dlls/ntdll/unix/signal_x86_64.c | 15 ++++++++++++++-
>>>     dlls/ntdll/unix/unix_private.h  |  1 +
>>>     6 files changed, 56 insertions(+), 14 deletions(-)
>>>
>>>
>>
>> Just to clarify—our NtSetContextThread does set nonvolatile registers,
>> and continues to do so after this series. Is that incorrect behaviour?
> 
> 
> No, it does not continue to do so after this series. This is covered by
> the test at the end of the series. NtSetContextThread does set
> nonvolatile registers in syscall frame so that if you set them and then
> query them before syscall returns, you will get modified values.
> However, syscall dispatcher does not restore them from syscall frame. It
> means that if you call NtSetContectThread on current thread, it will not
> set nonvolatile registers to passed values.

Ah, I see. That is a bit confusing, because it does still set them in 
the stored syscall frame, but doesn't restore them from the syscall frame.

I guess that kind of answers my below question too—I had assumed that 
the syscall frame was actually restoring these nonvolatile registers, 
and managed to not actually correctly interpret the tests to realize 
that wasn't the case. In that case it does make sense to have two 
different syscall exit paths, I think...?

> 
> 
>> Also:
>>
>>> diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c
>>> index db09d7759da..fe581cca7b2 100644
>>> --- a/dlls/ntdll/unix/server.c
>>> +++ b/dlls/ntdll/unix/server.c
>>> @@ -726,7 +726,7 @@ NTSTATUS WINAPI NtContinue( CONTEXT *context,
>>> BOOLEAN alertable )
>>>           status = server_select( NULL, 0, SELECT_INTERRUPTIBLE |
>>> SELECT_ALERTABLE, 0, NULL, NULL, &apc );
>>>           if (status == STATUS_USER_APC) invoke_apc( context, &apc );
>>>       }
>>> -    return NtSetContextThread( GetCurrentThread(), context );
>>> +    signal_set_cpu_context( context );
>>>   }
>>>
>>>
>>
>> Would it be simpler just to move NtContinue() to signal_*.c instead?
> 
> 
> We'd then need to expose invoke_apc somehow, so I'm not sure if that
> would be better.
> 
> 
>> One further question regarding this patch series / patch 0017: I
>> assume you're also planning to also change NtContinue() to use the
>> syscall exit code instead of set_full_cpu_context()?
> 
> 
> I considered it. My very first prototype worked like that because it
> always restored all registers, except for rax/eax which becomes a bit
> tricky for that use case. I was thinking about providing an alternative
> syscall leave implementation (NtContinue could replace its own return
> address to opt-in to this), which would do full restore. I didn't try
> implementing that variant yet to see if it's worth it.
> 
> 
> For potential future plans, there are also call_user_apc_dispatcher and
> call_user_exception_dispatcher. They could potentially use the same
> mechanism as well instead of manually implementing crossing syscall
> barrier. They are more tricky to change, because we'd need to allow
> using stack is a more similar way to actual syscalls first. A simple
> solution would be to preserve enough unused stack on syscall entry, but
> a more complete solution like separated stacks could be even better.
> 
> 
> BTW, as far as this series is considered, I think we will want a few
> specialized implementations for different CPU generations, so I plan to
> look at moving syscall dispatcher code from winebuild to ntdll first.
> ntdll.so would then set proper dispatcher depending on CPU capabilities.
> That requires changes to the way we access syscall table. I didn't do
> the actual implementation yet.
> 
> 
> Jacek
> 



More information about the wine-devel mailing list