From b1c5ff0fff6c2df4ed2dcbcf162cfb6e45e40da4 Mon Sep 17 00:00:00 2001 From: mstarzinger Date: Wed, 26 Aug 2015 02:35:30 -0700 Subject: [PATCH] [heap] Prevent direct access to ExternalStringTable. R=mlippautz@chromium.org Review URL: https://codereview.chromium.org/1312553003 Cr-Commit-Position: refs/heads/master@{#30372} --- src/api.cc | 8 +- src/extensions/externalize-string-extension.cc | 4 +- src/heap/heap-inl.h | 15 ++-- src/heap/heap.cc | 4 +- src/heap/heap.h | 108 ++++++++++++------------- 5 files changed, 72 insertions(+), 67 deletions(-) diff --git a/src/api.cc b/src/api.cc index 7937c1b..c73860f 100644 --- a/src/api.cc +++ b/src/api.cc @@ -5785,7 +5785,7 @@ MaybeLocal v8::String::NewExternalTwoByte( i::Handle string = i_isolate->factory() ->NewExternalStringFromTwoByte(resource) .ToHandleChecked(); - i_isolate->heap()->external_string_table()->AddString(*string); + i_isolate->heap()->RegisterExternalString(*string); return Utils::ToLocal(string); } @@ -5809,7 +5809,7 @@ MaybeLocal v8::String::NewExternalOneByte( i::Handle string = i_isolate->factory() ->NewExternalStringFromOneByte(resource) .ToHandleChecked(); - i_isolate->heap()->external_string_table()->AddString(*string); + i_isolate->heap()->RegisterExternalString(*string); return Utils::ToLocal(string); } @@ -5837,7 +5837,7 @@ bool v8::String::MakeExternal(v8::String::ExternalStringResource* resource) { DCHECK(!CanMakeExternal() || result); if (result) { DCHECK(obj->IsExternalString()); - isolate->heap()->external_string_table()->AddString(*obj); + isolate->heap()->RegisterExternalString(*obj); } return result; } @@ -5861,7 +5861,7 @@ bool v8::String::MakeExternal( DCHECK(!CanMakeExternal() || result); if (result) { DCHECK(obj->IsExternalString()); - isolate->heap()->external_string_table()->AddString(*obj); + isolate->heap()->RegisterExternalString(*obj); } return result; } diff --git a/src/extensions/externalize-string-extension.cc b/src/extensions/externalize-string-extension.cc index 3eaa70e..9241e9f 100644 --- a/src/extensions/externalize-string-extension.cc +++ b/src/extensions/externalize-string-extension.cc @@ -98,7 +98,7 @@ void ExternalizeStringExtension::Externalize( result = string->MakeExternal(resource); if (result) { i::Isolate* isolate = reinterpret_cast(args.GetIsolate()); - isolate->heap()->external_string_table()->AddString(*string); + isolate->heap()->RegisterExternalString(*string); } if (!result) delete resource; } else { @@ -109,7 +109,7 @@ void ExternalizeStringExtension::Externalize( result = string->MakeExternal(resource); if (result) { i::Isolate* isolate = reinterpret_cast(args.GetIsolate()); - isolate->heap()->external_string_table()->AddString(*string); + isolate->heap()->RegisterExternalString(*string); } if (!result) delete resource; } diff --git a/src/heap/heap-inl.h b/src/heap/heap-inl.h index f40ac56..a3785d0 100644 --- a/src/heap/heap-inl.h +++ b/src/heap/heap-inl.h @@ -323,6 +323,11 @@ void Heap::UpdateAllocationsHash(uint32_t value) { } +void Heap::RegisterExternalString(String* string) { + external_string_table_.AddString(string); +} + + void Heap::FinalizeExternalString(String* string) { DCHECK(string->IsExternalString()); v8::String::ExternalStringResourceBase** resource_addr = @@ -608,7 +613,7 @@ Isolate* Heap::isolate() { CALL_AND_RETRY_OR_DIE(ISOLATE, FUNCTION_CALL, return, return) -void ExternalStringTable::AddString(String* string) { +void Heap::ExternalStringTable::AddString(String* string) { DCHECK(string->IsExternalString()); if (heap_->InNewSpace(string)) { new_space_strings_.Add(string); @@ -618,7 +623,7 @@ void ExternalStringTable::AddString(String* string) { } -void ExternalStringTable::Iterate(ObjectVisitor* v) { +void Heap::ExternalStringTable::Iterate(ObjectVisitor* v) { if (!new_space_strings_.is_empty()) { Object** start = &new_space_strings_[0]; v->VisitPointers(start, start + new_space_strings_.length()); @@ -632,7 +637,7 @@ void ExternalStringTable::Iterate(ObjectVisitor* v) { // Verify() is inline to avoid ifdef-s around its calls in release // mode. -void ExternalStringTable::Verify() { +void Heap::ExternalStringTable::Verify() { #ifdef DEBUG for (int i = 0; i < new_space_strings_.length(); ++i) { Object* obj = Object::cast(new_space_strings_[i]); @@ -648,14 +653,14 @@ void ExternalStringTable::Verify() { } -void ExternalStringTable::AddOldString(String* string) { +void Heap::ExternalStringTable::AddOldString(String* string) { DCHECK(string->IsExternalString()); DCHECK(!heap_->InNewSpace(string)); old_space_strings_.Add(string); } -void ExternalStringTable::ShrinkNewStrings(int position) { +void Heap::ExternalStringTable::ShrinkNewStrings(int position) { new_space_strings_.Rewind(position); #ifdef VERIFY_HEAP if (FLAG_verify_heap) { diff --git a/src/heap/heap.cc b/src/heap/heap.cc index 30a6544..b7e0b11 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -6460,7 +6460,7 @@ void DescriptorLookupCache::Clear() { } -void ExternalStringTable::CleanUp() { +void Heap::ExternalStringTable::CleanUp() { int last = 0; for (int i = 0; i < new_space_strings_.length(); ++i) { if (new_space_strings_[i] == heap_->the_hole_value()) { @@ -6495,7 +6495,7 @@ void ExternalStringTable::CleanUp() { } -void ExternalStringTable::TearDown() { +void Heap::ExternalStringTable::TearDown() { for (int i = 0; i < new_space_strings_.length(); ++i) { heap_->FinalizeExternalString(ExternalString::cast(new_space_strings_[i])); } diff --git a/src/heap/heap.h b/src/heap/heap.h index aa63a00..68de80b 100644 --- a/src/heap/heap.h +++ b/src/heap/heap.h @@ -422,16 +422,13 @@ namespace internal { PRIVATE_SYMBOL_LIST(V) // Forward declarations. +class HeapObjectsFilter; class HeapStats; class Isolate; class MemoryReducer; class WeakObjectRetainer; -typedef String* (*ExternalStringTableUpdaterCallback)(Heap* heap, - Object** pointer); - - // A queue of objects promoted during scavenge. Each object is accompanied // by it's size to avoid dereferencing a map pointer for scanning. // The last page in to-space is used for the promotion queue. On conflict @@ -541,46 +538,6 @@ typedef void (*ScavengingCallback)(Map* map, HeapObject** slot, HeapObject* object); -// External strings table is a place where all external strings are -// registered. We need to keep track of such strings to properly -// finalize them. -class ExternalStringTable { - public: - // Registers an external string. - inline void AddString(String* string); - - inline void Iterate(ObjectVisitor* v); - - // Restores internal invariant and gets rid of collected strings. - // Must be called after each Iterate() that modified the strings. - void CleanUp(); - - // Destroys all allocated memory. - void TearDown(); - - private: - explicit ExternalStringTable(Heap* heap) : heap_(heap) {} - - friend class Heap; - - inline void Verify(); - - inline void AddOldString(String* string); - - // Notifies the table that only a prefix of the new list is valid. - inline void ShrinkNewStrings(int position); - - // To speed up scavenge collections new space string are kept - // separate from old space strings. - List new_space_strings_; - List old_space_strings_; - - Heap* heap_; - - DISALLOW_COPY_AND_ASSIGN(ExternalStringTable); -}; - - enum ArrayStorageAllocationMode { DONT_INITIALIZE_ARRAY_ELEMENTS, INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE @@ -870,10 +827,6 @@ class Heap { // index. void MoveElements(FixedArray* array, int dst_index, int src_index, int len); - // Finalizes an external string by deleting the associated external - // data and clearing the resource pointer. - inline void FinalizeExternalString(String* string); - // Initialize a filler object to keep the ability to iterate over the heap // when introducing gaps within pages. void CreateFillerObjectAt(Address addr, int size); @@ -1220,10 +1173,6 @@ class Heap { return &mark_compact_collector_; } - ExternalStringTable* external_string_table() { - return &external_string_table_; - } - // =========================================================================== // Root set access. ========================================================== // =========================================================================== @@ -1393,6 +1342,17 @@ class Heap { IncrementalMarking* incremental_marking() { return &incremental_marking_; } // =========================================================================== + // External string table API. ================================================ + // =========================================================================== + + // Registers an external string. + inline void RegisterExternalString(String* string); + + // Finalizes an external string by deleting the associated external + // data and clearing the resource pointer. + inline void FinalizeExternalString(String* string); + + // =========================================================================== // Methods checking/returning the space of a given object/address. =========== // =========================================================================== @@ -1613,6 +1573,45 @@ class Heap { private: class UnmapFreeMemoryTask; + // External strings table is a place where all external strings are + // registered. We need to keep track of such strings to properly + // finalize them. + class ExternalStringTable { + public: + // Registers an external string. + inline void AddString(String* string); + + inline void Iterate(ObjectVisitor* v); + + // Restores internal invariant and gets rid of collected strings. + // Must be called after each Iterate() that modified the strings. + void CleanUp(); + + // Destroys all allocated memory. + void TearDown(); + + private: + explicit ExternalStringTable(Heap* heap) : heap_(heap) {} + + inline void Verify(); + + inline void AddOldString(String* string); + + // Notifies the table that only a prefix of the new list is valid. + inline void ShrinkNewStrings(int position); + + // To speed up scavenge collections new space string are kept + // separate from old space strings. + List new_space_strings_; + List old_space_strings_; + + Heap* heap_; + + friend class Heap; + + DISALLOW_COPY_AND_ASSIGN(ExternalStringTable); + }; + struct StrongRootsList; struct StringTypeTable { @@ -1646,6 +1645,9 @@ class Heap { bool pass_isolate; }; + typedef String* (*ExternalStringTableUpdaterCallback)(Heap* heap, + Object** pointer); + static const int kInitialStringTableSize = 2048; static const int kInitialEvalCacheSize = 64; static const int kInitialNumberStringCacheSize = 256; @@ -2569,8 +2571,6 @@ class SpaceIterator : public Malloced { // nodes filtering uses GC marks, it can't be used during MS/MC GC // phases. Also, it is forbidden to interrupt iteration in this mode, // as this will leave heap objects marked (and thus, unusable). -class HeapObjectsFilter; - class HeapIterator BASE_EMBEDDED { public: enum HeapObjectsFiltering { kNoFiltering, kFilterUnreachable }; -- 2.7.4