[runtime] Harden JSFunction::CalculateInstanceSizeHelper(...)
Bug: chromium:808192
Change-Id: I80136d291d5c21c311903bffc96d86d109f5cdc9
Reviewed-on: https://chromium-review.googlesource.com/902103
Commit-Queue: Jakob Kummerow <[email protected]>
Reviewed-by: Jakob Kummerow <[email protected]>
Cr-Commit-Position: refs/heads/master@{#51255}diff --git a/src/objects.cc b/src/objects.cc
index e4a7e6f..71965e1 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -12985,6 +12985,56 @@
map->StartInobjectSlackTracking();
}
+namespace {
+bool FastInitializeDerivedMap(Isolate* isolate, Handle<JSFunction> new_target,
+ Handle<JSFunction> constructor,
+ Handle<Map> constructor_initial_map) {
+ // Check that |function|'s initial map still in sync with the |constructor|,
+ // otherwise we must create a new initial map for |function|.
+ if (new_target->has_initial_map() &&
+ new_target->initial_map()->GetConstructor() == *constructor) {
+ DCHECK(new_target->instance_prototype()->IsJSReceiver());
+ return true;
+ }
+ InstanceType instance_type = constructor_initial_map->instance_type();
+ DCHECK(CanSubclassHaveInobjectProperties(instance_type));
+ // Create a new map with the size and number of in-object properties
+ // suggested by |function|.
+
+ // Link initial map and constructor function if the new.target is actually a
+ // subclass constructor.
+ if (!IsDerivedConstructor(new_target->shared()->kind())) return false;
+
+ int instance_size;
+ int in_object_properties;
+ int embedder_fields =
+ JSObject::GetEmbedderFieldCount(*constructor_initial_map);
+ bool success = JSFunction::CalculateInstanceSizeForDerivedClass(
+ new_target, instance_type, embedder_fields, &instance_size,
+ &in_object_properties);
+
+ Handle<Map> map;
+ if (success) {
+ int pre_allocated = constructor_initial_map->GetInObjectProperties() -
+ constructor_initial_map->UnusedPropertyFields();
+ CHECK_LE(constructor_initial_map->instance_size(), instance_size);
+ int unused_property_fields = in_object_properties - pre_allocated;
+ map = Map::CopyInitialMap(constructor_initial_map, instance_size,
+ in_object_properties, unused_property_fields);
+ } else {
+ map = Map::CopyInitialMap(constructor_initial_map);
+ }
+ map->set_new_target_is_base(false);
+ Handle<Object> prototype(new_target->instance_prototype(), isolate);
+ JSFunction::SetInitialMap(new_target, map, prototype);
+ DCHECK(new_target->instance_prototype()->IsJSReceiver());
+ map->SetConstructor(*constructor);
+ map->set_construction_counter(Map::kNoSlackTracking);
+ map->StartInobjectSlackTracking();
+ return true;
+}
+
+} // namespace
// static
MaybeHandle<Map> JSFunction::GetDerivedMap(Isolate* isolate,
@@ -12995,55 +13045,16 @@
Handle<Map> constructor_initial_map(constructor->initial_map(), isolate);
if (*new_target == *constructor) return constructor_initial_map;
+ Handle<Map> result_map;
// Fast case, new.target is a subclass of constructor. The map is cacheable
// (and may already have been cached). new.target.prototype is guaranteed to
// be a JSReceiver.
if (new_target->IsJSFunction()) {
Handle<JSFunction> function = Handle<JSFunction>::cast(new_target);
-
- // Check that |function|'s initial map still in sync with the |constructor|,
- // otherwise we must create a new initial map for |function|.
- if (function->has_initial_map() &&
- function->initial_map()->GetConstructor() == *constructor) {
+ if (FastInitializeDerivedMap(isolate, function, constructor,
+ constructor_initial_map)) {
return handle(function->initial_map(), isolate);
}
-
- // Create a new map with the size and number of in-object properties
- // suggested by |function|.
-
- // Link initial map and constructor function if the new.target is actually a
- // subclass constructor.
- if (IsDerivedConstructor(function->shared()->kind())) {
- Handle<Object> prototype(function->instance_prototype(), isolate);
- InstanceType instance_type = constructor_initial_map->instance_type();
- DCHECK(CanSubclassHaveInobjectProperties(instance_type));
- int embedder_fields =
- JSObject::GetEmbedderFieldCount(*constructor_initial_map);
- int pre_allocated = constructor_initial_map->GetInObjectProperties() -
- constructor_initial_map->UnusedPropertyFields();
- int instance_size;
- int in_object_properties;
- bool success = CalculateInstanceSizeForDerivedClass(
- function, instance_type, embedder_fields, &instance_size,
- &in_object_properties);
-
- int unused_property_fields = in_object_properties - pre_allocated;
-
- Handle<Map> map;
- if (success) {
- map = Map::CopyInitialMap(constructor_initial_map, instance_size,
- in_object_properties, unused_property_fields);
- } else {
- map = Map::CopyInitialMap(constructor_initial_map);
- }
- map->set_new_target_is_base(false);
-
- JSFunction::SetInitialMap(function, map, prototype);
- map->SetConstructor(*constructor);
- map->set_construction_counter(Map::kNoSlackTracking);
- map->StartInobjectSlackTracking();
- return map;
- }
}
// Slow path, new.target is either a proxy or can't cache the map.
@@ -13085,7 +13096,7 @@
Handle<Map> map = Map::CopyInitialMap(constructor_initial_map);
map->set_new_target_is_base(false);
- DCHECK(prototype->IsJSReceiver());
+ CHECK(prototype->IsJSReceiver());
if (map->prototype() != *prototype) Map::SetPrototype(map, prototype);
map->SetConstructor(*constructor);
return map;
@@ -13779,15 +13790,17 @@
int* instance_size,
int* in_object_properties) {
int header_size = JSObject::GetHeaderSize(instance_type, has_prototype_slot);
- DCHECK_LE(requested_embedder_fields,
- (JSObject::kMaxInstanceSize - header_size) >> kPointerSizeLog2);
+ int max_nof_fields =
+ (JSObject::kMaxInstanceSize - header_size) >> kPointerSizeLog2;
+ CHECK_LE(max_nof_fields, JSObject::kMaxInObjectProperties);
+ *in_object_properties = Min(requested_in_object_properties, max_nof_fields);
+ CHECK_LE(requested_embedder_fields, max_nof_fields - *in_object_properties);
*instance_size =
- Min(header_size +
- ((requested_embedder_fields + requested_in_object_properties)
- << kPointerSizeLog2),
- JSObject::kMaxInstanceSize);
- *in_object_properties = ((*instance_size - header_size) >> kPointerSizeLog2) -
- requested_embedder_fields;
+ header_size +
+ ((requested_embedder_fields + *in_object_properties) << kPointerSizeLog2);
+ CHECK_EQ(*in_object_properties,
+ ((*instance_size - header_size) >> kPointerSizeLog2) -
+ requested_embedder_fields);
}
// static
@@ -13797,7 +13810,6 @@
int* in_object_properties) {
Isolate* isolate = function->GetIsolate();
int expected_nof_properties = 0;
- bool result = true;
for (PrototypeIterator iter(isolate, function, kStartAtReceiver);
!iter.IsAtEnd(); iter.Advance()) {
Handle<JSReceiver> current =
@@ -13810,21 +13822,24 @@
if (shared->is_compiled() ||
Compiler::Compile(func, Compiler::CLEAR_EXCEPTION)) {
DCHECK(shared->is_compiled());
- expected_nof_properties += shared->expected_nof_properties();
+ int count = shared->expected_nof_properties();
+ // Check that the estimate is sane.
+ if (expected_nof_properties <= JSObject::kMaxInObjectProperties - count) {
+ expected_nof_properties += count;
+ } else {
+ expected_nof_properties = JSObject::kMaxInObjectProperties;
+ }
} else if (!shared->is_compiled()) {
// In case there was a compilation error for the constructor we will
// throw an error during instantiation. Hence we directly return 0;
- result = false;
- break;
+ return false;
}
- if (!IsDerivedConstructor(shared->kind())) {
- break;
- }
+ if (!IsDerivedConstructor(shared->kind())) break;
}
CalculateInstanceSizeHelper(instance_type, true, requested_embedder_fields,
expected_nof_properties, instance_size,
in_object_properties);
- return result;
+ return true;
}
diff --git a/src/objects.h b/src/objects.h
index f242843..94059dae 100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -2707,6 +2707,7 @@
STATIC_ASSERT(kHeaderSize == Internals::kJSObjectHeaderSize);
static const int kMaxInObjectProperties =
(kMaxInstanceSize - kHeaderSize) >> kPointerSizeLog2;
+ STATIC_ASSERT(kMaxInObjectProperties <= kMaxNumberOfDescriptors);
class BodyDescriptor;
// No weak fields.
diff --git a/test/cctest/test-inobject-slack-tracking.cc b/test/cctest/test-inobject-slack-tracking.cc
index 48ec9e1..be6a71b 100644
--- a/test/cctest/test-inobject-slack-tracking.cc
+++ b/test/cctest/test-inobject-slack-tracking.cc
@@ -622,6 +622,12 @@
TestClassHierarchy(hierarchy_desc, static_cast<int>(hierarchy_desc.size()));
}
+TEST(Subclasses) {
+ std::vector<int> hierarchy_desc;
+ hierarchy_desc.push_back(50);
+ hierarchy_desc.push_back(128);
+ TestSubclassChain(hierarchy_desc);
+}
TEST(LongSubclassChain1) {
std::vector<int> hierarchy_desc;
diff --git a/test/mjsunit/es6/destructuring-assignment.js b/test/mjsunit/es6/destructuring-assignment.js
index 579c877..dee7a0b 100644
--- a/test/mjsunit/es6/destructuring-assignment.js
+++ b/test/mjsunit/es6/destructuring-assignment.js
@@ -513,25 +513,31 @@
}
function FakeNewTarget() {}
- assertEquals(undefined, ReturnNewTarget1());
- assertEquals(ReturnNewTarget1, new ReturnNewTarget1());
- assertEquals(FakeNewTarget,
- Reflect.construct(ReturnNewTarget1, [], FakeNewTarget));
- assertEquals(undefined, ReturnNewTarget2());
- assertEquals(ReturnNewTarget2, new ReturnNewTarget2());
- assertEquals(FakeNewTarget,
- Reflect.construct(ReturnNewTarget2, [], FakeNewTarget));
+ function construct() {
+ assertEquals(undefined, ReturnNewTarget1());
+ assertEquals(ReturnNewTarget1, new ReturnNewTarget1());
+ assertEquals(FakeNewTarget,
+ Reflect.construct(ReturnNewTarget1, [], FakeNewTarget));
- assertEquals(undefined, ReturnNewTarget3());
- assertEquals(ReturnNewTarget3, new ReturnNewTarget3());
- assertEquals(FakeNewTarget,
- Reflect.construct(ReturnNewTarget3, [], FakeNewTarget));
+ assertEquals(undefined, ReturnNewTarget2());
+ assertEquals(ReturnNewTarget2, new ReturnNewTarget2());
+ assertEquals(FakeNewTarget,
+ Reflect.construct(ReturnNewTarget2, [], FakeNewTarget));
- assertEquals(undefined, ReturnNewTarget4());
- assertEquals(ReturnNewTarget4, new ReturnNewTarget4());
- assertEquals(FakeNewTarget,
- Reflect.construct(ReturnNewTarget4, [], FakeNewTarget));
+ assertEquals(undefined, ReturnNewTarget3());
+ assertEquals(ReturnNewTarget3, new ReturnNewTarget3());
+ assertEquals(FakeNewTarget,
+ Reflect.construct(ReturnNewTarget3, [], FakeNewTarget));
+
+ assertEquals(undefined, ReturnNewTarget4());
+ assertEquals(ReturnNewTarget4, new ReturnNewTarget4());
+ assertEquals(FakeNewTarget,
+ Reflect.construct(ReturnNewTarget4, [], FakeNewTarget));
+ }
+ construct();
+ FakeNewTarget.prototype = 1;
+ construct();
})();
(function testSuperCall() {
diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status
index 98ba88a..6410235 100644
--- a/test/mjsunit/mjsunit.status
+++ b/test/mjsunit/mjsunit.status
@@ -86,6 +86,9 @@
# TODO(arm): This seems to flush out a bug on arm with simulator.
'array-constructor': [PASS, SLOW, ['arch == arm and simulator == True', SKIP]],
+ # Very slow test
+ 'regress/regress-crbug-808192' : [PASS, NO_VARIANTS, ['arch == arm or arch == arm64 or arch == android_arm or arch == android_arm64 or arch == mipsel or arch == mips64el or arch == mips64 or arch == mips', SKIP]],
+
# Very slow on ARM and MIPS, contains no architecture dependent code.
'unicode-case-overoptimization': [PASS, NO_VARIANTS, ['arch == arm or arch == arm64 or arch == android_arm or arch == android_arm64 or arch == mipsel or arch == mips64el or arch == mips64 or arch == mips', SKIP]],
'regress/regress-3976': [PASS, NO_VARIANTS, ['arch == arm or arch == arm64 or arch == android_arm or arch == android_arm64 or arch == mipsel or arch == mips64el or arch == mips64 or arch == mips', SKIP]],
diff --git a/test/mjsunit/regress/regress-crbug-808192.js b/test/mjsunit/regress/regress-crbug-808192.js
new file mode 100644
index 0000000..3336c00
--- /dev/null
+++ b/test/mjsunit/regress/regress-crbug-808192.js
@@ -0,0 +1,32 @@
+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// TODO(cbruni): enable always opt once v8:7438
+// Flags: --expose-gc --no-always-opt
+
+const f = eval(`(function f(i) {
+ if (i == 0) {
+ class Derived extends Object {
+ constructor() {
+ super();
+ ${"this.a=1;".repeat(0x3fffe-8)}
+ }
+ }
+ return Derived;
+ }
+
+ class DerivedN extends f(i-1) {
+ constructor() {
+ super();
+ ${"this.a=1;".repeat(0x40000-8)}
+ }
+ }
+
+ return DerivedN;
+})`);
+
+let a = new (f(0x7ff))();
+a.a = 1;
+gc();
+assertEquals(1, a.a);
