Closed Bug 546615 Opened 11 years ago Closed 11 years ago

Crash [@ BindNameToSlot] or "Assertion failure: cg->staticLevel >= level, at ../jsemit.cpp"

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
Tracking Status
blocking2.0 --- betaN+
blocking1.9.2 --- -
status1.9.2 --- wanted
blocking1.9.1 --- -
status1.9.1 --- wanted

People

(Reporter: gkw, Assigned: jorendorff)

References

Details

(4 keywords, Whiteboard: [ccbr][sg:dos] fixed-in-tracemonkey)

Crash Data

Attachments

(2 files)

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
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
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
A similar assert is in bug 533862 but they might not be related.
See Also: → 533862
Assignee: general → jorendorff
OK, it's not recursion or eval:

if(<y l={0
}/>)/*123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678*/if(x(function(){})(x
for(x in x)));
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.
Attached patch v1Splinter Review
It looks like this is the only place where we adjust ts->flags this way. Test case included.
Attachment #427628 - Flags: review?(brendan)
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(" ");
    }
Duplicate of this bug: 531516
Comment on attachment 427628 [details] [diff] [review]
v1

Thanks.

/be
Attachment #427628 - Flags: review?(brendan) → review+
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
http://hg.mozilla.org/tracemonkey/rev/36487442aeb0
Group: core-security
Whiteboard: fixed-in-tracemonkey
Whiteboard: fixed-in-tracemonkey → [ccbr] fixed-in-tracemonkey
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.
blocking1.9.1: ? → -
blocking1.9.2: ? → -
Flags: wanted1.9.0.x-
Whiteboard: [ccbr] fixed-in-tracemonkey → [ccbr][sg:dos] fixed-in-tracemonkey
Blocks: 533862
Duplicate of this bug: 533862
http://hg.mozilla.org/mozilla-central/rev/36487442aeb0
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
blocking2.0: ? → betaN+
Crash Signature: [@ BindNameToSlot]
A testcase for this bug was automatically identified at js/src/tests/js1_8_5/regress/regress-546615.js.
Flags: in-testsuite+
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.