930993 - (jit) Dangerous Crash on Heap with Typed Arrays and GC

Status: VERIFIED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 5a9ac6fed6ff (run with --fuzzing-safe --ion-eager):


x = {}; 
y = x;
x.toString = (function(heap) {
  Int8ArrayView = Int8Array(heap);
  Float32ArrayView = Float32Array(heap);
  function f() {
      Int8ArrayView[0] = Float32ArrayView[0]
  }
  return f;
})(ArrayBuffer);
gczeal(4);
function compareSAndSet() {
    return x << y;
}
evaluate("compareSAndSet()");

Comment 1

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 2

[Christian Holler (:decoder)]

The reduced crash is at 0x0:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff6bb0ff4 in ?? ()
#0  0x00007ffff6bb0ff4 in ?? ()
#1  0x00007ffff6bc0534 in ?? ()
#2  0x0000000000000183 in ?? ()
#3  0x00007ffff694c780 in ?? ()
#4  0x0000000000000000 in ?? ()
rax     0x0     0
rcx	0x0	0
=> 0x7ffff6bb0ff4:      mov    %cl,(%rax)


However, the original crash looked like this (same instruction, dangerous address):

Program terminated with signal 11, Segmentation fault.
#0  0x00007f79914c31c8 in ?? ()
#0  0x00007f79914c31c8 in ?? ()
#1  0x00007f79933e8574 in ?? ()
#2  0x0000000000000183 in ?? ()
#3  0x00007f797f103c40 in ?? ()
#4  0x0000000000000000 in ?? ()
rax	0xf586eda7	4602678818996940199
rcx	0x0	0
=> 0x7f79914c31c8:	mov    %cl,(%rax)


Marking s-s and sec-critical based on that.

Comment 3

[Hannes Verschore [:h4writer]]

Adding Bouvier, since he recently landed the Float32 patches. Looking at Float32Array it could be related to that.

Comment 4

[Christian Holler (:decoder)]

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

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/77b72ea510d0
user:        Benjamin Bouvier
date:        Thu Jul 18 16:45:16 2013 -0700
summary:     Bug 913282: More Float32 operators: TruncateToInt32; p=dougc,bbouvier, r=jonco,jandem

This iteration took 421.576 seconds to run.

Comment 5

[Jan de Mooij [:jandem]]

Benjamin can you take a look at this? :)

Comment 6

[Benjamin Bouvier [:bbouvier] (inactive)]

I will try to see what's going on.

Comment 7

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: bug930993.patch

Very nice catch!

This occurs because stack order isn't preserved in the OOL patch of truncate to int32. The fix just consists in changing the order of stack operations (one line move).

Comment 8

[Jan de Mooij [:jandem]]

Comment on attachment 823372 [details] [diff] [review]
bug930993.patch

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

Thanks and nice find!

Comment 9

[Benjamin Bouvier [:bbouvier] (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/101106bdce31

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/101106bdce31

Comment 11

[Christian Holler (:decoder)]

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

Comment 12

[bhavana bajaj [:bajaj]]

(In reply to Benjamin Bouvier [:bbouvier] from comment #9)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/101106bdce31

Can you please help request uplift approval on aurora to resolve this on FX 27. Looks like this is goon on central :)

Comment 13

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 823372 [details] [diff] [review]
bug930993.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 888109 (Float32 optimizations)
User impact if declined: correctness bugs + possible exploits on x86 and x64 machines
Testing completed (on m-c, etc.): testing completed on m-i, merged to and completed on m-c, all tests pass.
Risk to taking this patch (and alternatives if risky): no risk
String or IDL/UUID changes made by this patch: N/A

Comment 14

[David Chan [:dchan]]

Per comment 4, this bug was introduced post b2g18, though the timeline means b2g1.1+ may be affected.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e7be5919512b

Comment 16

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 17

[u279076]

Marking verified for Firefox 28 as per comment 16.