From 6c39aefd4f8b04adc10564fc46c2d856e1272709 Mon Sep 17 00:00:00 2001 From: "iposva@chromium.org" Date: Tue, 12 May 2009 22:07:10 +0000 Subject: [PATCH] Fix for issue 339: - Move GetHiddenProperties functionality from object.cc to handle.cc to be more robust in the presence of GC in the middle of the function. Review URL: http://codereview.chromium.org/115267 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@1924 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/handles.cc | 30 +++++++++++++++++++++++++++++- src/handles.h | 3 +++ src/objects.cc | 36 ------------------------------------ src/objects.h | 5 ----- test/cctest/test-api.cc | 32 ++++++++++++++++++++++++++++++++ 5 files changed, 64 insertions(+), 42 deletions(-) diff --git a/src/handles.cc b/src/handles.cc index 773483d..2d9efe6 100644 --- a/src/handles.cc +++ b/src/handles.cc @@ -273,7 +273,35 @@ Handle GetPrototype(Handle obj) { Handle GetHiddenProperties(Handle obj, bool create_if_needed) { - CALL_HEAP_FUNCTION(obj->GetHiddenProperties(create_if_needed), Object); + Handle key = Factory::hidden_symbol(); + + if (obj->HasFastProperties()) { + // If the object has fast properties, check whether the first slot + // in the descriptor array matches the hidden symbol. Since the + // hidden symbols hash code is zero (and no other string has hash + // code zero) it will always occupy the first entry if present. + DescriptorArray* descriptors = obj->map()->instance_descriptors(); + DescriptorReader r(descriptors); + if (!r.eos() && (r.GetKey() == *key) && r.IsProperty()) { + ASSERT(r.type() == FIELD); + return Handle(obj->FastPropertyAt(r.GetFieldIndex())); + } + } + + // Only attempt to find the hidden properties in the local object and not + // in the prototype chain. Note that HasLocalProperty() can cause a GC in + // the general case in the presence of interceptors. + if (!obj->HasLocalProperty(*key)) { + // Hidden properties object not found. Allocate a new hidden properties + // object if requested. Otherwise return the undefined value. + if (create_if_needed) { + Handle hidden_obj = Factory::NewJSObject(Top::object_function()); + return SetProperty(obj, key, hidden_obj, DONT_ENUM); + } else { + return Factory::undefined_value(); + } + } + return GetProperty(obj, key); } diff --git a/src/handles.h b/src/handles.h index 652d6c7..cf5ed56 100644 --- a/src/handles.h +++ b/src/handles.h @@ -228,6 +228,9 @@ Handle GetPropertyWithInterceptor(Handle receiver, Handle GetPrototype(Handle obj); +// Return the object's hidden properties object. If the object has no hidden +// properties and create_if_needed is true, then a new hidden property object +// will be allocated. Otherwise the Heap::undefined_value is returned. Handle GetHiddenProperties(Handle obj, bool create_if_needed); Handle DeleteElement(Handle obj, uint32_t index); diff --git a/src/objects.cc b/src/objects.cc index 28a397e..0a8ee83 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -5145,42 +5145,6 @@ bool JSObject::HasLocalElement(uint32_t index) { } -Object* JSObject::GetHiddenProperties(bool create_if_needed) { - String* key = Heap::hidden_symbol(); - if (this->HasFastProperties()) { - // If the object has fast properties, check whether the first slot - // in the descriptor array matches the hidden symbol. Since the - // hidden symbols hash code is zero (and no other string has hash - // code zero) it will always occupy the first entry if present. - DescriptorArray* descriptors = this->map()->instance_descriptors(); - DescriptorReader r(descriptors); - if (!r.eos() && (r.GetKey() == key) && r.IsProperty()) { - ASSERT(r.type() == FIELD); - return FastPropertyAt(r.GetFieldIndex()); - } - } - - // Only attempt to find the hidden properties in the local object and not - // in the prototype chain. Note that HasLocalProperty() can cause a GC in - // the general case, but in this case we know it won't hit an interceptor. - if (!this->HasLocalProperty(key)) { - // Hidden properties object not found. Allocate a new hidden properties - // object if requested. Otherwise return the undefined value. - if (create_if_needed) { - Object* obj = Heap::AllocateJSObject( - Top::context()->global_context()->object_function()); - if (obj->IsFailure()) { - return obj; - } - return this->SetProperty(key, obj, DONT_ENUM); - } else { - return Heap::undefined_value(); - } - } - return this->GetProperty(key); -} - - bool JSObject::HasElementWithReceiver(JSObject* receiver, uint32_t index) { // Check access rights if needed. if (IsAccessCheckNeeded() && diff --git a/src/objects.h b/src/objects.h index 3e132ff..2619c89 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1286,11 +1286,6 @@ class JSObject: public HeapObject { // Return the object's prototype (might be Heap::null_value()). inline Object* GetPrototype(); - // Return the object's hidden properties object. If the object has no hidden - // properties and create_if_needed is true, then a new hidden property object - // will be allocated. Otherwise the Heap::undefined_value is returned. - Object* GetHiddenProperties(bool create_if_needed); - // Tells whether the index'th element is present. inline bool HasElement(uint32_t index); bool HasElementWithReceiver(JSObject* receiver, uint32_t index); diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 4b55b6b..7c834c7 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -1327,6 +1327,38 @@ THREADED_TEST(HiddenProperties) { } +static v8::Handle InterceptorForHiddenProperties( + Local name, const AccessorInfo& info) { + // Make sure objects move. + bool saved_always_compact = i::FLAG_always_compact; + if (!i::FLAG_never_compact) { + i::FLAG_always_compact = true; + } + // The whole goal of this interceptor is to cause a GC during local property + // lookup. + i::Heap::CollectAllGarbage(); + i::FLAG_always_compact = saved_always_compact; + return v8::Handle(); +} + + +THREADED_TEST(HiddenPropertiesWithInterceptors) { + v8::HandleScope scope; + LocalContext context; + + v8::Local key = v8_str("api-test::hidden-key"); + + // Associate an interceptor with an object and start setting hidden values. + Local fun_templ = v8::FunctionTemplate::New(); + Local instance_templ = fun_templ->InstanceTemplate(); + instance_templ->SetNamedPropertyHandler(InterceptorForHiddenProperties); + Local function = fun_templ->GetFunction(); + Local obj = function->NewInstance(); + CHECK(obj->SetHiddenValue(key, v8::Integer::New(2302))); + CHECK_EQ(2302, obj->GetHiddenValue(key)->Int32Value()); +} + + THREADED_TEST(External) { v8::HandleScope scope; int x = 3; -- 2.7.4