Fix intermittent crashes caused by unexpected GCs in

HasLocalProperty (bug introduced in r1882 et al.)
Review URL: http://codereview.chromium.org/115106

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

src/objects.cc

@@ -5161,7 +5161,8 @@ Object* JSObject::GetHiddenProperties(bool create_if_needed) {
   }
 
   // Only attempt to find the hidden properties in the local object and not
-  // in the prototype chain.
+  // 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.

src/objects.h

@@ -1262,6 +1262,7 @@ class JSObject: public HeapObject {
     return GetPropertyAttribute(name) != ABSENT;
   }
 
+  // Can cause a GC if it hits an interceptor.
   bool HasLocalProperty(String* name) {
     return GetLocalPropertyAttribute(name) != ABSENT;
   }

src/runtime.cc

@@ -2832,19 +2832,37 @@ static Object* Runtime_DeleteProperty(Arguments args) {
 }
 
 
-static Object* HasLocalPropertyImplementation(Object* obj, String* key) {
+static Object* HasLocalPropertyImplementation(Handle<JSObject> object,
+                                              Handle<String> key) {
+  if (object->HasLocalProperty(*key)) return Heap::true_value();
+  // Handle hidden prototypes.  If there's a hidden prototype above this thing
+  // then we have to check it for properties, because they are supposed to
+  // look like they are on this object.
+  Handle<Object> proto(object->GetPrototype());
+  if (proto->IsJSObject() &&
+      Handle<JSObject>::cast(proto)->map()->is_hidden_prototype()) {
+    return HasLocalPropertyImplementation(Handle<JSObject>::cast(proto), key);
+  }
+  return Heap::false_value();
+}
+
+
+static Object* Runtime_HasLocalProperty(Arguments args) {
+  NoHandleAllocation ha;
+  ASSERT(args.length() == 2);
+  CONVERT_CHECKED(String, key, args[1]);
+
+  Object* obj = args[0];
   // Only JS objects can have properties.
   if (obj->IsJSObject()) {
     JSObject* object = JSObject::cast(obj);
-    if (object->HasLocalProperty(key)) return Heap::true_value();
-    // Handle hidden prototypes.  If there's a hidden prototype above this thing
-    // then we have to check it for properties, because they are supposed to
-    // look like they are on this object.
-    Object* proto = object->GetPrototype();
-    if (proto->IsJSObject() &&
-        JSObject::cast(proto)->map()->is_hidden_prototype()) {
-      return HasLocalPropertyImplementation(object->GetPrototype(), key);
-    }
+    // Fast case - no interceptors.
+    if (object->HasRealNamedProperty(key)) return Heap::true_value();
+    // Slow case.  Either it's not there or we have an interceptor.  We should
+    // have handles for this kind of deal.
+    HandleScope scope;
+    return HasLocalPropertyImplementation(Handle<JSObject>(object),
+                                          Handle<String>(key));
   } else if (obj->IsString()) {
     // Well, there is one exception:  Handle [] on strings.
     uint32_t index;
@@ -2858,15 +2876,6 @@ static Object* HasLocalPropertyImplementation(Object* obj, String* key) {
 }
 
 
-static Object* Runtime_HasLocalProperty(Arguments args) {
-  NoHandleAllocation ha;
-  ASSERT(args.length() == 2);
-  CONVERT_CHECKED(String, key, args[1]);
-
-  return HasLocalPropertyImplementation(args[0], key);
-}
-
-
 static Object* Runtime_HasProperty(Arguments args) {
   NoHandleAllocation na;
   ASSERT(args.length() == 2);

test/cctest/test-api.cc

@@ -4739,6 +4739,44 @@ THREADED_TEST(InterceptorHasOwnProperty) {
 }
 
 
+static v8::Handle<Value> InterceptorHasOwnPropertyGetterGC(
+    Local<String> name,
+    const AccessorInfo& info) {
+  ApiTestFuzzer::Fuzz();
+  i::Heap::CollectAllGarbage();
+  return v8::Handle<Value>();
+}
+
+
+THREADED_TEST(InterceptorHasOwnPropertyCausingGC) {
+  v8::HandleScope scope;
+  LocalContext context;
+  Local<v8::FunctionTemplate> fun_templ = v8::FunctionTemplate::New();
+  Local<v8::ObjectTemplate> instance_templ = fun_templ->InstanceTemplate();
+  instance_templ->SetNamedPropertyHandler(InterceptorHasOwnPropertyGetterGC);
+  Local<Function> function = fun_templ->GetFunction();
+  context->Global()->Set(v8_str("constructor"), function);
+  // Let's first make some stuff so we can be sure to get a good GC.
+  CompileRun(
+      "function makestr(size) {"
+      "  switch (size) {"
+      "    case 1: return 'f';"
+      "    case 2: return 'fo';"
+      "    case 3: return 'foo';"
+      "  }"
+      "  return makestr(size >> 1) + makestr((size + 1) >> 1);"
+      "}"
+      "var x = makestr(12345);"
+      "x = makestr(31415);"
+      "x = makestr(23456);");
+  v8::Handle<Value> value = CompileRun(
+      "var o = new constructor();"
+      "o.__proto__ = new String(x);"
+      "o.hasOwnProperty('ostehaps');");
+  CHECK_EQ(false, value->BooleanValue());
+}
+
+
 static v8::Handle<Value> InterceptorLoadICGetter(Local<String> name,
                                                  const AccessorInfo& info) {
   ApiTestFuzzer::Fuzz();