From a28143c77cf3b60531fbbb78e186e5536bae8e02 Mon Sep 17 00:00:00 2001
From: "ricow@chromium.org"
 <ricow@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Thu, 4 Feb 2010 19:43:56 +0000
Subject: [PATCH] Added extra tests to the DefineOrRedefineAccessorProperty and
 DefineOrRedefineDataProperty to avoid invalid input.

Added tests to object-define-property.js to test that it does not crash
on invalid input.


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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3798 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/runtime.cc                         | 14 ++++----
 test/mjsunit/object-define-property.js | 47 +++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/src/runtime.cc b/src/runtime.cc
index fb690c980d..908414a934 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -2898,7 +2898,7 @@ static Object* Runtime_DefineOrRedefineAccessorProperty(Arguments args) {
   CONVERT_CHECKED(Smi, flag_attr, args[4]);
   int unchecked = flag_attr->value();
   RUNTIME_ASSERT((unchecked & ~(READ_ONLY | DONT_ENUM | DONT_DELETE)) == 0);
-
+  RUNTIME_ASSERT(!obj->IsNull());
   LookupResult result;
   obj->LocalLookupRealNamedProperty(name, &result);
 
@@ -2917,18 +2917,16 @@ static Object* Runtime_DefineOrRedefineAccessorProperty(Arguments args) {
 static Object* Runtime_DefineOrRedefineDataProperty(Arguments args) {
   ASSERT(args.length() == 4);
   HandleScope scope;
-  Handle<Object> obj = args.at<Object>(0);
-  Handle<Object> name = args.at<Object>(1);
+  CONVERT_ARG_CHECKED(JSObject, js_object, 0);
+  CONVERT_ARG_CHECKED(String, name, 1);
   Handle<Object> obj_value = args.at<Object>(2);
-  Handle<JSObject> js_object = Handle<JSObject>::cast(obj);
-  Handle<String> key_string = Handle<String>::cast(name);
-
+   
   CONVERT_CHECKED(Smi, flag, args[3]);
   int unchecked = flag->value();
   RUNTIME_ASSERT((unchecked & ~(READ_ONLY | DONT_ENUM | DONT_DELETE)) == 0);
 
   LookupResult result;
-  js_object->LocalLookupRealNamedProperty(*key_string, &result);
+  js_object->LocalLookupRealNamedProperty(*name, &result);
 
   PropertyAttributes attr = static_cast<PropertyAttributes>(unchecked);
 
@@ -2942,7 +2940,7 @@ static Object* Runtime_DefineOrRedefineDataProperty(Arguments args) {
     PropertyDetails details = PropertyDetails(attr, NORMAL);
     // New attributes - normalize to avoid writing to instance descriptor
     js_object->NormalizeProperties(KEEP_INOBJECT_PROPERTIES, 0);
-    return js_object->SetNormalizedProperty(*key_string, *obj_value, details);
+    return js_object->SetNormalizedProperty(*name, *obj_value, details);
   }
 
   return Runtime::SetObjectProperty(js_object, name, obj_value, attr);
diff --git a/test/mjsunit/object-define-property.js b/test/mjsunit/object-define-property.js
index 089fb7e44b..43b1c7f09d 100644
--- a/test/mjsunit/object-define-property.js
+++ b/test/mjsunit/object-define-property.js
@@ -27,7 +27,7 @@
 
 // Tests the object.defineProperty method - ES 15.2.3.6
 
-
+// Flags: --allow-natives-syntax
 
 // Check that an exception is thrown when null is passed as object.
 try {
@@ -451,4 +451,49 @@ try {
 }
 
 
+// Test runtime calls to DefineOrRedefineDataProperty and
+// DefineOrRedefineAccessorProperty - make sure we don't 
+// crash
+try {
+  %DefineOrRedefineAccessorProperty(0, 0, 0, 0, 0);
+} catch (e) {
+  assertTrue(/illegal access/.test(e));
+}
 
+try {
+  %DefineOrRedefineDataProperty(0, 0, 0, 0);
+} catch (e) {
+  assertTrue(/illegal access/.test(e));
+}
+
+try {
+  %DefineOrRedefineDataProperty(null, null, null, null);
+} catch (e) {
+  assertTrue(/illegal access/.test(e));
+}
+
+try {
+  %DefineOrRedefineAccessorProperty(null, null, null, null, null);
+} catch (e) {
+  assertTrue(/illegal access/.test(e));
+}
+
+try {
+  %DefineOrRedefineDataProperty({}, null, null, null);
+} catch (e) {
+  assertTrue(/illegal access/.test(e));
+}
+
+// Defining properties null should fail even when we have
+// other allowed values
+try {
+  %DefineOrRedefineAccessorProperty(null, 'foo', 0, func, 0);
+} catch (e) {
+  assertTrue(/illegal access/.test(e));
+}
+
+try {
+  %DefineOrRedefineDataProperty(null, 'foo', 0, 0);
+} catch (e) {
+  assertTrue(/illegal access/.test(e));
+}