1126251 - Crash [@ ??] (SIGTRAP) with Float32 +inf with --ion-check-range-analysis

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 494632b9afed (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --enable-debug, run with --fuzzing-safe --thread-count=2 --ion-check-range-analysis):

(function(global) {
    "use asm";
    var frd = global.Math.fround;
    var fx4 = global.SIMD.float32x4;
    var fsp = fx4.splat;
    function d(x){x=fx4(x);}
    function e() {
        var x = frd(.1e+71 );
        x = frd(x / x);
        d(fsp(x));
    }
    return e;
})(this)();



Backtrace:

Program received signal SIGTRAP, Trace/breakpoint trap.
0x00007ffff7ff60de in ?? ()
[...]
#8  0x0000000000000000 in ?? ()
rax	0x7ffff7ff7000	140737354100736
rbx	0x19f4740	27215680
rcx	0x2	2
rdx	0x7ffff7ff6000	140737354096640
rsi	0x7ffff7ff7660	140737354102368
rdi	0x7fffffffcf78	140737488342904
rbp	0x7fffffffd040	140737488343104
rsp	0x7fffffffcad0	140737488341712
r8	0xd6ef58	14085976
r9	0x0	0
r10	0x7fffffffcf78	140737488342904
r11	0x19cf498	27063448
r12	0x7fffffffcb50	140737488341840
r13	0x19f4758	27215704
r14	0x0	0
r15	0x0	0
rip	0x7ffff7ff60de	140737354096862
=> 0x7ffff7ff60de:	movsd  0x1562(%rip),%xmm1        # 0x7ffff7ff7648
   0x7ffff7ff60e6:	ucomisd %xmm0,%xmm0

Comment 1

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/881bdc3b017f
user:        Benjamin Bouvier
date:        Wed Sep 03 13:35:12 2014 +0200
summary:     Bug 1060789: OdinMonkey SIMD: Add support for splat; r=luke

This iteration took 279.863 seconds to run.

Comment 2

[Christian Holler (:decoder)]

Needinfo from Benjamin based on comment 1 :)

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

// Related to float32 range analysis actually, not SIMD.
// Run with --ion-check-range-analysis.
(function(global) {
    "use asm";
    var frd = global.Math.fround;
    function e() {
        var x = frd(.1e+71 );
        x = frd(x / x);
        return +x;
    }
    return e;
})(this)();

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Working on a fix.

Comment 5

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: force-float32-asmjsnumlit.patch

Actually, an issue only with asm.js, as when I've tried to reproduce within Ion, I couldn't.

In asm.js, when we create a new AsmJSNumLit with type float, we assign it the double value in the JSValue, and a MConstant with this double value is recorded, although the double -> float32 conversion is not taken into account in the MConstant creation.  Then RangeAnalysis sees this finite double value, which would be +inf as a float32, and when checking the ranges in assembly, we assert because we see an infinite value, while expecting a finite one.  To fix this, let's convert explicitly directly in the AsmJSNumLit Create function.

In Ion, the same code would create a MToFloat32(MConstant(.1e+71)), which is correct as we have a Math.fround call around the constant value.  Then GVN would replace the MConstant by another typed with MIRType_Float32 and it would do the coercion at that time.  RangeAnalysis happens way later, so we're fine there.  Not even an issue if we disable GVN: in this case, the MToFloat32 will be jitted and the conversion will take place in assembly directly.

Comment 6

[Luke Wagner [:luke]]

Comment on attachment 8555746 [details] [diff] [review]
force-float32-asmjsnumlit.patch

Hah, wow, that is quite subtle.

Comment 7

[Benjamin Bouvier [:bbouvier] (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0740b0fe3052

Comment 8

[Carsten Book [:Tomcat]]

sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=6095356&repo=mozilla-inbound

Comment 9

[Benjamin Bouvier [:bbouvier] (inactive)]

Oops, forgot the SIMD check... Sorry for the bustage and thanks for the backout.

https://hg.mozilla.org/integration/mozilla-inbound/rev/8497232f9bb2

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8497232f9bb2

Comment 11

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/projects/cypress/rev/49d423e29580