Crash [@ js::frontend::Parser<js::frontend::FullParseHandler>::cloneParseTree] or Assertion failure: pn->pn_pos.begin >= pn_pos.begin, at frontend/ParseNode.h

RESOLVED FIXED in mozilla38

Status

()

defect
--
critical
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: gkw, Assigned: jorendorff)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
mozilla38
x86_64
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox36 affected)

Details

(Whiteboard: [jsbugmon:update], crash signature)

Attachments

(3 attachments)

Reporter

Description

5 years ago
for (let {
    [
        function(x) {;
        }
    ]: {}
} in 0

asserts js debug shell on m-c changeset 20408ad61ce5 with --ion-eager --no-threads at Assertion failure: pn->pn_pos.begin >= pn_pos.begin, at frontend/ParseNode.h.

Debug configure options:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-optimize --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/7079b7552946
user:        Guptha Rajagopal
date:        Fri Aug 08 09:15:00 2014 -0400
summary:     Bug 924688 - Implement ES6 computed property names. r=jorendorff

Guptha, is bug 924688 a possible regressor?
Flags: needinfo?(gupta.rajagopal)
Reporter

Comment 1

5 years ago
Posted file stack for assertion
(lldb) bt 5
* thread #1: tid = 0x5e077, 0x00000001001803de js-dbg-opt-64-dm-nsprBuild-darwin-20408ad61ce5`js::frontend::ParseNode::append(this=<unavailable>, pn=<unavailable>) + 174 at ParseNode.h:827, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001001803de js-dbg-opt-64-dm-nsprBuild-darwin-20408ad61ce5`js::frontend::ParseNode::append(this=<unavailable>, pn=<unavailable>) + 174 at ParseNode.h:827
    frame #1: 0x000000010014cc8f js-dbg-opt-64-dm-nsprBuild-darwin-20408ad61ce5`js::frontend::Parser<js::frontend::FullParseHandler>::cloneParseTree(this=0x00007fff5fbfe1a0, opn=0x0000000104005d30) + 1039 at ParseNode.cpp:366
    frame #2: 0x000000010014cbf9 js-dbg-opt-64-dm-nsprBuild-darwin-20408ad61ce5`js::frontend::Parser<js::frontend::FullParseHandler>::cloneParseTree(this=0x00007fff5fbfe1a0, opn=0x0000000104005c90) + 889 at ParseNode.cpp:355
    frame #3: 0x000000010014caab js-dbg-opt-64-dm-nsprBuild-darwin-20408ad61ce5`js::frontend::Parser<js::frontend::FullParseHandler>::cloneParseTree(this=0x00007fff5fbfe1a0, opn=0x0000000104005e18) + 555 at ParseNode.cpp:396
    frame #4: 0x000000010014d1a2 js-dbg-opt-64-dm-nsprBuild-darwin-20408ad61ce5`js::frontend::Parser<js::frontend::FullParseHandler>::cloneLeftHandSide(this=0x00007fff5fbfe1a0, opn=0x0000000104005c58) + 802 at ParseNode.cpp:472
(lldb)
Reporter

Comment 2

5 years ago
for (let {
    [
        function() {;
        }
    ]: {}
} in 0

This variant causes a crash which might be a null-deref. It bisects to the same changeset.
Crash Signature: [@ js::frontend::Parser<js::frontend::FullParseHandler>::cloneParseTree]
Keywords: crash
Summary: Assertion failure: pn->pn_pos.begin >= pn_pos.begin, at frontend/ParseNode.h → Crash [@ js::frontend::Parser<js::frontend::FullParseHandler>::cloneParseTree] or Assertion failure: pn->pn_pos.begin >= pn_pos.begin, at frontend/ParseNode.h
Reporter

Comment 3

5 years ago
Posted file stack for crash
(lldb) bt 5
* thread #1: tid = 0x5e186, 0x000000010014c957 js-dbg-opt-64-dm-nsprBuild-darwin-20408ad61ce5`js::frontend::Parser<js::frontend::FullParseHandler>::cloneParseTree(js::frontend::ParseNode*) [inlined] js::frontend::ParseNode::getKind(this=0x0000000000000000, this=<unavailable>, this=<unavailable>, p1=<unavailable>) const at ParseNode.h:492, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010014c957 js-dbg-opt-64-dm-nsprBuild-darwin-20408ad61ce5`js::frontend::Parser<js::frontend::FullParseHandler>::cloneParseTree(js::frontend::ParseNode*) [inlined] js::frontend::ParseNode::getKind(this=0x0000000000000000, this=<unavailable>, this=<unavailable>, p1=<unavailable>) const at ParseNode.h:492
    frame #1: 0x000000010014c957 js-dbg-opt-64-dm-nsprBuild-darwin-20408ad61ce5`js::frontend::Parser<js::frontend::FullParseHandler>::cloneParseTree(this=0x00007fff5fbfe1a0, opn=0x0000000000000000) + 215 at ParseNode.cpp:340
    frame #2: 0x000000010014caab js-dbg-opt-64-dm-nsprBuild-darwin-20408ad61ce5`js::frontend::Parser<js::frontend::FullParseHandler>::cloneParseTree(this=0x00007fff5fbfe1a0, opn=0x00000001020419a0) + 555 at ParseNode.cpp:396
    frame #3: 0x000000010014cc7b js-dbg-opt-64-dm-nsprBuild-darwin-20408ad61ce5`js::frontend::Parser<js::frontend::FullParseHandler>::cloneParseTree(this=0x00007fff5fbfe1a0, opn=0x0000000102041968) + 1019 at ParseNode.cpp:365
    frame #4: 0x000000010014cc7b js-dbg-opt-64-dm-nsprBuild-darwin-20408ad61ce5`js::frontend::Parser<js::frontend::FullParseHandler>::cloneParseTree(this=0x00007fff5fbfe1a0, opn=0x0000000102041930) + 1019 at ParseNode.cpp:365
(lldb)

Comment 4

5 years ago
Reflect.parse output for the underlying computed property name:
http://pastebin.mozilla.org/6987745

Nothing there should hit an assertion. Of course, Reflect.parse doesn't completely show the internal structure of the parse tree, so I'll have to look around a bit in the code.
Reporter

Comment 5

5 years ago
Also asserts with --no-ion.
Component: JavaScript Engine: JIT → JavaScript Engine
Assignee

Updated

5 years ago
Assignee: nobody → jorendorff
Assignee

Comment 6

5 years ago
OK, lots of distractions, but I want to get back to this on Monday.
Flags: needinfo?(jorendorff)
Comment on attachment 8552034 [details] [diff] [review]
Fix null crash in cloneParseTree with computed property names in destructuring

Review of attachment 8552034 [details] [diff] [review]:
-----------------------------------------------------------------

Nice catch. This will save classes as well.

::: js/src/frontend/ParseNode.cpp
@@ +374,2 @@
>          else
> +            pn->pn_binary_obj = opn->pn_binary_obj;

Arity assertion here just for good measure?

::: js/src/frontend/Parser.cpp
@@ -2252,5 @@
>  
>      MOZ_ASSERT(pn->pn_funbox == funbox);
>      MOZ_ASSERT(pn->pn_body->isKind(PNK_ARGSBODY));
>      pn->pn_body->append(body);
> -    pn->pn_body->pn_pos = body->pn_pos;

Why is this removed? Presumably it's duplicitous with append()?
Attachment #8552034 - Flags: review?(efaustbmo) → review+
Assignee

Comment 9

4 years ago
> >      pn->pn_body->append(body);
> > -    pn->pn_body->pn_pos = body->pn_pos;
> 
> Why is this removed? Presumably it's duplicitous with append()?

Yeah, it's redundant. ("Duplicitous" means deceptive.)
Flags: needinfo?(jorendorff)
https://hg.mozilla.org/mozilla-central/rev/4c573585e7dc
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Reporter

Updated

4 years ago
Flags: needinfo?(gupta.rajagopal)
You need to log in before you can comment on or make changes to this bug.