884920 - Crash [@ js::frontend::EmitTree]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision d2a7cfa34154 (no options required):


function testcase({}, a = b, ... x)  {
  "use strict";
  function f() { };
  with ( f(3) );
}

Comment 1

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 2

[Christian Holler (:decoder)]

Crash trace:


Program received signal SIGSEGV, Segmentation fault.
0x00000000004402e3 in js::frontend::EmitTree (cx=0x18741a0, bce=0x7fffffffbf10, pn=0x1893a00) at js/src/frontend/BytecodeEmitter.cpp:5696
5696                    while (rest->pn_next != pnlast)
#0  0x00000000004402e3 in js::frontend::EmitTree (cx=0x18741a0, bce=0x7fffffffbf10, pn=0x1893a00) at js/src/frontend/BytecodeEmitter.cpp:5696
#1  0x0000000000442e4d in EmitTree (pn=0x1893a00, bce=0x7fffffffbf10, cx=0x18741a0) at js/src/frontend/BytecodeEmitter.cpp:5631
#2  EmitFunctionScript (body=0x1893a00, bce=0x7fffffffbf10, cx=0x18741a0) at js/src/frontend/BytecodeEmitter.cpp:2585
#3  EmitFunc (cx=0x18741a0, bce=0x7fffffffc3a0, pn=0x1893668) at js/src/frontend/BytecodeEmitter.cpp:4568
#4  0x000000000043ed16 in js::frontend::EmitTree (cx=0x18741a0, bce=0x7fffffffc3a0, pn=0x1893668) at js/src/frontend/BytecodeEmitter.cpp:5645
#5  0x0000000000a2b98b in js::frontend::CompileScript (cx=0x18741a0, scopeChain=..., evalCaller=0x0, options=..., chars=0x1885800, length=<optimized out>, source_=0x0, staticLevel=0, extraSct=0x0) at js/src/frontend/BytecodeCompiler.cpp:311
#6  0x000000000059b252 in JS::Compile (cx=0x18741a0, obj=(JSObject * const) 0x7ffff5e4d060 [object global] delegate, options=..., chars=0x1885800, length=99) at js/src/jsapi.cpp:5336
#7  0x000000000059b5cc in JS::Compile (cx=0x18741a0, obj=(JSObject * const) 0x7ffff5e4d060 [object global] delegate, options=..., bytes=<optimized out>, length=99) at js/src/jsapi.cpp:5351
rdx     0x0     0
=> 0x4402e3 <js::frontend::EmitTree(JSContext*, js::frontend::BytecodeEmitter*, js::frontend::ParseNode*)+5971>:        mov    0x10(%rdx),%rax

Comment 3

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/ce43d28276e4
user:        Brian Hackett
date:        Fri Jun 14 05:58:28 2013 -0600
summary:     Bug 678037 - Enable lazy JS parsing and fix various bugs, r=waldo,evilpie,nobody.

This iteration took 11.878 seconds to run.

Comment 4

[Christian Holler (:decoder)]

Needinfo from Brian based on comment 3 :)

Comment 5

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Another testcase:

function d(c = (function h() {
    return {
        r: (1 for (x in []))
    }
}), ...x) {
    "use strict"
}

Comment 6

[Brian Hackett [Laid off!]]

Attachment: nuke lastFunctionKeyword

If we abort a syntax parse then the tokenStream's lastFunctionKeyword isn't reset and the restarted parse gets confused --- it parses the inner function as if it's the outer function.  lastFunctionKeyword is pretty hacky, and with the current state of the parser is totally unnecessary --- it lets the parser find the position of the last |function|, but the parser only ever tries to do this immediately after it gets a |function| token.  The attached patch removes this field.

Comment 7

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2c6a7766ae44

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/2c6a7766ae44

Comment 9

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Let's backport this to ESR 24, although the crash volume is low (~20 crashes in the past month, about 5 a week - see URL), a crash is a crash is a crash, and there's a patch here anyway.

https://crash-stats.mozilla.com/query/?product=Firefox&version=Firefox%3A24.2.0esr&version=Firefox%3A24.1.1esr&version=Firefox%3A24.1.0esr&version=Firefox%3A24.0esr&version=Firefox%3A24.0&range_value=1&range_unit=weeks&date=12%2F26%2F2013+22%3A00%3A00&query_search=signature&query_type=contains&query=js%3A%3Afrontend%3A%3AEmitTree&reason=&release_channels=&build_id=&process_type=any&hang_type=any

Comment 10

[Brian Hackett [Laid off!]]

Comment on attachment 766456 [details] [diff] [review]
nuke lastFunctionKeyword

[Approval Request Comment]
User impact if declined: See comment 9
Fix Landed on Version: 25
Risk to taking this patch (and alternatives if risky): none

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 766456 [details] [diff] [review]
nuke lastFunctionKeyword

Actually, that's explicitly not how the ESR branch works and this doesn't meet the criteria[1].  Especially considering the low volume of crashes, this will be in the next major update.  

[1] https://wiki.mozilla.org/Release_Management/ESR_Landing_Process