From 199e30d36fe8a360797b723ea19cbe106504dc43 Mon Sep 17 00:00:00 2001 From: bmeurer Date: Wed, 15 Jul 2015 04:05:00 -0700 Subject: [PATCH] [handles] Sanitize Handle and friends. Bunch of cleanups to allow us to get rid of handles-inl.h at some point (in the not so far future); but more importantly to sanitize uses of handles and prepare for handle canonicalization support. R=yangguo@chromium.org Committed: https://crrev.com/3283195d0408333cce552cf4087577e6f41054e5 Cr-Commit-Position: refs/heads/master@{#28222} Committed: https://crrev.com/d940c6d3bcc227b459cb4123d9a8332d9ed0d5f8 Cr-Commit-Position: refs/heads/master@{#29666} Review URL: https://codereview.chromium.org/1128533002 Cr-Commit-Position: refs/heads/master@{#29675} --- src/api.cc | 3 +- src/api.h | 11 -- src/handles-inl.h | 76 +----------- src/handles.cc | 30 ++++- src/handles.h | 298 ++++++++++++++++++++++++++--------------------- test/cctest/test-api.cc | 2 +- test/cctest/test-heap.cc | 16 --- 7 files changed, 206 insertions(+), 230 deletions(-) diff --git a/src/api.cc b/src/api.cc index 79f8247..e2abb98 100644 --- a/src/api.cc +++ b/src/api.cc @@ -3797,7 +3797,8 @@ MaybeLocal v8::Object::ObjectProtoToString(Local context) { isolate, self, toStringTag).ToHandle(&tag); RETURN_ON_FAILED_EXECUTION(String); if (tag->IsString()) { - class_name = i::Handle::cast(tag).EscapeFrom(&handle_scope); + class_name = Utils::OpenHandle(*handle_scope.Escape( + Utils::ToLocal(i::Handle::cast(tag)))); } } const char* prefix = "[object "; diff --git a/src/api.h b/src/api.h index b20ef5c..438c4f3 100644 --- a/src/api.h +++ b/src/api.h @@ -309,17 +309,6 @@ OPEN_HANDLE_LIST(DECLARE_OPEN_HANDLE) template -v8::internal::Handle v8::internal::Handle::EscapeFrom( - v8::EscapableHandleScope* scope) { - v8::internal::Handle handle; - if (!is_null()) { - handle = *this; - } - return Utils::OpenHandle(*scope->Escape(Utils::ToLocal(handle)), true); -} - - -template inline T* ToApi(v8::internal::Handle obj) { return reinterpret_cast(obj.location()); } diff --git a/src/handles-inl.h b/src/handles-inl.h index 3022f28..b905c16 100644 --- a/src/handles-inl.h +++ b/src/handles-inl.h @@ -1,7 +1,6 @@ // Copyright 2006-2008 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. -// #ifndef V8_HANDLES_INL_H_ #define V8_HANDLES_INL_H_ @@ -14,72 +13,8 @@ namespace v8 { namespace internal { -template -Handle::Handle(T* obj) { - location_ = HandleScope::CreateHandle(obj->GetIsolate(), obj); -} - - -template -Handle::Handle(T* obj, Isolate* isolate) { - location_ = HandleScope::CreateHandle(isolate, obj); -} - - -template -inline bool Handle::is_identical_to(const Handle o) const { - // Dereferencing deferred handles to check object equality is safe. - SLOW_DCHECK( - (location_ == NULL || IsDereferenceAllowed(NO_DEFERRED_CHECK)) && - (o.location_ == NULL || o.IsDereferenceAllowed(NO_DEFERRED_CHECK))); - if (location_ == o.location_) return true; - if (location_ == NULL || o.location_ == NULL) return false; - return *location_ == *o.location_; -} - - -template -inline T* Handle::operator*() const { - SLOW_DCHECK(IsDereferenceAllowed(INCLUDE_DEFERRED_CHECK)); - return *bit_cast(location_); -} - -template -inline T** Handle::location() const { - SLOW_DCHECK(location_ == NULL || - IsDereferenceAllowed(INCLUDE_DEFERRED_CHECK)); - return location_; -} - -#ifdef DEBUG -template -bool Handle::IsDereferenceAllowed(DereferenceCheckMode mode) const { - DCHECK(location_ != NULL); - Object* object = *bit_cast(location_); - if (object->IsSmi()) return true; - HeapObject* heap_object = HeapObject::cast(object); - Heap* heap = heap_object->GetHeap(); - Object** handle = reinterpret_cast(location_); - Object** roots_array_start = heap->roots_array_start(); - if (roots_array_start <= handle && - handle < roots_array_start + Heap::kStrongRootListLength && - heap->RootCanBeTreatedAsConstant( - static_cast(handle - roots_array_start))) { - return true; - } - if (!AllowHandleDereference::IsAllowed()) return false; - if (mode == INCLUDE_DEFERRED_CHECK && - !AllowDeferredHandleDereference::IsAllowed()) { - // Accessing cells, maps and internalized strings is safe. - if (heap_object->IsCell()) return true; - if (heap_object->IsMap()) return true; - if (heap_object->IsInternalizedString()) return true; - return !heap->isolate()->IsDeferredHandle(handle); - } - return true; -} -#endif - +HandleBase::HandleBase(Object* object, Isolate* isolate) + : location_(HandleScope::CreateHandle(isolate, object)) {} HandleScope::HandleScope(Isolate* isolate) { @@ -136,7 +71,7 @@ Handle HandleScope::CloseAndEscape(Handle handle_value) { CloseScope(isolate_, prev_next_, prev_limit_); // Allocate one handle in the parent scope. DCHECK(current->level > 0); - Handle result(CreateHandle(isolate_, value)); + Handle result(value, isolate_); // Reinitialize the current scope (so that it's ready // to be used or closed again). prev_next_ = current->next; @@ -151,7 +86,7 @@ T** HandleScope::CreateHandle(Isolate* isolate, T* value) { DCHECK(AllowHandleAllocation::IsAllowed()); HandleScopeData* current = isolate->handle_scope_data(); - internal::Object** cur = current->next; + Object** cur = current->next; if (cur == current->limit) cur = Extend(isolate); // Update the current next field, set the value in the created // handle, and return the result. @@ -190,6 +125,7 @@ inline SealHandleScope::~SealHandleScope() { #endif -} } // namespace v8::internal +} // namespace internal +} // namespace v8 #endif // V8_HANDLES_INL_H_ diff --git a/src/handles.cc b/src/handles.cc index d415315..ca23a6f 100644 --- a/src/handles.cc +++ b/src/handles.cc @@ -9,6 +9,34 @@ namespace v8 { namespace internal { +#ifdef DEBUG +bool HandleBase::IsDereferenceAllowed(DereferenceCheckMode mode) const { + DCHECK_NOT_NULL(location_); + Object* object = *location_; + if (object->IsSmi()) return true; + HeapObject* heap_object = HeapObject::cast(object); + Heap* heap = heap_object->GetHeap(); + Object** roots_array_start = heap->roots_array_start(); + if (roots_array_start <= location_ && + location_ < roots_array_start + Heap::kStrongRootListLength && + heap->RootCanBeTreatedAsConstant( + static_cast(location_ - roots_array_start))) { + return true; + } + if (!AllowHandleDereference::IsAllowed()) return false; + if (mode == INCLUDE_DEFERRED_CHECK && + !AllowDeferredHandleDereference::IsAllowed()) { + // Accessing cells, maps and internalized strings is safe. + if (heap_object->IsCell()) return true; + if (heap_object->IsMap()) return true; + if (heap_object->IsInternalizedString()) return true; + return !heap->isolate()->IsDeferredHandle(location_); + } + return true; +} +#endif + + int HandleScope::NumberOfHandles(Isolate* isolate) { HandleScopeImplementer* impl = isolate->handle_scope_implementer(); int n = impl->blocks()->length(); @@ -67,7 +95,7 @@ void HandleScope::DeleteExtensions(Isolate* isolate) { void HandleScope::ZapRange(Object** start, Object** end) { DCHECK(end - start <= kHandleBlockSize); for (Object** p = start; p != end; p++) { - *reinterpret_cast(p) = v8::internal::kHandleZapValue; + *reinterpret_cast(p) = kHandleZapValue; } } #endif diff --git a/src/handles.h b/src/handles.h index 162b6d2..841a83b 100644 --- a/src/handles.h +++ b/src/handles.h @@ -10,172 +10,211 @@ namespace v8 { namespace internal { -// A Handle can be converted into a MaybeHandle. Converting a MaybeHandle -// into a Handle requires checking that it does not point to NULL. This -// ensures NULL checks before use. -// Do not use MaybeHandle as argument type. +// Forward declarations. +class DeferredHandles; +class HandleScopeImplementer; -template -class MaybeHandle { - public: - INLINE(MaybeHandle()) : location_(NULL) { } - // Constructor for handling automatic up casting from Handle. - // Ex. Handle can be passed when MaybeHandle is expected. - template MaybeHandle(Handle handle) { -#ifdef DEBUG - T* a = NULL; - S* b = NULL; - a = b; // Fake assignment to enforce type checks. - USE(a); -#endif - this->location_ = reinterpret_cast(handle.location()); - } +// ---------------------------------------------------------------------------- +// Base class for Handle instantiations. Don't use directly. +class HandleBase { + public: + V8_INLINE explicit HandleBase(Object** location) : location_(location) {} + V8_INLINE explicit HandleBase(Object* object, Isolate* isolate); - // Constructor for handling automatic up casting. - // Ex. MaybeHandle can be passed when Handle is expected. - template MaybeHandle(MaybeHandle maybe_handle) { -#ifdef DEBUG - T* a = NULL; - S* b = NULL; - a = b; // Fake assignment to enforce type checks. - USE(a); -#endif - location_ = reinterpret_cast(maybe_handle.location_); + // Check if this handle refers to the exact same object as the other handle. + V8_INLINE bool is_identical_to(const HandleBase that) const { + // Dereferencing deferred handles to check object equality is safe. + SLOW_DCHECK((this->location_ == NULL || + this->IsDereferenceAllowed(NO_DEFERRED_CHECK)) && + (that.location_ == NULL || + that.IsDereferenceAllowed(NO_DEFERRED_CHECK))); + if (this->location_ == that.location_) return true; + if (this->location_ == NULL || that.location_ == NULL) return false; + return *this->location_ == *that.location_; } - INLINE(void Assert() const) { DCHECK(location_ != NULL); } - INLINE(void Check() const) { CHECK(location_ != NULL); } - - INLINE(Handle ToHandleChecked()) const { - Check(); - return Handle(location_); - } + V8_INLINE bool is_null() const { return location_ == nullptr; } - // Convert to a Handle with a type that can be upcasted to. - template - V8_INLINE bool ToHandle(Handle* out) const { - if (location_ == NULL) { - *out = Handle::null(); - return false; - } else { - *out = Handle(location_); - return true; - } + protected: + // Provides the C++ dereference operator. + V8_INLINE Object* operator*() const { + SLOW_DCHECK(IsDereferenceAllowed(INCLUDE_DEFERRED_CHECK)); + return *location_; } - bool is_null() const { return location_ == NULL; } - - template - bool operator==(MaybeHandle that) const { - return this->location_ == that.location_; - } - template - bool operator!=(MaybeHandle that) const { - return !(*this == that); + // Returns the address to where the raw pointer is stored. + V8_INLINE Object** location() const { + SLOW_DCHECK(location_ == NULL || + IsDereferenceAllowed(INCLUDE_DEFERRED_CHECK)); + return location_; } + enum DereferenceCheckMode { INCLUDE_DEFERRED_CHECK, NO_DEFERRED_CHECK }; +#ifdef DEBUG + bool IsDereferenceAllowed(DereferenceCheckMode mode) const; +#else + V8_INLINE + bool IsDereferenceAllowed(DereferenceCheckMode mode) const { return true; } +#endif // DEBUG - protected: - T** location_; - - // MaybeHandles of different classes are allowed to access each - // other's location_. - template friend class MaybeHandle; - template - friend size_t hash_value(MaybeHandle); + Object** location_; }; -template -inline size_t hash_value(MaybeHandle maybe_handle) { - return bit_cast(maybe_handle.location_); -} - // ---------------------------------------------------------------------------- // A Handle provides a reference to an object that survives relocation by // the garbage collector. // Handles are only valid within a HandleScope. // When a handle is created for an object a cell is allocated in the heap. - -template -class Handle { +template +class Handle final : public HandleBase { public: - INLINE(explicit Handle(T** location)) { location_ = location; } - INLINE(explicit Handle(T* obj)); - INLINE(Handle(T* obj, Isolate* isolate)); - - // TODO(yangguo): Values that contain empty handles should be declared as - // MaybeHandle to force validation before being used as handles. - INLINE(Handle()) : location_(NULL) { } + V8_INLINE explicit Handle(T** location = nullptr) + : HandleBase(reinterpret_cast(location)) { + Object* a = nullptr; + T* b = nullptr; + a = b; // Fake assignment to enforce type checks. + USE(a); + } + V8_INLINE explicit Handle(T* object) : Handle(object, object->GetIsolate()) {} + V8_INLINE Handle(T* object, Isolate* isolate) : HandleBase(object, isolate) {} // Constructor for handling automatic up casting. // Ex. Handle can be passed when Handle is expected. - template Handle(Handle handle) { -#ifdef DEBUG - T* a = NULL; - S* b = NULL; + template + V8_INLINE Handle(Handle handle) + : HandleBase(handle) { + T* a = nullptr; + S* b = nullptr; a = b; // Fake assignment to enforce type checks. USE(a); -#endif - location_ = reinterpret_cast(handle.location_); } - INLINE(T* operator->() const) { return operator*(); } - - // Check if this handle refers to the exact same object as the other handle. - INLINE(bool is_identical_to(const Handle other) const); + V8_INLINE T* operator->() const { return operator*(); } // Provides the C++ dereference operator. - INLINE(T* operator*() const); + V8_INLINE T* operator*() const { + return reinterpret_cast(HandleBase::operator*()); + } // Returns the address to where the raw pointer is stored. - INLINE(T** location() const); + V8_INLINE T** location() const { + return reinterpret_cast(HandleBase::location()); + } - template static Handle cast(Handle that) { + template + static const Handle cast(Handle that) { T::cast(*reinterpret_cast(that.location_)); return Handle(reinterpret_cast(that.location_)); } // TODO(yangguo): Values that contain empty handles should be declared as // MaybeHandle to force validation before being used as handles. - static Handle null() { return Handle(); } - bool is_null() const { return location_ == NULL; } - - // Closes the given scope, but lets this handle escape. See - // implementation in api.h. - inline Handle EscapeFrom(v8::EscapableHandleScope* scope); - -#ifdef DEBUG - enum DereferenceCheckMode { INCLUDE_DEFERRED_CHECK, NO_DEFERRED_CHECK }; - - bool IsDereferenceAllowed(DereferenceCheckMode mode) const; -#endif // DEBUG + static const Handle null() { return Handle(); } private: - T** location_; - // Handles of different classes are allowed to access each other's location_. - template friend class Handle; + template + friend class Handle; + // MaybeHandle is allowed to access location_. + template + friend class MaybeHandle; }; +template +V8_INLINE Handle handle(T* object, Isolate* isolate) { + return Handle(object, isolate); +} -// Convenience wrapper. -template -inline Handle handle(T* t, Isolate* isolate) { - return Handle(t, isolate); +template +V8_INLINE Handle handle(T* object) { + return Handle(object); } -// Convenience wrapper. -template -inline Handle handle(T* t) { - return Handle(t, t->GetIsolate()); -} +// ---------------------------------------------------------------------------- +// A Handle can be converted into a MaybeHandle. Converting a MaybeHandle +// into a Handle requires checking that it does not point to NULL. This +// ensures NULL checks before use. +// Do not use MaybeHandle as argument type. +template +class MaybeHandle final { + public: + V8_INLINE MaybeHandle() {} + V8_INLINE ~MaybeHandle() {} + // Constructor for handling automatic up casting from Handle. + // Ex. Handle can be passed when MaybeHandle is expected. + template + V8_INLINE MaybeHandle(Handle handle) + : location_(reinterpret_cast(handle.location_)) { + T* a = nullptr; + S* b = nullptr; + a = b; // Fake assignment to enforce type checks. + USE(a); + } -class DeferredHandles; -class HandleScopeImplementer; + // Constructor for handling automatic up casting. + // Ex. MaybeHandle can be passed when Handle is expected. + template + V8_INLINE MaybeHandle(MaybeHandle maybe_handle) + : location_(reinterpret_cast(maybe_handle.location_)) { + T* a = nullptr; + S* b = nullptr; + a = b; // Fake assignment to enforce type checks. + USE(a); + } + + V8_INLINE void Assert() const { DCHECK_NOT_NULL(location_); } + V8_INLINE void Check() const { CHECK_NOT_NULL(location_); } + + V8_INLINE Handle ToHandleChecked() const { + Check(); + return Handle(location_); + } + + // Convert to a Handle with a type that can be upcasted to. + template + V8_INLINE bool ToHandle(Handle* out) const { + if (location_ == nullptr) { + *out = Handle::null(); + return false; + } else { + *out = Handle(location_); + return true; + } + } + + bool is_null() const { return location_ == nullptr; } + + template + V8_INLINE bool operator==(MaybeHandle that) const { + return this->location_ == that.location_; + } + template + V8_INLINE bool operator!=(MaybeHandle that) const { + return this->location_ != that.location_; + } + + protected: + T** location_ = nullptr; + + // MaybeHandles of different classes are allowed to access each + // other's location_. + template + friend class MaybeHandle; + // Utility functions are allowed to access location_. + template + friend size_t hash_value(MaybeHandle); +}; + +template +V8_INLINE size_t hash_value(MaybeHandle maybe_handle) { + uintptr_t v = bit_cast(maybe_handle.location_); + DCHECK_EQ(0u, v & ((1u << kPointerSizeLog2) - 1)); + return v >> kPointerSizeLog2; +} // A stack-allocated class that governs a number of local handles. @@ -241,7 +280,7 @@ class HandleScope { Object** prev_limit); // Extend the handle scope making room for more handles. - static internal::Object** Extend(Isolate* isolate); + static Object** Extend(Isolate* isolate); #ifdef ENABLE_HANDLE_ZAPPING // Zaps the handles in the half-open interval [start, end). @@ -249,16 +288,13 @@ class HandleScope { #endif friend class v8::HandleScope; - friend class v8::internal::DeferredHandles; - friend class v8::internal::HandleScopeImplementer; - friend class v8::internal::Isolate; + friend class DeferredHandles; + friend class HandleScopeImplementer; + friend class Isolate; }; -class DeferredHandles; - - -class DeferredHandleScope { +class DeferredHandleScope final { public: explicit DeferredHandleScope(Isolate* isolate); // The DeferredHandles object returned stores the Handles created @@ -283,7 +319,7 @@ class DeferredHandleScope { // Seal off the current HandleScope so that new handles can only be created // if a new HandleScope is entered. -class SealHandleScope BASE_EMBEDDED { +class SealHandleScope final { public: #ifndef DEBUG explicit SealHandleScope(Isolate* isolate) {} @@ -298,9 +334,10 @@ class SealHandleScope BASE_EMBEDDED { #endif }; -struct HandleScopeData { - internal::Object** next; - internal::Object** limit; + +struct HandleScopeData final { + Object** next; + Object** limit; int level; void Initialize() { @@ -309,6 +346,7 @@ struct HandleScopeData { } }; -} } // namespace v8::internal +} // namespace internal +} // namespace v8 #endif // V8_HANDLES_H_ diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 3d94b44..6f5ac70 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -1965,7 +1965,7 @@ THREADED_TEST(UndefinedIsNotEnumerable) { v8::Handle