Fix issue 6264 with a test case.

The problem is that Disable/EnableAccessCheck on an object may chnage its constructor's behavior
if object's map is the same as constructor's initial map. By copying maps, the constructor's
initial map is not changed.

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

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

diff --git a/src/runtime.cc b/src/runtime.cc
index 3269217..f75aecf 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -343,8 +343,16 @@ static Object* Runtime_GetTemplateField(Arguments args) {
 static Object* Runtime_DisableAccessChecks(Arguments args) {
   ASSERT(args.length() == 1);
   CONVERT_CHECKED(HeapObject, object, args[0]);
-  bool needs_access_checks = object->map()->is_access_check_needed();
-  object->map()->set_is_access_check_needed(false);
+  Map* old_map = object->map();
+  bool needs_access_checks = old_map->is_access_check_needed();
+  if (needs_access_checks) {
+    // Copy map so it won't interfere constructor's initial map.
+    Object* new_map = old_map->CopyDropTransitions();
+    if (new_map->IsFailure()) return new_map;
+
+    Map::cast(new_map)->set_is_access_check_needed(false);
+    object->set_map(Map::cast(new_map));
+  }
   return needs_access_checks ? Heap::true_value() : Heap::false_value();
 }
 
@@ -352,7 +360,15 @@ static Object* Runtime_DisableAccessChecks(Arguments args) {
 static Object* Runtime_EnableAccessChecks(Arguments args) {
   ASSERT(args.length() == 1);
   CONVERT_CHECKED(HeapObject, object, args[0]);
-  object->map()->set_is_access_check_needed(true);
+  Map* old_map = object->map();
+  if (!old_map->is_access_check_needed()) {
+    // Copy map so it won't interfere constructor's initial map.
+    Object* new_map = old_map->CopyDropTransitions();
+    if (new_map->IsFailure()) return new_map;
+
+    Map::cast(new_map)->set_is_access_check_needed(true);
+    object->set_map(Map::cast(new_map));
+  }
   return Heap::undefined_value();
 }
 

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 926272e..6daa0d1 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -5448,6 +5448,50 @@ THREADED_TEST(DisableAccessChecksWhileConfiguring) {
   CHECK(value->BooleanValue());
 }
 
+static bool NamedGetAccessBlocker(Local<v8::Object> obj,
+                                  Local<Value> name,
+                                  v8::AccessType type,
+                                  Local<Value> data) {
+  return false;
+}
+
+
+static bool IndexedGetAccessBlocker(Local<v8::Object> obj,
+                                    uint32_t key,
+                                    v8::AccessType type,
+                                    Local<Value> data) {
+  return false;
+}
+
+
+
+THREADED_TEST(AccessChecksReenabledCorrectly) {
+  v8::HandleScope scope;
+  LocalContext context;
+  Local<ObjectTemplate> templ = ObjectTemplate::New();
+  templ->SetAccessCheckCallbacks(NamedGetAccessBlocker,
+                                 IndexedGetAccessBlocker);
+  templ->Set(v8_str("a"), v8_str("a"));
+  // Add more than 8 (see kMaxFastProperties) properties
+  // so that the constructor will force copying map.
+  char buf[5];
+  for (int i = 0; i < 999; i++) {
+    sprintf_s(buf, 5, "x%3d", i);
+    templ->Set(v8_str(buf), v8::Number::New(i));
+  }
+
+  Local<v8::Object> instance_1 = templ->NewInstance();
+  context->Global()->Set(v8_str("obj_1"), instance_1);
+
+  Local<Value> value_1 = CompileRun("obj_1.a");
+  CHECK(value_1->IsUndefined());
+
+  Local<v8::Object> instance_2 = templ->NewInstance();
+  context->Global()->Set(v8_str("obj_2"), instance_2);
+
+  Local<Value> value_2 = CompileRun("obj_2.a");
+  CHECK(value_2->IsUndefined());
+}
 
 // This tests that access check information remains on the global
 // object template when creating contexts.