[ion] 25975: - Fix critical bugs in DR_TRAP_FIXUP, TRAP_PROLOG and TRAP_EPILOG which would either cause infinite loops during exceptions or corruption of the correct code path when dealing with debug registers. - Fix a bug in KiRecordDr7 setting the new DR7 mask. - Make KiEspToTrapFrame thread-safe by raising to APC_LEVEL to make sure a thread/set context doesn't corrupt the state. - Fix thread-safe IRQL Code in KeContexToTrapFrame/KeTrapFrameToContext. - Fix KiDispatchException to properly hand

Author: ion
Date: Sat Mar  3 20:24:58 2007
New Revision: 25975

URL: http://svn.reactos.org/svn/reactos?rev=25975&view=rev
Log:
- Fix critical bugs in DR_TRAP_FIXUP, TRAP_PROLOG and TRAP_EPILOG which would either cause infinite loops during exceptions or corruption of the correct code path when dealing with debug registers.
- Fix a bug in KiRecordDr7 setting the new DR7 mask.
- Make KiEspToTrapFrame thread-safe by raising to APC_LEVEL to make sure a thread/set context doesn't corrupt the state.
- Fix thread-safe IRQL Code in KeContexToTrapFrame/KeTrapFrameToContext.
- Fix KiDispatchException to properly handle KI_EXCEPTION_ACCESS_VIOLATION and convert it back to STATUS_ACCESS_VIOLATION which is what the system expects.
- Also fix the way we do bugchecks so the the trapframe gets properly put as a parameter.
- Make KiDebugService call into KiTrap3 to share code (merge from kd-branch).
- Changes to the KdpEnterDebuggerException hack we have to handle this change.
- Temporarily disable DebugPrint functionality (sorry, I'm onto a big bug here!)

Modified:
    trunk/reactos/lib/rtl/debug.c
    trunk/reactos/ntoskrnl/include/internal/i386/asmmacro.S
    trunk/reactos/ntoskrnl/kd/kdmain.c
    trunk/reactos/ntoskrnl/ke/i386/exp.c
    trunk/reactos/ntoskrnl/ke/i386/trap.s

Modified: trunk/reactos/lib/rtl/debug.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/lib/rtl/debug.c?rev=25975&a...
==============================================================================
--- trunk/reactos/lib/rtl/debug.c (original)
+++ trunk/reactos/lib/rtl/debug.c Sat Mar  3 20:24:58 2007
@@ -23,7 +23,7 @@
            IN ULONG Level)
 {
     /* Call the INT2D Service */
-    //return STATUS_SUCCESS;
+    return STATUS_SUCCESS;
     return DebugService(BREAKPOINT_PRINT,
                         DebugString->Buffer,
                         DebugString->Length,

Modified: trunk/reactos/ntoskrnl/include/internal/i386/asmmacro.S
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/i...
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/i386/asmmacro.S (original)
+++ trunk/reactos/ntoskrnl/include/internal/i386/asmmacro.S Sat Mar  3 20:24:58 2007
@@ -304,7 +304,7 @@
     /* Set them */
     mov dr6, ebx
     mov dr7, ecx
-    jz 3f
+    jmp 3f
 .endm

//
@@ -482,12 +482,12 @@
     /* Flush DR7 */
     and dword ptr [ebp+KTRAP_FRAME_DR7], 0

-3:
     /* Check if the thread was being debugged */
     test byte ptr [ecx+KTHREAD_DEBUG_ACTIVE], 0xFF
     jnz Dr_&Label

/* Set the Trap Frame Debug Header */
+3:
     SET_TF_DEBUG_HEADER
 .endm

@@ -1171,7 +1171,7 @@
     mov dr3, esi
     mov dr6, edi
     mov dr7, ebx
-    jz 4b
+    jmp 4b

7:
     /* Restore real CS value */

Modified: trunk/reactos/ntoskrnl/kd/kdmain.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/kd/kdmain.c?rev=25...
==============================================================================
--- trunk/reactos/ntoskrnl/kd/kdmain.c (original)
+++ trunk/reactos/ntoskrnl/kd/kdmain.c Sat Mar  3 20:24:58 2007
@@ -106,9 +106,28 @@
                           IN BOOLEAN SecondChance)
 {
     KD_CONTINUE_TYPE Return;
-
-    /* HACK (just like all this routine */
-    if (ExceptionRecord->ExceptionCode == STATUS_BREAKPOINT) Context->Eip++;
+    ULONG ExceptionCommand = ExceptionRecord->ExceptionInformation[0];
+
+    /* Check if this was a breakpoint due to DbgPrint or Load/UnloadSymbols */
+    if ((ExceptionRecord->ExceptionCode == STATUS_BREAKPOINT) &&
+        (ExceptionRecord->NumberParameters > 0) &&
+        ((ExceptionCommand == BREAKPOINT_LOAD_SYMBOLS) ||
+         (ExceptionCommand == BREAKPOINT_UNLOAD_SYMBOLS) ||
+         (ExceptionCommand == BREAKPOINT_COMMAND_STRING) ||
+         (ExceptionCommand == BREAKPOINT_PRINT)))
+    {
+        /* Check if this is a debug print */
+        if (ExceptionCommand == BREAKPOINT_PRINT)
+        {
+            /* Print the string */
+            KdpServiceDispatcher(BREAKPOINT_PRINT,
+                                 (PVOID)ExceptionRecord->ExceptionInformation[1],
+                                 ExceptionRecord->ExceptionInformation[2]);
+        }
+
+        /* This we can handle: simply bump EIP */
+        Context->Eip++;
+    }

/* Get out of here if the Debugger isn't connected */
     if (KdDebuggerNotPresent) return FALSE;

Modified: trunk/reactos/ntoskrnl/ke/i386/exp.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/exp.c?rev=...
==============================================================================
--- trunk/reactos/ntoskrnl/ke/i386/exp.c (original)
+++ trunk/reactos/ntoskrnl/ke/i386/exp.c Sat Mar  3 20:24:58 2007
@@ -140,7 +140,7 @@
             NewMask |= DR_MASK(DR7_OVERRIDE_V);

/* Set DR7 override */
-            *DrMask = DR7_OVERRIDE_MASK;
+            *DrMask |= DR7_OVERRIDE_MASK;
         }
         else
         {
@@ -210,10 +210,19 @@
 KiEspToTrapFrame(IN PKTRAP_FRAME TrapFrame,
                  IN ULONG Esp)
 {
-    ULONG Previous = KiEspFromTrapFrame(TrapFrame);
+    KIRQL OldIrql;
+    ULONG Previous;
+
+    /* Raise to APC_LEVEL if needed */
+    OldIrql = KeGetCurrentIrql();
+    if (OldIrql < APC_LEVEL) KeRaiseIrql(APC_LEVEL, &OldIrql);
+
+    /* Get the old ESP */
+    Previous = KiEspFromTrapFrame(TrapFrame);

/* Check if this is user-mode or V86 */
-    if ((TrapFrame->SegCs & MODE_MASK) || (TrapFrame->EFlags & EFLAGS_V86_MASK))
+    if ((TrapFrame->SegCs & MODE_MASK) ||
+        (TrapFrame->EFlags & EFLAGS_V86_MASK))
     {
         /* Write it directly */
         TrapFrame->HardwareEsp = Esp;
@@ -221,7 +230,11 @@
     else
     {
         /* Don't allow ESP to be lowered, this is illegal */
-        if (Esp < Previous) KeBugCheck(SET_OF_INVALID_CONTEXT);
+        if (Esp < Previous) KeBugCheckEx(SET_OF_INVALID_CONTEXT,
+                                         Esp,
+                                         Previous,
+                                         (ULONG_PTR)TrapFrame,
+                                         0);

/* Create an edit frame, check if it was alrady */
         if (!(TrapFrame->SegCs & FRAME_EDITED))
@@ -243,6 +256,9 @@
             }
         }
     }
+
+    /* Restore IRQL */
+    if (OldIrql < APC_LEVEL) KeLowerIrql(OldIrql);
 }

ULONG
@@ -316,12 +332,13 @@
     PFX_SAVE_AREA FxSaveArea;
     ULONG i;
     BOOLEAN V86Switch = FALSE;
-    KIRQL OldIrql = APC_LEVEL;
+    KIRQL OldIrql;
     ULONG DrMask = 0;
     PVOID SafeDr;

/* Do this at APC_LEVEL */
-    if (KeGetCurrentIrql() < APC_LEVEL) KeRaiseIrql(APC_LEVEL, &OldIrql);
+    OldIrql = KeGetCurrentIrql();
+    if (OldIrql < APC_LEVEL) KeRaiseIrql(APC_LEVEL, &OldIrql);

/* Start with the basic Registers */
     if ((ContextFlags & CONTEXT_CONTROL) == CONTEXT_CONTROL)
@@ -544,7 +561,7 @@
         else
         {
             /* FIXME: Handle FPU Emulation */
-            ASSERT(FALSE);
+            //ASSERT(FALSE);
         }
     }

@@ -600,11 +617,12 @@
         FLOATING_SAVE_AREA UnalignedArea;
     } FloatSaveBuffer;
     FLOATING_SAVE_AREA *FloatSaveArea;
-    KIRQL OldIrql = APC_LEVEL;
+    KIRQL OldIrql;
     ULONG i;

/* Do this at APC_LEVEL */
-    if (KeGetCurrentIrql() < APC_LEVEL) KeRaiseIrql(APC_LEVEL, &OldIrql);
+    OldIrql = KeGetCurrentIrql();
+    if (OldIrql < APC_LEVEL) KeRaiseIrql(APC_LEVEL, &OldIrql);

/* Start with the Control flags */
     if ((Context->ContextFlags & CONTEXT_CONTROL) == CONTEXT_CONTROL)
@@ -817,11 +835,26 @@
     /* Get a Context */
     KeTrapFrameToContext(TrapFrame, ExceptionFrame, &Context);

-    /* Fix up EIP */
-    if (ExceptionRecord->ExceptionCode == STATUS_BREAKPOINT)
-    {
-        /* Decrement EIP by one */
-        Context.Eip--;
+    /* Look at our exception code */
+    switch (ExceptionRecord->ExceptionCode)
+    {
+        /* Breapoint */
+        case STATUS_BREAKPOINT:
+
+            /* Decrement EIP by one */
+            Context.Eip--;
+            break;
+
+        /* Internal exception */
+        case KI_EXCEPTION_ACCESS_VIOLATION:
+
+            /* Set correct code */
+            ExceptionRecord->ExceptionCode = STATUS_ACCESS_VIOLATION;
+            if (PreviousMode == UserMode)
+            {
+                /* FIXME: Handle no execute */
+            }
+            break;
     }

/* Sanity check */
@@ -866,8 +899,8 @@
         KeBugCheckEx(KMODE_EXCEPTION_NOT_HANDLED,
                      ExceptionRecord->ExceptionCode,
                      (ULONG_PTR)ExceptionRecord->ExceptionAddress,
-                     ExceptionRecord->ExceptionInformation[0],
-                     ExceptionRecord->ExceptionInformation[1]);
+                     (ULONG_PTR)TrapFrame,
+                     0);
     }
     else
     {
@@ -989,8 +1022,8 @@
         KeBugCheckEx(KMODE_EXCEPTION_NOT_HANDLED,
                      ExceptionRecord->ExceptionCode,
                      (ULONG_PTR)ExceptionRecord->ExceptionAddress,
-                     ExceptionRecord->ExceptionInformation[0],
-                     ExceptionRecord->ExceptionInformation[1]);
+                     (ULONG_PTR)TrapFrame,
+                     0);
     }

Handled:

Modified: trunk/reactos/ntoskrnl/ke/i386/trap.s
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/trap.s?rev...
==============================================================================
--- trunk/reactos/ntoskrnl/ke/i386/trap.s (original)
+++ trunk/reactos/ntoskrnl/ke/i386/trap.s Sat Mar  3 20:24:58 2007
@@ -473,43 +473,8 @@
     mov ecx, [ebp+KTRAP_FRAME_ECX]
     mov edx, [ebp+KTRAP_FRAME_EDX]

-    /* Check for V86 mode */
-    test dword ptr [ebp+KTRAP_FRAME_EFLAGS], EFLAGS_V86_MASK
-    jnz NotUserMode
-
-    /* Check if this is kernel or user-mode */
-    test byte ptr [ebp+KTRAP_FRAME_CS], 1
-    jz CallDispatch
-    cmp word ptr [ebp+KTRAP_FRAME_CS], KGDT_R3_CODE + RPL_MASK
-    jnz NotUserMode
-
-    /* Re-enable interrupts */
-VdmProc:
-    sti
-
-    /* Call the debug routine */
-CallDispatch:
-    mov esi, ecx
-    mov edi, edx
-    mov edx, eax
-    mov ecx, 3
-    push edi
-    push esi
-    push edx
-    call _KdpServiceDispatcher@12
-
-NotUserMode:
-
-    /* Get the current process */
-    mov ebx, [fs:KPCR_CURRENT_THREAD]
-    mov ebx, [ebx+KTHREAD_APCSTATE_PROCESS]
-
-    /* Check if this is a VDM Process */
-    //cmp dword ptr [ebx+EPROCESS_VDM_OBJECTS], 0
-    //jz VdmProc
-
-    /* Exit through common routine */
-    jmp _Kei386EoiHelper@0
+    /* Jump to INT3 handler */
+    jmp PrepareInt3
 .endfunc

.func NtRaiseException@12