Closed
Bug 757431
Opened 13 years ago
Closed 13 years ago
Assertion failure: v->toGCThing(), at gc/Marking.cpp:327 or Crash [@ js::gc::ArenaHeader::allocated]
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
mozilla16
| Tracking | Status | |
|---|---|---|
| firefox14 | --- | fixed |
| firefox15 | --- | fixed |
| firefox16 | --- | fixed |
| firefox-esr10 | --- | unaffected |
People
(Reporter: decoder, Assigned: billm)
Details
(4 keywords, Whiteboard: [jsbugmon:ignore][advisory-tracking+])
Crash Data
Attachments
(2 files)
|
1.38 KB,
application/javascript
|
Details | |
|
2.85 KB,
patch
|
bhackett1024
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
The attached test asserts/crashes on mozilla-central revision 5586efaf687d (options -m -n -a).
Crash trace (debug build):
Program received signal SIGSEGV, Segmentation fault.
0x0804c753 in js::gc::ArenaHeader::allocated (this=0x0) at ../../gc/Heap.h:467
467 JS_ASSERT(allocKind <= size_t(FINALIZE_LIMIT));
(gdb) bt 8
#0 0x0804c753 in js::gc::ArenaHeader::allocated (this=0x0) at ../../gc/Heap.h:467
#1 0x0804c7b7 in js::gc::ArenaHeader::getAllocKind (this=0x0) at ../../gc/Heap.h:497
#2 0x0806b1f5 in js::gc::Cell::getAllocKind (this=0x0) at ../gc/Heap.h:941
#3 0x080f29e0 in js::gc::GetGCThingTraceKind (thing=0x0) at /srv/repos/mozilla-central/js/src/jsgcinlines.h:30
#4 0x0832568e in js::gc::MarkKind (trc=0x86495b8, thingp=0xffffb48c, kind=JSTRACE_OBJECT) at /srv/repos/mozilla-central/js/src/gc/Marking.cpp:231
#5 0x08325c00 in js::gc::MarkValueInternal (trc=0x86495b8, v=0xf77021a0) at /srv/repos/mozilla-central/js/src/gc/Marking.cpp:330
#6 0x08325d2c in js::gc::MarkValueRoot (trc=0x86495b8, v=0xf77021a0, name=0x848f00a "vm_stack") at /srv/repos/mozilla-central/js/src/gc/Marking.cpp:352
#7 0x082b20cf in js::StackSpace::markFrameSlots (this=0x8616b30, trc=0x86495b8, fp=0xf7702160, slotsEnd=0xf77021b0, pc=0x8647dfe "W") at /srv/repos/mozilla-central/js/src/vm/Stack.cpp:501
(More stack frames follow...)
(gdb) x /i $pc
=> 0x804c753 <js::gc::ArenaHeader::allocated() const+21>: movzbl 0xc(%eax),%eax
(gdb) info reg eax
eax 0x0 0
S-s due to GC-related assertion with crash, until the crash is confirmed to be harmless and always a null-deref.
| Assignee | ||
Updated•13 years ago
|
Assignee: general → wmccloskey
| Reporter | ||
Comment 1•13 years ago
|
||
Fuzzer came up with an even simpler test that shows the same assertion and crash:
function setterFunction(v) { called = true; }
function getterFunction(v) { return "getter"; }
Object.defineProperty(Array.prototype, 1,{
get: getterFunction,
set: setterFunction
});
gczeal(4);
var N = 350;
var source = "".concat(
repeat_str("try { f(); } finally {\n", N),
repeat_str("}", N));
function repeat_str(str, repeat_count) {
var arr = new Array(--repeat_count);
while (repeat_count != 0)
arr[--repeat_count] = str;
return str.concat.apply(str, arr);
}
If you think this is not the same bug though, let me know and I'll clone :)
| Assignee | ||
Comment 2•13 years ago
|
||
The first bug is a dupe of bug 753283, which I'm having trouble landing. I'm working on it, though.
The second one is a separate bug, as far as I can tell. Let's make this bug be about the second test case and ignore the first one. Hopefully that won't screw up your tools too much.
| Assignee | ||
Comment 3•13 years ago
|
||
Another VM stack scanning bug. We can GC inside of GetElements before all the elements have been initialized.
Attachment #632088 -
Flags: review?(bhackett1024)
| Reporter | ||
Comment 4•13 years ago
|
||
No problem, we'll just ignore this bug for the tool so it does not get accidentally verified the wrong way.
Whiteboard: js-triage-needed → [jsbugmon:ignore]
Updated•13 years ago
|
Attachment #632088 -
Flags: review?(bhackett1024) → review+
| Assignee | ||
Comment 5•13 years ago
|
||
Target Milestone: --- → mozilla16
Comment 6•13 years ago
|
||
Comment 7•13 years ago
|
||
Guessing sec-critical since the fix is "MakeRangeGCSafe(f.regs.sp, delta);", and we appear to have this code back to at least ESR. Is there any reason we shouldn't take this fix on old branches?
status-firefox-esr10:
--- → affected
status-firefox14:
--- → affected
status-firefox15:
--- → affected
tracking-firefox-esr10:
--- → ?
Keywords: sec-critical
| Assignee | ||
Comment 8•13 years ago
|
||
Comment on attachment 632088 [details] [diff] [review]
fix
[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 723313
User impact if declined: Low likelihood of GC crashes, exploits.
Testing completed (on m-c, etc.): On m-c.
Risk to taking this patch (and alternatives if risky): Low. It just overwrites unused memory with NULLs.
String or UUID changes made by this patch: None.
No need to take on ESR. We used to handle the undefined memory more gracefully before bug 723313.
Attachment #632088 -
Flags: approval-mozilla-beta?
Attachment #632088 -
Flags: approval-mozilla-aurora?
Comment 9•13 years ago
|
||
Comment on attachment 632088 [details] [diff] [review]
fix
low risk, sec critical - approved.
Attachment #632088 -
Flags: approval-mozilla-beta?
Attachment #632088 -
Flags: approval-mozilla-beta+
Attachment #632088 -
Flags: approval-mozilla-aurora?
Attachment #632088 -
Flags: approval-mozilla-aurora+
Comment 10•13 years ago
|
||
adjusting the esr tracking/status flags according to comment 8
| Assignee | ||
Comment 11•13 years ago
|
||
| Reporter | ||
Comment 12•13 years ago
|
||
JSBugMon: This bug has been automatically verified fixed.
| Reporter | ||
Updated•13 years ago
|
Status: RESOLVED → VERIFIED
Updated•13 years ago
|
Whiteboard: [jsbugmon:ignore] → [jsbugmon:ignore][advisory-tracking+]
Updated•13 years ago
|
Group: core-security
| Reporter | ||
Comment 13•12 years ago
|
||
A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/bug757431.js.
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•