Closed Bug 966926 Opened 6 years ago Closed 6 years ago

Assertion failure: consumer->isConsistentFloat32Use(), at jit/IonAnalysis.cpp:979 or Crash [@ js::LifoAlloc::getOrCreateChunk]

Categories

(Core :: JavaScript Engine: JIT, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla30
Tracking Status
firefox27 --- unaffected
firefox28 --- unaffected
firefox29 + fixed
firefox30 --- fixed
firefox-esr24 --- unaffected
b2g18 --- unaffected
b2g-v1.1hd --- unaffected
b2g-v1.2 --- unaffected
b2g-v1.3 --- unaffected
b2g-v1.3T --- ?
b2g-v1.4 --- fixed

People

(Reporter: decoder, Assigned: bbouvier)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [jsbugmon:update])

Crash Data

Attachments

(2 files)

The following testcase asserts on mozilla-central revision 44ba69cacd7e (run with --fuzzing-safe --ion-eager):


var f32 = new Float32Array(32);
function f(n) {
    var x;
    if (n > 10000) {
        x = (0);
    } else {
        x = f32[0];
    }
    g('' + (x));
}
for (var n = 0; n < 100; n++)
  f(n);
Crash trace:


Program received signal SIGSEGV, Segmentation fault.
js::LifoAlloc::getOrCreateChunk (this=0x152a230 <vtable for js::jit::MToString+16>, n=88) at js/src/ds/LifoAlloc.cpp:82
82                  latest->resetBump();    // This was an unused BumpChunk on the chain.
#0  js::LifoAlloc::getOrCreateChunk (this=0x152a230 <vtable for js::jit::MToString+16>, n=88) at js/src/ds/LifoAlloc.cpp:82
#1  0x00000000004c8d59 in allocInfallible (this=0x152a230 <vtable for js::jit::MToString+16>, n=88) at js/src/ds/LifoAlloc.h:281
#2  allocateInfallible (bytes=88, this=<optimized out>) at js/src/jit/IonAllocPolicy.h:37
#3  js::jit::TempObject::operator new (nbytes=88, alloc=...) at js/src/jit/IonAllocPolicy.h:180
#4  0x0000000000603df8 in js::jit::LIRGenerator::visitGetPropertyPolymorphic (this=0x15c85e0, ins=0x15c7ac0) at js/src/jit/Lowering.cpp:2865
#5  0x00000000015c67f0 in ?? ()
#6  0x00000000015c7728 in ?? ()
#7  0x00000000015c8640 in ?? ()
rdx     0xb6048d48      1173903797291093320
=> 0x458a7d <js::LifoAlloc::getOrCreateChunk(unsigned long)+45>:        mov    (%rdx),%r8


Without closer inspection, I'd say the trace looks like a memory corruption in the internal JS allocator, marking sec-critical based on that.
Crash Signature: [@ js::LifoAlloc::getOrCreateChunk]
Keywords: crash, sec-critical
Whiteboard: [jsbugmon:update,bisect]
Float32 so NI from Benjamin :)
Flags: needinfo?(benj)
Temporarily avoids Float32 to flow into ToString instructions. Bug 966957 has been filed to fix it properly.

The interesting thing it that it couldn't happen with a simple Plus, as an addition between a Float32 and a String would just be specialized as a Values add.
Assignee: nobody → benj
Status: NEW → ASSIGNED
Attachment #8369391 - Flags: review?(hv1989)
Flags: needinfo?(benj)
Reading the explanation this is caused by bug 964229.
Blocks: 964229
Comment on attachment 8369391 [details] [diff] [review]
Patch + simpler test

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

Thanks for catching this! Today is merge day, so look carefully if this lands in FF29 or FF30. In the later case you'll need to uplift ;)
Attachment #8369391 - Flags: review?(hv1989) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7485fef3041

Will need to uplift, the tree was mostly closed yesterday :/
Wouldn't have mattered. You missed the uplift by the time this got r+ anyway.
https://hg.mozilla.org/mozilla-central/rev/e7485fef3041

What branches does this affect? Should it have had sec-approval first?
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Flags: needinfo?(benj)
What are the conditions for sec-approval? As far as I understand, in the worst case a Float32 is interpreted as a Double in a concatenation operation, so that's just a correctness issue, but it seems unlikely to crash. In the mean time, decoder provided a crash trace in the first comment, so there might be something else going wrong.

It doesn't appear with bug 888109, a.k.a the usual suspect, so I am bisecting by hand.
Flags: needinfo?(benj)
Comment on attachment 8369391 [details] [diff] [review]
Patch + simpler test

Caused by bug 964229, landed in mozilla 29 -> only aurora is affected.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 964229
User impact if declined: correctness issues, if not crashes
Testing completed (on m-c, etc.): m-i completed
Risk to taking this patch (and alternatives if risky): low, if not no risk at all 
String or IDL/UUID changes made by this patch: n/a
Attachment #8369391 - Flags: approval-mozilla-aurora?
Attachment #8369391 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Benjamin Bouvier [:bbouvier] from comment #11)
> What are the conditions for sec-approval?

https://wiki.mozilla.org/Security/Bug_Approval_Process

If this only affected trunk (at the time) it wouldn't need approval. It probably shouldn't have been checked into Aurora without approval though I don't think we've had it come up often that trunk was unaffected but Aurora was. As a sec-critical, it would all under the sec-approval process though.
Duplicate of this bug: 973620
Group: core-security
You need to log in before you can comment on or make changes to this bug.