From 57edd3268de55ef2efa24fb210587c392feabee3 Mon Sep 17 00:00:00 2001 From: Art Yerkes Date: Sat, 2 Aug 2008 22:01:46 +0000 Subject: [PATCH] Fix step and next commands. Not perfect, but working. Also, make all printing go through KdbpPrint, and switch to KeStallExecutionProcessor for checking on messages from the terminal. Use the classic unix 100ms time limit for messages. We need to refactor debug entry. There's a whole bunch of intermixed code that basically handles 5 distinct states here: 1) Unexpected stop against a loose breakpoint instruction 2) Expected stop against a breakpoint instruction we placed 3) Singlestep after an expected breakpoint, user was not stepping 4) Singlestep after an expected breakpoint, user was stepping 5) Singlestep trap, user was not stepping (memory breakpoint). svn path=/trunk/; revision=35048 --- reactos/ntoskrnl/kdbg/kdb.c | 52 +++++++++++++++-------------- reactos/ntoskrnl/kdbg/kdb_cli.c | 46 ++++++++++++++----------- reactos/ntoskrnl/kdbg/kdb_symbols.c | 16 +++++++-- 3 files changed, 68 insertions(+), 46 deletions(-) diff --git a/reactos/ntoskrnl/kdbg/kdb.c b/reactos/ntoskrnl/kdbg/kdb.c index a07882a5e69..2973c50a39b 100644 --- a/reactos/ntoskrnl/kdbg/kdb.c +++ b/reactos/ntoskrnl/kdbg/kdb.c @@ -1203,9 +1203,9 @@ KdbpInternalEnter() Thread->Tcb.StackLimit = (ULONG)KdbStack; Thread->Tcb.KernelStack = (char*)KdbStack + KDB_STACK_SIZE; - /*KdbpPrint("Switching to KDB stack 0x%08x-0x%08x\n", Thread->Tcb.StackLimit, Thread->Tcb.StackBase);*/ + /*KdbpPrint("Switching to KDB stack 0x%08x-0x%08x (Current Stack is 0x%08x)\n", Thread->Tcb.StackLimit, Thread->Tcb.StackBase, Esp);*/ - KdbpStackSwitchAndCall(Thread->Tcb.KernelStack, KdbpCallMainLoop); + KdbpStackSwitchAndCall(KdbStack + KDB_STACK_SIZE - sizeof(ULONG), KdbpCallMainLoop); Thread->Tcb.InitialStack = SavedInitialStack; Thread->Tcb.StackBase = SavedStackBase; @@ -1330,7 +1330,7 @@ KdbEnterDebuggerException( if (!NT_SUCCESS(KdbpOverwriteInstruction(KdbCurrentProcess, BreakPoint->Address, BreakPoint->Data.SavedInstruction, NULL))) { - DbgPrint("Couldn't restore original instruction after INT3! Cannot continue execution.\n"); + KdbpPrint("Couldn't restore original instruction after INT3! Cannot continue execution.\n"); KEBUGCHECK(0); } } @@ -1361,7 +1361,7 @@ KdbEnterDebuggerException( if ((KdbSingleStepOver && !KdbpStepOverInstruction(TrapFrame->Eip)) || (!KdbSingleStepOver && !KdbpStepIntoInstruction(TrapFrame->Eip))) { - TrapFrame->EFlags |= X86_EFLAGS_TF; + Context->EFlags |= X86_EFLAGS_TF; } goto continue_execution; /* return */ } @@ -1377,7 +1377,7 @@ KdbEnterDebuggerException( BreakPoint->Type == KdbBreakPointTemporary) { ASSERT(ExceptionCode == STATUS_BREAKPOINT); - TrapFrame->EFlags |= X86_EFLAGS_TF; + Context->EFlags |= X86_EFLAGS_TF; KdbBreakPointToReenable = BreakPoint; } @@ -1410,12 +1410,12 @@ KdbEnterDebuggerException( if (BreakPoint->Type == KdbBreakPointSoftware) { - DbgPrint("Entered debugger on breakpoint #%d: EXEC 0x%04x:0x%08x\n", + KdbpPrint("Entered debugger on breakpoint #%d: EXEC 0x%04x:0x%08x\n", KdbLastBreakPointNr, TrapFrame->SegCs & 0xffff, TrapFrame->Eip); } else if (BreakPoint->Type == KdbBreakPointHardware) { - DbgPrint("Entered debugger on breakpoint #%d: %s 0x%08x\n", + KdbpPrint("Entered debugger on breakpoint #%d: %s 0x%08x\n", KdbLastBreakPointNr, (BreakPoint->Data.Hw.AccessType == KdbAccessRead) ? "READ" : ((BreakPoint->Data.Hw.AccessType == KdbAccessWrite) ? "WRITE" : @@ -1444,36 +1444,38 @@ KdbEnterDebuggerException( if (!NT_SUCCESS(KdbpOverwriteInstruction(KdbCurrentProcess, BreakPoint->Address, 0xCC, &BreakPoint->Data.SavedInstruction))) { - DbgPrint("Warning: Couldn't reenable breakpoint %d\n", + KdbpPrint("Warning: Couldn't reenable breakpoint %d\n", BreakPoint - KdbBreakPoints); } /* Unset TF if we are no longer single stepping. */ if (KdbNumSingleSteps == 0) - TrapFrame->EFlags &= ~X86_EFLAGS_TF; + Context->EFlags &= ~X86_EFLAGS_TF; goto continue_execution; /* return */ } /* Check if we expect a single step */ if ((TrapFrame->Dr6 & 0xf) == 0 && KdbNumSingleSteps > 0) { - /*ASSERT((TrapFrame->Eflags & X86_EFLAGS_TF) != 0);*/ + /*ASSERT((Context->Eflags & X86_EFLAGS_TF) != 0);*/ if (--KdbNumSingleSteps > 0) { if ((KdbSingleStepOver && KdbpStepOverInstruction(TrapFrame->Eip)) || (!KdbSingleStepOver && KdbpStepIntoInstruction(TrapFrame->Eip))) { - TrapFrame->EFlags &= ~X86_EFLAGS_TF; + Context->EFlags &= ~X86_EFLAGS_TF; } else { - TrapFrame->EFlags |= X86_EFLAGS_TF; + Context->EFlags |= X86_EFLAGS_TF; } - goto continue_execution; /* return */ + goto continue_execution; /* return */ } - - TrapFrame->EFlags &= ~X86_EFLAGS_TF; - KdbEnteredOnSingleStep = TRUE; + else + { + Context->EFlags &= ~X86_EFLAGS_TF; + KdbEnteredOnSingleStep = TRUE; + } } else { @@ -1481,7 +1483,7 @@ KdbEnterDebuggerException( { return kdHandleException; } - DbgPrint("Entered debugger on unexpected debug trap!\n"); + KdbpPrint("Entered debugger on unexpected debug trap!\n"); } } else if (ExceptionCode == STATUS_BREAKPOINT) @@ -1496,7 +1498,7 @@ KdbEnterDebuggerException( return kdHandleException; } - DbgPrint("Entered debugger on embedded INT3 at 0x%04x:0x%08x.\n", + KdbpPrint("Entered debugger on embedded INT3 at 0x%04x:0x%08x.\n", TrapFrame->SegCs & 0xffff, TrapFrame->Eip - 1); } else @@ -1510,7 +1512,7 @@ KdbEnterDebuggerException( return ContinueType; } - DbgPrint("Entered debugger on %s-chance exception (Exception Code: 0x%x) (%s)\n", + KdbpPrint("Entered debugger on %s-chance exception (Exception Code: 0x%x) (%s)\n", FirstChance ? "first" : "last", ExceptionCode, ExceptionString); if (ExceptionCode == STATUS_ACCESS_VIOLATION && ExceptionRecord != NULL && ExceptionRecord->NumberParameters != 0) @@ -1528,15 +1530,15 @@ KdbEnterDebuggerException( #endif Err = TrapFrame->ErrCode; - DbgPrint("Memory at 0x%p could not be %s: ", TrapCr2, (Err & (1 << 1)) ? "written" : "read"); + KdbpPrint("Memory at 0x%p could not be %s: ", TrapCr2, (Err & (1 << 1)) ? "written" : "read"); if ((Err & (1 << 0)) == 0) - DbgPrint("Page not present.\n"); + KdbpPrint("Page not present.\n"); else { if ((Err & (1 << 3)) != 0) - DbgPrint("Reserved bits in page directory set.\n"); + KdbpPrint("Reserved bits in page directory set.\n"); else - DbgPrint("Page protection violation.\n"); + KdbpPrint("Page protection violation.\n"); } } } @@ -1577,7 +1579,7 @@ KdbEnterDebuggerException( } else { - KdbCurrentTrapFrame->Tf.EFlags |= X86_EFLAGS_TF; + Context->EFlags |= X86_EFLAGS_TF; } } @@ -1601,7 +1603,7 @@ KdbEnterDebuggerException( continue_execution: /* Clear debug status */ - if (ExceptionCode == STATUS_SINGLE_STEP || ExceptionCode == STATUS_BREAKPOINT) /* FIXME: Why clear DR6 on INT3? */ + if (ExceptionCode == STATUS_BREAKPOINT) /* FIXME: Why clear DR6 on INT3? */ { /* Set the RF flag so we don't trigger the same breakpoint again. */ if (Resume) diff --git a/reactos/ntoskrnl/kdbg/kdb_cli.c b/reactos/ntoskrnl/kdbg/kdb_cli.c index 6fbce3da0ae..e44f0de68e3 100644 --- a/reactos/ntoskrnl/kdbg/kdb_cli.c +++ b/reactos/ntoskrnl/kdbg/kdb_cli.c @@ -1395,8 +1395,10 @@ KdbpCmdMod(ULONG Argc, PCHAR Argv[]) { if (!KdbpSymFindModuleByIndex(0, &Info)) { - KdbpPrint("No modules.\n"); - return TRUE; + ULONG_PTR ntoskrnlBase = ((ULONG_PTR)KdbpCmdMod) & 0xfff00000; + KdbpPrint(" Base Size Name\n"); + KdbpPrint(" %08x %08x %s\n", ntoskrnlBase, 0, "ntoskrnl.exe"); + return TRUE; } i = 1; } @@ -1915,6 +1917,8 @@ KdbpPrint( TerminalInitialized = TRUE; Length = 0; + KeStallExecutionProcessor(100000); + for (;;) { c = KdbpTryGetCharSerial(5000); @@ -1937,6 +1941,7 @@ KdbpPrint( { /* Try to query number of rows from terminal. A reply looks like "\x1b[8;24;80t" */ TerminalReportsSize = FALSE; + KeStallExecutionProcessor(100000); DbgPrint("\x1b[18t"); c = KdbpTryGetCharSerial(5000); if (c == KEY_ESC) @@ -1968,6 +1973,8 @@ KdbpPrint( } } } + /* Clear further characters */ + while ((c = KdbpTryGetCharSerial(5000)) != -1); } } @@ -2066,7 +2073,7 @@ KdbpPrint( } } - DbgPrint("%s", p); + DbgPrint("%s", p); if (c != '\0') p[i + 1] = c; @@ -2229,13 +2236,14 @@ KdbpReadCommand( /* Read the next char - this is to throw away a \n which most clients should * send after \r. */ + KeStallExecutionProcessor(100000); if (KdbDebugState & KD_DEBUG_KDSERIAL) NextKey = KdbpTryGetCharSerial(5); else NextKey = KdbpTryGetCharKeyboard(&ScanCode, 5); if (NextKey == '\n' || NextKey == -1) /* \n or no response at all */ NextKey = '\0'; - DbgPrint("\n"); + KdbpPrint("\n"); /* * Repeat the last command if the user presses enter. Reduces the * risk of RSI when single-stepping. @@ -2260,9 +2268,9 @@ KdbpReadCommand( Buffer--; *Buffer = 0; if (EchoOn) - DbgPrint("%c %c", KEY_BS, KEY_BS); + KdbpPrint("%c %c", KEY_BS, KEY_BS); else - DbgPrint(" %c", KEY_BS); + KdbpPrint(" %c", KEY_BS); } } else if (ScanCode == KEY_SCAN_UP) @@ -2287,15 +2295,15 @@ KdbpReadCommand( Buffer--; *Buffer = 0; if (EchoOn) - DbgPrint("%c %c", KEY_BS, KEY_BS); + KdbpPrint("%c %c", KEY_BS, KEY_BS); else - DbgPrint(" %c", KEY_BS); + KdbpPrint(" %c", KEY_BS); } i = min(strlen(KdbCommandHistory[CmdHistIndex]), Size - 1); memcpy(Orig, KdbCommandHistory[CmdHistIndex], i); Orig[i] = '\0'; Buffer = Orig + i; - DbgPrint("%s", Orig); + KdbpPrint("%s", Orig); } } else if (ScanCode == KEY_SCAN_DOWN) @@ -2313,22 +2321,22 @@ KdbpReadCommand( Buffer--; *Buffer = 0; if (EchoOn) - DbgPrint("%c %c", KEY_BS, KEY_BS); + KdbpPrint("%c %c", KEY_BS, KEY_BS); else - DbgPrint(" %c", KEY_BS); + KdbpPrint(" %c", KEY_BS); } i = min(strlen(KdbCommandHistory[CmdHistIndex]), Size - 1); memcpy(Orig, KdbCommandHistory[CmdHistIndex], i); Orig[i] = '\0'; Buffer = Orig + i; - DbgPrint("%s", Orig); + KdbpPrint("%s", Orig); } } } else { if (EchoOn) - DbgPrint("%c", Key); + KdbpPrint("%c", Key); *Buffer = Key; Buffer++; @@ -2406,14 +2414,14 @@ KdbpCliMainLoop( { if (!KdbSymPrintAddress((PVOID)KdbCurrentTrapFrame->Tf.Eip)) { - DbgPrint("<%x>", KdbCurrentTrapFrame->Tf.Eip); + KdbpPrint("<%x>", KdbCurrentTrapFrame->Tf.Eip); } - DbgPrint(": "); + KdbpPrint(": "); if (KdbpDisassemble(KdbCurrentTrapFrame->Tf.Eip, KdbUseIntelSyntax) < 0) { - DbgPrint(""); + KdbpPrint(""); } - DbgPrint("\n"); + KdbpPrint("\n"); } /* Flush the input buffer */ @@ -2431,7 +2439,7 @@ KdbpCliMainLoop( do { /* Print the prompt */ - DbgPrint("kdb:> "); + KdbpPrint("kdb:> "); /* Read a command and remember it */ KdbpReadCommand(Command, sizeof (Command)); @@ -2456,7 +2464,7 @@ KdbpCliModuleLoaded(IN PUNICODE_STRING Name) if (!KdbBreakOnModuleLoad) return; - DbgPrint("Module %wZ loaded.\n", Name); + KdbpPrint("Module %wZ loaded.\n", Name); DbgBreakPointWithStatus(DBG_STATUS_CONTROL_C); } diff --git a/reactos/ntoskrnl/kdbg/kdb_symbols.c b/reactos/ntoskrnl/kdbg/kdb_symbols.c index 2d1ba0a796c..597f7a90592 100644 --- a/reactos/ntoskrnl/kdbg/kdb_symbols.c +++ b/reactos/ntoskrnl/kdbg/kdb_symbols.c @@ -26,7 +26,7 @@ typedef struct _IMAGE_SYMBOL_INFO_CACHE { static BOOLEAN LoadSymbols; static LIST_ENTRY SymbolFileListHead; static KSPIN_LOCK SymbolFileListLock; - +BOOLEAN KdbpSymbolsInitialized = FALSE; /* FUNCTIONS ****************************************************************/ @@ -57,6 +57,9 @@ KdbpSymFindUserModule(IN PVOID Address OPTIONAL, INT Count = 0; INT Length; + if (!KdbpSymbolsInitialized) + return FALSE; + CurrentProcess = PsGetCurrentProcess(); if (CurrentProcess != NULL) { @@ -110,6 +113,9 @@ KdbpSymFindModule(IN PVOID Address OPTIONAL, INT Count = 0; INT Length; + if (!KdbpSymbolsInitialized) + return FALSE; + current_entry = PsLoadedModuleList.Flink; while (current_entry != &PsLoadedModuleList) @@ -213,7 +219,7 @@ KdbSymPrintAddress(IN PVOID Address) CHAR FileName[256]; CHAR FunctionName[256]; - if (!KdbpSymFindModuleByAddress(Address, &Info)) + if (!KdbpSymbolsInitialized || !KdbpSymFindModuleByAddress(Address, &Info)) return FALSE; RelativeAddress = (ULONG_PTR) Address - Info.Base; @@ -259,6 +265,11 @@ KdbSymGetAddressInformation(IN PROSSYM_INFO RosSymInfo, OUT PCH FileName OPTIONAL, OUT PCH FunctionName OPTIONAL) { + if (!KdbpSymbolsInitialized) + { + return STATUS_UNSUCCESSFUL; + } + if (NULL == RosSymInfo) { return STATUS_UNSUCCESSFUL; @@ -772,6 +783,7 @@ KdbInitialize(PKD_DISPATCH_TABLE DispatchTable, SymbolsInfo.SizeOfImage = DataTableEntry->SizeOfImage; KdbSymProcessSymbols(NULL, &SymbolsInfo); + KdbpSymbolsInitialized = TRUE; } }