Yes, I'm sure. Consider the case when a page fault occurs. The page fault handler can choose to map the page the instruction attempted to write to and then retry the instruction. Now, if ESP was already lowered, it will be lowered again, which is wrong. In fact, this patch fixes the linux kernel fork() copy on write problem (when running linux under NTVDM).
Regards, Alex On Fri, Nov 21, 2014 at 11:55:18PM +0100, Timo Kreuzer wrote: > Are you sure this is the correct order of execution? The instruction is > documented as first lowering the stack pointer, then writing to the new > location. > If a stack exception occurs, you might have esp off by 4. > > > Am 21.11.2014 10:46, schrieb aandreje...@svn.reactos.org: > >Author: aandrejevic > >Date: Fri Nov 21 09:46:57 2014 > >New Revision: 65441 > > > >URL: http://svn.reactos.org/svn/reactos?rev=65441&view=rev > >Log: > >[FAST486] > >Attempt to write the value to the stack before subtracting from the actual > >ESP register > >in Fast486StackPush. > > > > > >Modified: > > trunk/reactos/lib/fast486/common.inl > > > >Modified: trunk/reactos/lib/fast486/common.inl > >URL: > >http://svn.reactos.org/svn/reactos/trunk/reactos/lib/fast486/common.inl?rev=65441&r1=65440&r2=65441&view=diff > >============================================================================== > >--- trunk/reactos/lib/fast486/common.inl [iso-8859-1] (original) > >+++ trunk/reactos/lib/fast486/common.inl [iso-8859-1] Fri Nov 21 > >09:46:57 2014 > >@@ -317,15 +317,19 @@ > > return FALSE; > > } > >+ /* Store the value in SS:[ESP - 4] */ > >+ if (!Fast486WriteMemory(State, > >+ FAST486_REG_SS, > >+ State->GeneralRegs[FAST486_REG_ESP].Long - > >sizeof(ULONG), > >+ &Value, > >+ sizeof(ULONG))) > >+ { > >+ /* Exception occurred */ > >+ return FALSE; > >+ } > >+ > > /* Subtract ESP by 4 */ > > State->GeneralRegs[FAST486_REG_ESP].Long -= sizeof(ULONG); > >- > >- /* Store the value in SS:ESP */ > >- return Fast486WriteMemory(State, > >- FAST486_REG_SS, > >- State->GeneralRegs[FAST486_REG_ESP].Long, > >- &Value, > >- sizeof(ULONG)); > > } > > else > > { > >@@ -339,16 +343,22 @@ > > return FALSE; > > } > >+ /* Store the value in SS:[SP - 2] */ > >+ if (!Fast486WriteMemory(State, > >+ FAST486_REG_SS, > >+ > >LOWORD(State->GeneralRegs[FAST486_REG_ESP].LowWord - sizeof(USHORT)), > >+ &ShortValue, > >+ sizeof(USHORT))) > >+ { > >+ /* Exception occurred */ > >+ return FALSE; > >+ } > >+ > > /* Subtract SP by 2 */ > > State->GeneralRegs[FAST486_REG_ESP].LowWord -= sizeof(USHORT); > >- > >- /* Store the value in SS:SP */ > >- return Fast486WriteMemory(State, > >- FAST486_REG_SS, > >- > >State->GeneralRegs[FAST486_REG_ESP].LowWord, > >- &ShortValue, > >- sizeof(USHORT)); > >- } > >+ } > >+ > >+ return TRUE; > > } > > FORCEINLINE > > > > > > > > > _______________________________________________ > Ros-dev mailing list > Ros-dev@reactos.org > http://www.reactos.org/mailman/listinfo/ros-dev -- Alexander Andrejevic <thefl...@sdf.lonestar.org> SDF Public Access UNIX System - http://sdf.lonestar.org _______________________________________________ Ros-dev mailing list Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
