1446372 - Audit interaction of Nursery and TI

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P2)

Description

[Ted Campbell [:tcampbell]]

Jan and I are investigating some potential issues with TI and GC interactions. Currently we have TypeRefSet entries in StoreBuffer to link type data and nursery objects.

The TI data is *not* a GCThing and actually is moved/compacted during incremental sweep. Generally, things all work out fine, but the reasons are very fragile.

Eg. Bug 1407143 will potentially open us up to corruption.

Comment 1

[Ted Campbell [:tcampbell]]

One concern is |ConstraintDataFreezeObjectForTypedArrayData::obj|. Currently TypeSetRef::sweep will visit this, but TypeSetRef::trace(TenuringTracer) will miss it. This may lead to stale GC pointers.

Comment 2

[Ted Campbell [:tcampbell]]

(In reply to Ted Campbell [:tcampbell] from comment #1)
> One concern is |ConstraintDataFreezeObjectForTypedArrayData::obj|. Currently
> TypeSetRef::sweep will visit this, but TypeSetRef::trace(TenuringTracer)
> will miss it. This may lead to stale GC pointers.

This is addressed by |StoreBuffer::setShouldCancelIonCompilations()| being set before adding any from TI to nursery objects.

Of course this raises the follow-up questions of "Is is possible the cancel compilation mechanism has a bug?" and "Is it possible we missing a |ConstraintTypeSet::postWriteBarrier|?". For the first one we should probably add a RELEASE_ASSERT in the Ion Linker that the StoreBuffer hasn't indicated Ion compiles should be cancelled. For the second one, I just need to audit the sites that add types or group object sets.

Comment 3

[Ted Campbell [:tcampbell]]

postWriteBarriers look currently used for all TypeSet::add (that aren't TemporaryTypeSet).
We allow Ion compiles to complete while having nursery pointers since they will be caught in sweep.

As far as I can tell, it all looks right, and I don't see any accidental regressions over the years. Hopefully the TI poisoning assert gives something useful.

Comment 4

[Ted Campbell [:tcampbell]]

The solution to this complex mess was to remove TI entirely