547911 - TM: Crash [@ js_CallIteratorNext] or "Assertion failure: !JSVAL_IS_PRIMITIVE(regs.sp[-2]), at ../jsops.cpp" or "Assertion failure: regs.sp == StackBase(fp), at ../jsops.cpp" with defineGetter, StopIteration

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

e = 1
function f(code) {
  var g = Function(code);
  try {
    g()
  } catch(e) {}
}
(function() {
  x = let(e) 3
})()
f("for(q=0;;){l}")
c = 1
f("__defineGetter__(\"\",(Function(\"throw StopIteration\")))")
for each(a in [0]) {
  for each(b in this);
}


crashes js opt shell with -j on TM tip near null at js_CallIteratorNext and asserts js debug shell with -j on TM tip at Assertion failure: !JSVAL_IS_PRIMITIVE(regs.sp[-2]), at ../jsops.cpp:489

To reproduce this, pass the testcase as a CLI argument. (e.g. ./js -j testcase.js)

===

$ ./js-dbg-32-tm-darwin -j testcase.js 
Assertion failure: !JSVAL_IS_PRIMITIVE(regs.sp[-2]), at ../jsops.cpp:489
Bus error
$ ./js-opt-32-tm-darwin -j testcase.js 
Bus error

===

js opt shell crash stack:

Exception Type:  EXC_BAD_ACCESS (SIGBUS)
Exception Codes: KERN_PROTECTION_FAILURE at 0x0000000000000004
Crashed Thread:  0  Dispatch queue: com.apple.main-thread

Thread 0 Crashed:  Dispatch queue: com.apple.main-thread
0   js-opt-32-tm-darwin           	0x00061792 js_CallIteratorNext + 18
1   js-opt-32-tm-darwin           	0x00055441 js_Interpret + 24193
2   js-opt-32-tm-darwin           	0x0005f7f1 js_Execute + 625
3   js-opt-32-tm-darwin           	0x0000d5bc JS_ExecuteScript + 60
4   js-opt-32-tm-darwin           	0x00004515 Process(JSContext*, JSObject*, char*, int) + 1621
5   js-opt-32-tm-darwin           	0x00008386 main + 1734
6   js-opt-32-tm-darwin           	0x0000229d _start + 208
7   js-opt-32-tm-darwin           	0x000021cc start + 40

Comment 1

[Andreas Gal :gal]

WFM with TM tip (macosx debug shell 64-bit). Please check again.

Comment 2

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

(In reply to comment #1)
> WFM with TM tip (macosx debug shell 64-bit). Please check again.

I reproduce this on 32-bit shell. I had to pass the testcase in as a CLI argument.

autoBisect shows this is probably related to bug 479109:

The first bad revision is:
changeset:   25410:094c2e9de304
user:        Andreas Gal
date:        Wed Feb 25 18:47:22 2009 -0800
summary:     Improve blacklisting (479109, r=graydon).

Comment 3

[Jason Orendorff [:jorendorff]]

I reproduce this. Minimized test case:

a = b = c = d = 0;
__defineGetter__("e", function () { throw StopIteration; })
for each(f in this);

Comment 4

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

(In reply to comment #3)
> I reproduce this. Minimized test case:
> 
> a = b = c = d = 0;
> __defineGetter__("e", function () { throw StopIteration; })
> for each(f in this);

This doesn't crash js opt shell with -j, and instead asserts js debug shells with -j at Assertion failure: regs.sp == StackBase(fp), at ../jsops.cpp:219

Comment 5

[Jason Orendorff [:jorendorff]]

I noticed it asserts in a different place, but it's still the same bug. I think this should reproduce the behavior of the test case in comment 0:

  a = b = c = d = 0;
  __defineGetter__("e", function () { throw StopIteration; })
  for each(a in [0])
    for each(f in this);

But in either case, what's happening is that we throw StopIteration and fall off trace, and we leave the interpreter's sp pointing to the wrong thing. You don't have to be iterating over the global:

  var obj = {a: 0, b: 0, c: 0, d: 0, get e() { throw StopIteration; }};
  for each (x in obj) {}  // asserts

I found the likely cause. We have an unconscionable hack in record_NativeCallComplete where we emit code to recover from StopIteration. It's leaving the stack in the wrong state.

The hack was added in bug 485022 without a test. Booooo! Any test at all would have caught this bug when it was later introduced.

Comment 6

[Jason Orendorff [:jorendorff]]

Hey, apologies -- a test for that particular bug wouldn't have caught this one after all. There was no specific previous bug in this code that really should have resulted in a test for this.

However I now know whom to blame for the bug -- it's the joker who checked in
http://hg.mozilla.org/tracemonkey/rev/2773d74789f4 ...

Comment 7

[Jason Orendorff [:jorendorff]]

Attachment: v1

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 428816 [details] [diff] [review]
v1

Oh good -- I never understood that JSOP_TRUE case. A mystery solved.

/be

Comment 9

[Jason Orendorff [:jorendorff]]

http://hg.mozilla.org/tracemonkey/rev/337699f6a88a

Comment 10

[Jason Orendorff [:jorendorff]]

An earlier version of the custom_next_imacro used JSOP_TRUE to tell the interpreter that next() returned a value. That was before we were catching StopIteration thrown from custom next() methods.

Comment 11

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/337699f6a88a

Comment 12

[Christian Holler (:decoder)]

A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/bug547911-1.js.