Closed Bug 451806 Opened 13 years ago Closed 13 years ago

TM: assert on wikipedia

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9.1

People

(Reporter: dvander, Assigned: brendan)

References

Details

Attachments

(1 file)

Stack trace: 

Assertion failure: sprop->getter == js_GetCallVar, at /home/dvander/tracemonkey/js/src/jstracer.cpp:2372

Program received signal SIGTRAP, Trace/breakpoint trap.
JS_Assert (s=0xb7f11a48 "sprop->getter == js_GetCallVar", file=0xb7f10f10 "/home/dvander/tracemonkey/js/src/jstracer.cpp", ln=2372)
    at /home/dvander/tracemonkey/js/src/jsutil.cpp:63
63          abort();
(gdb) bt
#0  JS_Assert (s=0xb7f11a48 "sprop->getter == js_GetCallVar", file=0xb7f10f10 "/home/dvander/tracemonkey/js/src/jstracer.cpp", ln=2372)
    at /home/dvander/tracemonkey/js/src/jsutil.cpp:63
#1  0xb7e9f60b in TraceRecorder::activeCallOrGlobalSlot (this=0xaedae590, obj=0xaedcc760, vp=@0xbfffd2b4) at /home/dvander/tracemonkey/js/src/jstracer.cpp:2372
#2  0xb7ea4333 in TraceRecorder::name (this=0xaedae590, vp=@0xbfffd2b4) at /home/dvander/tracemonkey/js/src/jstracer.cpp:4284
#3  0xb7eabaca in TraceRecorder::record_JSOP_FORNAME (this=0xaedae590) at /home/dvander/tracemonkey/js/src/jstracer.cpp:4816

(gdb) print *sprop
$1 = {id = -1238133892, getter = 0, setter = 0, slot = 9, attrs = 5 '\005', flags = 0 '\0', shortid = 0, parent = 0x0, kids = 0xb04b5b70, shape = 23064}

(gdb) print cx->fp->script->filename
$2 = 0xaed16171 "http://meta.wikimedia.org/w/index.php?title=MediaWiki:Wikiminiatlas.js&action=raw&ctype=text/javascript&smaxage=21600&maxage=86400"

(gdb) print JS_PCToLineNumber(cx, cx->fp->script, cx->fp->regs->pc)
$4 = 239
Good old with (wikiminiatlas) around for (var key in ...) means key gets hoisted above the with, so for-in can't use JSOP_FORVAR, but must use JSOP_FORNAME, which binds a named property of the Call object induced by the with statement. Blech!

Easy to fix.

/be
Status: NEW → ASSIGNED
OS: Linux → All
Priority: -- → P1
Hardware: Other → All
Target Milestone: --- → mozilla1.9.1
Attached patch fixSplinter Review
This will conflict with trace-test.js on tracemonkey, sigh.

I also followed Andreas's lead there and used four-space indentation units in trace-test.js. It's what vim does by default.

There's a bug I'll file hiding here: even in a with, where for-in using a local must select JSOP_FORNAME, we should be able to avoid creating an expando on the Call object. I need to debug a bit and file that, but this should go in anyway. You can use eval("var foo = ...") in a function to make an expando on the Call object, and that would botch the assertion hard.

/be
If you prefer 8-space indentation just re-format the whole file as one commit. I think my eclipse does default 4 on .js but I am pretty sure I can convince it to do 8.
Shaver used two-space indentation units in trace-test.js and we mostly stuck to that. 8 spaces (or tabs) is too much, I left that style school when I retired from pre-Linux kernel hacking ;-). 4 is consistent with the rest of the code.

/be
Comment on attachment 335279 [details] [diff] [review]
fix

No mystery, with makes the function heavyweight, and that means locals are plain old properties. Followup bug fodder, probably in the upvar followup I need to clone and mutate.

/be
Attachment #335279 - Flags: review?(mrbkap)
Attachment #335279 - Flags: review?(mrbkap) → review+
Duplicate of this bug: 451864
Fixed on mozilla-central:

http://hg.mozilla.org/mozilla-central/index.cgi/rev/e49ce187bae5

/be
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
test also included in js1_8_1/trace/trace-test.js
Flags: in-testsuite+
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.