952823 - Crash [@ js::jit::IonBuilder::processReturn] or [@ js::jit::IonBuilder::traverseBytecode] or [@ js::jit::AttachFinishedCompilations] or [@ js_InvokeOperationCallback] or Assertion failure: args_[i]->isPassArg() == wrapped, at jit/IonBuilder.h

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

Attachment: lldb stack

x(["" for (e in 3) if ("")])

asserts js debug shell on m-c changeset 599100c4ebfe with --ion-eager --ion-parallel-compile=off at Assertion failure: args_[i]->isPassArg() == wrapped, at jit/IonBuilder.h and crashes js opt shell at js::jit::IonBuilder::processReturn with js::jit::IonBuilder::traverseBytecode on the stack.

My configure flags are: (debug)

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 --enable-more-deterministic --enable-exact-rooting --enable-elf-hack --enable-stdcxx-compat --enable-warnings-as-errors --enable-signmar --disable-elf-hack --enable-js-diagnostics --with-intl-api=build --enable-ctypes --disable-shared-js --enable-jemalloc --with-ccache --enable-threadsafe <other NSPR options>

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/eb804b2f1e96
user:        Andy Wingo
date:        Wed Dec 18 14:45:09 2013 +0100
summary:     Bug 942804 - Ion-compile scripts with unaliased let bindings. r=jandem

I think this might be a null deref, but locking s-s to be safe, till further analysis.

Is bug 942804 a likely regressor?

Comment 1

[Andy Wingo [:wingo]]

I can reproduce this here with a debug build.  Backtrace in GDB attached; it is slightly different.  This is in an --enable-optimize debug build; will redo to get a trace without.

Comment 2

[Andy Wingo [:wingo]]

Attachment: gdb-optimized-backtrace

Comment 3

[Andy Wingo [:wingo]]

I think that patch is a likely regressor, yes.  Looking at the bytecode for this test case:

js> function foo() { x(["" for (e in 3) if ("")]) }
[New Thread 0x7ffff5b8f700 (LWP 4178)]
[New Thread 0x7ffff59ff700 (LWP 4179)]
[New Thread 0x7ffff57ff700 (LWP 4180)]
[New Thread 0x7ffff577e700 (LWP 4181)]
js> print(disassemble(foo))
flags:
loc     op
-----   --
main:
00000:  callgname "x"
00005:  undefined
00006:  notearg
00007:  newinit 3
00012:  undefined
00013:  int8 3
00015:  iter 1
00017:  goto 28 (+11)
00022:  loophead
00023:  iternext
00024:  setlocal 3
00027:  pop
00028:  loopentry 1
00030:  moreiter
00031:  ifne 22 (-9)
00036:  enditer
00037:  debugleaveblock
00038:  popn 1
00041:  endinit
00042:  notearg
00043:  call 1
00046:  pop
00047:  retrval

Source notes:
 ofs line    pc  delta desc     args
---- ---- ----- ------ -------- ------
  0:    1     0 [   0] colspan 17
  2:    1    17 [  17] xdelta  
  3:    1    17 [   0] for-in   closingjump 14
  5:    1    47 [  30] xdelta  
  6:    1    47 [   0] colspan 28

Exception table:
kind      stack    start      end
 iter         5       22       36

we see that this function would not be ion-compiled beforehand, due to the block scope, and that my patch enabled ion compilation for this code.  I wonder if there is a simple fix on the Ion side.

Comment 4

[Jan de Mooij [:jandem]]

When we see JSOP_NOTEARG we add a MPassArg instruction that wraps the pushed argument. Then when we compile the loop (array comprehension), we add phis for everything on the stack, including the MPassArg for the call's |this| value, breaking the code in isWrapped.

The simplest fix is probably to not create phis for the call's arguments, since there's no way this "loop" can modify these stack slots (the phi is always redundant).

Comment 5

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

Sometimes js::jit::AttachFinishedCompilations and js_InvokeOperationCallback are also on the stack.

Comment 6

[Christian Holler (:decoder)]

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

Comment 7

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

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

The "bad" changeset has the timestamp "20131227100226", the hash "07f8896db19c", and the reason for the result is: Negative exit code -11
The "good" changeset has the timestamp "20131227115226", the hash "1ed3e04b050b", and the reason for the result is: Acceptable exit code 3

Likely fix window: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=07f8896db19c&tochange=1ed3e04b050b

Jan, is bug 952992 a likely fix?

Comment 8

[Jan de Mooij [:jandem]]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #7)
> Jan, is bug 952992 a likely fix?

Yup, this bug was why I filed/fixed bug 952992.

Comment 9

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.