514790 - "Assertion failure: (uint32)(index_) < atoms_->length" on mbc.net

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jesse Ruderman]

On mozilla-central, loading http://mbc.net/ triggers:

Assertion failure: (uint32)(index_) < atoms_->length, at js/src/jsobj.cpp:2318

Comment 1

[David Mandelin [:dmandelin]]

Stack trace:

0   libmozjs.dylib                	0x00394851 JS_Assert + 67 (jsutil.cpp:69)
1   libmozjs.dylib                	0x0032663c Detecting(JSContext*, unsigned char*) + 518
2   libmozjs.dylib                	0x00326851 js_InferFlags + 329 (jsobj.cpp:2367)
3   libmozjs.dylib                	0x003c2c26 TraceRecorder::snapshot(ExitType) + 1862 (jstracer.cpp:3420)
4   libmozjs.dylib                	0x003c6582 TraceRecorder::guard(bool, nanojit::LIns*, ExitType) + 30 (jstracer.cpp:3515)
5   libmozjs.dylib                	0x003cf408 TraceRecorder::switchop() + 540 (jstracer.cpp:7474)
6   libmozjs.dylib                	0x003cf541 TraceRecorder::record_JSOP_LOOKUPSWITCH() + 17 (jstracer.cpp:11513)
7   libmozjs.dylib                	0x003dbd70 TraceRecorder::monitorRecording(JSContext*, TraceRecorder*, JSOp) + 2960 (jsopcode.tbl:194)
8   libmozjs.dylib                	0x002e60c1 js_Interpret + 1365 (jsops.cpp:79)
9   libmozjs.dylib                	0x00313113 js_Execute + 781 (jsinterp.cpp:1606)
10  libmozjs.dylib                	0x0028e99d JS_EvaluateUCScriptForPrincipals + 327 (jsapi.cpp:5054)

Comment 2

[Andreas Gal :gal]

Wow. Scary. I wrote that code. I do inferflags and store the results in the side exit and pick them up from there when code is called from trace.

Comment 3

[Brendan Eich [:brendan]]

Did I just regress this with the patch for bug 510644 ? Seems likely.

/be

Comment 4

[Brendan Eich [:brendan]]

Capturing the script that was running (script->{filename,lineno} in frame 8 and then wget if static) would probably lead to a reduced testcase.

/be

Comment 5

[Brendan Eich [:brendan]]

No, my patch for bug 510644 was innocent victim to a latent bug:

JS_Assert (s=0x42fe98 "(uint32)(index_) < atoms_->length", file=0x4314ac "../../../tracemonkey/js/src/jsobj.cpp", ln=2308) at ../../../tracemonkey/js/src/jsutil.cpp:69
69	    abort();
(gdb) up
#1  0x0032ca65 in Detecting (cx=0x23a84a00, pc=0x1eb7b6c3 ";G\001'") at ../../../tracemonkey/js/src/jsobj.cpp:2308
2308	            GET_ATOM_FROM_BYTECODE(script, pc, 0, atom);
(gdb) p index_
$1 = 18177
(gdb) p/x pc[1]
$2 = 0x47
(gdb) p/x pc[2]
$3 = 0x1

Yikes. Those look like opcodes, at least 0x47 does.

(gdb) p (JSOp)pc[0]
$5 = JSOP_NAME
(gdb) p cx.fp.regs.pc == pc
$6 = false
(gdb) p cx.fp.imacpc == pc
$7 = false
(gdb) p pc - cx.fp.regs.pc
$8 = -1

Uh oh.

(gdb) up
#2  0x0032cc7b in js_InferFlags (cx=0x23a84a00, defaultFlags=0) at ../../../tracemonkey/js/src/jsobj.cpp:2364
2364	        if (pc < cx->fp->script->code + cx->fp->script->length && Detecting(cx, pc))
(gdb) p pc - cx.fp.script.code
$9 = 883
(gdb) p cx.fp.script.length
$10 = 1406
(gdb) p (JSOp)*pc
$11 = JSOP_NAME
(gdb) p pc[1]
$12 = 71 'G'
(gdb) p pc[2]
$13 = 1 '\001'
(gdb) p (JSOp)$12
$14 = JSOP_LOOKUPSWITCH

Aha!

(gdb) p (JSOp)pc[-2]
$15 = JSOP_GETPROP
(gdb) p pc[-1]
$16 = 0 '\0'
(gdb) p pc[0]
$17 = 59 ';'

Valid.

(gdb) up
#3  0x003c99ea in TraceRecorder::snapshot (this=0x1a50a2a0, exitType=BRANCH_EXIT) at ../../../tracemonkey/js/src/jstracer.cpp:3419
3419	    exit->lookupFlags = js_InferFlags(cx, 0);
(gdb) down
#2  0x0032cc7b in js_InferFlags (cx=0x23a84a00, defaultFlags=0) at ../../../tracemonkey/js/src/jsobj.cpp:2364
2364	        if (pc < cx->fp->script->code + cx->fp->script->length && Detecting(cx, pc))
(gdb) l
2359	    if ((format & (JOF_SET | JOF_FOR)) ||
2360	        (fp->flags & JSFRAME_ASSIGNING)) {
2361	        flags |= JSRESOLVE_ASSIGNING;
2362	    } else {
2363	        pc += cs->length;

Evil! We are advancing by js_CodeSpec[JSOP_LOOKUPSWITCH].length, which is -1.

2364	        if (pc < cx->fp->script->code + cx->fp->script->length && Detecting(cx, pc))
2365	            flags |= JSRESOLVE_DETECTING;
2366	    }
2367	    if (format & JOF_DECLARING)
2368	        flags |= JSRESOLVE_DECLARING;
(gdb) p cs.length
$18 = -1 '?'
(gdb) p *cs
$19 = {
  length = -1 '?', 
  nuses = 1 '\001', 
  ndefs = 0 '\0', 
  prec = 0 '\0', 
  format = 1064965
}

Working on a patch now.

/be

Comment 6

[Brendan Eich [:brendan]]

Attachment: minimal fix

js_InferFlags is called on a great many more ops than used to trigger Detecting. This fix just avoids the inadvertent back-up by adding -1 to pc. Could be more aggressive in inferring no flags for ops other than those for which we do need to infer flags.

/be

Comment 7

[Brendan Eich [:brendan]]

True regressor was bug 502145:

changeset:   29681:753d18be63fc
user:        Andreas Gal <[e-mail]>
date:        Mon Jul 06 06:42:13 2009 +0200
summary:     Shortcut lookup flag calculation when on trace (bug 502145, r=igor).

/be

Comment 8

[Brendan Eich [:brendan]]

http://hg.mozilla.org/tracemonkey/rev/e376269c43ac

/be

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/e376269c43ac