On 11 December 2013 14:02, <[email protected]> wrote: > On 2013/12/11 12:16:03, rossberg wrote: >> >> On 2013/12/11 11:56:29, titzer wrote: >> > On 2013/12/11 11:50:32, rossberg wrote: >> > > https://codereview.chromium.org/110913004/diff/1/src/typing.cc >> > > File src/typing.cc (left): >> > > >> > > >> > > https://codereview.chromium.org/110913004/diff/1/src/typing.cc#oldcode53 >> > > src/typing.cc:53: if (visitor->HasStackOverflow()) return; \ >> > > Hm, is this really correct? It means that you do not bail out right >> > > away >> after >> > > detecting a stack overflow, but instead each iteration will continue >> trucking >> > > along until it either returns or performs the next attempt to do a > > recursive >> >> > > call. >> > > >> > > If you view stack overflow as an exception-like condition, that isn't > > quite >> >> > the >> > > right behaviour, and it seems fishy. In particular, consecutive code >> > > could >> > > happily try to call other auxiliary functions (with no stack check), >> although >> > > the stack space is already used up. >> > >> > Maybe we should check before and after then. I saw a crasher that failed >> > the >> > ASSERT that was there previously. I think it makes sense to do a check > > before, >> >> > otherwise one could recurse down the first child all the way without > > noticing >> >> > the overflow yet. > > >> Hm, if there can be early stack overflow conditions without the function > > having >> >> bailed out already then the same bad scenario could happen _before_ the >> first >> recursive call. > > What bad scenarios are you envisioning? Incorrect type information on the > AST? > > My understanding was that the check was to terminate the recursion, so as to > at > least avoid overflowing the C++ stack, and that any "bad" things that have > happened would already have signaled a bailout in the state of the visitor.
If the current assertion can ever fail, then only because a stack overflow must have been detected (and the condition set) by some function other than those of the current visitor. The bug must then be that whatever call (in the visitor) is leading to this function is not accompanied by a consecutive check of the stack condition. All function calls after this one are potentially fatal, not just recursive visitor calls. So AFAICS, merely guarding all recursive calls does only fix a symptom, but likely not the real bug. For which call did you see the assertion fail exactly? >> So I don't think doing a check beforehand fixes the actual bug (rather, it > > hides >> >> it) -- which seems to be that we do some other call(s) that can detect a >> stack >> overflow without checking for that condition after the call(s). We should >> find >> out which calls these are. > > >> It would be great if we could somehow instrument the language to flag >> missing >> checks, but I don't have a good idea off-hand. Maybe returning a > > MUST_USE_RESULT >> >> of some auxiliary type with the check being the only way to consume that >> type. >> But that would be tedious for functions that also want to return a real > > result. > > > > https://codereview.chromium.org/110913004/ > > -- > -- > v8-dev mailing list > [email protected] > http://groups.google.com/group/v8-dev > ---You received this message because you are subscribed to the Google Groups > "v8-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/groups/opt_out. -- -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/groups/opt_out.
