Closed
Bug 605011
Opened 14 years ago
Closed 14 years ago
"Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy),"
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
| Tracking | Status | |
|---|---|---|
| blocking2.0 | --- | beta8+ |
People
(Reporter: gkw, Assigned: gwagner)
References
Details
(Keywords: assertion, regression, testcase, Whiteboard: [sg:nse (bogus assertion)?] fixed-in-tracemonkey)
Attachments
(1 file, 1 obsolete file)
|
(deleted),
patch
|
igor
:
review+
|
Details | Diff | Splinter Review |
function g(x, n) {
for (var i = 0; i < n; ++i) {
x = {
a: x
};
}
return x;
}
d = g(0, 672);
(function() {
gczeal(2)(uneval(this))
})()
asserts js debug shell on TM changeset 47a8311cf0bb without -m or -j at Assertion failure: JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy),
|
Reporter | |
Comment 1•14 years ago
|
||
Also, locking s-s due to presence of gczeal in testcase.
autoBisect shows this is probably related to the following changeset:
The first bad revision is:
changeset: 54771:fd4510c77054
user: Gregor Wagner
date: Tue Oct 05 10:09:50 2010 -0700
summary: Bug 600310 - TM: don't perform GC outside of stack quota r=igor
Blocks: 600310
|
|
Reporter | |
Updated•14 years ago
|
Group: core-security
|
||
Updated•14 years ago
|
blocking2.0: ? → beta8+
This is the same as seen in bug 600139, an edge case where the assert is guaranteed to hit. Throwing an over-recursed error can cause a GC, and while you're overrecursed, you're probably beyond the stack limit.
I don't know whether it's a security issue or not though.
|
||
Comment 3•14 years ago
|
||
I think we should replace the assert from the bug 600139, with one that alows for few more kilobytes of the stack space. That is, the functions that sets the stack limit should make it slightly smaller so the GC will have an extra room after the error.
Gregor, do you have time to patch this?
|
Assignee | |
Updated•14 years ago
|
Assignee: general → anygregor
|
|
Assignee | |
Comment 4•14 years ago
|
||
This patch adds 4k to the stackLimit.
Another possibility would be to check if we are already in an over-recursion error.
Attachment #484378 -
Flags: review?(igor)
|
||
Updated•14 years ago
|
OS: Mac OS X → Windows 7
Whiteboard: [sg:nse (bogus assertion)?]
|
|
||
Updated•14 years ago
|
Attachment #484378 -
Flags: review?(igor) → review+
|
|
Assignee | |
Comment 5•14 years ago
|
||
It seems cx->stackLimit is not always set (jsapi-tests).
Attachment #484378 -
Attachment is obsolete: true
Attachment #484423 -
Flags: review?(igor)
|
|
||
Comment 6•14 years ago
|
||
Comment on attachment 484423 [details] [diff] [review]
patch
>diff -r 5bca7c8bd4c8 js/src/jsgc.cpp
>--- a/js/src/jsgc.cpp Tue Oct 19 11:39:55 2010 -0700
>+++ b/js/src/jsgc.cpp Tue Oct 19 12:49:16 2010 -0700
>@@ -2558,17 +2558,22 @@ js_GC(JSContext *cx, JSGCInvocationKind
> */
> if (rt->state != JSRTS_UP && gckind != GC_LAST_CONTEXT)
> return;
>
> RecordNativeStackTopForGC(cx);
>
> #ifdef DEBUG
> int stackDummy;
>- JS_ASSERT(JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy));
>+/* +-4k because it is possible to perform a GC during an overrecursion report. */
>+# if JS_STACK_GROWTH_DIRECTION > 0
>+ JS_ASSERT_IF(cx->stackLimit, JS_CHECK_STACK_SIZE(cx->stackLimit + 4096, &stackDummy));
For upward growing stacks cx->stackLimit is jsuword(-1) by default, the max value. r+ with that fixed.
Attachment #484423 -
Flags: review?(igor) → review+
|
|
Assignee | |
Comment 7•14 years ago
|
||
Whiteboard: [sg:nse (bogus assertion)?] → [sg:nse (bogus assertion)?] fixed-in-tracemonkey
|
|
||
Comment 8•14 years ago
|
||
(In reply to comment #7)
> http://hg.mozilla.org/tracemonkey/rev/0bb000635fbd
I was not clear in the comment 5. The suggestion was to change the assert into:
JS_ASSERT_IF(cx->stackLimit != jsuword(-1), JS_CHECK_STACK_SIZE(cx->stackLimit + 4096, &stackDummy));
|
|
Assignee | |
Comment 9•14 years ago
|
||
Yeah that makes more sense. Thx!
http://hg.mozilla.org/tracemonkey/rev/0ae037c29ab0
|
|
||
Comment 10•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
|
||
Updated•14 years ago
|
Group: core-security
|
||
Comment 11•12 years ago
|
||
Automatically extracted testcase for this bug was committed:
https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•