Commit 1f858b3f authored by Mythri A's avatar Mythri A Committed by Commit Bot

[handles] Fix is_identical_to to correctly work with Smis

HandleBase::is_identical_to() is_identical_to in handle base is used to
do the exact comparison by just comparing the locations. When the
locations are different the values are compared. For Smis it
compares 64 bits which might lead to incorrect behaviour. Smis loaded as
a TaggedField has the root address added. It is expexted that we don't
use higher order bits on Smi. Hence, is_identical_to shouldn't use these
bits when comparing Smis. This cl fixes it by comparing the objects
created from the given location. That takes care of correctly comparing
the required bits.

Change-Id: I574dfbea4c1fffc7a9e3a6a10ad7631d40c518ce
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2461743
Commit-Queue: Mythri Alle <mythria@chromium.org>
Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70422}
parent da3c7318
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "src/execution/local-isolate.h" #include "src/execution/local-isolate.h"
#include "src/handles/handles.h" #include "src/handles/handles.h"
#include "src/handles/local-handles-inl.h" #include "src/handles/local-handles-inl.h"
#include "src/objects/objects.h"
#include "src/sanitizer/msan.h" #include "src/sanitizer/msan.h"
namespace v8 { namespace v8 {
...@@ -25,8 +26,15 @@ HandleBase::HandleBase(Address object, LocalIsolate* isolate) ...@@ -25,8 +26,15 @@ HandleBase::HandleBase(Address object, LocalIsolate* isolate)
HandleBase::HandleBase(Address object, LocalHeap* local_heap) HandleBase::HandleBase(Address object, LocalHeap* local_heap)
: location_(LocalHandleScope::GetHandle(local_heap, object)) {} : location_(LocalHandleScope::GetHandle(local_heap, object)) {}
// Allocate a new handle for the object, do not canonicalize. bool HandleBase::is_identical_to(const HandleBase that) const {
SLOW_DCHECK((this->location_ == nullptr || this->IsDereferenceAllowed()) &&
(that.location_ == nullptr || that.IsDereferenceAllowed()));
if (this->location_ == that.location_) return true;
if (this->location_ == nullptr || that.location_ == nullptr) return false;
return Object(*this->location_) == Object(*that.location_);
}
// Allocate a new handle for the object, do not canonicalize.
template <typename T> template <typename T>
Handle<T> Handle<T>::New(T object, Isolate* isolate) { Handle<T> Handle<T>::New(T object, Isolate* isolate) {
return Handle(HandleScope::CreateHandle(isolate, object.ptr())); return Handle(HandleScope::CreateHandle(isolate, object.ptr()));
......
...@@ -44,14 +44,7 @@ class HandleBase { ...@@ -44,14 +44,7 @@ class HandleBase {
V8_INLINE explicit HandleBase(Address object, LocalHeap* local_heap); V8_INLINE explicit HandleBase(Address object, LocalHeap* local_heap);
// Check if this handle refers to the exact same object as the other handle. // Check if this handle refers to the exact same object as the other handle.
V8_INLINE bool is_identical_to(const HandleBase that) const { V8_INLINE bool is_identical_to(const HandleBase that) const;
SLOW_DCHECK((this->location_ == nullptr || this->IsDereferenceAllowed()) &&
(that.location_ == nullptr || that.IsDereferenceAllowed()));
if (this->location_ == that.location_) return true;
if (this->location_ == nullptr || that.location_ == nullptr) return false;
return *this->location_ == *that.location_;
}
V8_INLINE bool is_null() const { return location_ == nullptr; } V8_INLINE bool is_null() const { return location_ == nullptr; }
// Returns the raw address where this handle is stored. This should only be // Returns the raw address where this handle is stored. This should only be
......
...@@ -53,6 +53,15 @@ MaybeObjectHandle MaybeObjectHandle::Weak(Object object, Isolate* isolate) { ...@@ -53,6 +53,15 @@ MaybeObjectHandle MaybeObjectHandle::Weak(Object object, Isolate* isolate) {
return MaybeObjectHandle(object, HeapObjectReferenceType::WEAK, isolate); return MaybeObjectHandle(object, HeapObjectReferenceType::WEAK, isolate);
} }
bool MaybeObjectHandle::is_identical_to(const MaybeObjectHandle& other) const {
Handle<Object> this_handle;
Handle<Object> other_handle;
return reference_type_ == other.reference_type_ &&
handle_.ToHandle(&this_handle) ==
other.handle_.ToHandle(&other_handle) &&
this_handle.is_identical_to(other_handle);
}
MaybeObject MaybeObjectHandle::operator*() const { MaybeObject MaybeObjectHandle::operator*() const {
if (reference_type_ == HeapObjectReferenceType::WEAK) { if (reference_type_ == HeapObjectReferenceType::WEAK) {
return HeapObjectReference::Weak(*handle_.ToHandleChecked()); return HeapObjectReference::Weak(*handle_.ToHandleChecked());
......
...@@ -100,15 +100,7 @@ class MaybeObjectHandle { ...@@ -100,15 +100,7 @@ class MaybeObjectHandle {
inline MaybeObject operator->() const; inline MaybeObject operator->() const;
inline Handle<Object> object() const; inline Handle<Object> object() const;
bool is_identical_to(const MaybeObjectHandle& other) const { inline bool is_identical_to(const MaybeObjectHandle& other) const;
Handle<Object> this_handle;
Handle<Object> other_handle;
return reference_type_ == other.reference_type_ &&
handle_.ToHandle(&this_handle) ==
other.handle_.ToHandle(&other_handle) &&
this_handle.is_identical_to(other_handle);
}
bool is_null() const { return handle_.is_null(); } bool is_null() const { return handle_.is_null(); }
private: private:
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment