Closed Bug 781248 Opened 12 years ago Closed 12 years ago

IonMonkey: Assertion failure: !allocated(), at ../gc/Heap.h:482 or Crash [@ js::gc::Chunk::allocateArena]

Categories

(Core :: JavaScript Engine, defect)

Other Branch
x86_64
Linux
defect
Not set
major

Tracking

()

VERIFIED FIXED
Tracking Status
firefox-esr10 --- unaffected

People

(Reporter: decoder, Assigned: h4writer)

References

Details

(Keywords: assertion, testcase, Whiteboard: [ion:p1:fx18] [jsbugmon:update,ignore])

Attachments

(2 files)

The following testcase asserts on ionmonkey revision 15db586f9a12 (run with --ion -n):


function newArrayTest()
{
  var a = [];
  for (var i = 0; i < 10; newArrayTest ++)
    a[i] = new Array(0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9,
       0,1,2,3,4,5,6,7,8,9,0,1,2,3,4,5,6,7,8,9);
}
assertEq(newArrayTest(), "0,0,0,0,0,0,0,0,0,0");
Opt64 crash:

==23202== Invalid read of size 8
==23202==    at 0x46FB3E: js::gc::Chunk::allocateArena(JSCompartment*, js::gc::AllocKind) (jsgc.cpp:742)
==23202==    by 0x47A76E: js::gc::ArenaLists::refillFreeList(JSContext*, js::gc::AllocKind) (jsgc.cpp:1595)
==23202==    by 0x437788: js::NewDenseAllocatedArray(JSContext*, unsigned int, JSObject*) (jsgcinlines.h:419)
==23202==    by 0x72E43F: js::ion::NewInitArray(JSContext*, unsigned int, js::types::TypeObject*) (VMFunctions.cpp:236)
==23202==    by 0x4033769: ???
==23202==    by 0x7FEFFEF17: ???
==23202==    by 0x7FEFFEF17: ???
==23202==    by 0x1: ???
==23202==    by 0xAB00DF: ???
==23202==    by 0xC98D0D7: ???
==23202==    by 0x40342A1: ???
==23202==    by 0x1FF: ???
==23202==  Address 0xfff8800000000009 is not stack'd, malloc'd or (recently) free'd
Whiteboard: [jsbugmon:update] → [jsbugmon:update][ion:p1:fx18]
Assignee: general → dvander
Status: NEW → ASSIGNED
Attached patch fixSplinter Review
Bug is that we accidentally take an inline allocation path, but the template object requires non-inline slots. So we end up painting over unallocated memory.
Attachment #650398 - Flags: review?(nicolas.b.pierron)
Comment on attachment 650398 [details] [diff] [review]
fix

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

Sounds good.  I don't know exactly the details of the template Object allocation policy but if we are supposed to only allocate under a specific boundary, then this patch might remove some other memory corruption.  Would be nice to find the reason of this bound check and to add a reference/comment above it.
Attachment #650398 - Flags: review?(nicolas.b.pierron) → review+
It's okay to allocate over the boundary, but we don't have a great facility for allocating dynamic slots, thus the callVM if we are over the max inline slots.

https://hg.mozilla.org/projects/ionmonkey/rev/b07af1efa3dd
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Bug #742829 introduced this inline path. This was to equalize the allocation path with the one used in JM.
Attached patch Proper fixSplinter Review
This should be the proper fix for this bug. Only use the lazily allocating path when only one argument is provided.
Assignee: dvander → hv1989
Status: VERIFIED → REOPENED
Attachment #650737 - Flags: review?(dvander)
Resolution: FIXED → ---
Attachment #650737 - Flags: review?(dvander) → review+
Whiteboard: [jsbugmon:update][ion:p1:fx18] → [ion:p1:fx18] [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision ed05182dac42).
Revert b07af1efa3dd:
https://hg.mozilla.org/projects/ionmonkey/rev/b087c794dd80

Fix
https://hg.mozilla.org/projects/ionmonkey/rev/98004d8529b5
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Thanks for catching this, Hannes!
Group: core-security
You need to log in before you can comment on or make changes to this bug.