On Mar 14 21:52, Takashi Yano via Cygwin wrote:
> On Fri, 14 Mar 2025 13:19:28 +0100
> Corinna Vinschen wrote:
> > On Mar 14 20:35, Takashi Yano via Cygwin wrote:
> > > On Fri, 14 Mar 2025 11:01:25 +0100
> > > Corinna Vinschen wrote:
> > > > I don't think so. I was mulling in circles over this tonight
> > > > (don't ask me how I slept!) and came to the same conclusion.
> > > > But here's the problem:
> > > >
> > > > I'm simply not 100% sure.
> > > >
> > > > What concerns me is that stackptr points beyond stack if the stack
> > > > is full (i.e., sigdelayed + return address).
> > > >
> > > > That was what happened before I applied a942476236b5: stackptr was
> > > > incremented until it pointed at _cygtls::initialized, and eventually it
> > > > overwrote it. Fortunately, that stopped further incrementing due to the
> > > > isinitialized() test.
> > > >
> > > > So, if there *is* a twisted situation which results in pushing another
> > > > return address onto the stack, a stack size of 2 would again result in
> > > > initialized being overwritten. So I wonder if we should keep kind of
> > > > an airbag for an unusual situation. Plus trying to keep stackptr inside
> > > > stack even if it's full. So that stackptr never grows into initialized:
> > > >
> > > > #define TLS_STACK_SIZE 5
> > > >
> > > > and
> > > >
> > > > void push (__tlsstack_t addr)
> > > > {
> > > > if (stackptr < (__tlsstack_t *) &initialized)
> > > > *stackptr++ = (__tlsstack_t) addr;
> > > > }
> > > >
> > > > What do you think?
> > >
> > > Yeah. We do not have to minimize the stack space at the cost of
> > > taking risks.
> > >
> > > One more thing. I am also concerned that pop() lacks a guard.
> > > If pop() calls when stack is empty, then push() destroys the
> > > stackptr pointer value.
> >
> > Good point. I attach a new proposal. It also doesn't check against
> > &initialized (becasue that doesn't avoid an overflow into initialized,
> > but against the last slot of the stack. This also moves pop into
> > the C++ code and drops its assembler counterpart.
> >
> > Ok?
>
> LGTM. Thanks!
Pushed. I didn't create a test release, because I was sure you
would push your other signal patches ASAP anyway.
Thanks,
Corinna
--
Problem reports: https://cygwin.com/problems.html
FAQ: https://cygwin.com/faq/
Documentation: https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple
