625399 - TM: Crash [@ JSObject::isNative] or [@ js::js_Unbrand] or "Assertion failure: LIR type error (start of writer pipeline): arg 2 of 'calli' is 'immi' which has type int (expected quad)"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

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

(function() {
  "use strict";
  function a(bb) {
    {
      return []
    }
    this.d = function() {
      bb
    }
  }
  for (let c in [0, 0, 0, 0, 0, 0, 0, 0, 0]) {
    a()
  }
})()


crashes 32-bit js opt shell at js::js_Unbrand on TM changeset e60709603fe1 with -j and crashes 32-bit js debug shell at JSObject::isNative and asserts 64-bit js debug shell at Assertion failure: LIR type error (start of writer pipeline): arg 2 of 'calli' is 'immi' which has type int (expected quad)

s-s and assuming sg:critical? because 64-bit LIR type error message is scary.

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   55713:38cbd4e02afc
user:        Jeff Walden
date:        Tue Oct 12 11:50:03 2010 -0700
summary:     Bug 514570 - 3 - Don't box |this| for strict mode functions. r=jorendorff


===

Debug stack:

(gdb) bt
#0  0x001a1bb1 in JSObject::isNative (this=0x0) at jsobj.h:389
#1  0x00228a5e in JSObject::unbrand (this=0x0, cx=0x70a8e0) at jsobjinlines.h:107
#2  0x001e3347 in js::js_Unbrand (cx=0x70a8e0, obj=0x0) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-60415-e60709603fe1/compilePath/jstracer.cpp:16146
#3  0x0161ff8e in ?? ()
#4  0x001db927 in js::ExecuteTrace (cx=0x70a8e0, f=0x884f54, state=@0xbfffe7bc) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-60415-e60709603fe1/compilePath/jstracer.cpp:6467
#5  0x001e9b46 in js::ExecuteTree (cx=0x70a8e0, f=0x884f54, inlineCallCount=@0xbffff508, innermostNestedGuardp=0xbfffe8b8, lrp=0xbfffe8bc) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-60415-e60709603fe1/compilePath/jstracer.cpp:6576
#6  0x00211645 in js::RecordLoopEdge (cx=0x70a8e0, inlineCallCount=@0xbffff508) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-60415-e60709603fe1/compilePath/jstracer.cpp:7134
#7  0x0021182c in js::MonitorLoopEdge (cx=0x70a8e0, inlineCallCount=@0xbffff508) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-60415-e60709603fe1/compilePath/jstracer.cpp:17093
#8  0x000c3df3 in js::Interpret () at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-60415-e60709603fe1/compilePath/jsinterp.cpp:2954
#9  0x000ea3e8 in js::RunScript (cx=0x70a8e0, script=0x70e510, fp=0x1020030) at jsinterp.cpp:657
#10 0x000eaa94 in js::Execute (cx=0x70a8e0, chain=0x1502028, script=0x70e510, prev=0x0, flags=0, result=0xbffff6c0) at jsinterp.cpp:1023
#11 0x000241e2 in JS_ExecuteScript (cx=0x70a8e0, obj=0x1502028, script=0x70e510, rval=0xbffff6c0) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-60415-e60709603fe1/compilePath/jsapi.cpp:4883
#12 0x0001679a in Process (cx=0x70a8e0, obj=0x1502028, filename=0x0, forceTTY=0) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-60415-e60709603fe1/compilePath/shell/js.cpp:548
#13 0x00017256 in ProcessArgs (cx=0x70a8e0, obj=0x1502028, argv=0xbffff84c, argc=1) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-60415-e60709603fe1/compilePath/shell/js.cpp:951
#14 0x00017394 in Shell (cx=0x70a8e0, argc=1, argv=0xbffff84c, envp=0xbffff854) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-60415-e60709603fe1/compilePath/shell/js.cpp:5437
#15 0x000174fb in main (argc=1, argv=0xbffff84c, envp=0xbffff854) at /Users/fuzz3/Desktop/jsfunfuzz-dbg-32-tm-60415-e60709603fe1/compilePath/shell/js.cpp:5545

Comment 1

[Julian Seward [:jseward]]

What does valgrind say, for both the 32- and 64-bit cases?

Comment 2

[Jason Orendorff [:jorendorff]]

Attachment: v1

Pretty straightforward.

Comment 3

[Jeff Walden [:Waldo]]

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

Looking at the third (ugh) implementation in the tree of this (it seems I fixed interpreter but not tracer when writing this originally), does stubs::Unbrand need to compute this?  Seems to me it does, unbrandthis is in the prolog so you can't have a this op that already computed, or whatever.

Comment 4

[Jason Orendorff [:jorendorff]]

I spent about an hour on IRC pestering everybody about comment 3, and I just spent about another hour in the MSVC debugger, driving around in marshes, setting data breakpoints and pulling up stumps, and I just now figured it out.

stubs::Unbrand() doesn't need to do this because the compiler does:

          BEGIN_CASE(JSOP_UNBRANDTHIS)
            jsop_this();
            jsop_unbrand();
            frame.pop();
          END_CASE(JSOP_UNBRANDTHIS)

jsop_this(), of course, emits code to compute this.

I'll push this patch as-is tomorrow.

Comment 5

[Jason Orendorff [:jorendorff]]

http://hg.mozilla.org/tracemonkey/rev/4cda18415cd6

Comment 6

[Chris Leary [:cdleary] (not checking bugmail)]

cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/4cda18415cd6

Comment 7

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.