[PATCH 1/2] ntdll/tests: Add a test for segment register contents in x86_64 exception handlers.
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/ntdll/tests/exception.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index da88ca449fb..e2a8071168d 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -2812,7 +2812,7 @@ static void test___C_specific_handler(void) /* This is heavily based on the i386 exception tests. */ static const struct exception { - BYTE code[18]; /* asm code */ + BYTE code[24]; /* asm code */ BYTE offset; /* offset of faulting instruction */ BYTE length; /* length of faulting instruction */ NTSTATUS status; /* expected status code */ @@ -2921,6 +2921,14 @@ static const struct exception /* 35 */ { { 0xa3, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xc3 }, /* movl %eax,0xffffffffffffffff; ret */ 0, 9, STATUS_ACCESS_VIOLATION, 2, { 1, 0xffffffffffffffff } }, + + /* test exception with cleared %ds and %es */ + { { 0x8c, 0xd8, 0x50, 0x8c, 0xc0, 0x50, 0x31, 0xc0, 0x8e, 0xd8, 0x8e, 0xc0, + 0xfa, 0x58, 0x8e, 0xc0, 0x58, 0x8e, 0xd8, 0xc3 }, + /* mov %ds,%eax; pushq %rax; mov %es,%eax; pushq %rax; xorl %eax,%eax; mov %eax,%ds; mov %eax,%es; + * cli; popq %rax; mov %eax,%es; popq %rax; mov %eax,%ds; ret */ + 12, 1, STATUS_PRIVILEGED_INSTRUCTION, 0 }, + { { 0xf1, 0x90, 0xc3 }, /* icebp; nop; ret */ 1, 1, STATUS_SINGLE_STEP, 0 }, { { 0xcd, 0x2c, 0xc3 }, @@ -2992,6 +3000,9 @@ static DWORD WINAPI handler( EXCEPTION_RECORD *rec, ULONG64 frame, "%u: Unexpected exception address %p/%p\n", entry, rec->ExceptionAddress, (char*)context->Rip ); + todo_wine ok( context->SegDs == context->SegSs, + "%u: ds %#x does not match ss %#x\n", entry, context->SegDs, context->SegSs ); + if (except->status == STATUS_BREAKPOINT && is_wow64) parameter_count = 1; else if (except->alt_status == 0 || rec->ExceptionCode != except->alt_status) -- 2.30.2
Based on a patch by Dávid Török. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47970 Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- Actually, the behaviour of segment registers on x86_64 Windows is more complicated: * By default, %ss == %ds == %es == %gs == 0x2b; %cs == 0x33; %fs == 0x53. * NtGetContextThread() does not capture the actual values, but instead returns the default values. * All system calls restore all segments to their default values, including NtContinue(). * RtlRestoreContext() also restores all segments to their default values. * SegDs, SegEs, SegFs, SegGs all restore default values in exception handlers; SegSs returns the actual value of %ss. * RtlCaptureContext() does capture the actual values. I did not test: * whether %cs is correctly captured or restored in any functions; * whether segment registers are restored from exception handlers (either to the default values or to the values set in the context). We could, to a point, emulate the above, but it gets tricky. According to the Intel 64 and IA-32 Architectures Software Developer's Manual: * %es, %ds, %ss can point to any segment—the entire contents of the descriptor table are effectively ignored (if I'm reading Volume 3 §3.4.4 right). However, they have to point to a *valid* segment (cf. Volume 1 §3.4.2.1). * %cs is more restricted—the attributes in the descriptor table are also validated (Volume 3 §5.2.1). * I don't even know about %fs and %gs; the manual is too vague and confusing. dlls/ntdll/tests/exception.c | 2 +- dlls/ntdll/unix/signal_x86_64.c | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index e2a8071168d..90af9de1bba 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -3000,7 +3000,7 @@ static DWORD WINAPI handler( EXCEPTION_RECORD *rec, ULONG64 frame, "%u: Unexpected exception address %p/%p\n", entry, rec->ExceptionAddress, (char*)context->Rip ); - todo_wine ok( context->SegDs == context->SegSs, + ok( context->SegDs == context->SegSs, "%u: ds %#x does not match ss %#x\n", entry, context->SegDs, context->SegSs ); if (except->status == STATUS_BREAKPOINT && is_wow64) diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 7dab5fbf4ed..814f2824129 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -1500,11 +1500,6 @@ static void save_context( struct xcontext *xcontext, const ucontext_t *sigcontex context->SegFs = FS_sig(sigcontext); context->SegGs = GS_sig(sigcontext); context->EFlags = EFL_sig(sigcontext); -#ifdef DS_sig - context->SegDs = DS_sig(sigcontext); -#else - __asm__("movw %%ds,%0" : "=m" (context->SegDs)); -#endif #ifdef ES_sig context->SegEs = ES_sig(sigcontext); #else @@ -1515,6 +1510,9 @@ static void save_context( struct xcontext *xcontext, const ucontext_t *sigcontex #else __asm__("movw %%ss,%0" : "=m" (context->SegSs)); #endif + /* Legends of Runeterra depends on having SegDs == SegSs in an exception + * handler. */ + context->SegDs = context->SegSs; context->Dr0 = amd64_thread_data()->dr0; context->Dr1 = amd64_thread_data()->dr1; context->Dr2 = amd64_thread_data()->dr2; -- 2.30.2
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=87681 Your paranoid android. === debiant2 (build log) === Task: WineTest did not produce the win32 report === debiant2 (build log) === Task: Could not create the wow32 wineprefix: Failed to disable the crash dialogs: Task: WineTest did not produce the 0 report
Zebediah Figura <z.figura12(a)gmail.com> writes:
@@ -1500,11 +1500,6 @@ static void save_context( struct xcontext *xcontext, const ucontext_t *sigcontex context->SegFs = FS_sig(sigcontext); context->SegGs = GS_sig(sigcontext); context->EFlags = EFL_sig(sigcontext); -#ifdef DS_sig - context->SegDs = DS_sig(sigcontext); -#else - __asm__("movw %%ds,%0" : "=m" (context->SegDs)); -#endif #ifdef ES_sig context->SegEs = ES_sig(sigcontext); #else @@ -1515,6 +1510,9 @@ static void save_context( struct xcontext *xcontext, const ucontext_t *sigcontex #else __asm__("movw %%ss,%0" : "=m" (context->SegSs)); #endif + /* Legends of Runeterra depends on having SegDs == SegSs in an exception + * handler. */ + context->SegDs = context->SegSs;
Is there a reason you are not fixing %es too while you are at it? -- Alexandre Julliard julliard(a)winehq.org
On 3/26/21 6:19 AM, Alexandre Julliard wrote:
Zebediah Figura <z.figura12(a)gmail.com> writes:
@@ -1500,11 +1500,6 @@ static void save_context( struct xcontext *xcontext, const ucontext_t *sigcontex context->SegFs = FS_sig(sigcontext); context->SegGs = GS_sig(sigcontext); context->EFlags = EFL_sig(sigcontext); -#ifdef DS_sig - context->SegDs = DS_sig(sigcontext); -#else - __asm__("movw %%ds,%0" : "=m" (context->SegDs)); -#endif #ifdef ES_sig context->SegEs = ES_sig(sigcontext); #else @@ -1515,6 +1510,9 @@ static void save_context( struct xcontext *xcontext, const ucontext_t *sigcontex #else __asm__("movw %%ss,%0" : "=m" (context->SegSs)); #endif + /* Legends of Runeterra depends on having SegDs == SegSs in an exception + * handler. */ + context->SegDs = context->SegSs;
Is there a reason you are not fixing %es too while you are at it?
The application doesn't care about %es, and I wanted to make the patch as uninvasive as possible. A proper fix could mean quite a lot of changes (some riskier than others), and it isn't clear to me where a line can reasonably be drawn.
"Zebediah Figura (she/her)" <zfigura(a)codeweavers.com> writes:
On 3/26/21 6:19 AM, Alexandre Julliard wrote:
Zebediah Figura <z.figura12(a)gmail.com> writes:
@@ -1500,11 +1500,6 @@ static void save_context( struct xcontext *xcontext, const ucontext_t *sigcontex context->SegFs = FS_sig(sigcontext); context->SegGs = GS_sig(sigcontext); context->EFlags = EFL_sig(sigcontext); -#ifdef DS_sig - context->SegDs = DS_sig(sigcontext); -#else - __asm__("movw %%ds,%0" : "=m" (context->SegDs)); -#endif #ifdef ES_sig context->SegEs = ES_sig(sigcontext); #else @@ -1515,6 +1510,9 @@ static void save_context( struct xcontext *xcontext, const ucontext_t *sigcontex #else __asm__("movw %%ss,%0" : "=m" (context->SegSs)); #endif + /* Legends of Runeterra depends on having SegDs == SegSs in an exception + * handler. */ + context->SegDs = context->SegSs; Is there a reason you are not fixing %es too while you are at it?
The application doesn't care about %es, and I wanted to make the patch as uninvasive as possible. A proper fix could mean quite a lot of changes (some riskier than others), and it isn't clear to me where a line can reasonably be drawn.
It would be interesting to have at least tests for the other registers. As far as the fix, I'm not sure we want to do it so early in the signal handling, we may need the actual segment values when running 32-bit code inside a 64-bit process. -- Alexandre Julliard julliard(a)winehq.org
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=87680 Your paranoid android. === debiant2 (build log) === Task: WineTest did not produce the win32 report === debiant2 (build log) === Task: Could not create the wow32 wineprefix: Failed to disable the crash dialogs: Task: WineTest did not produce the 0 report
participants (4)
-
Alexandre Julliard -
Marvin -
Zebediah Figura -
Zebediah Figura (she/her)