[PATCH] ntdll: Fixup instruction pointer for EXCEPTION_BREAKPOINT in the unix part on x86/x86_64.

[Paul Gofman]

Signed-off-by: Paul Gofman 
---
 dlls/ntdll/signal_i386.c        |  3 --
 dlls/ntdll/signal_x86_64.c      |  3 --
 dlls/ntdll/tests/exception.c    | 53 ++++++++++++++++++++++++++-------
 dlls/ntdll/unix/signal_i386.c   | 14 ++++++++-
 dlls/ntdll/unix/signal_x86_64.c | 13 ++++++++
 5 files changed, 68 insertions(+), 18 deletions(-)

diff --git a/dlls/ntdll/signal_i386.c b/dlls/ntdll/signal_i386.c
index 5e0c0bd0907..a130638cb31 100644
--- a/dlls/ntdll/signal_i386.c
+++ b/dlls/ntdll/signal_i386.c
@@ -230,9 +230,6 @@ NTSTATUS WINAPI dispatch_exception( EXCEPTION_RECORD *rec, CONTEXT *context )
               context->SegEs, context->SegFs, context->SegGs, context->EFlags );
     }

-    /* fix up instruction pointer in context for EXCEPTION_BREAKPOINT */
-    if (rec->ExceptionCode == EXCEPTION_BREAKPOINT) context->Eip--;
-
     if (call_vectored_handlers( rec, context ) == EXCEPTION_CONTINUE_EXECUTION)
         NtContinue( context, FALSE );

diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c
index c7c8b964056..57f9aee8105 100644
--- a/dlls/ntdll/signal_x86_64.c
+++ b/dlls/ntdll/signal_x86_64.c
@@ -564,9 +564,6 @@ NTSTATUS WINAPI dispatch_exception( EXCEPTION_RECORD *rec, CONTEXT *context )
               context->R12, context->R13, context->R14, context->R15 );
     }

-    /* fix up instruction pointer in context for EXCEPTION_BREAKPOINT */
-    if (rec->ExceptionCode == EXCEPTION_BREAKPOINT) context->Rip--;
-
     if (call_vectored_handlers( rec, context ) == EXCEPTION_CONTINUE_EXECUTION)
         NtContinue( context, FALSE );

diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c
index bf8ccf03053..5de885aadf2 100644
--- a/dlls/ntdll/tests/exception.c
+++ b/dlls/ntdll/tests/exception.c
@@ -1673,6 +1673,9 @@ static DWORD dbg_except_continue_handler(EXCEPTION_RECORD *rec, EXCEPTION_REGIST
         CONTEXT *context, EXCEPTION_REGISTRATION_RECORD **dispatcher)
 {
     ok(hook_called, "Hook was not called.\n");
+
+    ok(rec->ExceptionCode == 0x80000003, "Got unexpected ExceptionCode %#x.\n", rec->ExceptionCode);
+
     got_exception = 1;
     dbg_except_continue_handler_eip = (void *)context->Eip;
     ++context->Eip;
@@ -1684,11 +1687,22 @@ static LONG WINAPI dbg_except_continue_vectored_handler(struct _EXCEPTION_POINTE
     EXCEPTION_RECORD *rec = e->ExceptionRecord;
     CONTEXT *context = e->ContextRecord;

-    trace("dbg_except_continue_vectored_handler, code %#x, eip %#x.\n", rec->ExceptionCode, context->Eip);
+    trace("dbg_except_continue_vectored_handler, code %#x, eip %#x, ExceptionAddress %p.\n",
+            rec->ExceptionCode, context->Eip, rec->ExceptionAddress);
+
+    ok(rec->ExceptionCode == 0x80000003, "Got unexpected ExceptionCode %#x.\n", rec->ExceptionCode);

got_exception = 1;
-    ++context->Eip;

+    if ((ULONG_PTR)rec->ExceptionAddress == context->Eip + 1)
+    {
+        /* XP and Vista+ have ExceptionAddress == Eip + 1, Eip is adjusted even
+         * for software raised breakpoint exception.
+         * Win2003 has Eip not adjusted and matching ExceptionAddress.
+         * Win2008 has Eip not adjuated and ExceptionAddress not filled for
+         * software raised exception. */
+        context->Eip = (ULONG_PTR)rec->ExceptionAddress;
+    }
     return EXCEPTION_CONTINUE_EXECUTION;
 }

@@ -1777,7 +1791,7 @@ static void test_kiuserexceptiondispatcher(void)
     ok(hook_exception_address == code_mem || broken(!hook_exception_address) /* Win2008 */,
             "Got unexpected exception address %p, expected %p.\n",
             hook_exception_address, code_mem);
-    todo_wine ok(hook_KiUserExceptionDispatcher_eip == code_mem, "Got unexpected exception address %p, expected %p.\n",
+    ok(hook_KiUserExceptionDispatcher_eip == code_mem, "Got unexpected exception address %p, expected %p.\n",
             hook_KiUserExceptionDispatcher_eip, code_mem);
     ok(dbg_except_continue_handler_eip == code_mem, "Got unexpected exception address %p, expected %p.\n",
             dbg_except_continue_handler_eip, code_mem);
@@ -1798,7 +1812,7 @@ static void test_kiuserexceptiondispatcher(void)
     pRtlRaiseException(&record);

ok(got_exception, "Handler was not called.\n");
-    ok(hook_called, "Hook was not called.\n");
+    ok(hook_called || broken(!hook_called) /* 2003 */, "Hook was not called.\n");

RemoveVectoredExceptionHandler(dbg_except_continue_vectored_handler);
     ret = VirtualProtect(pKiUserExceptionDispatcher, sizeof(saved_KiUserExceptionDispatcher_bytes),
@@ -2834,13 +2848,13 @@ static LONG WINAPI dbg_except_continue_vectored_handler(struct _EXCEPTION_POINTE

trace("dbg_except_continue_vectored_handler, code %#x, Rip %#lx.\n", rec->ExceptionCode, context->Rip);

+    ok(rec->ExceptionCode == 0x80000003, "Got unexpected exception code %#x.\n", rec->ExceptionCode);
+
     got_exception = 1;
-    if (NtCurrentTeb()->Peb->BeingDebugged || !strcmp( winetest_platform, "wine" ))
-    {
-        todo_wine_if(!NtCurrentTeb()->Peb->BeingDebugged)
-        ok(NtCurrentTeb()->Peb->BeingDebugged, "context->Rip misplaced for dbg breakpoint exception.\n");
+    dbg_except_continue_handler_rip = (void *)context->Rip;
+    if (NtCurrentTeb()->Peb->BeingDebugged)
         ++context->Rip;
-    }
+
     return EXCEPTION_CONTINUE_EXECUTION;
 }

@@ -2936,7 +2950,7 @@ static void test_kiuserexceptiondispatcher(void)
     ok(hook_exception_address == code_mem || broken(!hook_exception_address) /* Win2008 */,
             "Got unexpected exception address %p, expected %p.\n",
             hook_exception_address, code_mem);
-    todo_wine ok(hook_KiUserExceptionDispatcher_rip == code_mem, "Got unexpected exception address %p, expected %p.\n",
+    ok(hook_KiUserExceptionDispatcher_rip == code_mem, "Got unexpected exception address %p, expected %p.\n",
             hook_KiUserExceptionDispatcher_rip, code_mem);
     ok(dbg_except_continue_handler_rip == code_mem, "Got unexpected exception address %p, expected %p.\n",
             dbg_except_continue_handler_rip, code_mem);
@@ -2971,10 +2985,27 @@ static void test_kiuserexceptiondispatcher(void)
     ok(got_exception, "Handler was not called.\n");
     ok(hook_called, "Hook was not called.\n");

-    NtCurrentTeb()->Peb->BeingDebugged = 0;
+    ok(hook_exception_address == (BYTE *)hook_KiUserExceptionDispatcher_rip + 1
+            || broken(!hook_exception_address) /* 2008 */, "Got unexpected addresses %p, %p.\n",
+            hook_KiUserExceptionDispatcher_rip, hook_exception_address);

RemoveVectoredExceptionHandler(dbg_except_continue_vectored_handler);

+    memcpy(pKiUserExceptionDispatcher, patched_KiUserExceptionDispatcher_bytes,
+            sizeof(patched_KiUserExceptionDispatcher_bytes));
+    got_exception = 0;
+    hook_called = FALSE;
+    run_exception_test(dbg_except_continue_handler, NULL, except_code, ARRAY_SIZE(except_code), PAGE_EXECUTE_READ);
+
+    ok(got_exception, "Handler was not called.\n");
+    ok(hook_called, "Hook was not called.\n");
+    ok(hook_KiUserExceptionDispatcher_rip == code_mem, "Got unexpected exception address %p, expected %p.\n",
+            hook_KiUserExceptionDispatcher_rip, code_mem);
+    ok(dbg_except_continue_handler_rip == code_mem, "Got unexpected exception address %p, expected %p.\n",
+            dbg_except_continue_handler_rip, code_mem);
+
+    NtCurrentTeb()->Peb->BeingDebugged = 0;
+
     ret = VirtualProtect(pKiUserExceptionDispatcher, sizeof(saved_KiUserExceptionDispatcher_bytes),
             old_protect2, &old_protect2);
     ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError());
diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c
index 1e0774e84da..cafaaa0ea55 100644
--- a/dlls/ntdll/unix/signal_i386.c
+++ b/dlls/ntdll/unix/signal_i386.c
@@ -1584,11 +1584,23 @@ static void setup_raise_exception( ucontext_t *sigcontext, struct stack_layout *
     SS_sig(sigcontext)  = get_ds();
     stack->rec_ptr      = &stack->rec;         /* arguments for KiUserExceptionDispatcher */
     stack->context_ptr  = &stack->context;
+
+    if (stack->rec.ExceptionCode == EXCEPTION_BREAKPOINT)
+    {
+        /* fix up instruction pointer in context for EXCEPTION_BREAKPOINT */
+        stack->context.Eip--;
+    }
 }

__ASM_GLOBAL_FUNC( call_user_exception_dispatcher,
                    "add $4,%esp\n\t"
-                   "jmp *8(%esp)")
+                   "movl (%esp),%eax\n\t" /* rec */
+                   "movl (%eax),%eax\n\t" /* ExceptionCode */
+                   "cmpl $0x80000003,%eax\n\t"
+                   "jne 1f\n\t"
+                   "movl 4(%esp),%eax\n\t" /* context */
+                   "decl 0xb8(%eax)\n\t" /* Eip */
+                   "1:\tjmp *8(%esp)")

/**********************************************************************
  *		get_fpu_code
diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c
index 6e103a78fb5..eed622851f0 100644
--- a/dlls/ntdll/unix/signal_x86_64.c
+++ b/dlls/ntdll/unix/signal_x86_64.c
@@ -1958,6 +1958,12 @@ static void setup_raise_exception( ucontext_t *sigcontext, struct stack_layout *
         return;
     }

+    if (stack->rec.ExceptionCode == EXCEPTION_BREAKPOINT)
+    {
+        /* fix up instruction pointer in context for EXCEPTION_BREAKPOINT */
+        stack->context.Rip--;
+    }
+
     /* now modify the sigcontext to return to the raise function */
     RIP_sig(sigcontext) = (ULONG_PTR)raise_func_trampoline;
     R8_sig(sigcontext)  = (ULONG_PTR)pKiUserExceptionDispatcher;
@@ -1983,6 +1989,13 @@ void WINAPI do_call_user_exception_dispatcher( EXCEPTION_RECORD *rec, CONTEXT *c
 {
     memmove(&stack->context, context, sizeof(*context));
     memcpy(&stack->rec, rec, sizeof(*rec));
+
+    if (stack->rec.ExceptionCode == EXCEPTION_BREAKPOINT)
+    {
+        /* fix up instruction pointer in context for EXCEPTION_BREAKPOINT */
+        stack->context.Rip--;
+    }
+
     user_exception_dispatcher_trampoline( stack, dispatcher );
 }

[Marvin]

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=75096

Your paranoid android.

=== debiant (32 bit report) ===

ntdll:
om.c:2118: Test failed: got 88

=== debiant (32 bit WoW report) ===

ntdll:
om.c:2133: Test failed: got 89