On 11.08.2016 19:27, Daniel Lehman wrote:
Some tests will fail but are unrelated to this patch: https://urldefense.proofpoint.com/v2/url?u=http-3A__test.winehq.org_data_882... https://urldefense.proofpoint.com/v2/url?u=http-3A__test.winehq.org_data_882... https://urldefense.proofpoint.com/v2/url?u=http-3A__test.winehq.org_data_882...
0001-ntdll-Set-Rip-in-for-longjmp-in-RtlRestoreContext.txt
From eaab70ee4ddc866526df0341853ccdb599cf7674 Mon Sep 17 00:00:00 2001 From: Daniel Lehman dlehman@esri.com Date: Mon, 1 Aug 2016 14:52:32 -0700 Subject: [PATCH] ntdll: Set Rip in for longjmp in RtlRestoreContext
Signed-off-by: Daniel Lehman dlehman@esri.com
dlls/ntdll/signal_x86_64.c | 1 + dlls/ntdll/tests/exception.c | 202 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 203 insertions(+)
diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c index 4c88536..598e08a 100644 --- a/dlls/ntdll/signal_x86_64.c +++ b/dlls/ntdll/signal_x86_64.c @@ -3513,6 +3513,7 @@ void WINAPI RtlRestoreContext( CONTEXT *context, EXCEPTION_RECORD *rec ) context->R13 = jmp->R13; context->R14 = jmp->R14; context->R15 = jmp->R15;
context->Rip = jmp->Rip; context->u.s.Xmm6 = jmp->Xmm6; context->u.s.Xmm7 = jmp->Xmm7; context->u.s.Xmm8 = jmp->Xmm8;
Please note that this also modifies the behavior of RtlUnwindEx (which gets the target_ip passed directly as a function argument). With your change it will be ignored, which might need additional (at least manual) tests to make sure this is correct.
diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c index a086436..4846769 100644 --- a/dlls/ntdll/tests/exception.c +++ b/dlls/ntdll/tests/exception.c @@ -53,10 +53,44 @@ static NTSTATUS (WINAPI *pNtSetInformationProcess)(HANDLE, PROCESSINFOCLASS, PV static BOOL (WINAPI *pIsWow64Process)(HANDLE, PBOOL);
#if defined(__x86_64__) +typedef struct _SETJMP_FLOAT128 +{
- unsigned __int64 DECLSPEC_ALIGN(16) Part[2];
+} SETJMP_FLOAT128;
+typedef struct _JUMP_BUFFER +{
- unsigned __int64 Frame;
- unsigned __int64 Rbx;
- unsigned __int64 Rsp;
- unsigned __int64 Rbp;
- unsigned __int64 Rsi;
- unsigned __int64 Rdi;
- unsigned __int64 R12;
- unsigned __int64 R13;
- unsigned __int64 R14;
- unsigned __int64 R15;
- unsigned __int64 Rip;
- unsigned __int64 Spare;
- SETJMP_FLOAT128 Xmm6;
- SETJMP_FLOAT128 Xmm7;
- SETJMP_FLOAT128 Xmm8;
- SETJMP_FLOAT128 Xmm9;
- SETJMP_FLOAT128 Xmm10;
- SETJMP_FLOAT128 Xmm11;
- SETJMP_FLOAT128 Xmm12;
- SETJMP_FLOAT128 Xmm13;
- SETJMP_FLOAT128 Xmm14;
- SETJMP_FLOAT128 Xmm15;
+} _JUMP_BUFFER;
static BOOLEAN (CDECL *pRtlAddFunctionTable)(RUNTIME_FUNCTION*, DWORD, DWORD64); static BOOLEAN (CDECL *pRtlDeleteFunctionTable)(RUNTIME_FUNCTION*); static BOOLEAN (CDECL *pRtlInstallFunctionTableCallback)(DWORD64, DWORD64, DWORD, PGET_RUNTIME_FUNCTION_CALLBACK, PVOID, PCWSTR); static PRUNTIME_FUNCTION (WINAPI *pRtlLookupFunctionEntry)(ULONG64, ULONG64*, UNWIND_HISTORY_TABLE*); +static VOID (WINAPI *pRtlCaptureContext)(CONTEXT*); +static VOID (CDECL *pRtlRestoreContext)(CONTEXT*, EXCEPTION_RECORD*); +static int (CDECL *p_setjmp)(_JUMP_BUFFER*); #endif
#ifdef __i386__ @@ -1631,6 +1665,164 @@ static void test_virtual_unwind(void) call_virtual_unwind( i, &tests[i] ); }
+static int consolidate_dummy_called; +static PVOID CALLBACK test_consolidate_dummy(EXCEPTION_RECORD *rec) +{
- CONTEXT *ctx = (CONTEXT *)rec->ExceptionInformation[1];
- consolidate_dummy_called = 1;
- ok(ctx->Rip == 0xdeadbeef, "test_consolidate_dummy failed for Rip, expected: 0xdeadbeef, got: %lx\n", ctx->Rip);
- return (PVOID)rec->ExceptionInformation[2];
+}
+static void test_restore_context(void) +{
- XMM_SAVE_AREA32 *fltsave;
- EXCEPTION_RECORD rec;
- volatile int done; /* gcc 4.5.2 rearranges 'done' and RtlCaptureContext */
- volatile int pass;
A better way to make sure the compiler does not rearrange the code would be to use Interlocked*() functions.
- _JUMP_BUFFER buf;
- CONTEXT ctx;
- if (!pRtlRestoreContext || !pRtlCaptureContext || !p_setjmp)
- {
skip("RtlCaptureContext/RtlRestoreContext/_setjmp not found\n");return;- }
- /* RtlRestoreContext(NULL, NULL); crashes on Windows */
- /* test simple case of capture and restore context */
- pass = 0;
- done = 0;
- pRtlCaptureContext(&ctx);
- switch (pass)
- {
case 0:ok(!done, "done on first pass\n");break;case 1:ok(done, "not done on second pass\n");break;default:ok(0, "bad pass = %i done = %i\n", pass, done);break;- }
- ++pass;
- if (done) goto test_longjmp;
- done = 1;
I don't think the second variable adds much value here. A simplified version like below would test the same things:
pass = 0; pRtlCaptureContext(&ctx); ok(pass < 2, "unexpected pass = %d\n", pass); if (!pass++) pRtlRestoreContext(&ctx, NULL);
- pRtlRestoreContext(&ctx, NULL);
+test_longjmp:
I do not see any need to use goto here, regular if () checks would also do the job.
- fltsave = (XMM_SAVE_AREA32 *)((char*)&ctx + 0x100);
It is better to avoid such hardcoded constants. Why not access the field directly?
- done = 0;
- pRtlCaptureContext(&ctx);
What is the purpose of the RtlCaptureContext call here? The result does not seem to be used. After RtlRestoreContext() the ctx struct has been filled with the jump buffer values.
- p_setjmp(&buf);
- if (done)
- {
ok(buf.Rbx == ctx.Rbx, "RtlRestoreContext:longjump failed for Rbx, expected: %lx, got: %lx\n", buf.Rbx, ctx.Rbx);ok(buf.Rsp == ctx.Rsp, "RtlRestoreContext:longjump failed for Rsp, expected: %lx, got: %lx\n", buf.Rsp, ctx.Rsp);ok(buf.Rbp == ctx.Rbp, "RtlRestoreContext:longjump failed for Rbp, expected: %lx, got: %lx\n", buf.Rbp, ctx.Rbp);ok(buf.Rsi == ctx.Rsi, "RtlRestoreContext:longjump failed for Rsi, expected: %lx, got: %lx\n", buf.Rsi, ctx.Rsi);ok(buf.Rdi == ctx.Rdi, "RtlRestoreContext:longjump failed for Rdi, expected: %lx, got: %lx\n", buf.Rdi, ctx.Rdi);ok(buf.R12 == ctx.R12, "RtlRestoreContext:longjump failed for R12, expected: %lx, got: %lx\n", buf.R12, ctx.R12);ok(buf.R13 == ctx.R13, "RtlRestoreContext:longjump failed for R13, expected: %lx, got: %lx\n", buf.R13, ctx.R13);ok(buf.R14 == ctx.R14, "RtlRestoreContext:longjump failed for R14, expected: %lx, got: %lx\n", buf.R14, ctx.R14);ok(buf.R15 == ctx.R15, "RtlRestoreContext:longjump failed for R15, expected: %lx, got: %lx\n", buf.R15, ctx.R15);ok(buf.Xmm6.Part[1] == fltsave->XmmRegisters[6].High && buf.Xmm6.Part[0] == fltsave->XmmRegisters[6].Low,"RtlRestoreContext:longjump failed for Xmm6, expected: %lx%08lx, got: %lx%08lx\n",buf.Xmm6.Part[1], buf.Xmm6.Part[0], fltsave->XmmRegisters[6].High, fltsave->XmmRegisters[6].Low);ok(buf.Xmm7.Part[1] == fltsave->XmmRegisters[7].High && buf.Xmm7.Part[0] == fltsave->XmmRegisters[7].Low,"RtlRestoreContext:longjump failed for Xmm7, expected: %lx%08lx, got: %lx%08lx\n",buf.Xmm7.Part[1], buf.Xmm7.Part[0], fltsave->XmmRegisters[7].High, fltsave->XmmRegisters[7].Low);ok(buf.Xmm8.Part[1] == fltsave->XmmRegisters[8].High && buf.Xmm8.Part[0] == fltsave->XmmRegisters[8].Low,"RtlRestoreContext:longjump failed for Xmm8, expected: %lx%08lx, got: %lx%08lx\n",buf.Xmm8.Part[1], buf.Xmm8.Part[0], fltsave->XmmRegisters[8].High, fltsave->XmmRegisters[8].Low);ok(buf.Xmm9.Part[1] == fltsave->XmmRegisters[9].High && buf.Xmm9.Part[0] == fltsave->XmmRegisters[9].Low,"RtlRestoreContext:longjump failed for Xmm9, expected: %lx%08lx, got: %lx%08lx\n",buf.Xmm9.Part[1], buf.Xmm9.Part[0], fltsave->XmmRegisters[9].High, fltsave->XmmRegisters[9].Low);ok(buf.Xmm10.Part[1] == fltsave->XmmRegisters[10].High && buf.Xmm10.Part[0] == fltsave->XmmRegisters[10].Low,"RtlRestoreContext:longjump failed for Xmm10, expected: %lx%08lx, got: %lx%08lx\n",buf.Xmm10.Part[1], buf.Xmm10.Part[0], fltsave->XmmRegisters[10].High, fltsave->XmmRegisters[10].Low);ok(buf.Xmm11.Part[1] == fltsave->XmmRegisters[11].High && buf.Xmm11.Part[0] == fltsave->XmmRegisters[11].Low,"RtlRestoreContext:longjump failed for Xmm11, expected: %lx%08lx, got: %lx%08lx\n",buf.Xmm11.Part[1], buf.Xmm11.Part[0], fltsave->XmmRegisters[11].High, fltsave->XmmRegisters[11].Low);ok(buf.Xmm12.Part[1] == fltsave->XmmRegisters[12].High && buf.Xmm12.Part[0] == fltsave->XmmRegisters[12].Low,"RtlRestoreContext:longjump failed for Xmm12, expected: %lx%08lx, got: %lx%08lx\n",buf.Xmm12.Part[1], buf.Xmm12.Part[0], fltsave->XmmRegisters[12].High, fltsave->XmmRegisters[12].Low);ok(buf.Xmm13.Part[1] == fltsave->XmmRegisters[13].High && buf.Xmm13.Part[0] == fltsave->XmmRegisters[13].Low,"RtlRestoreContext:longjump failed for Xmm13, expected: %lx%08lx, got: %lx%08lx\n",buf.Xmm13.Part[1], buf.Xmm13.Part[0], fltsave->XmmRegisters[13].High, fltsave->XmmRegisters[13].Low);ok(buf.Xmm14.Part[1] == fltsave->XmmRegisters[14].High && buf.Xmm14.Part[0] == fltsave->XmmRegisters[14].Low,"RtlRestoreContext:longjump failed for Xmm14, expected: %lx%08lx, got: %lx%08lx\n",buf.Xmm14.Part[1], buf.Xmm14.Part[0], fltsave->XmmRegisters[14].High, fltsave->XmmRegisters[14].Low);ok(buf.Xmm15.Part[1] == fltsave->XmmRegisters[15].High && buf.Xmm15.Part[0] == fltsave->XmmRegisters[15].Low,"RtlRestoreContext:longjump failed for Xmm15, expected: %lx%08lx, got: %lx%08lx\n",buf.Xmm15.Part[1], buf.Xmm15.Part[0], fltsave->XmmRegisters[15].High, fltsave->XmmRegisters[15].Low);goto test_consolidate;
Similar to above, it would be better to avoid goto. All the code below could just be moved to an else branch.
- }
- done = 1;
- rec.ExceptionCode = STATUS_LONGJUMP;
- rec.NumberParameters = 1;
- rec.ExceptionInformation[0] = (DWORD64)&buf;
- ctx.Rbx = 0xdeadbeef;
- ctx.Rsp = 0xdeadbeef;
- ctx.Rbp = 0xdeadbeef;
- ctx.Rsi = 0xdeadbeef;
- ctx.Rdi = 0xdeadbeef;
- ctx.R12 = 0xdeadbeef;
- ctx.R13 = 0xdeadbeef;
- ctx.R14 = 0xdeadbeef;
- ctx.R15 = 0xdeadbeef;
- fltsave->XmmRegisters[6].High = 0xbaadf00d;
- fltsave->XmmRegisters[6].Low = 0xdeadbeef;
- fltsave->XmmRegisters[7].High = 0xbaadf00d;
- fltsave->XmmRegisters[7].Low = 0xdeadbeef;
- fltsave->XmmRegisters[8].High = 0xbaadf00d;
- fltsave->XmmRegisters[8].Low = 0xdeadbeef;
- fltsave->XmmRegisters[9].High = 0xbaadf00d;
- fltsave->XmmRegisters[9].Low = 0xdeadbeef;
- fltsave->XmmRegisters[10].High = 0xbaadf00d;
- fltsave->XmmRegisters[10].Low = 0xdeadbeef;
- fltsave->XmmRegisters[11].High = 0xbaadf00d;
- fltsave->XmmRegisters[11].Low = 0xdeadbeef;
- fltsave->XmmRegisters[12].High = 0xbaadf00d;
- fltsave->XmmRegisters[12].Low = 0xdeadbeef;
- fltsave->XmmRegisters[13].High = 0xbaadf00d;
- fltsave->XmmRegisters[13].Low = 0xdeadbeef;
- fltsave->XmmRegisters[14].High = 0xbaadf00d;
- fltsave->XmmRegisters[14].Low = 0xdeadbeef;
- fltsave->XmmRegisters[15].High = 0xbaadf00d;
- fltsave->XmmRegisters[15].Low = 0xdeadbeef;
It would look better if you would use a loop or just a memset() for something like that.
- pRtlRestoreContext(&ctx, &rec);
+test_consolidate:
- done = 0;
- pRtlCaptureContext(&ctx);
- if (done)
- {
ok(consolidate_dummy_called, "test_consolidate_dummy not called\n");return;- }
- done = 1;
- rec.ExceptionCode = STATUS_UNWIND_CONSOLIDATE;
- rec.NumberParameters = 3;
- rec.ExceptionInformation[0] = (DWORD64)test_consolidate_dummy;
- rec.ExceptionInformation[1] = (DWORD64)&ctx;
- rec.ExceptionInformation[2] = ctx.Rip;
- ctx.Rip = 0xdeadbeef;
- pRtlRestoreContext(&ctx, &rec);
+}
static RUNTIME_FUNCTION* CALLBACK dynamic_unwind_callback( DWORD64 pc, PVOID context ) { static const int code_offset = 1024; @@ -2155,6 +2347,9 @@ static void test_vectored_continue_handler(void) START_TEST(exception) { HMODULE hntdll = GetModuleHandleA("ntdll.dll"); +#if defined(__x86_64__)
- HMODULE hmsvcrt = LoadLibraryA("msvcrt.dll");
+#endif
code_mem = VirtualAlloc(NULL, 65536, MEM_RESERVE | MEM_COMMIT, PAGE_EXECUTE_READWRITE); if(!code_mem) {@@ -2267,6 +2462,12 @@ START_TEST(exception) "RtlInstallFunctionTableCallback" ); pRtlLookupFunctionEntry = (void *)GetProcAddress( hntdll, "RtlLookupFunctionEntry" );
pRtlCaptureContext = (void *)GetProcAddress( hntdll,
"RtlCaptureContext" );pRtlRestoreContext = (void *)GetProcAddress( hntdll,
"RtlRestoreContext" );p_setjmp = (void *)GetProcAddress( hmsvcrt,
"_setjmp" );test_debug_registers(); test_outputdebugstring(1, FALSE);
@@ -2275,6 +2476,7 @@ START_TEST(exception) test_breakpoint(1); test_vectored_continue_handler(); test_virtual_unwind();
test_restore_context();
if (pRtlAddFunctionTable && pRtlDeleteFunctionTable && pRtlInstallFunctionTableCallback && pRtlLookupFunctionEntry) test_dynamic_unwind();
-- 1.9.5
-
Sebastian Lackner