[ros-diffs] [reactos] 01/01: [NTOS:KDBG] Fix parsing the boot command line for the (NO)LOADSYMBOLS options.

[Hermès Bélusca-Maïto]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=934812c4b22dadc37abb7…

commit 934812c4b22dadc37abb7a3c27272e843ecdf244
Author:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
AuthorDate: Mon Mar 20 02:28:16 2023 +0100
Commit:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
CommitDate: Mon Mar 20 02:44:41 2023 +0100

    [NTOS:KDBG] Fix parsing the boot command line for the (NO)LOADSYMBOLS options.
    
    Addendum to commit de892d5b.
    
    The boot options get stripped of their optional command switch '/'
    (and replaced by whitspace separation) by the NT loader. Also, forbid
    the presence of space between the optional '=' character following
    (NO)LOADSYMBOLS.
    
    In addition, fix the default initialization of LoadSymbols in KdbSymInit():
    we cannot rely on MmNumberOfPhysicalPages in BootPhase 0 since at this point,
    the Memory Manager hasn't been initialized and this variable is not yet set.
    (We are called by KdInitSystem(0) -> KdDebuggerInitialize0 at kernel init.)
    It gets initialized later on between BootPhase 0 and 1.
    
    Also display a nice KDBG signon showing the status of symbols loading.
---
 ntoskrnl/kd/kdio.c          |  3 ++
 ntoskrnl/kdbg/kdb.h         |  2 +-
 ntoskrnl/kdbg/kdb_cli.c     | 23 +++++++++++++--
 ntoskrnl/kdbg/kdb_symbols.c | 69 ++++++++++++++++++++++-----------------------
 4 files changed, 57 insertions(+), 40 deletions(-)

diff --git a/ntoskrnl/kd/kdio.c b/ntoskrnl/kd/kdio.c
index be47e04c1c7..4ce92e3f076 100644
--- a/ntoskrnl/kd/kdio.c
+++ b/ntoskrnl/kd/kdio.c
@@ -229,6 +229,7 @@ KdpDebugLogInit(
         /* Register for later BootPhase 2 reinitialization */
         DispatchTable->KdpInitRoutine = KdpDebugLogInit;
 
+        /* Announce ourselves */
         HalDisplayString("   File log debugging enabled\r\n");
     }
     else if (BootPhase >= 2)
@@ -413,6 +414,7 @@ KdpSerialInit(
     }
     else if (BootPhase == 1)
     {
+        /* Announce ourselves */
         HalDisplayString("   Serial debugging enabled\r\n");
     }
 
@@ -526,6 +528,7 @@ KdpScreenInit(
         /* Take control of the display */
         KdpScreenAcquire();
 
+        /* Announce ourselves */
         HalDisplayString("   Screen debugging enabled\r\n");
     }
 
diff --git a/ntoskrnl/kdbg/kdb.h b/ntoskrnl/kdbg/kdb.h
index b606d4a5156..2ec084fbb13 100644
--- a/ntoskrnl/kdbg/kdb.h
+++ b/ntoskrnl/kdbg/kdb.h
@@ -171,7 +171,7 @@ KdbSymProcessSymbols(
     _Inout_ PLDR_DATA_TABLE_ENTRY LdrEntry,
     _In_ BOOLEAN Load);
 
-VOID
+BOOLEAN
 KdbSymInit(
     _In_ ULONG BootPhase);
 
diff --git a/ntoskrnl/kdbg/kdb_cli.c b/ntoskrnl/kdbg/kdb_cli.c
index aaebf573cf8..f114a862958 100644
--- a/ntoskrnl/kdbg/kdb_cli.c
+++ b/ntoskrnl/kdbg/kdb_cli.c
@@ -3964,6 +3964,9 @@ KdbInitialize(
     _In_ PKD_DISPATCH_TABLE DispatchTable,
     _In_ ULONG BootPhase)
 {
+    /* Saves the different symbol-loading status across boot phases */
+    static ULONG LoadSymbols = 0;
+
     if (BootPhase == 0)
     {
         /* Write out the functions that we support for now */
@@ -3993,12 +3996,26 @@ KdbInitialize(
         KeInitializeSpinLock(&KdpDmesgLogSpinLock);
     }
 
+    /* Initialize symbols support in BootPhase 0 and 1 */
     if (BootPhase <= 1)
     {
-        /* Initialize symbols support */
-        KdbSymInit(BootPhase);
+        LoadSymbols <<= 1;
+        LoadSymbols |= KdbSymInit(BootPhase);
     }
-    else if (BootPhase >= 2)
+
+    if (BootPhase == 1)
+    {
+        /* Announce ourselves */
+        CHAR buffer[60];
+        RtlStringCbPrintfA(buffer, sizeof(buffer),
+                           "   KDBG debugger enabled - %s\r\n",
+                           !(LoadSymbols & 0x2) ? "No symbols loaded" :
+                           !(LoadSymbols & 0x1) ? "Kernel symbols loaded"
+                                                : "Loading symbols");
+        HalDisplayString(buffer);
+    }
+
+    if (BootPhase >= 2)
     {
         /* I/O is now set up for disk access: Read KDB Data */
         NTSTATUS Status = KdbpCliInit();
diff --git a/ntoskrnl/kdbg/kdb_symbols.c b/ntoskrnl/kdbg/kdb_symbols.c
index 2ac4d29b30f..3222c40ce6d 100644
--- a/ntoskrnl/kdbg/kdb_symbols.c
+++ b/ntoskrnl/kdbg/kdb_symbols.c
@@ -337,9 +337,10 @@ KdbSymProcessSymbols(
  * @param[in]   BootPhase
  * Phase of initialization.
  *
- * @return  None.
+ * @return
+ * TRUE if symbols are to be loaded at this given BootPhase; FALSE if not.
  **/
-VOID
+BOOLEAN
 KdbSymInit(
     _In_ ULONG BootPhase)
 {
@@ -347,61 +348,55 @@ KdbSymInit(
 
     if (BootPhase == 0)
     {
-        PCHAR p1, p2;
+        PSTR CommandLine;
         SHORT Found = FALSE;
         CHAR YesNo;
 
-        /*
-         * Default symbols loading strategy:
-         * In DBG builds, load symbols only if we have 96MB of RAM or more.
-         * In REL builds, do not load them by default.
-         */
+        /* By default, load symbols in DBG builds, but not in REL builds */
 #if DBG
-        LoadSymbols = (MmNumberOfPhysicalPages >= 0x6000);
+        LoadSymbols = TRUE;
 #else
         LoadSymbols = FALSE;
 #endif
 
-        /* Check the command line for /LOADSYMBOLS, /NOLOADSYMBOLS,
-         * /LOADSYMBOLS={YES|NO}, /NOLOADSYMBOLS={YES|NO} */
+        /* Check the command line for LOADSYMBOLS, NOLOADSYMBOLS,
+         * LOADSYMBOLS={YES|NO}, NOLOADSYMBOLS={YES|NO} */
         ASSERT(KeLoaderBlock);
-        p1 = KeLoaderBlock->LoadOptions;
-        while ('\0' != *p1 && NULL != (p2 = strchr(p1, '/')))
+        CommandLine = KeLoaderBlock->LoadOptions;
+        while (*CommandLine)
         {
-            p2++;
+            /* Skip any whitespace */
+            while (isspace(*CommandLine))
+                ++CommandLine;
+
             Found = 0;
-            if (0 == _strnicmp(p2, "LOADSYMBOLS", 11))
+            if (_strnicmp(CommandLine, "LOADSYMBOLS", 11) == 0)
             {
                 Found = +1;
-                p2 += 11;
+                CommandLine += 11;
             }
-            else if (0 == _strnicmp(p2, "NOLOADSYMBOLS", 13))
+            else if (_strnicmp(CommandLine, "NOLOADSYMBOLS", 13) == 0)
             {
                 Found = -1;
-                p2 += 13;
+                CommandLine += 13;
             }
-            if (0 != Found)
+            if (Found != 0)
             {
-                while (isspace(*p2))
+                if (*CommandLine == '=')
                 {
-                    p2++;
-                }
-                if ('=' == *p2)
-                {
-                    p2++;
-                    while (isspace(*p2))
-                    {
-                        p2++;
-                    }
-                    YesNo = toupper(*p2);
-                    if ('N' == YesNo || 'F' == YesNo || '0' ==
YesNo)
+                    ++CommandLine;
+                    YesNo = toupper(*CommandLine);
+                    if (YesNo == 'N' || YesNo == '0')
                     {
                         Found = -1 * Found;
                     }
                 }
                 LoadSymbols = (0 < Found);
             }
-            p1 = p2;
+
+            /* Move on to the next option */
+            while (*CommandLine && !isspace(*CommandLine))
+                ++CommandLine;
         }
     }
     else if (BootPhase == 1)
@@ -411,13 +406,13 @@ KdbSymInit(
         KIRQL OldIrql;
         PLIST_ENTRY ListEntry;
 
-        /* Do not load symbols if we have less than 96MB of RAM */
-        if (MmNumberOfPhysicalPages < 0x6000)
+        /* Do not continue loading symbols if we have less than 96MB of RAM */
+        if (MmNumberOfPhysicalPages < (96 * 1024 * 1024 / PAGE_SIZE))
             LoadSymbols = FALSE;
 
         /* Continue this phase only if we need to load symbols */
         if (!LoadSymbols)
-            return;
+            return LoadSymbols;
 
         /* Launch our worker thread */
         InitializeListHead(&SymbolsToLoad);
@@ -433,7 +428,7 @@ KdbSymInit(
         {
             DPRINT1("Failed starting symbols loader thread: 0x%08x\n",
Status);
             LoadSymbols = FALSE;
-            return;
+            return LoadSymbols;
         }
 
         RosSymInitKernelMode();
@@ -450,6 +445,8 @@ KdbSymInit(
 
         KeReleaseSpinLock(&PsLoadedModuleSpinLock, OldIrql);
     }
+
+    return LoadSymbols;
 }
 
 /* EOF */