xiaoxiang781216 commented on pull request #3471: URL: https://github.com/apache/incubator-nuttx/pull/3471#issuecomment-816379767
> @xiaoxiang781216 I am trying to review it..... > > if this `(sp < istackbase && sp >= istackbase - istacksize)` is `(sp < istackTOP && sp >= istackTOP - istacksize)` > Yes, itstackbase equal istackTOP, the valid stack range is [istackTOP - istacksize, istackTOP) > Then I think the change is wrong. > based on > > > Given FD-DB-IA - A test for a valid Stack would be clear: > > 'sp > lowerbound && sp <= upperbound` No, the valid range is `sp >= lowerbound && sp < upperbound` since local variables is accessed by sp[-offset] for FD stack where offset is always a negative number. Note: I assume the stack range is [lowerbound, upperbound) > > 'lowerbound == sp - is invalid as a push (DB) at this point would be out of bounds. It is valid if in the context of a handler that can NOT call deeper but will ONLY return pop (IA) Yes, it is invalid to allocate(sub sp) more local variables or make more function call, but it's valid in the current funciton context. So it's correct at this point: no stack overflow. > > upperbound` == sp - is valid if the stake is unused, as push is DB. It is invalid the context of a handler that will ONLY return pop (IA) But, the condition(the stack is unused) is impossible once the thread is running. On the other hand, it isn't only invalid at the return, but also invalid to access all local variables which's address equal or higher than upperbound. > > The correct test is: (sp <= istackTOP && sp > istackTOP - istacksize) > > I am missing something or do you agree? I still think the correct test is: (sp < istackTOP && sp >= istackTOP - istacksize). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
