Last Comment Bug 554670 - TM: "Assertion failed: LIR type error (start of writer pipeline): arg 1 of 'fmul' is 'int' which has type int32 (expected float64): 0 (../nanojit/LIR.cpp" or "Assertion failure: m != TT_INT32 || isInt32(*vp), at ../jstracer.cpp"
: TM: "Assertion failed: LIR type error (start of writer pipeline): arg 1 of 'f...
Status: VERIFIED FIXED
[sg:critical?]fixed-in-tracemonkey
: assertion, regression, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Mac OS X
: -- critical (vote)
: ---
Assigned To: Robert Sayre
:
:
Mentors:
Depends on: 547299
Blocks: jsfunfuzz 453730
  Show dependency treegraph
 
Reported: 2010-03-24 09:54 PDT by Gary Kwong [:gkw] [:nth10sd]
Modified: 2013-01-14 07:50 PST (History)
10 users (show)
choller: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta1+
.11+
.11-fixed
unaffected


Attachments
Patch (2.01 KB, patch)
2010-04-08 16:02 PDT, David Mandelin [:dmandelin]
jorendorff: review+
christian: approval1.9.2.11+
Details | Diff | Splinter Review

Description Gary Kwong [:gkw] [:nth10sd] 2010-03-24 09:54:17 PDT
x = true;
(function() {
  for each(let c in [0, x]) {
    (arguments)[4] *= c
  }
})()

asserts js debug shell on JM tip with -m and -j at Assertion failed: LIR type error (start of writer pipeline): arg 1 of 'fmul' is 'int' which has type int32 (expected float64): 0 (../nanojit/LIR.cpp:2610)
Comment 1 Gary Kwong [:gkw] [:nth10sd] 2010-03-24 10:02:36 PDT
A variant testcase asserts differently:

var c;
(function() {
  for each(e in [0, 0]) {
    (arguments)[1] *= c
  }
})()


Assertion failure: m != TT_INT32 || isInt32(*vp), at ../jstracer.cpp:3796
Comment 2 David Anderson [:dvander] 2010-03-24 13:05:22 PDT
This affects tm-tip too.
Comment 3 David Anderson [:dvander] 2010-03-24 13:38:45 PDT
Yuck. The problem is that the tracer expects arguments[n] to be undefined if |n| is out of range. In this case, it is _not_ undefined, it is NaN, because the interpreter did this:

  arguments[4] = undefined * 0
Comment 4 Gary Kwong [:gkw] [:nth10sd] 2010-03-24 17:48:04 PDT
You're right - it occurs on TM tip with -j. In the past, it used to assert at:

Assertion failed: ((unsigned)r)<8 (../nanojit/Nativei386.cpp:427)

or

Assertion failed: s0->isQuad() && s1->isQuad() (../nanojit/LIR.cpp:2182)

before morphing to the current form.

autoBisect shows this is probably related to bug 453730:

The first bad revision is:
changeset:   30020:c76558a87dd9
user:        David Mandelin
date:        Wed Jul 08 11:16:41 2009 -0700
summary:     Bug 453730: trace JSOP_ARGUMENTS, r=gal
Comment 5 David Mandelin [:dmandelin] 2010-04-08 15:55:10 PDT
dvander and I talked this over, and we think all that it takes to fix this is to abort tracing a getelem with arguments (args[expr]) if the index is a constant that is out of range. The tracer already aborts if the index is a non-constant that is out of range, so this just makes things consistent. 

ETA = patch up for review later today.
Comment 6 David Mandelin [:dmandelin] 2010-04-08 16:02:30 PDT
Created attachment 437988 [details] [diff] [review]
Patch
Comment 7 David Mandelin [:dmandelin] 2010-04-08 16:03:12 PDT
Adding bug 547299 as a dep because it needs to land on 1.9.2 before the patch for this bug can land there.
Comment 8 Jason Orendorff [:jorendorff] 2010-04-08 16:11:22 PDT
Comment on attachment 437988 [details] [diff] [review]
Patch

The patch looks good.

Please put newlines on the last line of the two test files.
Comment 9 David Mandelin [:dmandelin] 2010-04-09 11:16:13 PDT
http://hg.mozilla.org/tracemonkey/rev/bbdddef55da3
Comment 11 Daniel Veditz [:dveditz] 2010-06-11 15:45:29 PDT
(In reply to comment #7)
> Adding bug 547299 as a dep because it needs to land on 1.9.2 before the patch
> for this bug can land there.

That one's fixed in 1.9.2.4, please add a 1.9.2 patch for this bug (or request approval on the trunk patch if it applies)
Comment 12 Reed Loden [:reed] (use needinfo?) 2010-09-28 21:40:43 PDT
Why did this just land on 1.9.2 without approval? This isn't a new policy, and I've said this before. All branch patches must have explicit approval before landing. blocking+ is not enough.
Comment 13 Robert Sayre 2010-09-28 21:42:46 PDT
(In reply to comment #12)
> Why did this just land on 1.9.2 without approval? This isn't a new policy, and
> I've said this before. All branch patches must have explicit approval before
> landing. blocking+ is not enough.

It's set to blocking 1.9.2.11. I don't even know why am answering this nonsense.
Comment 15 Christian Holler (:decoder) 2012-03-28 12:02:36 PDT
Tracer has been long gone on trunk, marking verified.
Comment 16 Christian Holler (:decoder) 2013-01-14 07:50:33 PST
A testcase for this bug was automatically identified at js/src/jit-test/tests/arguments/bug554670-1.js.

Note You need to log in before you can comment on or make changes to this bug.