[arty] 35048: 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) Singlest

Author: arty
Date: Sat Aug  2 17:01:46 2008
New Revision: 35048

URL: http://svn.reactos.org/svn/reactos?rev=35048&view=rev
Log:
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).

Modified:
    trunk/reactos/ntoskrnl/kdbg/kdb.c
    trunk/reactos/ntoskrnl/kdbg/kdb_cli.c
    trunk/reactos/ntoskrnl/kdbg/kdb_symbols.c

Modified: trunk/reactos/ntoskrnl/kdbg/kdb.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/kdbg/kdb.c?rev=35…
==============================================================================
--- trunk/reactos/ntoskrnl/kdbg/kdb.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/kdbg/kdb.c [iso-8859-1] Sat Aug  2 17:01:46 2008
@@ -1203,9 +1203,9 @@
    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);*/
-
-   KdbpStackSwitchAndCall(Thread->Tcb.KernelStack, KdbpCallMainLoop);
+   /*KdbpPrint("Switching to KDB stack 0x%08x-0x%08x (Current Stack is
0x%08x)\n", Thread->Tcb.StackLimit, Thread->Tcb.StackBase, Esp);*/
+
+   KdbpStackSwitchAndCall(KdbStack + KDB_STACK_SIZE - sizeof(ULONG), KdbpCallMainLoop);
 
    Thread->Tcb.InitialStack = SavedInitialStack;
    Thread->Tcb.StackBase = SavedStackBase;
@@ -1330,7 +1330,7 @@
          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 @@
             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 @@
                BreakPoint->Type == KdbBreakPointTemporary)
       {
          ASSERT(ExceptionCode == STATUS_BREAKPOINT);
-         TrapFrame->EFlags |= X86_EFLAGS_TF;
+         Context->EFlags |= X86_EFLAGS_TF;
          KdbBreakPointToReenable = BreakPoint;
       }
 
@@ -1410,12 +1410,12 @@
 
       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 @@
          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 @@
          {
             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 @@
          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 @@
          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 @@
 #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 @@
       }
       else
       {
-         KdbCurrentTrapFrame->Tf.EFlags |= X86_EFLAGS_TF;
+         Context->EFlags |= X86_EFLAGS_TF;
       }
    }
 
@@ -1601,7 +1603,7 @@
 
 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)

Modified: trunk/reactos/ntoskrnl/kdbg/kdb_cli.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/kdbg/kdb_cli.c?re…
==============================================================================
--- trunk/reactos/ntoskrnl/kdbg/kdb_cli.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/kdbg/kdb_cli.c [iso-8859-1] Sat Aug  2 17:01:46 2008
@@ -1395,8 +1395,10 @@
    {
       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 @@
 
       TerminalInitialized = TRUE;
       Length = 0;
+	  KeStallExecutionProcessor(100000);
+
       for (;;)
       {
          c = KdbpTryGetCharSerial(5000);
@@ -1937,6 +1941,7 @@
       {
          /* 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 @@
                   }
                }
             }
+			/* Clear further characters */
+			while ((c = KdbpTryGetCharSerial(5000)) != -1);
          }
       }
 
@@ -2066,7 +2073,7 @@
          }
       }
 
-      DbgPrint("%s", p);
+	  DbgPrint("%s", p);
 
       if (c != '\0')
          p[i + 1] = c;
@@ -2229,13 +2236,14 @@
          /* 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 @@
             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 @@
                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 @@
                   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 @@
    {
       if (!KdbSymPrintAddress((PVOID)KdbCurrentTrapFrame->Tf.Eip))
       {
-         DbgPrint("<%x>", KdbCurrentTrapFrame->Tf.Eip);
-      }
-      DbgPrint(": ");
+         KdbpPrint("<%x>", KdbCurrentTrapFrame->Tf.Eip);
+      }
+      KdbpPrint(": ");
       if (KdbpDisassemble(KdbCurrentTrapFrame->Tf.Eip, KdbUseIntelSyntax) < 0)
       {
-         DbgPrint("<INVALID>");
-      }
-      DbgPrint("\n");
+         KdbpPrint("<INVALID>");
+      }
+      KdbpPrint("\n");
    }
 
    /* Flush the input buffer */
@@ -2431,7 +2439,7 @@
    do
    {
       /* Print the prompt */
-      DbgPrint("kdb:> ");
+      KdbpPrint("kdb:> ");
 
       /* Read a command and remember it */
       KdbpReadCommand(Command, sizeof (Command));
@@ -2456,7 +2464,7 @@
    if (!KdbBreakOnModuleLoad)
       return;
 
-   DbgPrint("Module %wZ loaded.\n", Name);
+   KdbpPrint("Module %wZ loaded.\n", Name);
    DbgBreakPointWithStatus(DBG_STATUS_CONTROL_C);
 }
 

Modified: trunk/reactos/ntoskrnl/kdbg/kdb_symbols.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/kdbg/kdb_symbols.…
==============================================================================
--- trunk/reactos/ntoskrnl/kdbg/kdb_symbols.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/kdbg/kdb_symbols.c [iso-8859-1] Sat Aug  2 17:01:46 2008
@@ -26,7 +26,7 @@
 static BOOLEAN LoadSymbols;
 static LIST_ENTRY SymbolFileListHead;
 static KSPIN_LOCK SymbolFileListLock;
-
+BOOLEAN KdbpSymbolsInitialized = FALSE;
 
 /* FUNCTIONS ****************************************************************/
 
@@ -56,6 +56,9 @@
   PPEB Peb = NULL;
   INT Count = 0;
   INT Length;
+
+  if (!KdbpSymbolsInitialized)
+	  return FALSE;
 
   CurrentProcess = PsGetCurrentProcess();
   if (CurrentProcess != NULL)
@@ -110,6 +113,9 @@
   INT Count = 0;
   INT Length;
 
+  if (!KdbpSymbolsInitialized)
+	  return FALSE;
+
   current_entry = PsLoadedModuleList.Flink;
 
   while (current_entry != &PsLoadedModuleList)
@@ -213,7 +219,7 @@
   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 @@
                             OUT PCH FileName  OPTIONAL,
                             OUT PCH FunctionName  OPTIONAL)
 {
+  if (!KdbpSymbolsInitialized)
+    {
+	  return STATUS_UNSUCCESSFUL;
+	}
+
   if (NULL == RosSymInfo)
     {
       return STATUS_UNSUCCESSFUL;
@@ -772,6 +783,7 @@
         SymbolsInfo.SizeOfImage = DataTableEntry->SizeOfImage;
 
         KdbSymProcessSymbols(NULL, &SymbolsInfo);
+		KdbpSymbolsInitialized = TRUE;
     }
 }