New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duktape C stack size gets too large reporting some errors #1994

Open
kinnison opened this Issue Nov 4, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@kinnison
Copy link

kinnison commented Nov 4, 2018

Hi,

In NetSurf we had an interesting bug report that a site was causing our browser to segfault/abort and die. We tracked it down to a piece of JavaScript likely using a browser feature we hadn't implemented yet, but the abort was seen in trying to report the error case to the log. Eventually we discovered that the problem was simply that the C stack size on our test computer was 8MB and that in order to hit the C call stack depth limit we needed 10 MB. With that in place, we get the following logged error:

js_exec: Failed to execute JavaScript
js_exec: Fetching name
js_exec: Success fetching name
js_exec: Fetching message
js_exec: Success fetching message
js_exec: Fetching fileName
js_exec: Error fetching fileName: RangeError: C call stack depth limit
js_exec: Fetching lineNumber
js_exec: Error fetching lineNumber: RangeError: C call stack depth limit
js_exec: Fetching stack
js_exec: Error fetching stack: RangeError: C call stack depth limit
js_exec: Uncaught error in JS: ReferenceError: identifier 'Number' undefined
js_exec:               was at: RangeError: C call stack depth limit line RangeError: C call stack depth limit
js_exec:          Stack trace: RangeError: C call stack depth limit

As you can see, while that helps us a little (we can look at implementing/adding in the Number API) it indicates that the default value of 1000 of DUK_USE_NATIVE_CALL_RECLIMIT is too high for an 8MB stack. We'll be dropping that in our builds if we can't find a nicer way to deal with this.

What's worrying though is that reporting this required such recursion that the C call stack limit was reached at all. The JS in question is a minified polyfill of some kind (test files are attached to our bug tracker but likely won't be very helpful for you since you need the DOM etc or it won't do anything interesting).

Is there anything we can do to help you track down how/why this is happening? Anything we can do to protect against such problems in the future?

Thanks,

Daniel.

https://bugs.netsurf-browser.org/mantis/view.php?id=2626

@svaarala

This comment has been minimized.

Copy link
Owner

svaarala commented Nov 4, 2018

It's relatively difficult to safeguard reliably against the native stack in a portable manner: there is no standard portable mechanism of knowing stack limits, there is no easy way to know the maximum stack frame size in the worst case recursion scenario, etc.

The best you can do for now is reduce the maximum C stack depth to a value which is safe in practice. To do this correctly one would need to know the maximum stack growth for each C recursion step - but because this ultimately depends on the compiler, it's hard to know reliably (some compilers even support split stacks).

Going forward, I'll be adding a feature where you can register a stack check macro in tools/configure.py. The idea is that Duktape will from time to time call that macro to check if there's enough native stack left to continue. This would be done every place where the C recursion depth is now increased, but also in any other places where significant stack growth happens.

This check will be more accurate because it doesn't rely on a fixed C recursion depth limit; rather the application can check that there's e.g. at least 4kB free stack before allowing Duktape to recurse further. The check itself will need to be unportable and provided by the application.

As for why native recursion happens, it is easy to arrange even from pure Ecmascript code. For example, an Array .forEach() is implemented as a Duktape/C function and will consume native stack. By doing recursive .forEach() calls one can easily exhaust the native stack from pure Ecmascript code. I'm not sure what's happening here specifically however.

@svaarala

This comment has been minimized.

Copy link
Owner

svaarala commented Nov 4, 2018

@kinnison I've pushed my unfinished work for the dynamic stack check macro in #1995. It's intended for 2.4.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment