chore: cherry-pick 27bc67f761e6 from v8 (#32738)

* chore: cherry-pick 27bc67f761e6 from v8

* chore: update patches

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Co-authored-by: Electron Bot <electron@github.com>
This commit is contained in:
Jeremy Rose
2022-02-09 09:04:48 -08:00
committed by GitHub
parent e8f90e57d8
commit 77451d2cf3
2 changed files with 254 additions and 0 deletions

View File

@@ -6,5 +6,6 @@ workaround_an_undefined_symbol_error.patch
do_not_export_private_v8_symbols_on_windows.patch
fix_build_deprecated_attirbute_for_older_msvc_versions.patch
fix_disable_implies_dcheck_for_node_stream_array_buffers.patch
cherry-pick-27bc67f761e6.patch
regexp_arm_fix_regexp_assembler_abortion.patch
regexp_ensure_regress-1255368_runs_only_with_irregexp.patch

View File

@@ -0,0 +1,253 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Jakob Gruber <jgruber@chromium.org>
Date: Thu, 13 Jan 2022 08:01:37 +0100
Subject: Merged: [maps] Lock map_updater_access in
CompleteInobjectSlackTracking
CompleteInobjectSlackTracking potentially shrinks multiple maps, and
the relation between these maps should be preserved in a concurrent
environment. Thus it is not enough to make each modification
atomically, but all related map modifications must be within a
critical section.
We do this by locking the map_updater_access mutex
CompleteInobjectSlackTracking, and hence moving the function to the
MapUpdater class.
(cherry picked from commit 4b8d04897cba70cac45eea33d78fa2354dfe2bd7)
No-Try: true
No-Presubmit: true
No-Treechecks: true
Bug: chromium:1274445,v8:7990
Change-Id: If99bb8b55e03180128ee397d845fa4c269c4241e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3379819
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#78597}
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3406537
Cr-Commit-Position: refs/branch-heads/9.8@{#16}
Cr-Branched-From: e218afa8473132b56a9e1532be7920dd130aeb7e-refs/heads/9.8.177@{#1}
Cr-Branched-From: 86ebfc969cde382122a4d429f2380f06175ea2a8-refs/heads/main@{#78312}
diff --git a/src/objects/js-function-inl.h b/src/objects/js-function-inl.h
index 15634b8f024705481e5f0da5008231c1e6ca2f0d..eddedfeabfb48f39418342c8d0044c941f449a5b 100644
--- a/src/objects/js-function-inl.h
+++ b/src/objects/js-function-inl.h
@@ -15,6 +15,7 @@
#include "src/ic/ic.h"
#include "src/init/bootstrapper.h"
#include "src/objects/feedback-cell-inl.h"
+#include "src/objects/map-updater.h"
#include "src/objects/shared-function-info-inl.h"
// Has to be the last include (doesn't have include guards):
@@ -124,7 +125,7 @@ bool JSFunction::IsInOptimizationQueue() {
void JSFunction::CompleteInobjectSlackTrackingIfActive() {
if (!has_prototype_slot()) return;
if (has_initial_map() && initial_map().IsInobjectSlackTrackingInProgress()) {
- initial_map().CompleteInobjectSlackTracking(GetIsolate());
+ MapUpdater::CompleteInobjectSlackTracking(GetIsolate(), initial_map());
}
}
diff --git a/src/objects/map-inl.h b/src/objects/map-inl.h
index c8eb40042410a684306ddb7fde69cff57ab146ce..5156cd9d9f4a5fed832466b16589c08ddd4c4faf 100644
--- a/src/objects/map-inl.h
+++ b/src/objects/map-inl.h
@@ -12,6 +12,7 @@
#include "src/objects/field-type.h"
#include "src/objects/instance-type-inl.h"
#include "src/objects/js-function-inl.h"
+#include "src/objects/map-updater.h"
#include "src/objects/map.h"
#include "src/objects/objects-inl.h"
#include "src/objects/property.h"
@@ -856,7 +857,7 @@ void Map::InobjectSlackTrackingStep(Isolate* isolate) {
int counter = construction_counter();
set_construction_counter(counter - 1);
if (counter == kSlackTrackingCounterEnd) {
- CompleteInobjectSlackTracking(isolate);
+ MapUpdater::CompleteInobjectSlackTracking(isolate, *this);
}
}
diff --git a/src/objects/map-updater.cc b/src/objects/map-updater.cc
index 3bfd3922a3dc2119250c20b97ac1d5061cae74db..21f2665454ae121ac448ad0df9c7d5459c72650d 100644
--- a/src/objects/map-updater.cc
+++ b/src/objects/map-updater.cc
@@ -420,21 +420,50 @@ MapUpdater::State MapUpdater::Normalize(const char* reason) {
return state_; // Done.
}
-void MapUpdater::ShrinkInstanceSize(base::SharedMutex* map_updater_access,
- Map map, int slack) {
+// static
+void MapUpdater::CompleteInobjectSlackTracking(Isolate* isolate,
+ Map initial_map) {
+ DisallowGarbageCollection no_gc;
+ // Has to be an initial map.
+ DCHECK(initial_map.GetBackPointer().IsUndefined(isolate));
+
+ const int slack = initial_map.ComputeMinObjectSlack(isolate);
DCHECK_GE(slack, 0);
+
+ TransitionsAccessor transitions(isolate, initial_map, &no_gc);
+ TransitionsAccessor::TraverseCallback callback;
+ if (slack != 0) {
+ // Resize the initial map and all maps in its transition tree.
+ callback = [slack](Map map) {
#ifdef DEBUG
- int old_visitor_id = Map::GetVisitorId(map);
- int new_unused = map.UnusedPropertyFields() - slack;
+ int old_visitor_id = Map::GetVisitorId(map);
+ int new_unused = map.UnusedPropertyFields() - slack;
#endif
+ map.set_instance_size(map.InstanceSizeFromSlack(slack));
+ map.set_construction_counter(Map::kNoSlackTracking);
+ DCHECK_EQ(old_visitor_id, Map::GetVisitorId(map));
+ DCHECK_EQ(new_unused, map.UnusedPropertyFields());
+ };
+ } else {
+ // Stop slack tracking for this map.
+ callback = [](Map map) {
+ map.set_construction_counter(Map::kNoSlackTracking);
+ };
+ }
{
- base::SharedMutexGuard<base::kExclusive> mutex_guard(map_updater_access);
- map.set_instance_size(map.InstanceSizeFromSlack(slack));
+ // The map_updater_access lock is taken here to guarantee atomicity of all
+ // related map changes (instead of guaranteeing only atomicity of each
+ // single map change). This is needed e.g. by InstancesNeedsRewriting,
+ // which expects certain relations between maps to hold.
+ //
+ // Note: Avoid locking the full_transition_array_access lock inside this
+ // call to TraverseTransitionTree to prevent dependencies between the two
+ // locks.
+ base::SharedMutexGuard<base::kExclusive> mutex_guard(
+ isolate->map_updater_access());
+ transitions.TraverseTransitionTree(callback);
}
- map.set_construction_counter(Map::kNoSlackTracking);
- DCHECK_EQ(old_visitor_id, Map::GetVisitorId(map));
- DCHECK_EQ(new_unused, map.UnusedPropertyFields());
}
MapUpdater::State MapUpdater::TryReconfigureToDataFieldInplace() {
diff --git a/src/objects/map-updater.h b/src/objects/map-updater.h
index 6f022e1d39f7222904143eb93b24a98d68ca9286..7136532bbe1c8ad97a86232110ea563ee281c3af 100644
--- a/src/objects/map-updater.h
+++ b/src/objects/map-updater.h
@@ -86,8 +86,9 @@ class V8_EXPORT_PRIVATE MapUpdater {
Representation new_representation,
Handle<FieldType> new_field_type);
- static void ShrinkInstanceSize(base::SharedMutex* map_updater_access, Map map,
- int slack);
+ // Completes inobject slack tracking for the transition tree starting at the
+ // initial map.
+ static void CompleteInobjectSlackTracking(Isolate* isolate, Map initial_map);
private:
enum State {
diff --git a/src/objects/map.cc b/src/objects/map.cc
index 0610e5968802753438b6e812a09a85400742892d..f1c52d69ad800d7345b64cf722e894f583da6517 100644
--- a/src/objects/map.cc
+++ b/src/objects/map.cc
@@ -2133,28 +2133,6 @@ int Map::ComputeMinObjectSlack(Isolate* isolate) {
return slack;
}
-void Map::CompleteInobjectSlackTracking(Isolate* isolate) {
- DisallowGarbageCollection no_gc;
- // Has to be an initial map.
- DCHECK(GetBackPointer().IsUndefined(isolate));
-
- int slack = ComputeMinObjectSlack(isolate);
- TransitionsAccessor transitions(isolate, *this, &no_gc);
- TransitionsAccessor::TraverseCallback callback;
- if (slack != 0) {
- // Resize the initial map and all maps in its transition tree.
- callback = [&](Map map) {
- MapUpdater::ShrinkInstanceSize(isolate->map_updater_access(), map, slack);
- };
- } else {
- callback = [](Map map) {
- // Stop slack tracking for this map.
- map.set_construction_counter(Map::kNoSlackTracking);
- };
- }
- transitions.TraverseTransitionTree(callback);
-}
-
void Map::SetInstanceDescriptors(Isolate* isolate, DescriptorArray descriptors,
int number_of_own_descriptors) {
set_instance_descriptors(descriptors, kReleaseStore);
diff --git a/src/objects/map.h b/src/objects/map.h
index d60890d9103e352215d8afda9c1c8f952b5ad2be..a8655262155ce897f3f2c36211336ae190c7e334 100644
--- a/src/objects/map.h
+++ b/src/objects/map.h
@@ -352,10 +352,6 @@ class Map : public TorqueGeneratedMap<Map, HeapObject> {
int ComputeMinObjectSlack(Isolate* isolate);
inline int InstanceSizeFromSlack(int slack) const;
- // Completes inobject slack tracking for the transition tree starting at this
- // initial map.
- V8_EXPORT_PRIVATE void CompleteInobjectSlackTracking(Isolate* isolate);
-
// Tells whether the object in the prototype property will be used
// for instances created from this function. If the prototype
// property is set to a value that is not a JSObject, the prototype
diff --git a/src/runtime/runtime-object.cc b/src/runtime/runtime-object.cc
index 3da21358d80fafbd8a7cc5f0d485bf417a26e27c..3c29f13814b4442dd11bbed711047f911be2015b 100644
--- a/src/runtime/runtime-object.cc
+++ b/src/runtime/runtime-object.cc
@@ -990,7 +990,7 @@ RUNTIME_FUNCTION(Runtime_CompleteInobjectSlackTrackingForMap) {
DCHECK_EQ(1, args.length());
CONVERT_ARG_HANDLE_CHECKED(Map, initial_map, 0);
- initial_map->CompleteInobjectSlackTracking(isolate);
+ MapUpdater::CompleteInobjectSlackTracking(isolate, *initial_map);
return ReadOnlyRoots(isolate).undefined_value();
}
diff --git a/src/runtime/runtime-test.cc b/src/runtime/runtime-test.cc
index 38cdf5767145df2df4f00f73729ced4d3b916923..b008965901de4d4a5274318f6ab124b77ce38fe3 100644
--- a/src/runtime/runtime-test.cc
+++ b/src/runtime/runtime-test.cc
@@ -1329,7 +1329,7 @@ RUNTIME_FUNCTION(Runtime_CompleteInobjectSlackTracking) {
DCHECK_EQ(1, args.length());
CONVERT_ARG_HANDLE_CHECKED(JSObject, object, 0);
- object->map().CompleteInobjectSlackTracking(isolate);
+ MapUpdater::CompleteInobjectSlackTracking(isolate, object->map());
return ReadOnlyRoots(isolate).undefined_value();
}
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 61398f2c5ea6cd4bbc57403eb0ddf97d2fb5f575..4b64d307887d28c844b08e9e0524e03fc2dd4183 100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -70,6 +70,7 @@
#include "src/objects/js-array-inl.h"
#include "src/objects/js-promise-inl.h"
#include "src/objects/lookup.h"
+#include "src/objects/map-updater.h"
#include "src/objects/module-inl.h"
#include "src/objects/objects-inl.h"
#include "src/objects/string-inl.h"
@@ -2981,9 +2982,9 @@ TEST(InternalFieldsSubclassing) {
CHECK_LE(i_value->map().GetInObjectProperties(), kMaxNofProperties);
}
- // Make Sure we get the precise property count.
- i_value->map().FindRootMap(i_isolate).CompleteInobjectSlackTracking(
- i_isolate);
+ // Make sure we get the precise property count.
+ i::MapUpdater::CompleteInobjectSlackTracking(
+ i_isolate, i_value->map().FindRootMap(i_isolate));
// TODO(cbruni): fix accounting to make this condition true.
// CHECK_EQ(0, i_value->map()->UnusedPropertyFields());
if (in_object_only) {