Change calls to undefined property setters to not throw (fixes issue 1355).

We currently throw when there is only a getter defined on the
property, but this should only be the case in strict mode.
Review URL: http://codereview.chromium.org/7064027

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

diff --git a/src/objects.cc b/src/objects.cc
index b5d3643..1c4b7a8 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -1771,7 +1771,8 @@ MaybeObject* JSObject::SetProperty(String* name,
 MaybeObject* JSObject::SetPropertyWithCallback(Object* structure,
                                                String* name,
                                                Object* value,
-                                               JSObject* holder) {
+                                               JSObject* holder,
+                                               StrictModeFlag strict_mode) {
   Isolate* isolate = GetIsolate();
   HandleScope scope(isolate);
 
@@ -1819,6 +1820,9 @@ MaybeObject* JSObject::SetPropertyWithCallback(Object* structure,
     if (setter->IsJSFunction()) {
      return SetPropertyWithDefinedSetter(JSFunction::cast(setter), value);
     } else {
+      if (strict_mode == kNonStrictMode) {
+        return value;
+      }
       Handle<String> key(name);
       Handle<Object> holder_handle(holder, isolate);
       Handle<Object> args[2] = { key, holder_handle };
@@ -1876,9 +1880,11 @@ void JSObject::LookupCallbackSetterInPrototypes(String* name,
 }
 
 
-MaybeObject* JSObject::SetElementWithCallbackSetterInPrototypes(uint32_t index,
-                                                                Object* value,
-                                                                bool* found) {
+MaybeObject* JSObject::SetElementWithCallbackSetterInPrototypes(
+    uint32_t index,
+    Object* value,
+    bool* found,
+    StrictModeFlag strict_mode) {
   Heap* heap = GetHeap();
   for (Object* pt = GetPrototype();
        pt != heap->null_value();
@@ -1892,8 +1898,11 @@ MaybeObject* JSObject::SetElementWithCallbackSetterInPrototypes(uint32_t index,
       PropertyDetails details = dictionary->DetailsAt(entry);
       if (details.type() == CALLBACKS) {
         *found = true;
-        return SetElementWithCallback(
-            dictionary->ValueAt(entry), index, value, JSObject::cast(pt));
+        return SetElementWithCallback(dictionary->ValueAt(entry),
+                                      index,
+                                      value,
+                                      JSObject::cast(pt),
+                                      strict_mode);
       }
     }
   }
@@ -2074,10 +2083,12 @@ void JSObject::LookupRealNamedPropertyInPrototypes(String* name,
 
 
 // We only need to deal with CALLBACKS and INTERCEPTORS
-MaybeObject* JSObject::SetPropertyWithFailedAccessCheck(LookupResult* result,
-                                                        String* name,
-                                                        Object* value,
-                                                        bool check_prototype) {
+MaybeObject* JSObject::SetPropertyWithFailedAccessCheck(
+    LookupResult* result,
+    String* name,
+    Object* value,
+    bool check_prototype,
+    StrictModeFlag strict_mode) {
   if (check_prototype && !result->IsProperty()) {
     LookupCallbackSetterInPrototypes(name, result);
   }
@@ -2093,7 +2104,8 @@ MaybeObject* JSObject::SetPropertyWithFailedAccessCheck(LookupResult* result,
               return SetPropertyWithCallback(result->GetCallbackObject(),
                                              name,
                                              value,
-                                             result->holder());
+                                             result->holder(),
+                                             strict_mode);
             }
           }
           break;
@@ -2104,8 +2116,11 @@ MaybeObject* JSObject::SetPropertyWithFailedAccessCheck(LookupResult* result,
           LookupResult r;
           LookupRealNamedProperty(name, &r);
           if (r.IsProperty()) {
-            return SetPropertyWithFailedAccessCheck(&r, name, value,
-                                                    check_prototype);
+            return SetPropertyWithFailedAccessCheck(&r,
+                                                    name,
+                                                    value,
+                                                    check_prototype,
+                                                    strict_mode);
           }
           break;
         }
@@ -2149,7 +2164,11 @@ MaybeObject* JSObject::SetProperty(LookupResult* result,
   // Check access rights if needed.
   if (IsAccessCheckNeeded()
       && !heap->isolate()->MayNamedAccess(this, name, v8::ACCESS_SET)) {
-    return SetPropertyWithFailedAccessCheck(result, name, value, true);
+    return SetPropertyWithFailedAccessCheck(result,
+                                            name,
+                                            value,
+                                            true,
+                                            strict_mode);
   }
 
   if (IsJSGlobalProxy()) {
@@ -2169,7 +2188,8 @@ MaybeObject* JSObject::SetProperty(LookupResult* result,
       return SetPropertyWithCallback(accessor_result.GetCallbackObject(),
                                      name,
                                      value,
-                                     accessor_result.holder());
+                                     accessor_result.holder(),
+                                     strict_mode);
     }
   }
   if (!result->IsFound()) {
@@ -2213,7 +2233,8 @@ MaybeObject* JSObject::SetProperty(LookupResult* result,
       return SetPropertyWithCallback(result->GetCallbackObject(),
                                      name,
                                      value,
-                                     result->holder());
+                                     result->holder(),
+                                     strict_mode);
     case INTERCEPTOR:
       return SetPropertyWithInterceptor(name, value, attributes, strict_mode);
     case CONSTANT_TRANSITION: {
@@ -2266,7 +2287,11 @@ MaybeObject* JSObject::SetLocalPropertyIgnoreAttributes(
   if (IsAccessCheckNeeded()) {
     Heap* heap = GetHeap();
     if (!heap->isolate()->MayNamedAccess(this, name, v8::ACCESS_SET)) {
-      return SetPropertyWithFailedAccessCheck(&result, name, value, false);
+      return SetPropertyWithFailedAccessCheck(&result,
+                                              name,
+                                              value,
+                                              false,
+                                              kNonStrictMode);
     }
   }
 
@@ -7406,7 +7431,8 @@ MaybeObject* JSObject::GetElementWithCallback(Object* receiver,
 MaybeObject* JSObject::SetElementWithCallback(Object* structure,
                                               uint32_t index,
                                               Object* value,
-                                              JSObject* holder) {
+                                              JSObject* holder,
+                                              StrictModeFlag strict_mode) {
   Isolate* isolate = GetIsolate();
   HandleScope scope(isolate);
 
@@ -7445,10 +7471,13 @@ MaybeObject* JSObject::SetElementWithCallback(Object* structure,
   }
 
   if (structure->IsFixedArray()) {
-    Object* setter = FixedArray::cast(structure)->get(kSetterIndex);
+    Handle<Object> setter(FixedArray::cast(structure)->get(kSetterIndex));
     if (setter->IsJSFunction()) {
-     return SetPropertyWithDefinedSetter(JSFunction::cast(setter), value);
+     return SetPropertyWithDefinedSetter(JSFunction::cast(*setter), value);
     } else {
+      if (strict_mode == kNonStrictMode) {
+        return value;
+      }
       Handle<Object> holder_handle(holder, isolate);
       Handle<Object> key(isolate->factory()->NewNumberFromUint(index));
       Handle<Object> args[2] = { key, holder_handle };
@@ -7482,8 +7511,10 @@ MaybeObject* JSObject::SetFastElement(uint32_t index,
   if (check_prototype &&
       (index >= elms_length || elms->get(index)->IsTheHole())) {
     bool found;
-    MaybeObject* result =
-        SetElementWithCallbackSetterInPrototypes(index, value, &found);
+    MaybeObject* result = SetElementWithCallbackSetterInPrototypes(index,
+                                                                   value,
+                                                                   &found,
+                                                                   strict_mode);
     if (found) return result;
   }
 
@@ -7627,7 +7658,11 @@ MaybeObject* JSObject::SetElementWithoutInterceptor(uint32_t index,
         Object* element = dictionary->ValueAt(entry);
         PropertyDetails details = dictionary->DetailsAt(entry);
         if (details.type() == CALLBACKS) {
-          return SetElementWithCallback(element, index, value, this);
+          return SetElementWithCallback(element,
+                                        index,
+                                        value,
+                                        this,
+                                        strict_mode);
         } else {
           dictionary->UpdateMaxNumberKey(index);
           // If put fails instrict mode, throw exception.
@@ -7647,7 +7682,10 @@ MaybeObject* JSObject::SetElementWithoutInterceptor(uint32_t index,
           bool found;
           MaybeObject* result =
               // Strict mode not needed. No-setter case already handled.
-              SetElementWithCallbackSetterInPrototypes(index, value, &found);
+              SetElementWithCallbackSetterInPrototypes(index,
+                                                       value,
+                                                       &found,
+                                                       strict_mode);
           if (found) return result;
         }
         // When we set the is_extensible flag to false we always force

diff --git a/src/objects.h b/src/objects.h
index 92a854a..77f25d5 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -1425,11 +1425,14 @@ class JSObject: public HeapObject {
       LookupResult* result,
       String* name,
       Object* value,
-      bool check_prototype);
-  MUST_USE_RESULT MaybeObject* SetPropertyWithCallback(Object* structure,
-                                                       String* name,
-                                                       Object* value,
-                                                       JSObject* holder);
+      bool check_prototype,
+      StrictModeFlag strict_mode);
+  MUST_USE_RESULT MaybeObject* SetPropertyWithCallback(
+      Object* structure,
+      String* name,
+      Object* value,
+      JSObject* holder,
+      StrictModeFlag strict_mode);
   MUST_USE_RESULT MaybeObject* SetPropertyWithDefinedSetter(JSFunction* setter,
                                                             Object* value);
   MUST_USE_RESULT MaybeObject* SetPropertyWithInterceptor(
@@ -1656,7 +1659,7 @@ class JSObject: public HeapObject {
   void LookupRealNamedPropertyInPrototypes(String* name, LookupResult* result);
   void LookupCallbackSetterInPrototypes(String* name, LookupResult* result);
   MUST_USE_RESULT MaybeObject* SetElementWithCallbackSetterInPrototypes(
-      uint32_t index, Object* value, bool* found);
+      uint32_t index, Object* value, bool* found, StrictModeFlag strict_mode);
   void LookupCallback(String* name, LookupResult* result);
 
   // Returns the number of properties on this object filtering out properties
@@ -1868,7 +1871,8 @@ class JSObject: public HeapObject {
   MaybeObject* SetElementWithCallback(Object* structure,
                                       uint32_t index,
                                       Object* value,
-                                      JSObject* holder);
+                                      JSObject* holder,
+                                      StrictModeFlag strict_mode);
   MUST_USE_RESULT MaybeObject* SetElementWithInterceptor(
       uint32_t index,
       Object* value,

diff --git a/test/mjsunit/getter-in-prototype.js b/test/mjsunit/getter-in-prototype.js
index 5563123..01a3473 100644 (file)
--- a/test/mjsunit/getter-in-prototype.js
+++ b/test/mjsunit/getter-in-prototype.js
@@ -25,8 +25,9 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-// Test that exceptions are thrown when setting properties on object
-// that have only a getter in a prototype object.
+// Test that exceptions are not thrown when setting properties on object
+// that have only a getter in a prototype object, except when we are in strict
+// mode where exceptsions should be thrown.
 
 var o = {};
 var p = {};
@@ -34,25 +35,44 @@ p.__defineGetter__('x', function(){});
 p.__defineGetter__(0, function(){});
 o.__proto__ = p;
 
-assertThrows("o.x = 42");
-assertThrows("o[0] = 42");
+assertDoesNotThrow("o.x = 42");
+assertDoesNotThrow("o[0] = 42");
+
+assertThrows(function() { 'use strict'; o.x = 42; });
+assertThrows(function() { 'use strict'; o[0] = 42; });
 
 function f() {
   with(o) {
     x = 42;
   }
 }
-assertThrows("f()");
+
+assertDoesNotThrow(f);
 
 __proto__ = p;
 function g() {
   eval('1');
   x = 42;
 }
-assertThrows("g()");
+
+function g_strict() {
+  'use strict';
+  eval('1');
+  x = 42;
+}
+
+assertDoesNotThrow(g);
+assertThrows(g_strict);
 
 __proto__ = p;
 function g2() {
   this[0] = 42;
 }
-assertThrows("g2()");
+
+function g2_strict() {
+  'use strict';
+  this[0] = 42;
+}
+
+assertDoesNotThrow(g2);
+assertThrows(g2_strict);

diff --git a/test/mjsunit/indexed-accessors.js b/test/mjsunit/indexed-accessors.js
index 1634857..b69695a 100644 (file)
--- a/test/mjsunit/indexed-accessors.js
+++ b/test/mjsunit/indexed-accessors.js
@@ -81,10 +81,11 @@ testArray();
 expected[0] = 111;
 testArray();
 
-// Using a setter where only a getter is defined throws an exception.
+// Using a setter where only a getter is defined does not throw an exception,
+// unless we are in strict mode.
 var q = {};
 q.__defineGetter__('0', function() { return 42; });
-assertThrows('q[0] = 7');
+assertDoesNotThrow('q[0] = 7');
 
 // Using a getter where only a setter is defined returns undefined.
 var q1 = {};

diff --git a/test/mjsunit/regress/regress-1355.js b/test/mjsunit/regress/regress-1355.js
new file mode 100644 (file)
index 0000000..de9364a
--- /dev/null
+++ b/test/mjsunit/regress/regress-1355.js
@@ -0,0 +1,44 @@
+// Copyright 2011 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Test that an exception is not thrown when trying to set a value for
+// a property that has only a defined getter, except when in strict mode.
+
+var foo = Object.defineProperty({}, "bar", {
+ get: function () {
+      return 10;
+    }
+  });
+
+assertDoesNotThrow("foo.bar = 20");
+
+function shouldThrow() {
+  'use strict';
+  foo.bar = 20;
+}
+
+assertThrows("shouldThrow()");