Runner-up to Arty's patch of the year. Best regards, Alex Ionescu
On Sun, Apr 29, 2012 at 3:00 AM, <[email protected]> wrote: > Author: tkreuzer > Date: Sat Apr 28 20:00:09 2012 > New Revision: 56442 > > URL: http://svn.reactos.org/svn/reactos?rev=56442&view=rev > Log: > [PSEH] > Fix a serious bug in PSEH that could lead to stack corruption and was causing > CSRSS to crash, when large amounts of text were output on the console. > Background: PSEH used __builtin_alloca to allocate an SEH registration frame > on the stack on demand, ie only for the first try level. But there are some > nasty things with __builtin_alloca: First it DOES NOT - as one might think - > free the allocated memory, once the allocation "goes out of scope", like with > local variables, but only on function exit. Therefore it cannot normally be > used inside a loop. The trick that PSEH used to "fix" this problem, was to > save the stack pointer and reset it back at the end. This is quite > problematic, since the rest of the code might assume that the stack pointer > is still where it was left off after the allocation. The other thing is that > __builtin_alloca() can allocate the memory whenever it likes to. It can > allocate everything on function entry or not allocate anything at all, when > other stack variables that went out of scope have left enough space to be > reused. In csrss it now happened that the allocation was done before the > stack pointer was saved, > so the memory could not be freed by restoring the old stack pointer value. > That lead to slowly eating up the stack since the code was inside a loop. > The allocation is now replaced with a variable sized array. The variable > stays in scope until the _SEH2_END and will be automaticall cleaned up by the > compiler. This also makes saving and restoring the stack pointer obsolete. > Reliability++ > > Modified: > trunk/reactos/include/reactos/libs/pseh/pseh2.h > > Modified: trunk/reactos/include/reactos/libs/pseh/pseh2.h > URL: > http://svn.reactos.org/svn/reactos/trunk/reactos/include/reactos/libs/pseh/pseh2.h?rev=56442&r1=56441&r2=56442&view=diff > ============================================================================== > --- trunk/reactos/include/reactos/libs/pseh/pseh2.h [iso-8859-1] (original) > +++ trunk/reactos/include/reactos/libs/pseh/pseh2.h [iso-8859-1] Sat Apr 28 > 20:00:09 2012 > @@ -192,7 +192,6 @@ > if(_SEHTopTryLevel) \ > { \ > _SEH2LeaveFrame(); \ > - __asm__ __volatile__("mov %0, %%esp" : : "g" > (_SEHStackPointer)); \ > } > > #define __SEH_END_SCOPE_CHAIN \ > @@ -219,16 +218,14 @@ > static const int _SEH2ScopeKind = 0; \ > volatile _SEH2TryLevel_t _SEHTryLevel; \ > volatile _SEH2HandleTryLevel_t _SEHHandleTryLevel; \ > - void * _SEHStackPointer; \ > + _SEH2Frame_t _SEH2Frame[_SEHTopTryLevel ? 1 : 0]; \ > volatile _SEH2TryLevel_t * _SEH2TryLevelP; \ > _SEH2Frame_t * const _SEH2FrameP = _SEHTopTryLevel ? \ > - ({ __asm__ __volatile__("mov %%esp, %0" : > "=g" (_SEHStackPointer)); __builtin_alloca(sizeof(_SEH2Frame_t)); }) : \ > - _SEHCurFrameP; \ > + _SEH2Frame : _SEHCurFrameP; \ > \ > (void)_SEH2ScopeKind; \ > (void)_SEHTryLevel; \ > (void)_SEHHandleTryLevel; \ > - (void)_SEHStackPointer; \ > (void)_SEH2FrameP; \ > (void)_SEH2TryLevelP; \ > \ > > _______________________________________________ Ros-dev mailing list [email protected] http://www.reactos.org/mailman/listinfo/ros-dev
