Closed Bug 1127167 Opened 5 years ago Closed 5 years ago

Crash [@ js::GCMarker::markAndScanString] with --unboxed-objects

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox38 --- fixed

People

(Reporter: jruderman, Assigned: bhackett)

References

(Blocks 2 open bugs)

Details

(Keywords: crash, testcase)

Attachments

(2 files)

Attached file stack
./js --unboxed-objects

gcslice(0);
function F() {
  this.a = "";
}
for (var i = 0; i < 30; i++) {
  new F();
}
gcslice(0x80000000);

Crash [@ js::GCMarker::markAndScanString]
Flags: needinfo?(bhackett1024)
Attached patch patchSplinter Review
During and before the new script properties analysis, there are several places where objects with the group are created and then have references to them discarded.  If we end up using an unboxed layout for the group, these objects will be mutants with a non-native group and a native shape and layout.  Since the objects don't have any incoming references, they will generally be collected by the next GC so this doesn't matter, but if that GC does mark them, as will be done if the objects were allocated during an incremental GC, then it will crash while doing so.

The attached patch fixes this by either avoiding allocation of these objects, or giving them a different group so that they will appear to be native objects to the GC.  This patch also avoids populating the new object cache for objects which haven't been analyzed yet, which can also be problematic.
Flags: needinfo?(bhackett1024)
Attachment #8561332 - Flags: review?(jdemooij)
Comment on attachment 8561332 [details] [diff] [review]
patch

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

::: js/src/jsinfer.cpp
@@ +3466,5 @@
>          destroyNewScript.group = nullptr;
>  
> +        // Clear out the template object, which is not used for TypeNewScripts
> +        // with an unboxed layout. Currently it is a mutant object with a
> +        // non-native type and native shape, so make it safe for GC by chaning

Nit: changing, typo
Attachment #8561332 - Flags: review?(jdemooij) → review+
Duplicate of this bug: 1131270
Backed out for causing massive octane regressions:

https://hg.mozilla.org/integration/mozilla-inbound/rev/2db20a9e01b9
https://hg.mozilla.org/mozilla-central/rev/1f7d87422dfb
Assignee: nobody → bhackett1024
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.