Make JSObjects with both indexed interceptors and indexed accessors work safely.

Review URL: http://codereview.chromium.org/118499

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@2138 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/objects.cc

@@ -5203,27 +5203,6 @@ bool JSObject::HasElementWithReceiver(JSObject* receiver, uint32_t index) {
 }
 
 
-Object* JSObject::SetElementPostInterceptor(uint32_t index, Object* value) {
-  if (HasFastElements()) return SetFastElement(index, value);
-
-  // Dictionary case.
-  ASSERT(!HasFastElements());
-
-  FixedArray* elms = FixedArray::cast(elements());
-  Object* result = Dictionary::cast(elms)->AtNumberPut(index, value);
-  if (result->IsFailure()) return result;
-  if (elms != FixedArray::cast(result)) {
-    set_elements(FixedArray::cast(result));
-  }
-
-  if (IsJSArray()) {
-    return JSArray::cast(this)->JSArrayUpdateLengthFromIndex(index, value);
-  }
-
-  return value;
-}
-
-
 Object* JSObject::SetElementWithInterceptor(uint32_t index, Object* value) {
   // Make sure that the top context does not change when doing
   // callbacks or interceptor calls.
@@ -5250,7 +5229,7 @@ Object* JSObject::SetElementWithInterceptor(uint32_t index, Object* value) {
     if (!result.IsEmpty()) return *value_handle;
   }
   Object* raw_result =
-      this_handle->SetElementPostInterceptor(index, *value_handle);
+      this_handle->SetElementWithoutInterceptor(index, *value_handle);
   RETURN_IF_SCHEDULED_EXCEPTION();
   return raw_result;
 }
@@ -5332,6 +5311,11 @@ Object* JSObject::SetElement(uint32_t index, Object* value) {
     return SetElementWithInterceptor(index, value);
   }
 
+  return SetElementWithoutInterceptor(index, value);
+}
+
+
+Object* JSObject::SetElementWithoutInterceptor(uint32_t index, Object* value) {
   // Fast case.
   if (HasFastElements()) return SetFastElement(index, value);
 
@@ -5438,7 +5422,21 @@ Object* JSObject::GetElementPostInterceptor(JSObject* receiver,
     Dictionary* dictionary = element_dictionary();
     int entry = dictionary->FindNumberEntry(index);
     if (entry != -1) {
-      return dictionary->ValueAt(entry);
+      Object* element = dictionary->ValueAt(entry);
+      PropertyDetails details = dictionary->DetailsAt(entry);
+      if (details.type() == CALLBACKS) {
+        // Only accessors allowed as elements.
+        FixedArray* structure = FixedArray::cast(element);
+        Object* getter = structure->get(kGetterIndex);
+        if (getter->IsJSFunction()) {
+          return GetPropertyWithDefinedGetter(receiver,
+                                              JSFunction::cast(getter));
+        } else {
+          // Getter is not a function.
+          return Heap::undefined_value();
+        }
+      }
+      return element;
     }
   }
 

src/objects.h

@@ -1531,7 +1531,7 @@ class JSObject: public HeapObject {
 
  private:
   Object* SetElementWithInterceptor(uint32_t index, Object* value);
-  Object* SetElementPostInterceptor(uint32_t index, Object* value);
+  Object* SetElementWithoutInterceptor(uint32_t index, Object* value);
 
   Object* GetElementPostInterceptor(JSObject* receiver, uint32_t index);
 

test/cctest/test-api.cc

@@ -551,6 +551,7 @@ THREADED_TEST(UsingExternalString) {
     CHECK(isymbol->IsSymbol());
   }
   i::Heap::CollectAllGarbage();
+  i::Heap::CollectAllGarbage();
 }
 
 
@@ -568,6 +569,7 @@ THREADED_TEST(UsingExternalAsciiString) {
     CHECK(isymbol->IsSymbol());
   }
   i::Heap::CollectAllGarbage();
+  i::Heap::CollectAllGarbage();
 }
 
 
@@ -2281,7 +2283,7 @@ static v8::Handle<Value> XPropertyGetter(Local<String> property,
 }
 
 
-THREADED_TEST(NamedInterceporPropertyRead) {
+THREADED_TEST(NamedInterceptorPropertyRead) {
   v8::HandleScope scope;
   Local<ObjectTemplate> templ = ObjectTemplate::New();
   templ->SetNamedPropertyHandler(XPropertyGetter);
@@ -2294,6 +2296,58 @@ THREADED_TEST(NamedInterceporPropertyRead) {
   }
 }
 
+
+static v8::Handle<Value> IndexedPropertyGetter(uint32_t index,
+                                               const AccessorInfo& info) {
+  ApiTestFuzzer::Fuzz();
+  if (index == 37) {
+    return v8::Handle<Value>(v8_num(625));
+  }
+  return v8::Handle<Value>();
+}
+
+
+static v8::Handle<Value> IndexedPropertySetter(uint32_t index,
+                                               Local<Value> value,
+                                               const AccessorInfo& info) {
+  ApiTestFuzzer::Fuzz();
+  if (index == 39) {
+    return value;
+  }
+  return v8::Handle<Value>();
+}
+
+
+THREADED_TEST(IndexedInterceptorWithIndexedAccessor) {
+  v8::HandleScope scope;
+  Local<ObjectTemplate> templ = ObjectTemplate::New();
+  templ->SetIndexedPropertyHandler(IndexedPropertyGetter,
+                                   IndexedPropertySetter);
+  LocalContext context;
+  context->Global()->Set(v8_str("obj"), templ->NewInstance());
+  Local<Script> getter_script = Script::Compile(v8_str(
+      "obj.__defineGetter__(\"3\", function(){return 5;});obj[3];"));
+  Local<Script> setter_script = Script::Compile(v8_str(
+      "obj.__defineSetter__(\"17\", function(val){this.foo = val;});"
+      "obj[17] = 23;"
+      "obj.foo;"));
+  Local<Script> interceptor_setter_script = Script::Compile(v8_str(
+      "obj.__defineSetter__(\"39\", function(val){this.foo = \"hit\";});"
+      "obj[39] = 47;"
+      "obj.foo;"));  // This setter should not run, due to the interceptor.
+  Local<Script> interceptor_getter_script = Script::Compile(v8_str(
+      "obj[37];"));
+  Local<Value> result = getter_script->Run();
+  CHECK_EQ(v8_num(5), result);
+  result = setter_script->Run();
+  CHECK_EQ(v8_num(23), result);
+  result = interceptor_setter_script->Run();
+  CHECK_EQ(v8_num(23), result);
+  result = interceptor_getter_script->Run();
+  CHECK_EQ(v8_num(625), result);
+}
+
+
 THREADED_TEST(MultiContexts) {
   v8::HandleScope scope;
   v8::Handle<ObjectTemplate> templ = ObjectTemplate::New();