From: ulan Date: Thu, 26 Feb 2015 13:16:32 +0000 (-0800) Subject: Fix memory leak caused by field type in descriptor array. X-Git-Tag: upstream/4.7.83~4161 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=77d3ae0e119893ac8d34ea6ca090cddd5bbf987e;p=platform%2Fupstream%2Fv8.git Fix memory leak caused by field type in descriptor array. When a field type is a map, it is wrapped in a weak cell upon storing to the descriptor array. Map::GetFieldType(i) does the unwrapping. BUG=v8:3877 LOG=N TEST=cctest/test-heap/Regress3877 Review URL: https://codereview.chromium.org/955063002 Cr-Commit-Position: refs/heads/master@{#26879} --- diff --git a/src/objects-inl.h b/src/objects-inl.h index 4ea0a08..dc73935 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -3136,7 +3136,12 @@ int DescriptorArray::GetFieldIndex(int descriptor_number) { HeapType* DescriptorArray::GetFieldType(int descriptor_number) { DCHECK(GetDetails(descriptor_number).location() == kField); - return HeapType::cast(GetValue(descriptor_number)); + Object* value = GetValue(descriptor_number); + if (value->IsWeakCell()) { + if (WeakCell::cast(value)->cleared()) return HeapType::Any(); + value = WeakCell::cast(value)->value(); + } + return HeapType::cast(value); } diff --git a/src/objects.cc b/src/objects.cc index bef1881..d93f99b 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -1704,6 +1704,12 @@ String* JSReceiver::constructor_name() { } +static Handle WrapType(Handle type) { + if (type->IsClass()) return Map::WeakCellForMap(type->AsClass()->Map()); + return type; +} + + MaybeHandle Map::CopyWithField(Handle map, Handle name, Handle type, @@ -1729,7 +1735,10 @@ MaybeHandle Map::CopyWithField(Handle map, type = HeapType::Any(isolate); } - DataDescriptor new_field_desc(name, index, type, attributes, representation); + Handle wrapped_type(WrapType(type)); + + DataDescriptor new_field_desc(name, index, wrapped_type, attributes, + representation); Handle new_map = Map::CopyAddDescriptor(map, &new_field_desc, flag); int unused_property_fields = new_map->unused_property_fields() - 1; if (unused_property_fields < 0) { @@ -2305,15 +2314,16 @@ Map* Map::FindFieldOwner(int descriptor) { void Map::UpdateFieldType(int descriptor, Handle name, Representation new_representation, - Handle new_type) { + Handle new_wrapped_type) { + DCHECK(new_wrapped_type->IsSmi() || new_wrapped_type->IsWeakCell()); DisallowHeapAllocation no_allocation; PropertyDetails details = instance_descriptors()->GetDetails(descriptor); if (details.type() != DATA) return; if (HasTransitionArray()) { TransitionArray* transitions = this->transitions(); for (int i = 0; i < transitions->number_of_transitions(); ++i) { - transitions->GetTarget(i) - ->UpdateFieldType(descriptor, name, new_representation, new_type); + transitions->GetTarget(i)->UpdateFieldType( + descriptor, name, new_representation, new_wrapped_type); } } // It is allowed to change representation here only from None to something. @@ -2321,9 +2331,9 @@ void Map::UpdateFieldType(int descriptor, Handle name, details.representation().IsNone()); // Skip if already updated the shared descriptor. - if (instance_descriptors()->GetFieldType(descriptor) == *new_type) return; + if (instance_descriptors()->GetValue(descriptor) == *new_wrapped_type) return; DataDescriptor d(name, instance_descriptors()->GetFieldIndex(descriptor), - new_type, details.attributes(), new_representation); + new_wrapped_type, details.attributes(), new_representation); instance_descriptors()->Replace(descriptor, &d); } @@ -2371,8 +2381,10 @@ void Map::GeneralizeFieldType(Handle map, int modify_index, PropertyDetails details = descriptors->GetDetails(modify_index); Handle name(descriptors->GetKey(modify_index)); + + Handle wrapped_type(WrapType(new_field_type)); field_owner->UpdateFieldType(modify_index, name, new_representation, - new_field_type); + wrapped_type); field_owner->dependent_code()->DeoptimizeDependentCodeGroup( isolate, DependentCode::kFieldTypeGroup); @@ -2764,7 +2776,8 @@ Handle Map::ReconfigureProperty(Handle old_map, int modify_index, next_field_type = GeneralizeFieldType(target_field_type, old_field_type, isolate); } - DataDescriptor d(target_key, current_offset, next_field_type, + Handle wrapped_type(WrapType(next_field_type)); + DataDescriptor d(target_key, current_offset, wrapped_type, next_attributes, next_representation); current_offset += d.GetDetails().field_width_in_words(); new_descriptors->Set(i, &d); @@ -2832,8 +2845,10 @@ Handle Map::ReconfigureProperty(Handle old_map, int modify_index, next_field_type = old_field_type; } - DataDescriptor d(old_key, current_offset, next_field_type, - next_attributes, next_representation); + Handle wrapped_type(WrapType(next_field_type)); + + DataDescriptor d(old_key, current_offset, wrapped_type, next_attributes, + next_representation); current_offset += d.GetDetails().field_width_in_words(); new_descriptors->Set(i, &d); } else { @@ -2964,33 +2979,41 @@ MaybeHandle Map::TryUpdate(Handle old_map) { if (!old_details.representation().fits_into(new_details.representation())) { return MaybeHandle(); } - Object* new_value = new_descriptors->GetValue(i); - Object* old_value = old_descriptors->GetValue(i); switch (new_details.type()) { case DATA: { - PropertyType old_type = old_details.type(); - if (old_type == DATA) { - if (!HeapType::cast(old_value)->NowIs(HeapType::cast(new_value))) { + HeapType* new_type = new_descriptors->GetFieldType(i); + PropertyType old_property_type = old_details.type(); + if (old_property_type == DATA) { + HeapType* old_type = old_descriptors->GetFieldType(i); + if (!old_type->NowIs(new_type)) { return MaybeHandle(); } } else { - DCHECK(old_type == DATA_CONSTANT); - if (!HeapType::cast(new_value)->NowContains(old_value)) { + DCHECK(old_property_type == DATA_CONSTANT); + Object* old_value = old_descriptors->GetValue(i); + if (!new_type->NowContains(old_value)) { return MaybeHandle(); } } break; } - case ACCESSOR: - DCHECK(HeapType::Any()->Is(HeapType::cast(new_value))); + case ACCESSOR: { +#ifdef DEBUG + HeapType* new_type = new_descriptors->GetFieldType(i); + DCHECK(HeapType::Any()->Is(new_type)); +#endif break; + } case DATA_CONSTANT: - case ACCESSOR_CONSTANT: + case ACCESSOR_CONSTANT: { + Object* old_value = old_descriptors->GetValue(i); + Object* new_value = new_descriptors->GetValue(i); if (old_details.location() == kField || old_value != new_value) { return MaybeHandle(); } break; + } } } if (new_map->NumberOfOwnDescriptors() != old_nof) return MaybeHandle(); diff --git a/src/objects.h b/src/objects.h index 4ead848..b8564d8 100644 --- a/src/objects.h +++ b/src/objects.h @@ -6443,9 +6443,12 @@ class Map: public HeapObject { Map* FindLastMatchMap(int verbatim, int length, DescriptorArray* descriptors); + // Update field type of the given descriptor to new representation and new + // type. The type must be prepared for storing in descriptor array: + // it must be either a simple type or a map wrapped in a weak cell. void UpdateFieldType(int descriptor_number, Handle name, Representation new_representation, - Handle new_type); + Handle new_wrapped_type); void PrintReconfiguration(FILE* file, int modify_index, PropertyKind kind, PropertyAttributes attributes); diff --git a/src/property.h b/src/property.h index 5f8e6da..c6556b3 100644 --- a/src/property.h +++ b/src/property.h @@ -79,10 +79,14 @@ class DataDescriptor FINAL : public Descriptor { PropertyAttributes attributes, Representation representation) : Descriptor(key, HeapType::Any(key->GetIsolate()), attributes, DATA, representation, field_index) {} - DataDescriptor(Handle key, int field_index, Handle field_type, + // The field type is either a simple type or a map wrapped in a weak cell. + DataDescriptor(Handle key, int field_index, + Handle wrapped_field_type, PropertyAttributes attributes, Representation representation) - : Descriptor(key, field_type, attributes, DATA, representation, - field_index) {} + : Descriptor(key, wrapped_field_type, attributes, DATA, representation, + field_index) { + DCHECK(wrapped_field_type->IsSmi() || wrapped_field_type->IsWeakCell()); + } }; diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc index b5e091f..2d8d7b9 100644 --- a/test/cctest/test-heap.cc +++ b/test/cctest/test-heap.cc @@ -5083,6 +5083,40 @@ TEST(NumberStringCacheSize) { } +TEST(Regress3877) { + CcTest::InitializeVM(); + Isolate* isolate = CcTest::i_isolate(); + Heap* heap = isolate->heap(); + Factory* factory = isolate->factory(); + HandleScope scope(isolate); + CompileRun("function cls() { this.x = 10; }"); + Handle weak_prototype; + { + HandleScope inner_scope(isolate); + v8::Local result = CompileRun("cls.prototype"); + Handle proto = + v8::Utils::OpenHandle(*v8::Handle::Cast(result)); + weak_prototype = inner_scope.CloseAndEscape(factory->NewWeakCell(proto)); + } + CHECK(!weak_prototype->cleared()); + CompileRun( + "var a = { };" + "a.x = new cls();" + "cls.prototype = null;"); + for (int i = 0; i < 4; i++) { + heap->CollectAllGarbage(Heap::kNoGCFlags); + } + // The map of a.x keeps prototype alive + CHECK(!weak_prototype->cleared()); + // Change the map of a.x and make the previous map garbage collectable. + CompileRun("a.x.__proto__ = {};"); + for (int i = 0; i < 4; i++) { + heap->CollectAllGarbage(Heap::kNoGCFlags); + } + CHECK(weak_prototype->cleared()); +} + + #ifdef DEBUG TEST(PathTracer) { CcTest::InitializeVM(); diff --git a/test/cctest/test-migrations.cc b/test/cctest/test-migrations.cc index 9eb3676..c7aaf29 100644 --- a/test/cctest/test-migrations.cc +++ b/test/cctest/test-migrations.cc @@ -233,12 +233,14 @@ class Expectations { representations_[descriptor])) { return false; } - Object* expected_value = *values_[descriptor]; Object* value = descriptors->GetValue(descriptor); + Object* expected_value = *values_[descriptor]; switch (type) { case DATA: - case ACCESSOR: - return HeapType::cast(expected_value)->Equals(HeapType::cast(value)); + case ACCESSOR: { + HeapType* type = descriptors->GetFieldType(descriptor); + return HeapType::cast(expected_value)->Equals(type); + } case DATA_CONSTANT: return value == expected_value;