Closed
Bug 631082
Opened 14 years ago
Closed 14 years ago
Assertion failure: fp && fp->isScriptFrame(), at ../jscntxt.h:1830 @findFrameAtLevel with generator on the stack
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: jorendorff, Assigned: dmandelin)
References
(Blocks 1 open bug)
Details
(Keywords: assertion, regression, testcase, Whiteboard: [sg:critical?][hardblocker][fixed-in-tracemonkey][has patch])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
var t;
(function () {
t = (function() {
yield k();
})();
function h() {
}
function k() {
return function() { h(); };
}
})();
t.next();
Assertion failure: fp && fp->isScriptFrame(), at ../jscntxt.h:1830
The compiler believes that neither k nor its caller can escape the enclosing function, and therefore any time k is running, the enclosing function is on the stack. This is wrong because k's caller is a generator-function, and the generator-iterator does escape.
Reporter | ||
Comment 1•14 years ago
|
||
In an opt build, this test case would crash in findFrameAtLevel() near null. But you could easily arrange for findFrameAtLevel() to find a frame with the appropriate level. Having accomplished that you could probably use the code to read arbitrary slots from the js operand stack and possibly past the end of it. I'm not sure it's exploitable but it doesn't seem good; therefore setting the s-g bit.
Updated•14 years ago
|
Blocks: about:startup
Reporter | ||
Comment 2•14 years ago
|
||
(Btw, db48x ran across this while writing actual code to do something useful. The test case in comment 0 is reduced from about:startup, bug 593743, which this bug now blocks.)
Updated•14 years ago
|
blocking2.0: --- → betaN+
Whiteboard: [sg:critical?][hardblocker]
Assignee | ||
Comment 3•14 years ago
|
||
Well, this fixes this particular test case. I'm not sure it's a complete fix, though, partly because I don't understand the code I patched and partly because it seems to have relied on TCF_FUN_HEAVYWEIGHT being propagated outward, and I assume TCF_FUN_IS_GENERATOR is not similarly propagated.
Assignee | ||
Comment 4•14 years ago
|
||
I think the v(-1) patch is actually correct. This is just that patch plus the test case. Argument for why it works:
The basic idea is to assume that generator functions always escape, as a conservative approximation. (I figured it would be hard to teach the analysis to track down exactly where such functions are called, and then see exactly where that result flowed, so it would be better to say they always escape.)
From there, the existing analysis does the rest, given the point where I encoded that assumption. That point marks the generator function and all its kids (immediately enclosed functions) as escaping. In general, when the analysis marks any function |f| as escaping, it also marks as escaping any functions that are defined or referred to inside |f|.
In the test case, this plays out as follows:
1. We the lambda inside k as escaping (it gets processed first) and enqueue it.
2. We mark the generator as escaping and enqueue it.
3. When we dequeue the lambda, we mark h as escaping because h is referred to inside the lambda.
4. When we dequeue the generator, we mark k as escaping because k is referred to inside the lambda.
Assignee: general → dmandelin
Attachment #509635 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #509654 -
Flags: review?(jorendorff)
Updated•14 years ago
|
Whiteboard: [sg:critical?][hardblocker] → [sg:critical?][hardblocker][has patch]
Comment 5•14 years ago
|
||
Not sure how reliable this is, but autoBisect (on windows) pinpoints:
Due to skipped revisions, the first bad revision could be any of:
changeset: 51604:e5958cd4a135
user: Brendan Eich
date: Sun Aug 29 11:57:08 2010 -0700
summary: Merge JSScope into JSObject and JSScopeProperty (now js::Shape; bug 558451, r=jorendorff).
changeset: 52694:d575f16c7f55
parent: 52693:ff1a35b9e9b8
parent: 51604:e5958cd4a135
user: David Mandelin
date: Mon Aug 30 15:13:32 2010 -0700
summary: [JAEGER] Merge from Tracemonkey.
changeset: 52695:01a86ce240da
user: David Anderson
date: Mon Aug 30 15:17:18 2010 -0700
summary: [JAEGER] Silence GCC warning about signed integer comparisons.
Comment 6•14 years ago
|
||
Comment on attachment 509654 [details] [diff] [review]
v1 (just adds test case)
Add a short para about generators to the comment, and r=me.
/be
Attachment #509654 -
Flags: review?(jorendorff) → review+
Assignee | ||
Comment 7•14 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/a46dd602c30d
(with added comment.)
Whiteboard: [sg:critical?][hardblocker][has patch] → [sg:critical?][hardblocker][fixed-in-tracemonkey]
Updated•14 years ago
|
Whiteboard: [sg:critical?][hardblocker][fixed-in-tracemonkey] → [sg:critical?][hardblocker][fixed-in-tracemonkey][has patch]
Comment 8•14 years ago
|
||
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/a46dd602c30d
Updated•14 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•