Closed Bug 748119 Opened 8 years ago Closed 8 years ago
Assertion failure: Is
Marked Or Allocated(static _cast<Cell *>(*thingp)), at jsgc .cpp:4284
The attached test asserts on mozilla-central revision 17af008937e3 (options -m -n -a).
8 years ago
Assignee: general → wmccloskey
This test exposed two problems. The first one is that we need read barriers on the TypeObjects and JSObjects that are stored in Types. Here's what can go wrong: 1. Incremental GC starts. 2. Type object that will eventually be swept is read out from the compiler. Some component of it (newScript->fun in this case) is copied to some other location so that it's now reachable. 3. We fail to mark the type object and its newScript->fun component, so potentially it gets collected. Later, it can be accessed. It seems pretty easy to fix the problem. We just need to invoke a read barrier whenever we read anything out of a Type. I'm guessing that this isn't a very hot path.
Attachment #620919 - Flags: review?(bhackett1024)
Unfortunately, even with the first patch, we still fail the test. The barrier verifier is actually doing two checks: 1. If some object has a field that changes while JS is running, then the original value of the field should be marked. This checks the correctness of write barriers. 2. Any object that is reachable at the end of verification was reachable at the beginning, or else it was marked. This is mostly to check that read barriers are invoked correctly. In this test, the read barrier causes us to mark the original TypeObject that we fetch out of the Type. But we don't transitively mark everything reachable from it. In a real GC that would happen, but the verifier doesn't mimic the GC closely enough. It just notices that that the object that was found via the newScript->fun access from the type object is reachable and unmarked, so it asserts. To really check #2 above, I should be transitively marking forward from everything marked via a read barrier. Then the assertion would pass. However, this is kind of a pain to do. So I'd like to just remove the #2 assertions. They've been the cause of a lot of false positives, including bug 729571, which is still outstanding. Brian, if you don't want to review this, I can give it to Igor. It's really just removing a bogus assert, though.
Attachment #620922 - Flags: review?(bhackett1024)
8 years ago
Duplicate of this bug: 746136
Attachment #620919 - Flags: review?(bhackett1024) → review+
Attachment #620922 - Flags: review?(bhackett1024) → review+
Target Milestone: --- → mozilla15
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
JSBugMon: This bug has been automatically verified fixed.
How far back does this problem go? Does it affect the ESR-10 branch? I see a reference in the patch to IsIncrementalGCSafe() so maybe this is all new code? Or maybe only that part of it?
This only affect incremental GC, which is still disabled by default.
Whiteboard: js-triage-needed, [sg:critical] → js-triage-needed, [sg:critical][advisory-tracking+]
Automatically extracted testcase for this bug was committed: https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
You need to log in before you can comment on or make changes to this bug.