diff --git a/src/code-stub-assembler.cc b/src/code-stub-assembler.cc index ccfefe08f1..0658d61a67 100644 --- a/src/code-stub-assembler.cc +++ b/src/code-stub-assembler.cc @@ -2387,6 +2387,7 @@ Node* CodeStubAssembler::AllocateNameDictionary(Node* at_least_space_for) { Node* CodeStubAssembler::AllocateNameDictionaryWithCapacity(Node* capacity) { CSA_ASSERT(this, WordIsPowerOfTwo(capacity)); + CSA_ASSERT(this, IntPtrGreaterThan(capacity, IntPtrConstant(0))); Node* length = EntryToIndex(capacity); Node* store_size = IntPtrAdd(TimesPointerSize(length), IntPtrConstant(NameDictionary::kHeaderSize)); diff --git a/src/factory.cc b/src/factory.cc index 6748e85458..f0a816b9b4 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -181,6 +181,7 @@ Handle Factory::NewFixedArray(int size, PretenureFlag pretenure) { Handle Factory::NewPropertyArray(int size, PretenureFlag pretenure) { DCHECK_LE(0, size); + if (size == 0) return empty_property_array(); CALL_HEAP_FUNCTION(isolate(), isolate()->heap()->AllocatePropertyArray(size, pretenure), PropertyArray); @@ -1331,6 +1332,7 @@ Handle Factory::CopyFixedArrayAndGrow(Handle array, Handle Factory::CopyPropertyArrayAndGrow( Handle array, int grow_by, PretenureFlag pretenure) { + DCHECK_LE(0, grow_by); CALL_HEAP_FUNCTION( isolate(), isolate()->heap()->CopyArrayAndGrow(*array, grow_by, pretenure), diff --git a/src/heap/heap.cc b/src/heap/heap.cc index 592a81a527..39fbbc3cd1 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -3724,7 +3724,10 @@ AllocationResult Heap::AllocateFixedArrayWithFiller(int length, AllocationResult Heap::AllocatePropertyArray(int length, PretenureFlag pretenure) { + // Allow length = 0 for the empty_property_array singleton. DCHECK_LE(0, length); + DCHECK_IMPLIES(length == 0, pretenure == TENURED); + DCHECK(!InNewSpace(undefined_value())); HeapObject* result = nullptr; { diff --git a/src/objects-debug.cc b/src/objects-debug.cc index 93387418c8..7fa48886d6 100644 --- a/src/objects-debug.cc +++ b/src/objects-debug.cc @@ -458,6 +458,12 @@ void FixedArray::FixedArrayVerify() { } void PropertyArray::PropertyArrayVerify() { + if (length() == 0) { + CHECK_EQ(this, this->GetHeap()->empty_property_array()); + return; + } + // There are no empty PropertyArrays. + CHECK_LT(0, length()); for (int i = 0; i < length(); i++) { Object* e = get(i); VerifyPointer(e); diff --git a/src/objects-printer.cc b/src/objects-printer.cc index 1d24791d24..77f9eb36b6 100644 --- a/src/objects-printer.cc +++ b/src/objects-printer.cc @@ -503,7 +503,12 @@ static void JSObjectPrintHeader(std::ostream& os, JSObject* obj, static void JSObjectPrintBody(std::ostream& os, JSObject* obj, // NOLINT bool print_elements = true) { - os << "\n - properties = " << Brief(obj->raw_properties_or_hash()) << " {"; + os << "\n - properties = "; + Object* properties_or_hash = obj->raw_properties_or_hash(); + if (!properties_or_hash->IsSmi()) { + os << Brief(properties_or_hash); + } + os << " {"; if (obj->PrintProperties(os)) os << "\n "; os << "}\n"; if (print_elements && obj->elements()->length() > 0) { diff --git a/src/objects.cc b/src/objects.cc index bcf7657f94..bf7ff88dce 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -4175,9 +4175,7 @@ void MigrateFastToFast(Handle object, Handle new_map) { } } - if (external > 0) { - object->SetProperties(*array); - } + object->SetProperties(*array); // Create filler object past the new instance size. int new_instance_size = new_map->instance_size(); @@ -4381,6 +4379,10 @@ int Map::NumberOfFields() const { return result; } +bool Map::HasOutOfObjectProperties() const { + return GetInObjectProperties() < NumberOfFields(); +} + void DescriptorArray::GeneralizeAllFields() { int length = number_of_descriptors(); for (int i = 0; i < length; i++) { @@ -5919,10 +5921,8 @@ void JSObject::AllocateStorageForMap(Handle object, Handle map) { storage = isolate->factory()->NewFixedArray(inobject); } - Handle array; - if (external > 0) { - array = isolate->factory()->NewPropertyArray(external); - } + Handle array = + isolate->factory()->NewPropertyArray(external); for (int i = 0; i < map->NumberOfOwnDescriptors(); i++) { PropertyDetails details = descriptors->GetDetails(i); @@ -5938,9 +5938,7 @@ void JSObject::AllocateStorageForMap(Handle object, Handle map) { } } - if (external > 0) { - object->SetProperties(*array); - } + object->SetProperties(*array); if (!FLAG_unbox_double_fields) { for (int i = 0; i < inobject; i++) { @@ -6464,13 +6462,16 @@ Object* SetHashAndUpdateProperties(HeapObject* properties, int hash) { DCHECK_NE(PropertyArray::kNoHashSentinel, hash); DCHECK(PropertyArray::HashField::is_valid(hash)); - if (properties == properties->GetHeap()->empty_fixed_array() || - properties == properties->GetHeap()->empty_property_dictionary()) { + Heap* heap = properties->GetHeap(); + if (properties == heap->empty_fixed_array() || + properties == heap->empty_property_array() || + properties == heap->empty_property_dictionary()) { return Smi::FromInt(hash); } if (properties->IsPropertyArray()) { PropertyArray::cast(properties)->SetHash(hash); + DCHECK_LT(0, PropertyArray::cast(properties)->length()); return properties; } @@ -6522,6 +6523,9 @@ void JSReceiver::SetIdentityHash(int hash) { } void JSReceiver::SetProperties(HeapObject* properties) { + DCHECK_IMPLIES(properties->IsPropertyArray() && + PropertyArray::cast(properties)->length() == 0, + properties == properties->GetHeap()->empty_property_array()); DisallowHeapAllocation no_gc; Isolate* isolate = properties->GetIsolate(); int hash = GetIdentityHashHelper(isolate, this); diff --git a/src/objects.h b/src/objects.h index 73ad334958..6fb23e7f36 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1994,6 +1994,9 @@ class JSReceiver: public HeapObject { // Gets slow properties for non-global objects. inline NameDictionary* property_dictionary() const; + // Sets the properties backing store and makes sure any existing hash is moved + // to the new properties store. To clear out the properties store, pass in the + // empty_fixed_array(), the hash will be maintained in this case as well. void SetProperties(HeapObject* properties); // There are five possible values for the properties offset. diff --git a/src/objects/map.h b/src/objects/map.h index 22d1cd62a9..4cbd07bad1 100644 --- a/src/objects/map.h +++ b/src/objects/map.h @@ -435,6 +435,8 @@ class Map : public HeapObject { int NumberOfFields() const; + bool HasOutOfObjectProperties() const; + // Returns true if transition to the given map requires special // synchronization with the concurrent marker. bool TransitionRequiresSynchronizationWithGC(Map* target) const; diff --git a/src/runtime/runtime-object.cc b/src/runtime/runtime-object.cc index 8e7f032540..7815e68d01 100644 --- a/src/runtime/runtime-object.cc +++ b/src/runtime/runtime-object.cc @@ -163,16 +163,23 @@ bool DeleteObjectPropertyFast(Isolate* isolate, Handle receiver, if (details.location() == kField) { isolate->heap()->NotifyObjectLayoutChange(*receiver, map->instance_size(), no_allocation); - Object* filler = isolate->heap()->one_pointer_filler_map(); FieldIndex index = FieldIndex::ForPropertyIndex(map, details.field_index()); - JSObject::cast(*receiver)->RawFastPropertyAtPut(index, filler); - // We must clear any recorded slot for the deleted property, because - // subsequent object modifications might put a raw double there. - // Slot clearing is the reason why this entire function cannot currently - // be implemented in the DeleteProperty stub. - if (index.is_inobject() && !map->IsUnboxedDoubleField(index)) { - isolate->heap()->ClearRecordedSlot( - *receiver, HeapObject::RawField(*receiver, index.offset())); + // Special case deleting the last out-of object property. + if (!index.is_inobject() && index.outobject_array_index() == 0) { + DCHECK(!Map::cast(backpointer)->HasOutOfObjectProperties()); + // Clear out the properties backing store. + receiver->SetProperties(isolate->heap()->empty_fixed_array()); + } else { + Object* filler = isolate->heap()->one_pointer_filler_map(); + JSObject::cast(*receiver)->RawFastPropertyAtPut(index, filler); + // We must clear any recorded slot for the deleted property, because + // subsequent object modifications might put a raw double there. + // Slot clearing is the reason why this entire function cannot currently + // be implemented in the DeleteProperty stub. + if (index.is_inobject() && !map->IsUnboxedDoubleField(index)) { + isolate->heap()->ClearRecordedSlot( + *receiver, HeapObject::RawField(*receiver, index.offset())); + } } } // If the map was marked stable before, then there could be optimized code @@ -182,6 +189,10 @@ bool DeleteObjectPropertyFast(Isolate* isolate, Handle receiver, map->NotifyLeafMapLayoutChange(); // Finally, perform the map rollback. receiver->synchronized_set_map(Map::cast(backpointer)); +#if VERIFY_HEAP + receiver->HeapObjectVerify(); + receiver->property_array()->PropertyArrayVerify(); +#endif return true; } diff --git a/src/runtime/runtime-test.cc b/src/runtime/runtime-test.cc index 2bf180df02..c825ffc9eb 100644 --- a/src/runtime/runtime-test.cc +++ b/src/runtime/runtime-test.cc @@ -1087,5 +1087,15 @@ RUNTIME_FUNCTION(Runtime_IsLiftoffFunction) { return isolate->heap()->ToBoolean(!wasm_code->is_turbofanned()); } +RUNTIME_FUNCTION(Runtime_CompleteInobjectSlackTracking) { + HandleScope scope(isolate); + DCHECK_EQ(1, args.length()); + + CONVERT_ARG_HANDLE_CHECKED(JSObject, object, 0); + object->map()->CompleteInobjectSlackTracking(); + + return isolate->heap()->undefined_value(); +} + } // namespace internal } // namespace v8 diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index a7a6136810..bae54a1c1e 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -633,6 +633,7 @@ namespace internal { F(WasmNumInterpretedCalls, 1, 1) \ F(RedirectToWasmInterpreter, 2, 1) \ F(WasmTraceMemory, 4, 1) \ + F(CompleteInobjectSlackTracking, 1, 1) \ F(IsLiftoffFunction, 1, 1) #define FOR_EACH_INTRINSIC_TYPEDARRAY(F) \ diff --git a/test/mjsunit/regress/regress-781218.js b/test/mjsunit/regress/regress-781218.js new file mode 100644 index 0000000000..ae00cc5c08 --- /dev/null +++ b/test/mjsunit/regress/regress-781218.js @@ -0,0 +1,43 @@ +// Copyright 2017 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. + +// Flags: --allow-natives-syntax + +var m = new Map(); + +function C() { } + +// Make sure slack tracking kicks in and shrinks the default size to prevent +// any further in-object properties. +%CompleteInobjectSlackTracking(new C()); + +function f(o) { + o.x = true; +} + +// Warm up {f}. +f(new C()); +f(new C()); + + +var o = new C(); +%HeapObjectVerify(o); + +m.set(o, 1); // This creates hash code on o. + +// Add an out-of-object property. +o.x = true; +%HeapObjectVerify(o); +// Delete the property (so we have no out-of-object properties). +delete o.x; +%HeapObjectVerify(o); + + +// Ensure that growing the properties backing store in optimized code preserves +// the hash. +%OptimizeFunctionOnNextCall(f); +f(o); + +%HeapObjectVerify(o); +assertEquals(1, m.get(o));