… tracking
Here are the steps that lead to the bug:
- main thread: map `a` was being slack-tracked
- background: a compilation job serializes `a` into a MapRef `aRef`
- main thread: slack tracking finished for this map.
- main thread: a store to an object of map `a` created a transition
from map `a` to map `b`, and the property stored was stored as the
1st item of the out-of-object properties.
- background: compilation reached JSNativeContextSpecialization,
which tried to optimize a JSSetNamedProperty (specifically, the
same operation that lead to the map transition on the main
thread). There was no feedback for this operation since it hadn't
been executed before (otherwise, the map transition would have had
happened before, and the MapRef would not have been out of date).
JSNativeCtxtSpec inferred maps of the receiver from previous
CheckMaps, and realized that the store was transitioning (from `a`
to `b`). It looked at the MapRef `aRef` to see how much unused
properties the object had. `aRef` still had the cached
slack-tracking data, and thus thought that it still had unused
properties, whereas in reality, `a` didn't have any left, and a
new property backing store should have been allocated.
- main thread: when executing the store generated, we tried to write
to the 1st item of the out-of-object properties of an object
with map `a`, which was the EmptyFixedArray root, which caused a
segfault, since this is in read-only space.
The fix is to add a compilation dependency for map slack-tracking when
deciding to extend (or not) the property backing store of an object.
At the end of compilation, if the construction_counter of the Map is 0
and the one of the MapRef is non-0, then slack tracking finished
during compilation, and we discard the optimized code.
While fixing this, I also found out that UnusedPropertyFields and
construction_counter were sometimes incoherent in the background,
because CSA was updating construction_counter without taking the
map_updater_access mutex (which means that when construction_counter
was 0 in the background, it wasn't always safe to look at
UnusedPropertyFields, since it could contain the old value).
Similarly, MapRef::IsInobjectSlackTrackingInProgress was looking at
the Map rather than the cached value for construction_counter, which
means that it could also be out of sync with UnusedPropertyFields.
Bug: chromium:1444366
(cherry picked from commit 7effdbf)
Change-Id: I186301bc5fca3836743f59ee46e3ddb35391229f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4650761
Reviewed-by: Jakob Kummerow <[email protected]>
Commit-Queue: Darius Mercadier <[email protected]>
Cr-Commit-Position: refs/branch-heads/11.6@{#8}
Cr-Branched-From: e29c028-refs/heads/11.6.189@{#3}
Cr-Branched-From: 95cbef2-refs/heads/main@{#88340}
