[greatlrd] 27618: patch from w3seek ------------------ Don't mess with the stack in inline assembly because it cannot be assumed that the compiler magically knows where the local variables are located after changing the stack pointer manually. See issue #2364 for more details.

Author: greatlrd
Date: Thu Jul 12 23:48:27 2007
New Revision: 27618

URL: http://svn.reactos.org/svn/reactos?rev=27618&view=rev
Log:
patch from w3seek
------------------
Don't mess with the stack in inline assembly because it cannot be
assumed that the compiler magically knows where the local variables are located
after changing the stack pointer manually.


See issue #2364 for more details.

Added:
    trunk/reactos/ntoskrnl/ke/i386/init.S   (with props)
Modified:
    trunk/reactos/ntoskrnl/ke/i386/kiinit.c
    trunk/reactos/ntoskrnl/ntoskrnl.rbuild

Added: trunk/reactos/ntoskrnl/ke/i386/init.S
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/init.S?re…
==============================================================================
--- trunk/reactos/ntoskrnl/ke/i386/init.S (added)
+++ trunk/reactos/ntoskrnl/ke/i386/init.S Thu Jul 12 23:48:27 2007
@@ -1,0 +1,39 @@
+/*
+ * FILE:            ntoskrnl/ke/i386/init.S
+ * COPYRIGHT:       See COPYING in the top level directory
+ * PURPOSE:         Kernel Initialization
+ * PROGRAMMER:      Thomas Weidenmueller <w3seek(a)reactos.org>
+ */
+
+/* INCLUDES ******************************************************************/
+
+#include <asm.h>
+#include <internal/i386/asmmacro.S>
+.intel_syntax noprefix
+
+/* FUNCTIONS ******************************************************************/
+
+.text
+.globl _KiSetupStackAndInitializeKernel@24
+.func KiSetupStackAndInitializeKernel@24
+_KiSetupStackAndInitializeKernel@24:
+
+    mov esi, esp
+
+    /* Setup the new stack */
+    mov esp, [esp + 12]
+    sub esp, NPX_FRAME_LENGTH + KTRAP_FRAME_ALIGN + KTRAP_FRAME_LENGTH
+    push CR0_EM + CR0_TS + CR0_MP
+
+    /* Copy all parameters to the new stack */
+    push [esi + 24]
+    push [esi + 20]
+    push [esi + 16]
+    push [esi + 12]
+    push [esi + 8]
+    push [esi + 4]
+    xor ebp, ebp
+    call _KiInitializeKernel@24
+
+    jmp _KiSystemStartupFinal@0
+.endfunc

Propchange: trunk/reactos/ntoskrnl/ke/i386/init.S
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: trunk/reactos/ntoskrnl/ke/i386/kiinit.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/kiinit.c?…
==============================================================================
--- trunk/reactos/ntoskrnl/ke/i386/kiinit.c (original)
+++ trunk/reactos/ntoskrnl/ke/i386/kiinit.c Thu Jul 12 23:48:27 2007
@@ -11,6 +11,16 @@
 #include <ntoskrnl.h>
 #define NDEBUG
 #include <debug.h>
+
+
+VOID
+NTAPI
+KiSetupStackAndInitializeKernel(IN PKPROCESS InitProcess,
+                                IN PKTHREAD InitThread,
+                                IN PVOID IdleStack,
+                                IN PKPRCB Prcb,
+                                IN CCHAR Number,
+                                IN PLOADER_PARAMETER_BLOCK LoaderBlock);
 
 /* GLOBALS *******************************************************************/
 
@@ -752,28 +762,32 @@
     KfRaiseIrql(HIGH_LEVEL);
 
     /* Align stack and make space for the trap frame and NPX frame */
-    InitialStack &= -KTRAP_FRAME_ALIGN;
-#ifdef __GNUC__
-    __asm__ __volatile__("xorl %ebp, %ebp");
-    __asm__ __volatile__("movl %0,%%esp" : :"r" (InitialStack));
-    __asm__ __volatile__("subl %0,%%esp" : :"r" (NPX_FRAME_LENGTH +
-                                                 KTRAP_FRAME_LENGTH +
-                                                 KTRAP_FRAME_ALIGN));
-    __asm__ __volatile__("push %0" : :"r" (CR0_EM + CR0_TS +
CR0_MP));
-#else
-    __asm xor ebp, ebp;
-    __asm mov esp, InitialStack;
-    __asm sub esp, NPX_FRAME_LENGTH + KTRAP_FRAME_ALIGN + KTRAP_FRAME_LENGTH;
-    __asm push CR0_EM + CR0_TS + CR0_MP;
-#endif
-
-    /* Call main kernel initialization */
-    KiInitializeKernel(&KiInitialProcess.Pcb,
-                       InitialThread,
-                       (PVOID)InitialStack,
-                       (PKPRCB)__readfsdword(KPCR_PRCB),
-                       (CCHAR)Cpu,
-                       KeLoaderBlock);
+    InitialStack &= ~(KTRAP_FRAME_ALIGN - 1);
+
+    /* NOTE: We cannot setup the stack using inline assembly and then later assume
+             that the compiler is smart enough to figure out how the stack layout
+             changed! This is to avoid generating wrong code. We cannot directly
+             call KiInitializeKernel from here! */
+
+    KiSetupStackAndInitializeKernel(&KiInitialProcess.Pcb,
+                                    InitialThread,
+                                    (PVOID)InitialStack,
+                                    (PKPRCB)__readfsdword(KPCR_PRCB),
+                                    (CCHAR)Cpu,
+                                    KeLoaderBlock);
+
+    /* NOTE: KiSetupStackAndInitializeKernel never returns! Do NOT add any code here! */
+    ASSERT(FALSE);
+}
+
+VOID
+NTAPI
+KiSystemStartupFinal(VOID)
+{
+    /* NOTE: This routine is called after setting up the stack in KiSystemStartup!
+             This code cannot be moved to KiSystemStartup because it cannot be assumed
+             that the compiler can generate working code after modifying ESP/EBP
+             using inline assembly! */
 
     /* Set the priority of this thread to 0 */
     KeGetCurrentThread()->Priority = 0;

Modified: trunk/reactos/ntoskrnl/ntoskrnl.rbuild
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ntoskrnl.rbuild?r…
==============================================================================
--- trunk/reactos/ntoskrnl/ntoskrnl.rbuild (original)
+++ trunk/reactos/ntoskrnl/ntoskrnl.rbuild Thu Jul 12 23:48:27 2007
@@ -36,6 +36,7 @@
                 <file>cpu.c</file>
                 <file>ctxswitch.S</file>
                 <file>exp.c</file>
+                <file>init.S</file>
                 <file>irqobj.c</file>
                 <file>kiinit.c</file>
                 <file>ldt.c</file>