This is the mail archive of the cygwin-developers@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: The new Arthur "two queues" Jackson* implementation of signals


Christopher Faylor wrote:
> 
> >The stack picture at label 3 when starting the handler,
> >or just after the call to _set_process_mask@4 when
> >terminating the handler, is as follows:
> >
> >ret addr
> >old ebp        <= ebp
> >flags
> >6 registers
> >errno          <= esp
> >oldmask
> >signal arg
> >signal handler
> >
> >So in the recursion elimination code the return address must be
> >overwritten at ebp + 4 (my solution) or esp + 36 (your solution)
> >
> >However your code
> >        movl    %%esp,%%ebp
> >        movl    %%eax,36(%%ebp) # Restore return address
> >produces an ebp different from the one in the sketch above,
> >so when you return from the eliminated recursion the stack format
> >is wrong and a stack walk crashes. This only happens when
> >there are two consecutive recursions.
> >
> >Just leave ebp alone and change the two lines above to
> >        movl    %%eax,36(%%esp)  or %%eax,4(%%ebp)
> >(untested)
> 
> I don't think it is a good idea rely on the contents of ebp even though
> it probably has been correctly restored.  In your patch, you could use
> ebp before because it was set prior to calling _set_process_mask and
> then 32 was subtracted from it.  I eliminated that code because it
> wasn't doing anything.  That was, in fact, a previous abortive attempt
> to eliminate recursion, I think.

Right. Good idea to remove that. It took me a while to see it's indeed
correct.

> I'm not sure why clobbering ebp should have any effect on a subsequent
> program since AFAICT, it should eventually be restored correctly when
> the (nested) signal handler returns.  However, it does seem to cause
> problems.

Yes, "old ebp" is eventually restored. 
But the crash happens during the call to set_process_mask (during termination),
which saves the (incorrect) ebp on the stack. 
When an interrupt is pending the sigthread walks the stack and expects
the ebp's on the stack to point next to a return address. So ebp must be correct
when set_process_mask is called.
The fact that the first recursion works fine indicates that ebp 
is indeed correct at sigreturn time (otherwise the code you have eliminated 
would be useful).

>From memory the crash occurs on line  
sf.AddrReturn.Offset = (DWORD) *++ebp;

> I've modified my test case to deal with this scenario.  It seems
> to work fine with the 36 offset. 

Yes, I don't have a problem with that approach.

> For the curious, I've included my test case below.

No time now... You need 3 interrupts to have 2 recursions and
demonstrate the problem.


Pierre


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]