From 83fc4205f69cd59503d16baf7c4e59349d05dd0a Mon Sep 17 00:00:00 2001
From: "svenpanne@chromium.org"
 <svenpanne@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Fri, 3 Aug 2012 09:45:08 +0000
Subject: [PATCH] Fixed compound/count operations with getter-only accessor
 properties.

The underlying problem is that for compound/count operations we use the *load*
type feedback for storing, too. For normal properties this doesn't matter, but
for accessor properties we should better use the *store* type feedback, which
would be available, too. This consistent feedback usage could be guaranteed if
we removed the heavy copy-n-paste in the crankshaft code generation for
compound/count operations and assignments/property loads.

To be on the safe side, we postpone this refactoring and do a quick and easily
mergeable fix.

BUG=140083
TEST=mjsunit/regress/regress-crbug-140083.js

Review URL: https://chromiumcodereview.appspot.com/10828146

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@12252 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/hydrogen.cc                              |  8 +++--
 test/mjsunit/regress/regress-crbug-140083.js | 44 ++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 2 deletions(-)
 create mode 100644 test/mjsunit/regress/regress-crbug-140083.js

diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 7d3f1f3..513e983 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -5422,7 +5422,9 @@ void HGraphBuilder::HandleCompoundAssignment(Assignment* expr) {
       } else {
         Handle<AccessorPair> accessors;
         Handle<JSObject> holder;
-        if (LookupAccessorPair(map, name, &accessors, &holder)) {
+        // Because we re-use the load type feedback, there might be no setter.
+        if (LookupAccessorPair(map, name, &accessors, &holder) &&
+            accessors->setter()->IsJSFunction()) {
           store = BuildCallSetter(object, instr, map, accessors, holder);
         } else {
           CHECK_ALIVE(store = BuildStoreNamedMonomorphic(object,
@@ -7883,7 +7885,9 @@ void HGraphBuilder::VisitCountOperation(CountOperation* expr) {
       } else {
         Handle<AccessorPair> accessors;
         Handle<JSObject> holder;
-        if (LookupAccessorPair(map, name, &accessors, &holder)) {
+        // Because we re-use the load type feedback, there might be no setter.
+        if (LookupAccessorPair(map, name, &accessors, &holder) &&
+            accessors->setter()->IsJSFunction()) {
           store = BuildCallSetter(object, after, map, accessors, holder);
         } else {
           CHECK_ALIVE(store = BuildStoreNamedMonomorphic(object,
diff --git a/test/mjsunit/regress/regress-crbug-140083.js b/test/mjsunit/regress/regress-crbug-140083.js
new file mode 100644
index 0000000..e38192c
--- /dev/null
+++ b/test/mjsunit/regress/regress-crbug-140083.js
@@ -0,0 +1,44 @@
+// Copyright 2012 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.
+
+// Flags: --allow-natives-syntax
+
+// Test that the absence of a setter in a compound/count operation works.
+
+Object.defineProperty(Object.prototype, "foo",
+                      { get: function() { return 123; } });
+
+function bar(o) {
+  o.foo += 42;
+  o.foo++;
+}
+
+var baz = {};
+bar(baz);
+bar(baz);
+%OptimizeFunctionOnNextCall(bar)
+bar(baz);
-- 
2.7.4