1181796 - OdinMonkey: Crash at SIGTRAP with testcase involving "use asm" and --ion-check-range-analysis

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

g = (function() {
    "use asm"
    function f(i0) {
        i0 = i0 | 0
        return ((i0 >>> 32) % 2) | 0
    }
    return f
})()
g(4294967295)

crashes js debug shell on m-c changeset f34a7120f46b with --fuzzing-safe --no-threads --ion-eager --ion-check-range-analysis at a SIGTRAP. (It also seems to crash opt shells.)

Configure options:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/funfuzz/js/compileShell.py -b "--enable-debug --enable-more-deterministic --enable-nspr-build" -r f34a7120f46b

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/596ee431b3d2
user:        Sander Mathijs van Veen
date:        Tue Jul 07 17:03:59 2015 +0200
summary:     bug 1180874 - Use DivOrModConstantI for unsigned division or modulo by constant. r=nbp

Setting s-s to be safe because this is a SIGTRAP. Please feel free to open it up if it is not s-s.

Nicolas, is bug 1180874 a likely regressor?

Comment 1

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

Attachment: stack

Comment 2

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

(lldb) bt
* thread #1: tid = 0x25b13e, 0x0000000101cf708a, queue = 'com.apple.main-thread', stop reason = EXC_BREAKPOINT (code=EXC_I386_BPT, subcode=0x0)
  * frame #0: 0x0000000101cf708a
    frame #1: 0x0000000100226439 js-dbg-64-dm-nsprBuild-darwin-f34a7120f46b`js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) [inlined] js::CallJSNative(cx=0x0000000103b23000, native=0x0000000102809640)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 203 at jscntxtinlines.h:235
    frame #2: 0x000000010022636e js-dbg-64-dm-nsprBuild-darwin-f34a7120f46b`js::Invoke(cx=0x0000000103b23000, args=CallArgs at 0x00007fff5fbfe4b0, construct=<unavailable>) + 526 at Interpreter.cpp:711
    frame #3: 0x000000010020a324 js-dbg-64-dm-nsprBuild-darwin-f34a7120f46b`js::Invoke(cx=0x00000001028a3180, thisv=0x00007fff5fbfe6b8, fval=<unavailable>, argc=<unavailable>, argv=<unavailable>, rval=<unavailable>) + 692 at Interpreter.cpp:768
    frame #4: 0x00000001004b4fd6 js-dbg-64-dm-nsprBuild-darwin-f34a7120f46b`js::jit::DoCallFallback(cx=0x00000001028a3180, frame=0x00007fff5fbfe9b8, stub_=0x00000001028b7890, argc=1, vp=0x00007fff5fbfe968, res=<unavailable>) + 1958 at BaselineIC.cpp:9859
    frame #5: 0x0000000101fb820b
(lldb)

Comment 3

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

Setting [fuzzblocker] because this is also throwing up various testcases with empty stack traces.

Comment 4

[Sander Mathijs van Veen]

FYI The sigtrap is caused by the --ion-check-range-analysis flag:

> smvv@multivac ~/work/leaningtech/tests/modulo-with-int32 $ ../../sm-js --ion-check-range-analysis unsigned-mod-div-crash.js 
> Trace/breakpoint trap
> smvv@multivac ~/work/leaningtech/tests/modulo-with-int32 $ ../../sm-js -D unsigned-mod-div-crash.js
> --- SCRIPT unsigned-mod-div-crash.js:1 ---
> --- END SCRIPT unsigned-mod-div-crash.js:1 ---
> --- Asm.js Module ---
> IonScript [1 blocks]:
> BB #0 [00000] :: 1 hits
> [AsmJSParameter]
> [MoveGroup]
> movl       %edi, 0xc(%rsp)
> movl       %edi, %eax
> [ShiftI:ursh]
> [MoveGroup]
> movl       %eax, 0xc(%rsp)
> [ModPowTwoI]
> testl      %eax, %eax
> js         .Lfrom68
> andl       $0x1, %eax
> jmp        .Lfrom76
> .set .Llabel76, .
> .set .Lfrom68, .Llabel76
> negl       %eax
> andl       $0x1, %eax
> negl       %eax
> .set .Llabel83, .
> .set .Lfrom76, .Llabel83
> [MoveGroup]
> movl       %eax, 0xc(%rsp)
> [AsmJSReturn]
> 
> --- END Asm.js Module ---

Comment 5

[Nicolas B. Pierron [:nbp] {away}]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #0)
> Nicolas, is bug 1180874 a likely regressor?

Yes, jandem already backed it out.

Comment 6

[Christian Holler (:decoder)]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision adfdc7f29ba7).

Comment 7

[Benjamin Bouvier [:bbouvier] (inactive)]

Fixed by backout of bug 1180874. Test case should be useful, so I've copied it over there.

Comment 8

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 9

[Al Billings [:abillings - ex-MoCo]]

Did this affect Firefox 41 or did we avoid shipping this?

Comment 10

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

(In reply to Al Billings [:abillings] from comment #9)
> Did this affect Firefox 41 or did we avoid shipping this?

I think we did avoid shipping this, see bug 1180874 comment 17.

Moving needinfo? to :nbp to double check this.

Comment 11

[Nicolas B. Pierron [:nbp] {away}]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #10)
> (In reply to Al Billings [:abillings] from comment #9)
> > Did this affect Firefox 41 or did we avoid shipping this?
> 
> I think we did avoid shipping this, see bug 1180874 comment 17.
> 
> Moving needinfo? to :nbp to double check this.

AreWeFastYet confirms the backout in terms of performance regression:
  https://arewefastyet.com/#machine=29&view=single&suite=asmjs-ubench&subtest=fasta&start=1436326032&end=1436448687

and we had no merge day in July:
  https://wiki.mozilla.org/RapidRelease/Calendar