351794 - Assertion failure: CG_NOTE_COUNT(cg) == 0 || CG_LAST_NOTE_OFFSET(cg) != CG_OFFSET(cg)

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jesse Ruderman]

js> new Function("for(let y in [5,6,7,8]) x")  
Assertion failure: CG_NOTE_COUNT(cg) == 0 || CG_LAST_NOTE_OFFSET(cg) != CG_OFFSET(cg), at jsemit.c:5727
Bus error

Comment 1

[Brendan Eich [:brendan]]

Ok, this looks at first like "just" a bogus assertion (the line number from line 1 of this js shell session at this point is 3):

js> new Function ("for(let y in [5,6,7,8]) x")
Assertion failure: CG_NOTE_COUNT(cg) == 0 || CG_LAST_NOTE_OFFSET(cg) != CG_OFFSET(cg), at jsemit.c:5740

Program received signal SIGTRAP, Trace/breakpoint trap.
JS_Assert (s=0xe6cd0 "CG_NOTE_COUNT(cg) == 0 || CG_LAST_NOTE_OFFSET(cg) != CG_OFFSET(cg)", file=0xe5e88 "jsemit.c", ln=5740) at jsutil.c:63
63          abort();
(gdb) up
#1  0x000447e0 in js_EmitTree (cx=0x500210, cg=0xbfffd9d8, pn=0x18191b8) at jsemit.c:5739
5739                JS_ASSERT(CG_NOTE_COUNT(cg) == 0 ||
(gdb) p cg.main
$1 = {
  base = 0x0, 
  limit = 0x0, 
  next = 0x0, 
  notes = 0x181bc10 "?", 
  noteCount = 4, 
  noteMask = 63, 
  lastNoteOffset = 0, 
  currentLine = 3
}
(gdb) x/4b $.notes
0x181bc10:      0xb8    0x00    0xb8    0x03
(gdb) p js_SrcNoteSpec[0xb8>>3]
$2 = {
  name = 0xe6e54 "setline", 
  arity = 1 '\001', 
  offsetBias = 0 '\0', 
  isSpanDep = 0 '\0'
}

There's an old and minor bug here: we don't need two SRC_SETLINEs in a row for the same code offset 0 from start of bytecode.

But putting the code in a function instead of passing it quoted to new Function discloses a bug not fixed in the patch for bug 349634:

js> function f(){for(let y in [5,6,7,8]) x}
js> f
function f() {
    {
        for (let y in [5, 6, 7, 8]) {
            x;
        }
    }
}

/be

Comment 2

[Brendan Eich [:brendan]]

Attachment: fix

1.  Exclude TOK_FOR as well as TOK_CATCH and TOK_LET inside a TOK_LEXICALSCOPE from consideration for annotating with SRC_BRACE, because for loops (both kinds) and array comprehensions do not need extra decompiler bracing.

2.  Fix the bogus assertion, with a potentially expensive (O(n^2) worst case, until we fix source note lookup -- and even then, this is a special lookup done on the code generator, so it would need separate optimization) test that there are no gettable (non-line-number) source notes for the JSOP_ENTERBLOCK about to be emitted.  This is #ifdef me and mrbkap.

3.  Fix FunctionBody to set the beginning line number of the parse-node for the function's body.  It was defaulting to 0, wrongly, resulting in an extra pair of line-number source notes at the front of bodies compiled by Function.  This old buglet helped expose the current bug.

/be

Comment 3

[Brendan Eich [:brendan]]

Attachment: patch I'm checking in

I'll apply this on top of the patch for bug 349634 applied to the 1.8 branch, and attach that to bug 349634.

/be

Comment 4

[Brendan Eich [:brendan]]

Fixed on trunk.

/be

Comment 5

[Bob Clary [:bc] (inactive)]

Checking in regress-351794.js;
/cvsroot/mozilla/js/tests/js1_7/block/regress-351794.js,v  <--  regress-351794.js
initial revision: 1.1

Comment 6

[Bob Clary [:bc] (inactive)]

verified fixed 1.9 20060909 windows/mac*/linux, the regression isn't on 1.8 yet. ;-)

Comment 7

[Brendan Eich [:brendan]]

Trunk-only regression, fix went into the 1.8 branch just now as part of roll-up patch in bug 349634.

/be

Comment 8

[Bob Clary [:bc] (inactive)]

verified fixed 1.8 1.9 2006091022 windows/mac*/linux