Last Comment Bug 684922 - Crash due to too much recursion through GetCustomIterator
: Crash due to too much recursion through GetCustomIterator
Status: VERIFIED FIXED
[sg:dos]
: crash, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- critical (vote)
: mozilla11
Assigned To: David Mandelin [:dmandelin]
:
Mentors:
: 706306 (view as bug list)
Depends on:
Blocks: jsfunfuzz 630996
  Show dependency treegraph
 
Reported: 2011-09-06 10:54 PDT by Gary Kwong [:gkw] [:nth10sd]
Modified: 2012-12-13 17:06 PST (History)
11 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
-
-


Attachments
stacks (8.67 KB, text/plain)
2011-09-06 10:54 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
Patch (1.16 KB, patch)
2011-11-28 15:22 PST, David Mandelin [:dmandelin]
luke: review+
Details | Diff | Splinter Review

Description Gary Kwong [:gkw] [:nth10sd] 2011-09-06 10:54:37 PDT
Created attachment 558528 [details]
stacks

q = {}.__proto__
function h() {
    q.p = q
}
h()
function t() {
    for each(let y in []) {
        y.__iterator__ = Iterator
        for (c in []) {}
    }
}
t()

crashes js debug shell on m-c changeset db9e99d537f2 with the patch from bug 683966 without any CLI arguments at js::PropertyTable::search and crashes js opt shell at PrimitiveToObject.

Based on the stack, this seems to be a recursive overflow, but locking s-s just to be safe.
Comment 1 Daniel Veditz [:dveditz] 2011-09-07 18:05:56 PDT
recursive stack exhaustion would be sg:dos, but the stacks you attached don't show that kind of error.
Comment 2 Gary Kwong [:gkw] [:nth10sd] 2011-09-07 22:43:32 PDT
(In reply to Daniel Veditz from comment #1)
> recursive stack exhaustion would be sg:dos, but the stacks you attached
> don't show that kind of error.

I should probably attach longer stacks next time, but they seem to repeatedly go on for thousands of lines.
Comment 3 Jesse Ruderman 2011-09-08 01:24:54 PDT
One way to make "too much recursion" explicit is to strip out all the distractions, like I did in bug 671797 ;)

I happen to know that 0x00007fff5f3ffffc is typical "guard page at the end of stack memory" on Mac.  It would be nice if gdb gave more information about the inaccessible memory page.
Comment 4 Jesse Ruderman 2011-09-08 01:30:27 PDT
By the way, in fuzzing/js-known/mozilla-central/assertions.txt, you can write

# Bug 684922
[TMR] GetCustomIterator

The "TMR" is special thing for too-much-recursion crashes. It instructs detect_interesting_crashes to only consider a crash report to be a match if the offending function is seen on 9+ lines.

You have to pick a function from the repeating portion of the stack, but that's saner than picking a function from the top.
Comment 5 Jesse Ruderman 2011-09-08 01:35:10 PDT
Simpler testcase:

var op = Object.prototype;
op.b = op;
op.__iterator__ = Iterator;
for (var c in {}) {}
Comment 6 Daniel Veditz [:dveditz] 2011-09-08 13:21:44 PDT
Ok to unhide the bug then?
Comment 7 Jesse Ruderman 2011-09-08 14:09:32 PDT
Yes.
Comment 8 Scoobidiver (away) 2011-11-26 06:07:02 PST
The js::PropertyTable::search crash signature is #1 top crasher on Mac OS X in 9.0b2.
Comment 9 Gary Kwong [:gkw] [:nth10sd] 2011-11-28 14:51:05 PST
(In reply to Scoobidiver from comment #8)
> The js::PropertyTable::search crash signature is #1 top crasher on Mac OS X
> in 9.0b2.

Given that we have simple testcases in comment 0 and comment 5, plus it being a topcrasher (assuming the correlation), could someone please kindly take a look?

Nominating for tracking from versions 9 onwards.
Comment 10 David Mandelin [:dmandelin] 2011-11-28 15:22:16 PST
Created attachment 577397 [details] [diff] [review]
Patch

This is an infinite recursion bug. The cycle goes through GetCustomIterator, which seems like a reasonable place to put a JS_CHECK_RECURSION.

I'd be pretty surprised if this had an effect on the topcrashes, but I wouldn't mind being lucky.
Comment 11 David Mandelin [:dmandelin] 2011-11-28 15:59:38 PST
http://hg.mozilla.org/integration/mozilla-inbound/rev/720247db9dc3

I don't think we need to track this bug, but we should check out the crashstats after it lands.
Comment 12 David Mandelin [:dmandelin] 2011-11-28 17:00:09 PST
Followup fix: http://hg.mozilla.org/integration/mozilla-inbound/rev/ff1c18668452
Comment 14 Christian Holler (:decoder) 2011-11-29 15:55:52 PST
*** Bug 706306 has been marked as a duplicate of this bug. ***
Comment 15 Christian Holler (:decoder) 2011-11-29 16:26:34 PST
Do we want to fix this on Aurora as well?
Comment 16 Jesse Ruderman 2011-11-29 17:06:11 PST
(In reply to Scoobidiver from comment #8)
> The js::PropertyTable::search crash signature is #1 top crasher on Mac OS X
> in 9.0b2.

I looked at some of the js::PropertyTable::search crash reports and none of the stacks showed deep recursion. So the topcrash is not this bug.
Comment 17 Gary Kwong [:gkw] [:nth10sd] 2011-11-29 17:11:19 PST
fwiw I put the testcase in comment 5 in a <script> tag and opt Firefox crashed [@ PrimitiveToObject ] - the js::PropertyTable::search signature I got was off a debug shell.
Comment 18 Gary Kwong [:gkw] [:nth10sd] 2012-12-13 17:06:42 PST
A type of test for this bug has already been landed because it is already marked in-testsuite+ -> VERIFIED.

Note You need to log in before you can comment on or make changes to this bug.