From faeb567e1cbe09da47087231db81d0a541082a20 Mon Sep 17 00:00:00 2001 From: Gregor Anich Date: Fri, 17 Jun 2005 17:25:02 +0000 Subject: [PATCH] Fix setjmp (didn't save correct ESP, so it crashed after longjmp). Fix the kernel debugger. svn path=/trunk/; revision=16002 --- reactos/ntoskrnl/kdbg/i386/kdb_help.S | 33 +++----- reactos/ntoskrnl/kdbg/i386/setjmp.S | 4 +- reactos/ntoskrnl/kdbg/kdb.c | 109 +++++++++++++++++--------- reactos/ntoskrnl/ke/i386/ctxswitch.S | 75 +++++++++++------- 4 files changed, 132 insertions(+), 89 deletions(-) diff --git a/reactos/ntoskrnl/kdbg/i386/kdb_help.S b/reactos/ntoskrnl/kdbg/i386/kdb_help.S index cda895a42d9..a44bc86733d 100644 --- a/reactos/ntoskrnl/kdbg/i386/kdb_help.S +++ b/reactos/ntoskrnl/kdbg/i386/kdb_help.S @@ -8,15 +8,17 @@ _KdbEnter: /* * Set up a trap frame */ - /* Ss - space already reserved by return EIP */ - pushl %esp /* Esp */ pushfl /* Eflags */ pushl %cs /* Cs */ - pushl 12(%esp) /* Eip */ - movl %ss, 16(%esp) /* Save Ss */ pushl $0 /* ErrorCode */ pushl %ebp /* Ebp */ pushl %ebx /* Ebx */ + movl 20(%esp), %ebp /* Eip */ + movl 16(%esp), %ebx /* Eflags */ + movl %ebx, 20(%esp) + movl 12(%esp), %ebx /* Cs */ + movl %ebx, 16(%esp) + movl %ebp, 12(%esp) pushl %esi /* Esi */ pushl %edi /* Edi */ pushl %fs /* Fs */ @@ -43,8 +45,9 @@ _KdbEnter: pushl %eax /* Dr1 */ movl %dr0, %eax pushl %eax /* Dr0 */ - pushl $0 /* TempEip */ - pushl $0 /* TempCs */ + leal 0x58(%esp), %eax + pushl %eax /* TempEsp */ + pushl %ss /* TempSegSs */ pushl $0 /* DebugPointer */ pushl $3 /* DebugArgMark (Exception number) */ pushl 0x60(%esp) /* DebugEip */ @@ -67,8 +70,8 @@ _KdbEnter: * DebugEip * DebugArgMark * DebugPointer - * TempCs - * TempEip + * TempSegSs + * TempEsp */ addl $(11*4), %esp @@ -98,23 +101,13 @@ _KdbEnter: popl %edx /* Edx */ popl %ecx /* Ecx */ popl %eax /* Eax */ - addl $4, %esp /* PreviousMode */ - addl $4, %esp /* ExceptionList */ + addl $8, %esp /* PreviousMode, ExceptionList */ popl %fs /* Fs */ popl %edi /* Edi */ popl %esi /* Esi */ popl %ebx /* Ebx */ - - /* Remove SS:ESP from the stack */ - movl 16(%esp), %ebp - movl %ebp, 24(%esp) - movl 12(%esp), %ebp - movl %ebp, 20(%esp) - movl 8(%esp), %ebp - movl %ebp, 16(%esp) - popl %ebp /* Ebp */ - addl $12, %esp /* ErrorCode and SS:ESP */ + addl $4, %esp /* ErrorCode */ /* * Return to the caller. diff --git a/reactos/ntoskrnl/kdbg/i386/setjmp.S b/reactos/ntoskrnl/kdbg/i386/setjmp.S index 7a596b91cda..b577263c2b0 100644 --- a/reactos/ntoskrnl/kdbg/i386/setjmp.S +++ b/reactos/ntoskrnl/kdbg/i386/setjmp.S @@ -38,7 +38,9 @@ * return PC */ .globl _setjmp +.globl __setjmp _setjmp: +__setjmp: pushl %ebp movl %esp,%ebp @@ -55,5 +57,5 @@ _setjmp: xorl %eax,%eax /* return 0 the first time */ leave - ret $4 + ret diff --git a/reactos/ntoskrnl/kdbg/kdb.c b/reactos/ntoskrnl/kdbg/kdb.c index 82581034353..b3f7eded1f3 100644 --- a/reactos/ntoskrnl/kdbg/kdb.c +++ b/reactos/ntoskrnl/kdbg/kdb.c @@ -6,7 +6,7 @@ * * PROGRAMMERS: Gregor Anich */ - + /* INCLUDES ******************************************************************/ #include @@ -106,6 +106,65 @@ STATIC CONST PCHAR ExceptionNrToString[] = /* FUNCTIONS *****************************************************************/ +STATIC VOID +KdbpTrapFrameToKdbTrapFrame(PKTRAP_FRAME TrapFrame, PKDB_KTRAP_FRAME KdbTrapFrame) +{ + /* Copy the TrapFrame only up to Eflags and zero the rest*/ + RtlCopyMemory(&KdbTrapFrame->Tf, TrapFrame, FIELD_OFFSET(KTRAP_FRAME, Esp)); + RtlZeroMemory((PVOID)((ULONG_PTR)&KdbTrapFrame->Tf + FIELD_OFFSET(KTRAP_FRAME, Esp)), + sizeof (KTRAP_FRAME) - FIELD_OFFSET(KTRAP_FRAME, Esp)); + asm volatile( + "movl %%cr0, %0" "\n\t" + "movl %%cr2, %1" "\n\t" + "movl %%cr3, %2" "\n\t" + "movl %%cr4, %3" "\n\t" + : "=r"(KdbTrapFrame->Cr0), "=r"(KdbTrapFrame->Cr2), + "=r"(KdbTrapFrame->Cr3), "=r"(KdbTrapFrame->Cr4)); + + if (TrapFrame->PreviousMode == KernelMode) + { + /* If the trapframe is a kmode one use the temp ss:esp */ + KdbTrapFrame->Tf.Esp = (ULONG)TrapFrame->TempEsp; + KdbTrapFrame->Tf.Ss = (USHORT)((ULONG)TrapFrame->TempSegSs & 0xFFFF); + } + else + { + /* Otherwise use ss:esp pushed by the CPU */ + /* FIXME: maybe change all trapframes to always put ss:esp into tempss:tempesp so we + * can handle umode and kmode the same way */ + KdbTrapFrame->Tf.Esp = TrapFrame->Esp; + KdbTrapFrame->Tf.Ss = TrapFrame->Ss; + } + + /* FIXME: copy v86 registers if TrapFrame is a V86 trapframe */ +} + +STATIC VOID +KdbpKdbTrapFrameToTrapFrame(PKDB_KTRAP_FRAME KdbTrapFrame, PKTRAP_FRAME TrapFrame) +{ + /* Copy the TrapFrame only up to Eflags and zero the rest*/ + RtlCopyMemory(TrapFrame, &KdbTrapFrame->Tf, FIELD_OFFSET(KTRAP_FRAME, Esp)); + + /* FIXME: write cr0, cr2, cr3 and cr4 (not needed atm) */ + + if (TrapFrame->PreviousMode == KernelMode) + { + /* If the trapframe is a kmode one write to the temp ss:esp */ + TrapFrame->TempEsp = (PVOID)KdbTrapFrame->Tf.Esp; + TrapFrame->TempSegSs = (PVOID)(((ULONG)TrapFrame->TempSegSs & ~0xffff) | KdbTrapFrame->Tf.Ss); + } + else + { + /* Otherwise write to ss:esp pushed by the CPU */ + /* FIXME: maybe change all trap-epilogs to always put temp ss:esp into ss:esp so we + * can handle umode and kmode the same way */ + TrapFrame->Esp = KdbTrapFrame->Tf.Esp; + TrapFrame->Ss = KdbTrapFrame->Tf.Ss; + } + + /* FIXME: copy v86 registers if TrapFrame is a V86 trapframe */ +} + /*!\brief Overwrites the instruction at \a Address with \a NewInst and stores * the old instruction in *OldInst. * @@ -981,7 +1040,7 @@ KdbpAttachToThread( if (KdbCurrentThread != KdbOriginalThread) { ASSERT(KdbCurrentTrapFrame == &KdbThreadTrapFrame); - RtlCopyMemory(KdbCurrentThread->Tcb.TrapFrame, &KdbCurrentTrapFrame->Tf, sizeof (KTRAP_FRAME)); + KdbpKdbTrapFrameToTrapFrame(KdbCurrentTrapFrame, KdbCurrentThread->Tcb.TrapFrame); } else { @@ -991,15 +1050,12 @@ KdbpAttachToThread( /* Switch to the thread's context */ if (Thread != KdbOriginalThread) { - ASSERT(Thread->Tcb.TrapFrame != NULL); - RtlCopyMemory(&KdbThreadTrapFrame.Tf, Thread->Tcb.TrapFrame, sizeof (KTRAP_FRAME)); - asm volatile( - "movl %%cr0, %0" "\n\t" - "movl %%cr2, %1" "\n\t" - "movl %%cr3, %2" "\n\t" - "movl %%cr4, %3" "\n\t" - : "=r"(KdbTrapFrame.Cr0), "=r"(KdbTrapFrame.Cr2), - "=r"(KdbTrapFrame.Cr3), "=r"(KdbTrapFrame.Cr4)); + if (Thread->Tcb.TrapFrame == NULL) + { + KdbpPrint("Threads TrapFrame is NULL! Cannot attach.\n"); + return FALSE; + } + KdbpTrapFrameToKdbTrapFrame(Thread->Tcb.TrapFrame, &KdbThreadTrapFrame); KdbCurrentTrapFrame = &KdbThreadTrapFrame; } else /* Switching back to original thread */ @@ -1243,14 +1299,7 @@ KdbEnterDebuggerException( if (BreakPoint->Condition != NULL) { /* Setup the KDB trap frame */ - RtlCopyMemory(&KdbTrapFrame.Tf, TrapFrame, sizeof (KTRAP_FRAME)); - asm volatile( - "movl %%cr0, %0" "\n\t" - "movl %%cr2, %1" "\n\t" - "movl %%cr3, %2" "\n\t" - "movl %%cr4, %3" "\n\t" - : "=r"(KdbTrapFrame.Cr0), "=r"(KdbTrapFrame.Cr2), - "=r"(KdbTrapFrame.Cr3), "=r"(KdbTrapFrame.Cr4)); + KdbpTrapFrameToKdbTrapFrame(TrapFrame, &KdbTrapFrame); ull = 0; if (!KdbpRpnEvaluateParsedExpression(BreakPoint->Condition, &KdbTrapFrame, &ull, NULL, NULL)) @@ -1395,14 +1444,7 @@ KdbEnterDebuggerException( KdbCurrentTrapFrame = &KdbTrapFrame; /* Setup the KDB trap frame */ - RtlCopyMemory(&KdbTrapFrame.Tf, TrapFrame, sizeof(KTRAP_FRAME)); - asm volatile( - "movl %%cr0, %0" "\n\t" - "movl %%cr2, %1" "\n\t" - "movl %%cr3, %2" "\n\t" - "movl %%cr4, %3" "\n\t" - : "=r"(KdbTrapFrame.Cr0), "=r"(KdbTrapFrame.Cr2), - "=r"(KdbTrapFrame.Cr3), "=r"(KdbTrapFrame.Cr4)); + KdbpTrapFrameToKdbTrapFrame(TrapFrame, &KdbTrapFrame); /* Enter critical section */ Ke386SaveFlags(OldEflags); @@ -1436,7 +1478,7 @@ KdbEnterDebuggerException( /* Save the current thread's trapframe */ if (KdbCurrentTrapFrame == &KdbThreadTrapFrame) { - RtlCopyMemory(KdbCurrentThread->Tcb.TrapFrame, KdbCurrentTrapFrame, sizeof (KTRAP_FRAME)); + KdbpKdbTrapFrameToTrapFrame(KdbCurrentTrapFrame, KdbCurrentThread->Tcb.TrapFrame); } /* Detach from attached process */ @@ -1446,16 +1488,7 @@ KdbEnterDebuggerException( } /* Update the exception TrapFrame */ - RtlCopyMemory(TrapFrame, &KdbTrapFrame.Tf, sizeof(KTRAP_FRAME)); -#if 0 - asm volatile( - "movl %0, %%cr0" "\n\t" - "movl %1, %%cr2" "\n\t" - "movl %2, %%cr3" "\n\t" - "movl %3, %%cr4" "\n\t" - : : "r"(KdbTrapFrame.Cr0), "r"(KdbTrapFrame.Cr2), - "r"(KdbTrapFrame.Cr3), "r"(KdbTrapFrame.Cr4)); -#endif + KdbpKdbTrapFrameToTrapFrame(&KdbTrapFrame, TrapFrame); /* Decrement the entry count */ InterlockedDecrement(&KdbEntryCount); diff --git a/reactos/ntoskrnl/ke/i386/ctxswitch.S b/reactos/ntoskrnl/ke/i386/ctxswitch.S index 517f7a442b8..b6099561cd5 100644 --- a/reactos/ntoskrnl/ke/i386/ctxswitch.S +++ b/reactos/ntoskrnl/ke/i386/ctxswitch.S @@ -116,7 +116,7 @@ BadThread: .globl @KiSwapContextInternal@0 @KiSwapContextInternal@0: #ifdef KDBG - //jmp SaveTrapFrameForKDB + jmp SaveTrapFrameForKDB SaveTrapFrameForKDB_Return: #endif @@ -298,32 +298,55 @@ SameProcess: SaveTrapFrameForKDB: /* Set up a trap frame */ - - /* Fake Interrupt Stack */ - push esp // 0x74 pushf // 0x70 push cs // 0x6C - push [esp+12] /* EIP */ // 0x68 - mov [esp+16], ss // 0x78 - - /* Trap Frame */ - push 0 /* Error Code */ // 0x64 + push 0 /* Error Code */ // 0x64 push ebp // 0x60 push ebx + + /* Fake Interrupt Stack */ + mov ebp, [esp+20] /* Eip */ + mov ebx, [esp+16] /* Eflags */ + mov [esp+20], ebx + mov ebx, [esp+12] /* Cs */ + mov [esp+16], ebx + mov [esp+12], ebp + push esi push edi push fs - push -1 /* Exception List */ // 0x4C - push 0 /* Previous Mode */ // 0x48 + push -1 /* Exception List */ // 0x4C + push 0 /* Previous Mode */ // 0x48 push eax push ecx push edx push ds push es push gs // 0x30 - sub esp, 0x28 /* Debug Registers */ // 0x8 - push [esp+60] /* Debug EIP */ // 0x4 - push ebp /* Debug EBP */ // 0x0 + + mov eax, dr7 + push eax /* Dr7 */ + /* Clear breakpoint enables in dr7. */ + and eax, 0xffff0000 + mov dr7, eax + mov eax, dr6 + push eax /* Dr6 */ + mov eax, dr3 + push eax /* Dr3 */ + mov eax, dr2 + push eax /* Dr2 */ + mov eax, dr1 + push eax /* Dr1 */ + mov eax, dr0 + push eax /* Dr0 */ + + lea eax, [esp+0x58] + push eax /* TempEsp */ + push ss /* TempSegSs */ + push 0 /* DebugPointer */ + push -1 /* DebugArgMark */ + push [esp+60] /* Debug EIP */ // 0x4 + push ebp /* Debug EBP */ // 0x0 /* Set Stack */ mov ebp, esp @@ -334,19 +357,21 @@ SaveTrapFrameForKDB: /* Save new one */ mov [edi+KTHREAD_TRAP_FRAME], ebp + /* Restore EBP, EBX and EAX */ + mov ebp, [ebp+KTRAP_FRAME_EBP] + mov ebx, [ebp+KTRAP_FRAME_EBX] + mov eax, [ebp+KTRAP_FRAME_EAX] + /* Return EIP */ push offset RestoreTrapFrameForKDB - /* Restore EBP */ - mov ebp, [ebp+KTRAP_FRAME_EBP] - /* Jump to normal code */ jmp SaveTrapFrameForKDB_Return RestoreTrapFrameForKDB: /* Restore the old trapframe */ - pop [edi+KTHREAD_TRAP_FRAME] + pop [esi+KTHREAD_TRAP_FRAME] /* Pop unused portions of the trap frame */ add esp, 0x30 @@ -358,23 +383,13 @@ RestoreTrapFrameForKDB: pop edx pop ecx pop eax - add esp, 8 + add esp, 8 /* ExceptionList and PreviousMode */ pop fs pop edi pop esi pop ebx - - /* Remove SS:ESP from the stack */ - mov ebp, [esp+16] - mov [esp+24], ebp - mov ebp, [esp+12] - mov [esp+20], ebp - mov ebp, [esp+8] - mov [esp+16], ebp - - /* Restore Fake INT Stack */ pop ebp - add esp, 12 + add esp, 4 /* ErrorCode */ /* Return to the caller. */ iret