516262 - Crash [@ js_Interpret] or "Assertion failure: script->nfixed == 2, at ../jsinterp.cpp"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

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

y=0
var z
[#1#]

crashes js opt shell at js_Interpret at null on TM tip (changeset:   32240:01fc5bf1d21a) without -j and asserts debug at Assertion failure: script->nfixed == 2, at ../jsinterp.cpp:1568

(Pass in as a CLI argument to get it to assert/crash)

autoBisect shows this is probably related to bug 514981:

The first bad revision is:
changeset:   32201:c19b0d06d076
user:        Brendan Eich
date:        Wed Sep 09 20:21:15 2009 -0700
summary:     Bug 514981 - JSStackFrame::sharp{Array,Depth} should be locals allocated due to #n[#=] usage (r=igor).

===

$ cat 2interesting/w4811-reduced.js 
y=0
var z
[#1#]
$ Mozilla/32240/js-dbg-tm-darwin 2interesting/w4811-reduced.js 
Assertion failure: script->nfixed == 2, at ../jsinterp.cpp:1568
$ Mozilla/32240/js-opt-tm-darwin 2interesting/w4811-reduced.js 
Bus error

===

Exception Type:  EXC_BAD_ACCESS (SIGBUS)
Exception Codes: KERN_PROTECTION_FAILURE at 0x0000000000000000
Crashed Thread:  0

Thread 0 Crashed:
0   js-opt-tm-darwin              	0x00053c7a js_Interpret + 23498
1   js-opt-tm-darwin              	0x0005dc61 js_Execute + 385
2   js-opt-tm-darwin              	0x0000d53c JS_ExecuteScript + 60
3   js-opt-tm-darwin              	0x00004270 Process(JSContext*, JSObject*, char*, int) + 1616
4   js-opt-tm-darwin              	0x0000746f main + 879
5   js-opt-tm-darwin              	0x0000196b _start + 209
6   js-opt-tm-darwin              	0x00001899 start + 41

Comment 1

[Brendan Eich [:brendan]]

Attachment: fix

The code shown in the patch was not self-consistent (look for all occurrences of script->nfixed - 2). D'oh!

Gary, thanks for all the fuzzing!

/be

Comment 2

[Igor Bukanov]

Comment on attachment 400403 [details] [diff] [review]
fix

We should define a constant like SHARP_LOCAL_COUNT and use it, not just a plain 2, which is confusing with all those args - 2. Also the patch should use either memcpy/loop or a static assert that SHARP_LOCAL_COUNT is 2 when initializing the slots. At least this will be self-commenting. r+ with this fixed.

Comment 3

[Brendan Eich [:brendan]]

Thanks, I opted for static assert (in two files, mainly as you say to document the unrolled short loop or similar dependency on #array/#depth being the two and only two sharp locals). Fixed in tm:

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

/be

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

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

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Did/should this land on mozilla-1.9.2? @js_Interpret is the topcrash on Firefox 3.6b1 by an order of magnitude.

Comment 6

[Robert Sayre]

 Bug 514981 did not land on 192