1005590 - Crash [@ js::jit::MacroAssembler::branchIfTrueBool] or Assertion failure: lir->mir()->operand()->mightBeType(MIRType_Object), at jit/CodeGenerator.cpp

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

Attachment: stack

function f(x) {
    "use asm"
    return !(1 || x)
}
for (var j = 0; j < 1; j++) {
    (function(x) {
        +f(+x)
    })()
}

asserts js debug shell on m-c changeset 3285e030d9c0 with --ion-eager --ion-parallel-compile=off at Assertion failure: lir->mir()->operand()->mightBeType(MIRType_Object), at jit/CodeGenerator.cpp

(it goes away with --no-asmjs)

My configure flags are:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --disable-tests --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/4c874962ee02
user:        Boris Zbarsky
date:        Sat May 03 01:08:13 2014 -0400
summary:     Bug 1004198.  Improve codegen in testValueTruthyKernel to emit as few tests as we can get away with given our type inference information.  r=jandem

bz, is bug 1004198 a likely regressor?

(s-s and assuming sec-high first due to this being LIR / MIR-related.)

Comment 1

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

Attachment: Opt stack

This crashes opt shell at js::jit::MacroAssembler::branchIfTrueBool and seems to access weird memory addresses.

Configuration for opt shell:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-optimize --disable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --disable-tests --enable-more-deterministic --with-ccache --enable-threadsafe <other NSPR options>

Comment 2

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

> This crashes opt shell at js::jit::MacroAssembler::branchIfTrueBool and
> seems to access weird memory addresses.

-> 0x10019fafc:  movl   (%r13), %eax
   0x10019fb00:  leal   (%rax,%rax), %ebx
   0x10019fb03:  sarl   %ebx
   0x10019fb05:  testl  %eax, %eax
(lldb) x/b $r13
error: memory read failed for 0x7463656a624f5e00
(lldb) x/b $eax
error: memory read failed for 0x200
(lldb)

Let's assume sec-critical for now.

Comment 3

[Boris Zbarsky [:bzbarsky]]

> bz, is bug 1004198 a likely regressor?

Yes.

This looks like the same issue I ran into with MTest: we start off with an MPhi between an MConstant and an MParameter, then someone swizzles it out from under us.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: MNot can also end up with a known-not-object type even while we think it might emulate undefined. Guard against that

Jan, what's the right way to add this crashtest to our JS tests?

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 8417029 [details] [diff] [review]
MNot can also end up with a known-not-object type even while we think it might emulate undefined.  Guard against that

Review of attachment 8417029 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for fixing this so quickly.

(In reply to Boris Zbarsky [:bz] from comment #4)
> Jan, what's the right way to add this crashtest to our JS tests?

Copy the testcase in comment 0 (or a better one) to js/src/jit-test/tests/ion/bug1005590.js, and make sure the following command shows test failures without the patch:

jit_test.py --tbpl $path-to-js bug1005590

--tbpl runs jit-tests also with "--ion-eager --ion-parallel-compile=off" so you should get at least one failure.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Perfect, thanks.

https://hg.mozilla.org/integration/mozilla-inbound/rev/fbe9c7cc085d

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Gary, does this need to remain security-sensitive?

Comment 11

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

(In reply to Boris Zbarsky [:bz] from comment #10)
> Gary, does this need to remain security-sensitive?

Is this likely exploitable? The rating of sec-high was a guess, would something else be a better rating?

It can be opened up if it is not a security issue.

Comment 12

[Boris Zbarsky [:bzbarsky]]

> Is this likely exploitable? 

I don't know.  All the crash-stats crashes are at 0x4 or 0xffffffff, looks like.

More importantly, this was nightly-only and is fixed...

Comment 14

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed on Fx32