[PATCH] ntdll: Set Rip in for longjmp in RtlRestoreContext

Sebastian Lackner sebastian at fds-team.de
Mon Aug 15 11:37:07 CDT 2016


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_882980c17a9a033fa8e49a4c116af9583698d218_vista-5Fnewtb-2Dwvista_ntdll-3Aexception.html&d=CwIFAg&c=n6-cguzQvX_tUIrZOS_4Og&r=5dLcsNXR9pq24ykby7Q50BTTDDS13wpZmWQYxjgZHCU&m=SRFDFge6CZTog2m2CW8KGoOJtuNh7qepTaCM-kQpRkY&s=KxkYNPH69CO_qG7nBrElOWhFtZbU46ski-bMNN9B6j4&e= 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__test.winehq.org_data_882980c17a9a033fa8e49a4c116af9583698d218_vista-5Fnewtb-2Dwvistau64-2D32_ntdll-3Aexception.html&d=CwIFAg&c=n6-cguzQvX_tUIrZOS_4Og&r=5dLcsNXR9pq24ykby7Q50BTTDDS13wpZmWQYxjgZHCU&m=SRFDFge6CZTog2m2CW8KGoOJtuNh7qepTaCM-kQpRkY&s=dT15Yjer6cWckjuNF98FXPpxyP90qSizCd9wePGygco&e= 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__test.winehq.org_data_882980c17a9a033fa8e49a4c116af9583698d218_win8-5Fnewtb-2Dw8_ntdll-3Aexception.html&d=CwIFAg&c=n6-cguzQvX_tUIrZOS_4Og&r=5dLcsNXR9pq24ykby7Q50BTTDDS13wpZmWQYxjgZHCU&m=SRFDFge6CZTog2m2CW8KGoOJtuNh7qepTaCM-kQpRkY&s=tqoPjRIeFbCe5M5Am7HFaGN_0AFzXivu51O_x0t0R5A&e= 
> 
> 
> 0001-ntdll-Set-Rip-in-for-longjmp-in-RtlRestoreContext.txt
> 
> 
> From eaab70ee4ddc866526df0341853ccdb599cf7674 Mon Sep 17 00:00:00 2001
> From: Daniel Lehman <dlehman at 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 at 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
> 
> 
> 




More information about the wine-devel mailing list