1207572 - Crash [@ ??] (SIGTRAP) with forceinlineCaches

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 19b4265d0d56 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2 --ion-eager --baseline-eager --ion-offthread-compile=off):

setJitCompilerOption("ion.forceinlineCaches", 1);
function valueLength(a, l) {
    var res = 0;
    for (var i = Number == 0 & (this) & (this); i < l; i++)
        res += a.length;
    return res / l;
}
var denseArray = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9];
var hugeArray = new Array(4294967295);
assertEq(valueLength(denseArray, 10), 10);
assertEq(valueLength(hugeArray, 10), 4294967295);



Backtrace:

Program received signal SIGTRAP, Trace/breakpoint trap.
0x00007ffff7fc17ef in ?? ()
#0  0x00007ffff7fc17ef in ?? ()
[...]
#21 0x00007fffffffc5e0 in ?? ()
#22 0x0000000000975f67 in EnterIon (data=..., cx=0x7ffff6907000) at js/src/jit/Ion.cpp:2668
#23 js::jit::IonCannon (cx=0x7ffff6907000, state=...) at js/src/jit/Ion.cpp:2769
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
rax	0x7fffffe00000	140737486258176
rbx	0xffffffff	4294967295
rcx	0x7ffff4800260	140737295417952
rdx	0x7fffffffc368	140737488339816
rsi	0x7fffffffbe60	140737488338528
rdi	0x7fffffffc000	140737488338944
rbp	0x7fffffffc128	140737488339240
rsp	0x7fffffffc100	140737488339200
r8	0x1	1
r9	0x7ffff6957c89	140737330379913
r10	0x40	64
r11	0x3	3
r12	0x8	8
r13	0x7fffffffc8c0	140737488341184
r14	0x7ffff7e77a40	140737352530496
r15	0x7ffff6907000	140737330049024
rip	0x7ffff7fc17ef	140737353881583
=> 0x7ffff7fc17ef:	movabs $0xfff8800000000000,%rbx
   0x7ffff7fc17f9:	or     %rax,%rbx


Marking s-s because this is a SIGTRAP on the heap, possibly bad stuff going on here.

Comment 1

[Fuzzing Team]

JSBugMon: Bisection requested, result:
=== Treeherder Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20150827092037" and the hash "0ed63658fcba416b196c8e1fc33b78b873e95883".
The "bad" changeset has the timestamp "20150827102433" and the hash "73aa4c4fc3be34ae9cd4787540363be0664d6354".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=0ed63658fcba416b196c8e1fc33b78b873e95883&tochange=73aa4c4fc3be34ae9cd4787540363be0664d6354

Comment 2

[Jan de Mooij [:jandem]]

In that regression range, bug 1195545 seems most likely.

Comment 3

[Brian Hackett [Laid off!]]

Attachment: patch

I disabled the instruction reordering pass and the bug still reproduces.  This looks like an Ion cache bug.  When we generate an Array.length cache the code includes a check to make sure the result will fit in an int32.  However, when that cache is first generated, if the produced length does not fit in an int32 then the result of the call is never Monitor()'ed, the script's code is not invalidated, and the double length is interpreted as an int32, leading to this eventual debugging TRAP in MacroAssemblerX64::boxValue when it boxes what should be an int32 but finds that the upper 32 bits of the register are not clear.

AFAICT the 64 bit value which boxValue() sees will have the tag bits clear --- after the getproperty cache the double value will be "unboxed" to an int32 and we end up with a deformed value with the tag bits clear but not the upper part of the payload bits.  So when boxValue() or's the register with the tag bits the result still appears to be an int32 value, just not a valid one (since bits above the lower 32 bits of the payload are set).  I don't think this will cause any problems other than incorrect script behavior, but I'm not sure.

Comment 4

[Brian Hackett [Laid off!]]

Oh, and unboxed array length stubs already have a check to only generate the cache if the nominal object has a length that fits in an int32.

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 8669016 [details] [diff] [review]
patch

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

Subtle.

Comment 6

[Pulsebot]

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

Comment 7

[Carsten Book [:Tomcat]]

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