Closed
Bug 546615
Opened 15 years ago
Closed 15 years ago
Crash [@ BindNameToSlot] or "Assertion failure: cg->staticLevel >= level, at ../jsemit.cpp"
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
People
(Reporter: gkw, Assigned: jorendorff)
References
Details
(4 keywords, Whiteboard: [ccbr][sg:dos] fixed-in-tracemonkey)
Crash Data
Attachments
(2 files)
302 bytes,
text/plain
|
Details | |
2.51 KB,
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
function spaces(n) {
if (n == 0) return ""
if (n == 1) return " "
var w = spaces(Math.floor(n / 2));
return w + w + spaces(n % 2)
}
s = "if(<y {l}={(0)[(*\n)]}/>)/**/if(x(function(){})(x for(x in x))){}";
s = s.replace("/**/", "/*" + spaces(228) + "*/");
eval(s);
crashes js opt build at BindNameToSlot near null on TM tip without -j, and asserts js debug build at Assertion failure: cg->staticLevel >= level, at ../jsemit.cpp:2171 on TM tip without -j.
Turning security-sensitive because I'm not sure how bad this is, though it's crashing near null.
autoBisect shows this is probably related to bug 452498:
The first bad revision is:
changeset: 26784:2cf0bbe3772a
user: Brendan Eich
date: Sun Apr 05 21:17:22 2009 -0700
summary: upvar2, aka the big one take 2 (452498, r=mrbkap).
js shell opt crash stack:
Exception Type: EXC_BAD_ACCESS (SIGBUS)
Exception Codes: KERN_PROTECTION_FAILURE at 0x000000000000000c
Crashed Thread: 0 Dispatch queue: com.apple.main-thread
Thread 0 Crashed: Dispatch queue: com.apple.main-thread
0 js-opt-32-tm-darwin 0x000325b7 BindNameToSlot(JSContext*, JSCodeGenerator*, JSParseNode*) + 663
1 js-opt-32-tm-darwin 0x0003442c EmitNameOp(JSContext*, JSCodeGenerator*, JSParseNode*, int) + 28
2 js-opt-32-tm-darwin 0x0003baeb js_EmitTree + 23707
3 js-opt-32-tm-darwin 0x00038838 js_EmitTree + 10728
4 js-opt-32-tm-darwin 0x00036bb6 js_EmitTree + 3430
5 js-opt-32-tm-darwin 0x0003a4ea js_EmitTree + 18074
6 js-opt-32-tm-darwin 0x000a35ae JSCompiler::compileScript(JSContext*, JSObject*, JSStackFrame*, JSPrincipals*, unsigned int, unsigned short const*, unsigned long, __sFILE*, char const*, unsigned int, JSString*, unsigned int) + 990
7 js-opt-32-tm-darwin 0x00073e4a obj_eval(JSContext*, JSObject*, unsigned int, long*, long*) + 2330
8 js-opt-32-tm-darwin 0x0005fdbc js_Invoke + 1180
9 js-opt-32-tm-darwin 0x0004fe51 js_Interpret + 2209
10 js-opt-32-tm-darwin 0x0005f5d1 js_Execute + 625
11 js-opt-32-tm-darwin 0x0000d7bc JS_ExecuteScript + 60
12 js-opt-32-tm-darwin 0x00004658 Process(JSContext*, JSObject*, char*, int) + 1336
13 js-opt-32-tm-darwin 0x000085e6 main + 1734
14 js-opt-32-tm-darwin 0x000024fd _start + 208
15 js-opt-32-tm-darwin 0x0000242c start + 40
![]() |
Reporter | |
Comment 1•15 years ago
|
||
Yes, this affects 1.9.1 and 1.9.2 js shell builds, and I could also use this to make a testcase that crashes Firefox 3.6 on WinXP SP3.
OS: Mac OS X → All
Hardware: x86 → All
![]() |
Reporter | |
Comment 2•15 years ago
|
||
![]() |
Reporter | |
Comment 3•15 years ago
|
||
Comment on attachment 427322 [details]
HTML testcase
Seems to crash on load so switching to text/plain.
Attachment #427322 -
Attachment mime type: text/html → text/plain
![]() |
Reporter | |
Comment 4•15 years ago
|
||
A similar assert is in bug 533862 but they might not be related.
See Also: → 533862
Assignee | ||
Updated•15 years ago
|
Assignee: general → jorendorff
Assignee | ||
Comment 5•15 years ago
|
||
OK, it's not recursion or eval:
if(<y l={0
}/>)/*123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678*/if(x(function(){})(x
for(x in x)));
Assignee | ||
Comment 6•15 years ago
|
||
How do you find this stuff?! It turns out that when we do this:
oldflags = ts->flags;
ts->flags = oldflags & ~TSF_XMLTAGMODE;
pn2 = Expr(cx, ts, tc);
if (!pn2)
return NULL;
MUST_MATCH_TOKEN(TOK_RC, JSMSG_CURLY_IN_XML_EXPR);
ts->flags = oldflags;
the last line there can unintentionally clobber TSF_NLFLAG. The confusion about whether we saw a newline or not in turn causes the column numbers in token positions to be wrong for long lines, which in turn causes CompExprTransplanter to get confused and adjust the lexical depth of something that shouldn't be adjusted, which results in an assertion down the road.
Fix coming.
Assignee | ||
Comment 7•15 years ago
|
||
It looks like this is the only place where we adjust ts->flags this way. Test case included.
Attachment #427628 -
Flags: review?(brendan)
Assignee | ||
Comment 8•15 years ago
|
||
BTW, the spaces function can be implemented like this (unless you're just trying to test recursion).
function spaces(n) {
return Array(n+1).join(" ");
}
Comment 10•15 years ago
|
||
Comment on attachment 427628 [details] [diff] [review]
v1
Thanks.
/be
Attachment #427628 -
Flags: review?(brendan) → review+
Comment 11•15 years ago
|
||
More spaces-like fun, for anyone who feels like arguing algorithms or code (well, don't actually argue, I mention this only for pedagogical benefit ;-) ):
http://www.davidflanagan.com/2009/08/string-multipli.html
http://www.davidflanagan.com/2009/08/good-algorithms.html
Assignee | ||
Comment 12•15 years ago
|
||
Group: core-security
Whiteboard: fixed-in-tracemonkey
![]() |
Reporter | |
Updated•15 years ago
|
Whiteboard: fixed-in-tracemonkey → [ccbr] fixed-in-tracemonkey
Comment 13•15 years ago
|
||
Since this has been deemed not a security issue it's not a security-update blocker, but if you want to fix this crash on the old branches please request approval on the patch.
Updated•15 years ago
|
Flags: wanted1.9.0.x-
Whiteboard: [ccbr] fixed-in-tracemonkey → [ccbr][sg:dos] fixed-in-tracemonkey
Comment 14•15 years ago
|
||
fixed manifest http://hg.mozilla.org/tracemonkey/rev/3970759ca2c1
Comment 16•15 years ago
|
||
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•15 years ago
|
blocking2.0: ? → betaN+
Updated•14 years ago
|
Crash Signature: [@ BindNameToSlot]
Comment 17•12 years ago
|
||
A testcase for this bug was automatically identified at js/src/tests/js1_8_5/regress/regress-546615.js.
Flags: in-testsuite+
![]() |
Reporter | |
Comment 18•12 years ago
|
||
Testcases have been landed by virtue of being marked in-testsuite+ -> VERIFIED as well.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•