567577 - TM: (64-bit) "Assertion failed: LIR type error (start of writer pipeline): arg 1 of 'ui2uq' is 'cmovq' which has type int64 (expected int32): 0 ("

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

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

for (a in [/x/, null, new String - Infinity]) {
  print(new evalcx(" "))
}

asserts js debug shell on TM tip (64-bit) with -j at Assertion failed: LIR type error (start of writer pipeline): arg 1 of 'ui2uq' is 'cmovq' which has type int64 (expected int32): 0 (../nanojit/LIR.cpp:2642)

s-s because this is an LIR type error (generated code error)

Comment 1

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

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   33797:60ec3940a434
user:        Brendan Eich
date:        Sun Oct 18 17:41:24 2009 -0700
summary:     Don't build a stack frame to execute empty scripts (516827, r=igor).

Comment 2

[Brendan Eich [:brendan]]

Sayrer, can you get someone with 64-bit chops to debug this? I can't repro atm.

/be

Comment 3

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

I tried out the technique in bug 558633 to try and get more information, but seem to crash instead:


js> for (a in [/x/, null, new String - Infinity]) {
  print(new evalcx(" "))
}
undefined
undefined

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
0x00007fff846722b3 in __vfprintf ()
(gdb) 
(gdb) bt
#0  0x00007fff846722b3 in __vfprintf ()
#1  0x00007fff846b507b in vfprintf_l ()
#2  0x00007fff846e273b in printf ()
#3  0x00000001001c17d4 in nanojit::ValidateWriter::typeCheckArgs (this=0x1008c2b28, op=nanojit::LIR_ui2uq, nArgs=1, formals=0x7fff5fbfe640, args=0x7fff5fbfe630) at ../nanojit/LIR.cpp:2642
#4  0x00000001001c23ec in nanojit::ValidateWriter::ins1 (this=0x1008c2b28, op=nanojit::LIR_ui2uq, a=0x1008bea10) at ../nanojit/LIR.cpp:2882
#5  0x00000001001a7f79 in nanojit::LirWriter::insUI2P (this=0x1008c2b28, uintIns=0x1008bea10) at LIR.h:1520
#6  0x000000010019abb7 in js::TraceRecorder::box_jsval (this=0x10041f4c0, v=22, v_ins=0x1008bea10) at ../jstracer.cpp:9355
#7  0x000000010019f4ef in js::TraceRecorder::callNative (this=0x10041f4c0, argc=1, mode=JSOP_CALL) at ../jstracer.cpp:10877
#8  0x00000001001a02d3 in js::TraceRecorder::functionCall (this=0x10041f4c0, argc=1, mode=JSOP_CALL) at ../jstracer.cpp:11020
#9  0x00000001001a0386 in js::TraceRecorder::record_JSOP_CALL (this=0x10041f4c0) at ../jstracer.cpp:12616
#10 0x00000001001a5aca in js::TraceRecorder::monitorRecording (this=0x10041f4c0, op=JSOP_CALL) at jsopcode.tbl:179
#11 0x0000000100084c31 in js_Interpret (cx=0x10089f200) at jsops.cpp:78
#12 0x00000001000ad84a in js_Execute (cx=0x10089f200, chain=0x101402000, script=0x100415560, down=0x0, flags=0, result=0x7fff5fbff4b8) at jsinterp.cpp:837
#13 0x0000000100011b73 in JS_ExecuteScript (cx=0x10089f200, obj=0x101402000, script=0x100415560, rval=0x7fff5fbff4b8) at ../jsapi.cpp:4832
#14 0x0000000100009c9d in Process (cx=0x10089f200, obj=0x101402000, filename=0x0, forceTTY=0) at ../../shell/js.cpp:515
#15 0x000000010000a569 in ProcessArgs (cx=0x10089f200, obj=0x101402000, argv=0x7fff5fbff6a0, argc=1) at ../../shell/js.cpp:836
#16 0x000000010000a6e4 in main (argc=1, argv=0x7fff5fbff6a0, envp=0x7fff5fbff6b0) at ../../shell/js.cpp:5077
(gdb) f 3
#3  0x00000001001c17d4 in nanojit::ValidateWriter::typeCheckArgs (this=0x1008c2b28, op=nanojit::LIR_ui2uq, nArgs=1, formals=0x7fff5fbfe640, args=0x7fff5fbfe630) at ../nanojit/LIR.cpp:2642
2642                        type2string(actual), type2string(formal));
(gdb) p actual
$1 = nanojit::LTy_Q
(gdb) p formal
$2 = nanojit::LTy_I
(gdb) x actual
0x2:    Cannot access memory at address 0x2
(gdb) x formal
0x1:    Cannot access memory at address 0x1
(gdb) x/i $rip
0x1001c17d4 <_ZN7nanojit14ValidateWriter13typeCheckArgsENS_7LOpcodeEiPNS_3LTyEPPNS_4LInsE+312>: incl   -0x1c(%rbp)
(gdb) x/b $rbp
0x7fff5fbfe600: 0x50

Comment 4

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

Oops, the crash was my fault, here is the needed data:

(the "*"s denote where the instructions 'ui2uq' and 'cmovq' are located)

=== BEGIN Initial LIR ===
  00: start
  01: rbx = paramq 0 rbx
  02: r12 = paramq 1 r12
  03: r13 = paramq 2 r13
  04: r14 = paramq 3 r14
  05: r15 = paramq 4 r15
  06: state = paramq 0 rdi
  07: label1:
  08: sp = ldq.o state[16]
  09: rp = ldq.o state[48]
  10: cx = ldq.o state[0]
  11: eos = ldq.o state[24]
  12: eor = ldq.o state[56]
  13: ldi3 = ldi.csro cx[0]
  14: immi1 = immi 0
  15: eqi3 = eqi ldi3, immi1/*0*/
  16: xf8: xf eqi3 -> pc=0x1004149a3 imacpc=0x0 sp+8 rp+0 (GuardID=001)
  17: globalObj = immq 20979712LL
  18: $global0 = ldq.o eos[1144]
  19: $stack0_2 = ldq.s sp[0]
  20: ldq6 = ldq.o $stack0_2[8]
  21: ~JSSLOT_CLASS_MASK_BITS = immq -4LL
  22: andq4 = andq ldq6, ~JSSLOT_CLASS_MASK_BITS/*-4LL*/
  23: clasp = immq 2504192LL
  24: guard(class is Iterator) = eqq andq4, clasp/*2504192LL*/
  25: xf7: xf guard(class is Iterator) -> pc=0x1004149a4 imacpc=0x0 sp+8 rp+0 (GuardID=002)
  26: ldq1 = ldq.c $stack0_2[32]
  27: cursor = ldq.o ldq1[8]
  28: next = ldq.o cursor[0]
  29: JSVAL_INT = immq 1LL
  30: andq3 = andq next, JSVAL_INT/*1LL*/
  31: NULL = immq 0LL
  32: eqq7 = eqq andq3, NULL/*0LL*/
  33: xt3: xt eqq7 -> pc=0x1004149a4 imacpc=0x0 sp+8 rp+0 (GuardID=003)
  34: immi2 = immi 1
  35: rshq1 = rshq next, immi2/*1*/
  36: q2i1 = q2i rshq1
  37: js_IntToString1 = callq. #js_IntToString ( cx q2i1 )
  38: eqq6 = eqq js_IntToString1, NULL/*0LL*/
  39: xt2: xt eqq6 -> pc=0x1004149a4 imacpc=0x0 sp+8 rp+0 (GuardID=004)
  40: sizeof(jsval) = immq 8LL
  41: addq3 = addq cursor, sizeof(jsval)/*8LL*/
  42: stq.o ldq1[8] = addq3
  43: stq.o eos[1144] = js_IntToString1
  44: pcval.toObject() = immq 20987168LL
  45: stq.s sp[8] = pcval.toObject()/*20987168LL*/
  46: stq.s sp[16] = globalObj/*20979712LL*/
  47: $global1 = ldq.o eos[904]
  48: stq.s sp[24] = $global1
  49: stq.s sp[32] = NULL/*0LL*/
  50: ATOM_TO_STRING(atom) = immq 2517744LL
  51: stq.s sp[40] = ATOM_TO_STRING(atom)/*2517744LL*/
  52: callee_fun = immq 20990816LL
  53: ldq5 = ldq.o $global1[32]
  54: eqq5 = eqq ldq5, callee_fun/*20990816LL*/
  55: xf6: xf eqq5 -> pc=0x1004149b1 imacpc=0x0 sp+48 rp+0 (GuardID=005)
  56: allocp2 = allocp 32
  57: stq.o allocp2[0] = callee_fun/*20990816LL*/
  58: clasp2 = immq 2508096LL
  59: js_NewInstance1 = callq.sro #js_NewInstance ( cx clasp2/*2508096LL*/ callee_fun/*20990816LL*/ )
  60: eqq4 = eqq js_NewInstance1, NULL/*0LL*/
  61: xt1: xt eqq4 -> pc=0x1004149b1 imacpc=0x0 sp+48 rp+0 (GuardID=006)
  62: stq.o allocp2[8] = js_NewInstance1
  63: JSVAL_STRING = immq 4LL
  64: orq2 = orq ATOM_TO_STRING(atom)/*2517744LL*/, JSVAL_STRING/*4LL*/
  65: stq.o allocp2[16] = orq2
  66: JSVAL_VOID = immq 22LL
  67: stq.o allocp2[24] = JSVAL_VOID/*22LL*/
  68: offset = immq 24LL
  69: addq1 = addq allocp2, offset/*24LL*/
  70: 2 * sizeof(jsval) = immq 16LL
  71: addq2 = addq allocp2, 2 * sizeof(jsval)/*16LL*/
  72: vplen = immi 4
  73: sti.o state[160] = vplen/*4*/
  74: stq.o state[168] = allocp2
  75: exit = immq 9178928LL
  76: stq.o cx[872] = exit/*9178928LL*/
  77: xbarrier2: xbarrier  -> pc=0x1004149b1 imacpc=0x0 sp+48 rp+0 (GuardID=007)
  78: evalcx1 = calli.sro #evalcx ( cx js_NewInstance1 immi2/*1*/ addq2 addq1 )
  79: stq.o state[168] = NULL/*0LL*/
  80: sti.s sp[24] = evalcx1
  81: stq.o cx[872] = NULL/*0LL*/
  82: ldi2 = ldi.o state[144]
  83: ldq4 = ldq.o addq1[0]
  84: JSVAL_TAGMASK = immq 7LL
  85: andq2 = andq ldq4, JSVAL_TAGMASK/*7LL*/
  86: eqq3 = eqq andq2, NULL/*0LL*/
* 87: cmovq2 = cmovq eqq3 ? ldq4 : NULL/*0LL*/
* 88: eqq2 = eqq cmovq2, NULL/*0LL*/
* 89: cmovq1 = cmovq eqq2 ? js_NewInstance1 : cmovq2
* 90: stq.s sp[24] = cmovq1
  91: andi2 = andi evalcx1, immi2/*1*/
  92: xori2 = xori andi2, immi2/*1*/
  93: lshi2 = lshi xori2, immi2/*1*/
  94: ori2 = ori ldi2, lshi2
  95: sti.o state[144] = ori2
  96: eqi2 = eqi ori2, immi1/*0*/
  97: xf5: xf eqi2 -> pc=0x1004149b4 imacpc=0x0 sp+32 rp+0 (GuardID=008)
  98: allocp1 = allocp 24
  99: stq.o allocp1[0] = pcval.toObject()/*20987168LL*/
  100: stq.o allocp1[8] = globalObj/*20979712LL*/
  101: JSVAL_SPECIAL = immq 6LL
* 102: ui2uq1 = ui2uq cmovq1
  103: vplen2 = immi 3
* 104: lshq1 = lshq ui2uq1, vplen2/*3*/
  105: orq1 = orq lshq1, JSVAL_SPECIAL/*6LL*/
  106: stq.o allocp1[16] = orq1
  107: sti.o state[160] = vplen2/*3*/
  108: stq.o state[168] = allocp1
  109: exit2 = immq 9179240LL
  110: stq.o cx[872] = exit2/*9179240LL*/
  111: xbarrier1: xbarrier  -> pc=0x1004149b4 imacpc=0x0 sp+32 rp+0 (GuardID=009)
  112: print1 = calli.sro #print ( cx immi2/*1*/ allocp1 )
  113: stq.o state[168] = NULL/*0LL*/
  114: sti.s sp[8] = print1
  115: stq.o cx[872] = NULL/*0LL*/
  116: ldi1 = ldi.o state[144]
  117: ldq3 = ldq.o allocp1[0]
  118: stq.s sp[8] = ldq3
  119: andi1 = andi print1, immi2/*1*/
  120: xori1 = xori andi1, immi2/*1*/
  121: lshi1 = lshi xori1, immi2/*1*/
  122: ori1 = ori ldi1, lshi1
  123: sti.o state[144] = ori1
  124: eqi1 = eqi ori1, immi1/*0*/
  125: xf4: xf eqi1 -> pc=0x1004149b7 imacpc=0x0 sp+16 rp+0 (GuardID=010)
  126: eqq1 = eqq ldq3, JSVAL_VOID/*22LL*/
  127: xf3: xf eqq1 -> pc=0x1004149b7 imacpc=0x0 sp+16 rp+0 (GuardID=011)
  128: JSVAL_TO_SPECIAL(JSVAL_VOID) = immi 2
  129: sti.s sp[8] = JSVAL_TO_SPECIAL(JSVAL_VOID)/*2*/
  130: ldq2 = ldq.o $stack0_2[8]
  131: andq1 = andq ldq2, ~JSSLOT_CLASS_MASK_BITS/*-4LL*/
  132: guard(class is Iterator) = eqq andq1, clasp/*2504192LL*/
  133: xf2: xf guard(class is Iterator) -> pc=0x1004149b9 imacpc=0x0 sp+8 rp+0 (GuardID=012)
  134: cursor2 = ldq.o ldq1[8]
  135: end = ldq.o ldq1[16]
  136: ltq1 = ltq cursor2, end
  137: xf1: xf ltq1 -> pc=0x1004149b9 imacpc=0x0 sp+8 rp+0 (GuardID=013)
  138: j -> label1
  139: liveq state
  140: x1: x  -> pc=0x1004149a3 imacpc=0x0 sp+8 rp+0 (GuardID=014)
=== END Initial LIR ===

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

<dvander> jorendorff, bug 567577 - |new evalcx(" ")| returns undefined, this confuses the trace recorder
<jorendorff> it should be impossible for |new evalcx(" ")| to return undefined
<jorendorff> yet it does, or so it seems
<shaver> that is a bug in JSOP_NEW then
<jorendorff> I agree.
<jorendorff> dvander: This is an interpreter bug, to start with.
<jorendorff> the recorder is probably correct to assume that |new| can't produce anything but an object

Comment 6

[David Anderson [:dvander] - inactive, e-mail if emergency]

critsmash update: I'll have a patch for this by the end of the week.

Comment 7

[Damon Sicore (:damons)]

(In reply to comment #6)
> critsmash update: I'll have a patch for this by the end of the week.

Thanks, dvander.  This is helpful.

Comment 9

[Jeff Walden [:Waldo]]

Fun testcase for a semi-latent GC hazard, discovered while reading code related to this:

  gczeal(11);
  var o = { valueOf: function() { new Object(); gc(); return 17; } };
  (new String.prototype.substring(o)).toString;

Were we to pass JS_TRUE for clampReturn in JSOP_NEW's implementation -- which we do not, which seems to be the fundamental bug as far as the testcase in comment 0 is concerned, and which we've never done in the history of js_InvokeConstructor not always constraining primitive-value returns -- we'd construct the default return of the constructor and store it in a stack local.  Then we'd store that in vp[1] to root it -- but what if a fastnative is invoked to overwrite that location (and to overwrite the weakroot for it, natch)?  Running through the above in a debugger, with the clampReturn change as noted at start of paragraph, we only ever trace the object from the implicit stack-root noted by conservative stack scanning.

The not-constraining behavior dates back a ways (to bug 453462 in September 2008!), so if we change the clampReturn argument value anywhere without conservative stack scanning (thinking old branches here), I'm pretty sure we need to add some extra rooting chops for the default-return-object of the constructor.

Comment 10

[Jeff Walden [:Waldo]]

I should further note that this problem occurs when js_InvokeConstructor is called with clampReturn==JS_TRUE in unconstrained ways.  This seems only to be the case for fun_applyConstructor (Narcissus only, don't care), __noSuchMethod__ in the constructing case (is it possible?  bug 520778 says no for at least the obvious way, are there others I've missed?), and JS_New (very new, so not a worry as far as current exploitability since we can assume conservative stack-scanning).  The middle case is possibly worrisome as far as branches are concerned, but it may well be impossible.  More investigation is probably necessary to determine whether __noSuchMethod__ can ever be invoked in a constructing case, since nobody seems to know for certain (as far as I can tell).

So basically, comment 9 may require followup even if we were never to change to clampReturn==JS_TRUE on branches.  :-\

Comment 11

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

Thanks for noticing this rooting bug, Jeff. This indeed affects branches - though once we can rely on conservative stack scanning, the rooter can go 'way.

Comment 12

[Robert Sayre]

can we land this?

Comment 13

[David Anderson [:dvander] - inactive, e-mail if emergency]

http://hg.mozilla.org/tracemonkey/rev/839073dc9b77

Comment 14

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/839073dc9b77

Comment 15

[Daniel Veditz [:dveditz]]

It appears that the branch-affecting rooting bug from comment 9 was spun off and fixed in bug 572232. I can't get the original testcase to do anything bad in a 1.9.2 branch build and comment 1 indicates the symptoms started after that branch was cut. Can I safely say that the 1.9.2/1.9.1 branches are unaffected by this bug?

Comment 16

[Christian Holler (:decoder)]

Tracer has been long gone on trunk, marking verified.

Comment 17

[Christian Holler (:decoder)]

Verified 1.9.x to be unaffected by the test in comment 0.

Comment 18

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929

Comment 19

[Nicholas Nethercote [inactive]]

> Automatically extracted testcase for this bug was committed:
> 
> https://hg.mozilla.org/mozilla-central/rev/efaf8960a929

The bug was in the trace JIT, which is long gone.  So this test isn't much use.

Comment 20

[Christian Holler (:decoder)]

(In reply to Nicholas Nethercote [:njn] from comment #19)

> The bug was in the trace JIT, which is long gone.  So this test isn't much
> use.

These regression tests usually don't only detect the original regression but also other regressions. Furthermore, they serve as a basis for mutation based fuzzing. Overall if the test isn't causing any trouble or speed penalties I don't see a reason not to add it.