Revert of Fix memory leak caused by field type in descriptor array. (patchset #3 id:40001 of https://codereview.chromium.org/955063002/)
Reason for revert: Breaks test/mjsunit/debug-clearbreakpointgroup.js on arm64.debug. Original issue's description: > 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 > > Committed: https://crrev.com/77d3ae0e119893ac8d34ea6ca090cddd5bbf987e > Cr-Commit-Position: refs/heads/master@{#26879} TBR=verwaest@chromium.org,ulan@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=v8:3877 Review URL: https://codereview.chromium.org/960103003 Cr-Commit-Position: refs/heads/master@{#26883}
This commit is contained in:
marja
Commit bot
parent
6517181c49
commit
b57be748b1
@ -3136,12 +3136,7 @@ int DescriptorArray::GetFieldIndex(int descriptor_number) {
|
||||
|
||||
HeapType* DescriptorArray::GetFieldType(int descriptor_number) {
|
||||
DCHECK(GetDetails(descriptor_number).location() == kField);
|
||||
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);
|
||||
return HeapType::cast(GetValue(descriptor_number));
|
||||
}
|
||||
|
||||
|
||||
|
||||
@ -1704,12 +1704,6 @@ String* JSReceiver::constructor_name() {
|
||||
}
|
||||
|
||||
|
||||
static Handle<Object> WrapType(Handle<HeapType> type) {
|
||||
if (type->IsClass()) return Map::WeakCellForMap(type->AsClass()->Map());
|
||||
return type;
|
||||
}
|
||||
|
||||
|
||||
MaybeHandle<Map> Map::CopyWithField(Handle<Map> map,
|
||||
Handle<Name> name,
|
||||
Handle<HeapType> type,
|
||||
@ -1735,10 +1729,7 @@ MaybeHandle<Map> Map::CopyWithField(Handle<Map> map,
|
||||
type = HeapType::Any(isolate);
|
||||
}
|
||||
|
||||
Handle<Object> wrapped_type(WrapType(type));
|
||||
|
||||
DataDescriptor new_field_desc(name, index, wrapped_type, attributes,
|
||||
representation);
|
||||
DataDescriptor new_field_desc(name, index, type, attributes, representation);
|
||||
Handle<Map> new_map = Map::CopyAddDescriptor(map, &new_field_desc, flag);
|
||||
int unused_property_fields = new_map->unused_property_fields() - 1;
|
||||
if (unused_property_fields < 0) {
|
||||
@ -2314,16 +2305,15 @@ Map* Map::FindFieldOwner(int descriptor) {
|
||||
|
||||
void Map::UpdateFieldType(int descriptor, Handle<Name> name,
|
||||
Representation new_representation,
|
||||
Handle<Object> new_wrapped_type) {
|
||||
DCHECK(new_wrapped_type->IsSmi() || new_wrapped_type->IsWeakCell());
|
||||
Handle<HeapType> new_type) {
|
||||
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_wrapped_type);
|
||||
transitions->GetTarget(i)
|
||||
->UpdateFieldType(descriptor, name, new_representation, new_type);
|
||||
}
|
||||
}
|
||||
// It is allowed to change representation here only from None to something.
|
||||
@ -2331,9 +2321,9 @@ void Map::UpdateFieldType(int descriptor, Handle<Name> name,
|
||||
details.representation().IsNone());
|
||||
|
||||
// Skip if already updated the shared descriptor.
|
||||
if (instance_descriptors()->GetValue(descriptor) == *new_wrapped_type) return;
|
||||
if (instance_descriptors()->GetFieldType(descriptor) == *new_type) return;
|
||||
DataDescriptor d(name, instance_descriptors()->GetFieldIndex(descriptor),
|
||||
new_wrapped_type, details.attributes(), new_representation);
|
||||
new_type, details.attributes(), new_representation);
|
||||
instance_descriptors()->Replace(descriptor, &d);
|
||||
}
|
||||
|
||||
@ -2381,10 +2371,8 @@ void Map::GeneralizeFieldType(Handle<Map> map, int modify_index,
|
||||
|
||||
PropertyDetails details = descriptors->GetDetails(modify_index);
|
||||
Handle<Name> name(descriptors->GetKey(modify_index));
|
||||
|
||||
Handle<Object> wrapped_type(WrapType(new_field_type));
|
||||
field_owner->UpdateFieldType(modify_index, name, new_representation,
|
||||
wrapped_type);
|
||||
new_field_type);
|
||||
field_owner->dependent_code()->DeoptimizeDependentCodeGroup(
|
||||
isolate, DependentCode::kFieldTypeGroup);
|
||||
|
||||
@ -2776,8 +2764,7 @@ Handle<Map> Map::ReconfigureProperty(Handle<Map> old_map, int modify_index,
|
||||
next_field_type =
|
||||
GeneralizeFieldType(target_field_type, old_field_type, isolate);
|
||||
}
|
||||
Handle<Object> wrapped_type(WrapType(next_field_type));
|
||||
DataDescriptor d(target_key, current_offset, wrapped_type,
|
||||
DataDescriptor d(target_key, current_offset, next_field_type,
|
||||
next_attributes, next_representation);
|
||||
current_offset += d.GetDetails().field_width_in_words();
|
||||
new_descriptors->Set(i, &d);
|
||||
@ -2845,10 +2832,8 @@ Handle<Map> Map::ReconfigureProperty(Handle<Map> old_map, int modify_index,
|
||||
next_field_type = old_field_type;
|
||||
}
|
||||
|
||||
Handle<Object> wrapped_type(WrapType(next_field_type));
|
||||
|
||||
DataDescriptor d(old_key, current_offset, wrapped_type, next_attributes,
|
||||
next_representation);
|
||||
DataDescriptor d(old_key, current_offset, next_field_type,
|
||||
next_attributes, next_representation);
|
||||
current_offset += d.GetDetails().field_width_in_words();
|
||||
new_descriptors->Set(i, &d);
|
||||
} else {
|
||||
@ -2979,43 +2964,35 @@ MaybeHandle<Map> Map::TryUpdate(Handle<Map> old_map) {
|
||||
if (!old_details.representation().fits_into(new_details.representation())) {
|
||||
return MaybeHandle<Map>();
|
||||
}
|
||||
Object* new_value = new_descriptors->GetValue(i);
|
||||
Object* old_value = old_descriptors->GetValue(i);
|
||||
switch (new_details.type()) {
|
||||
case DATA: {
|
||||
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)) {
|
||||
PropertyType old_type = old_details.type();
|
||||
if (old_type == DATA) {
|
||||
if (!HeapType::cast(old_value)->NowIs(HeapType::cast(new_value))) {
|
||||
return MaybeHandle<Map>();
|
||||
}
|
||||
} else {
|
||||
DCHECK(old_property_type == DATA_CONSTANT);
|
||||
Object* old_value = old_descriptors->GetValue(i);
|
||||
if (!new_type->NowContains(old_value)) {
|
||||
DCHECK(old_type == DATA_CONSTANT);
|
||||
if (!HeapType::cast(new_value)->NowContains(old_value)) {
|
||||
return MaybeHandle<Map>();
|
||||
}
|
||||
}
|
||||
break;
|
||||
}
|
||||
case ACCESSOR: {
|
||||
#ifdef DEBUG
|
||||
HeapType* new_type = new_descriptors->GetFieldType(i);
|
||||
DCHECK(HeapType::Any()->Is(new_type));
|
||||
#endif
|
||||
case ACCESSOR:
|
||||
DCHECK(HeapType::Any()->Is(HeapType::cast(new_value)));
|
||||
break;
|
||||
}
|
||||
|
||||
case DATA_CONSTANT:
|
||||
case ACCESSOR_CONSTANT: {
|
||||
Object* old_value = old_descriptors->GetValue(i);
|
||||
Object* new_value = new_descriptors->GetValue(i);
|
||||
case ACCESSOR_CONSTANT:
|
||||
if (old_details.location() == kField || old_value != new_value) {
|
||||
return MaybeHandle<Map>();
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (new_map->NumberOfOwnDescriptors() != old_nof) return MaybeHandle<Map>();
|
||||
return handle(new_map);
|
||||
}
|
||||
|
||||
@ -6443,12 +6443,9 @@ 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> name,
|
||||
Representation new_representation,
|
||||
Handle<Object> new_wrapped_type);
|
||||
Handle<HeapType> new_type);
|
||||
|
||||
void PrintReconfiguration(FILE* file, int modify_index, PropertyKind kind,
|
||||
PropertyAttributes attributes);
|
||||
|
||||
@ -79,14 +79,10 @@ class DataDescriptor FINAL : public Descriptor {
|
||||
PropertyAttributes attributes, Representation representation)
|
||||
: Descriptor(key, HeapType::Any(key->GetIsolate()), attributes, DATA,
|
||||
representation, field_index) {}
|
||||
// The field type is either a simple type or a map wrapped in a weak cell.
|
||||
DataDescriptor(Handle<Name> key, int field_index,
|
||||
Handle<Object> wrapped_field_type,
|
||||
DataDescriptor(Handle<Name> key, int field_index, Handle<HeapType> field_type,
|
||||
PropertyAttributes attributes, Representation representation)
|
||||
: Descriptor(key, wrapped_field_type, attributes, DATA, representation,
|
||||
field_index) {
|
||||
DCHECK(wrapped_field_type->IsSmi() || wrapped_field_type->IsWeakCell());
|
||||
}
|
||||
: Descriptor(key, field_type, attributes, DATA, representation,
|
||||
field_index) {}
|
||||
};
|
||||
|
||||
|
||||
|
||||
@ -5083,40 +5083,6 @@ 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<WeakCell> weak_prototype;
|
||||
{
|
||||
HandleScope inner_scope(isolate);
|
||||
v8::Local<v8::Value> result = CompileRun("cls.prototype");
|
||||
Handle<JSObject> proto =
|
||||
v8::Utils::OpenHandle(*v8::Handle<v8::Object>::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();
|
||||
|
||||
@ -233,14 +233,12 @@ class Expectations {
|
||||
representations_[descriptor])) {
|
||||
return false;
|
||||
}
|
||||
Object* value = descriptors->GetValue(descriptor);
|
||||
Object* expected_value = *values_[descriptor];
|
||||
Object* value = descriptors->GetValue(descriptor);
|
||||
switch (type) {
|
||||
case DATA:
|
||||
case ACCESSOR: {
|
||||
HeapType* type = descriptors->GetFieldType(descriptor);
|
||||
return HeapType::cast(expected_value)->Equals(type);
|
||||
}
|
||||
case ACCESSOR:
|
||||
return HeapType::cast(expected_value)->Equals(HeapType::cast(value));
|
||||
|
||||
case DATA_CONSTANT:
|
||||
return value == expected_value;
|
||||
|
||||
Loading…
Reference in New Issue
Block a user