640075 - "Assertion failure: tp->begin.lineno == tp->end.lineno,"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

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

Attachment: stack

options("strict")
eval(
  uneval(
    (function() {
    eval()
    function eval() {}
    })
  )
)

asserts js debug shell on TM changeset 80d8431f209e without -m nor -j at Assertion failure: tp->begin.lineno == tp->end.lineno,


autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   63253:3035bb782013
user:        Nicholas Nethercote
date:        Tue Mar 08 16:10:51 2011 -0800
summary:     Bug 638034 - Make scanning safer.  r=brendan.

Comment 1

[Nicholas Nethercote [inactive]]

No time left today, I'll take a look tomorrow.  Thanks for the report, Gary!

Comment 2

[Nicholas Nethercote [inactive]]

I'm seeing this in a JSParseNode:

  pn_pos = {begin = {index = 9, lineno = 3}, end = {index = 18, lineno = 2}}

The end precedes the beginning!  I suspect this is a pre-existing bug in the parser -- whereby it gets some index and/or lineno values wrong -- that the increased checking in bug 638034 has exposed.

Comment 3

[Brendan Eich [:brendan]]

What is ('js::TokenKind'')pn->pn_type?

/be

Comment 4

[Nicholas Nethercote [inactive]]

(gdb) p *pn
$11 = {pn_type = 34, pn_op = 0, pn_arity = 4, pn_parens = 0, pn_used = 0, 
  pn_defn = 1, pn_pos = {begin = {index = 9, lineno = 3}, end = {index = 18, 
      lineno = 2}}, pn_offset = 0, pn_next = 0x0, pn_link = 0x8451980, 
  pn_u = {list = {head = 0xf76006e0, tail = 0x0, count = 4294967295, 
      xflags = 160, blockid = 1}, ternary = {kid1 = 0xf76006e0, kid2 = 0x0, 
      kid3 = 0xffffffff}, binary = {left = 0xf76006e0, right = 0x0, 
      pval = 0xffffffff, iflags = 4256}, unary = {kid = 0xf76006e0, num = 0, 
      hidden = -1}, name = {{atom = 0xf76006e0, funbox = 0xf76006e0, 
        objbox = 0xf76006e0}, {expr = 0x0, lexdef = 0x0}, cookie = {
        value = 4294967295, static FREE_VALUE = 4294967295, 
        static FREE_LEVEL = 16383, static UPVAR_LEVEL_LIMIT = 16, 
        static CALLEE_SLOT = 65535}, dflags = 160, blockid = 1}, nameset = {
      names = {list = 0xf76006e0, table = 0x0, count = 4294967295}, 
      tree = 0x10a0}, apair = {atom = 0xf76006e0, atom2 = 0x0}, 
    dval = 2.0505036382666079e-314}}


34 is TOK_FUNCTION.

Comment 5

[Brendan Eich [:brendan]]

Attachment: proposed fix

Also got rid of a single-use, downward and within same parent block, goto (in your favoriate and mine, TokenStream::reportCompileErrorNumberVA).

/be

Comment 6

[Brendan Eich [:brendan]]

Attachment: proposed fix

Comment 7

[Nicholas Nethercote [inactive]]

Comment on attachment 517997 [details] [diff] [review]
proposed fix

Looks good.  Extra points if you can move some of the variable declarations from the top of the function to a more local scope -- 'linelength' looks like a good candidate, for example.

Comment 8

[Brendan Eich [:brendan]]

Attachment: proposed fix with linelimit eliminated and linelength localized

Comment 9

[Brendan Eich [:brendan]]

http://hg.mozilla.org/tracemonkey/rev/5f49ed96bfbf

/be

Comment 10

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

http://hg.mozilla.org/mozilla-central/rev/5f49ed96bfbf

Comment 11

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

Testcases have been landed by virtue of being marked in-testsuite+ -> VERIFIED as well.