956173 - Assertion failure: !val.isMagic(), at jit/BaselineIC.cpp

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

Attachment: stack

(function() {
    let(x = arguments) {
        ((function() {
            return {
                y: function() {
                    x
                }
            }
        })(), x)
    }
})()

asserts js debug shell on m-c changeset 25524dc5c99f with --ion-parallel-compile=off --ion-eager at Assertion failure: !val.isMagic(), at jit/BaselineIC.cpp

My configure flags are:

CC="clang -Qunused-arguments" AR=ar CXX="clang++ -Qunused-arguments" sh ./configure --target=x86_64-apple-darwin12.5.0 --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --with-ccache --enable-threadsafe <other NSPR options>

=== Tinderbox Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20131213091706" and the hash "9fff05a1c0f3".
The "bad" changeset has the timestamp "20131213092106" and the hash "9e707fd8e62d".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9fff05a1c0f3&tochange=9e707fd8e62d

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/d0b854b106d4
user:        Andy Wingo
date:        Tue Nov 26 12:07:02 2013 +0100
summary:     Bug 927782 - Part 11: Optimize block scopes without aliased locals. r=luke

Andy, is bug 927782 a likely regressor?

Comment 1

[Andy Wingo [:wingo]]

Got a reduced case:

Starting program: /hack/mozilla-central/js/src/+debug/./js --ion-parallel-compile=off --ion-eager
warning: Could not load shared library symbols for linux-vdso.so.1.
Do you need "set solib-search-path" or "set sysroot"?
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[New Thread 0x7ffff6390700 (LWP 7052)]
js> function foo() { return 10 }
[New Thread 0x7ffff5b8f700 (LWP 7053)]
[New Thread 0x7ffff59ff700 (LWP 7054)]
[New Thread 0x7ffff57ff700 (LWP 7055)]
[New Thread 0x7ffff577e700 (LWP 7056)]
[New Thread 0x7ffff56fd700 (LWP 7057)]
[New Thread 0x7ffff567c700 (LWP 7058)]
[New Thread 0x7ffff55fb700 (LWP 7059)]
[New Thread 0x7ffff557a700 (LWP 7060)]
js> foo()
10
js> function f() { let (x=arguments) { return function g() { (foo(), x) } } }
js> print(disassemble(f))
flags:
loc     op
-----   --
00000:  arguments
00001:  setlocal 0
00004:  pop
main:
00005:  getlocal 0
00008:  pushblockscope depth 0 {x: 0}
00013:  lambda (function g() { (foo(), x) })
00018:  setrval
00019:  debugleaveblock
00020:  popblockscope
00021:  popn 1
00024:  retrval
00025:  debugleaveblock
00026:  popblockscope
00027:  popn 1
00030:  retrval

Source notes:
 ofs line    pc  delta desc     args
---- ---- ----- ------ -------- ------
  0:    3    13 [  13] xdelta  
  1:    3    13 [   0] colspan 35
  3:    3    30 [  17] xdelta  
  4:    3    30 [   0] colspan 36

js> var g = f();
js> print(disassemble(g))
flags: LAMBDA
loc     op
-----   --
main:
00000:  callgname "foo"
00005:  undefined
00006:  call 0
00009:  pop
00010:  getaliasedvar "x" (hops = 0, slot = 2)
00015:  pop
00016:  retrval

Source notes:
 ofs line    pc  delta desc     args
---- ---- ----- ------ -------- ------
  0:    3     0 [   0] colspan 58
  2:    3    16 [  16] xdelta  
  3:    3    16 [   0] colspan 9

js> g()
Assertion failure: !val.isMagic(), at /hack/mozilla-central/js/src/jit/BaselineIC.cpp:1138

Catchpoint 1 (signal SIGSEGV), 0x00000000005cdec1 in js::jit::ICTypeMonitor_Fallback::addMonitorStubForValue (this=this@entry=0x170dee8, cx=cx@entry=0x1645e90, script=..., script@entry=0x7ffff5842510, val=..., 
    val@entry=$jsmagic(JS_OPTIMIZED_ARGUMENTS)) at /hack/mozilla-central/js/src/jit/BaselineIC.cpp:1138
1138	        JS_ASSERT(!val.isMagic());

Catchpoint 1 (signal SIGSEGV), 0x00000000005cdec1 in js::jit::ICTypeMonitor_Fallback::addMonitorStubForValue (this=this@entry=0x170dee8, cx=cx@entry=0x1645e90, script=..., script@entry=0x7ffff5842510, val=..., 
    val@entry=$jsmagic(JS_OPTIMIZED_ARGUMENTS)) at /hack/mozilla-central/js/src/jit/BaselineIC.cpp:1138
1138	        JS_ASSERT(!val.isMagic());
(gdb) bt
#0  0x00000000005cdec1 in js::jit::ICTypeMonitor_Fallback::addMonitorStubForValue (this=this@entry=0x170dee8, cx=cx@entry=0x1645e90, script=..., script@entry=0x7ffff5842510, val=..., 
    val@entry=$jsmagic(JS_OPTIMIZED_ARGUMENTS)) at /hack/mozilla-central/js/src/jit/BaselineIC.cpp:1138
#1  0x00000000005ce0ec in js::jit::DoTypeMonitorFallback (cx=0x1645e90, frame=<optimized out>, stub=0x170dee8, value=$jsmagic(JS_OPTIMIZED_ARGUMENTS), res=JSVAL_VOID)
    at /hack/mozilla-central/js/src/jit/BaselineIC.cpp:1259
#2  0x00007ffff7ff0f49 in ?? ()
#3  0x0000000000000000 in ?? ()


The error comes when ion-compiling g().  Still working out what the deal is.

Comment 2

[Andy Wingo [:wingo]]

I do think that the Gary found the changeset that introduced the error:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/d0b854b106d4
user:        Andy Wingo
date:        Tue Nov 26 12:07:02 2013 +0100
summary:     Bug 927782 - Part 11: Optimize block scopes without aliased locals. r=luke

I'll see if I can come up with a fix.

Comment 3

[Andy Wingo [:wingo]]

I got a shorter test case:

  function f() { let (x=arguments) { return function g() { return x } } }        
  f()()                                                                   
  Assertion failure: !val.isMagic(), at /hack/mozilla-central/js/src/jit/BaselineIC.cpp:1138

The error happens on the "x" access in the nested function.  I think "x" is getting rendered as an optimized arguments object when it should be a heap arguments object, probably because f's ion compilation didn't see that pushblockscope aliased "x".

Comment 4

[Andy Wingo [:wingo]]

Pretty sure the fix will go in jsanalyze.cpp, to correctly mark PUSHBLOCKSCOPE as using values from the stack.  Will poke tomorrow.

Comment 5

[Christian Holler (:decoder)]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision bcbe93f41547).

Comment 6

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

I'm guessing, FIXED by bug 960168?

Comment 7

[Andy Wingo [:wingo]]

Yep.  Sorry, I didn't close as I was rushing out the door on thursday, and I didn't know whether you wanted to keep this open to set flags or something.  As you're on holiday now I'll just FIXED it :)

Comment 8

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.