886551 - GenerationalGC: Crash [@ GetGCThingRuntime] or Opt-Crash [@ tenuredZone]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central built with --enable-exact-rooting --enable-gcgenerational, revision 76820c6dff7b (run with --fuzzing-safe --ion-eager):


gc();
gcslice(0);
var thisValues = [ "x" ];
thisValues.forEach(function (value) {
    var format = Intl.DateTimeFormat.call(value);
});

Comment 1

[Christian Holler (:decoder)]

Another test, involving WeakMaps this time:


gc();
gcslice(0);
function isClone(a, b) {
    var rmemory = new WeakMap();
    rmemory.set(a,b);
}
isClone([]);

Comment 2

[Terrence Cole [:terrence]]

Attachment: v0

It turns out that the "fix" for bug 883472 caused this and probably some other fuzz bugs. The problem is that it is fine to fire the pre-barriers if the the object has not been relocated -- which is why this didn't immediately show up -- but after it has been relocated the pre-barrier will crash. The simple solution is to turn off the runtime needsBarrier, but leave the zone needsBarrier alone so that we do not regress bug 883472.

The attached patch undoes the runtime portion of bug 883472 and adds the two tests here to ensure that we don't regress again.

Comment 4

[Brian Hackett [Laid off!]]

Comment on attachment 767360 [details] [diff] [review]
v0

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

It seems strange that the minor collector triggers pre barriers at all and it would be nice if this wasn't possible / was asserted.  For now this seems fine though.

Comment 5

[Terrence Cole [:terrence]]

(In reply to Brian Hackett (:bhackett) from comment #4)
> 
> It seems strange that the minor collector triggers pre barriers at all and
> it would be nice if this wasn't possible / was asserted.  For now this seems
> fine though.

This is the result of two separate concessions to simplicity. (1) We want to re-use EncapsulatedPtr in HashTables rather than requiring a new type there. (2) We want to use a common implementation of the post-barrier logic for HashTable, WeakMap, Watchpoint, and probably other keys. The upshot is that HashKeyRef is currently templatized on the key and we can't easily cast out the EncapsulatedPtr to its underlying type. We can, of course, specialize all of these, but I agree that we should do that later. I've filed bug 887030 so we don't forget to revisit this.

https://hg.mozilla.org/integration/mozilla-inbound/rev/0b81bf199408

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0b81bf199408