Reviewers: danno,

Description:
Merged r11492 into 3.9 branch.

Fix register clobbering in LoadIC for interceptors.

[email protected]
BUG=chromium:125988
TEST=cctest/test-api/Regress125988


Please review this at https://chromiumcodereview.appspot.com/10417004/

SVN Base: https://v8.googlecode.com/svn/branches/3.9

Affected files:
  M src/arm/stub-cache-arm.cc
  M src/ia32/stub-cache-ia32.cc
  M src/version.cc
  M src/x64/stub-cache-x64.cc
  M test/cctest/test-api.cc


Index: src/arm/stub-cache-arm.cc
diff --git a/src/arm/stub-cache-arm.cc b/src/arm/stub-cache-arm.cc
index 98a3cdcbcd125dd173f6fe6ed47c904222bd2333..d514b607aea212e5dd166ef136df2d96c9145047 100644
--- a/src/arm/stub-cache-arm.cc
+++ b/src/arm/stub-cache-arm.cc
@@ -1266,12 +1266,19 @@ void StubCompiler::GenerateLoadInterceptor(Handle<JSObject> object,
                                           name, miss);
     ASSERT(holder_reg.is(receiver) || holder_reg.is(scratch1));

+ // Preserve the receiver register explicitly whenever it is different from + // the holder and it is needed should the interceptor return without any + // result. The CALLBACKS case needs the receiver to be passed into C++ code,
+    // the FIELD case might cause a miss during the prototype check.
+ bool must_perfrom_prototype_check = *interceptor_holder != lookup->holder();
+    bool must_preserve_receiver_reg = !receiver.is(holder_reg) &&
+        (lookup->type() == CALLBACKS || must_perfrom_prototype_check);
+
     // Save necessary data before invoking an interceptor.
     // Requires a frame to make GC aware of pushed pointers.
     {
       FrameScope frame_scope(masm(), StackFrame::INTERNAL);
-      if (lookup->type() == CALLBACKS && !receiver.is(holder_reg)) {
-        // CALLBACKS case needs a receiver to be passed into C++ callback.
+      if (must_preserve_receiver_reg) {
         __ Push(receiver, holder_reg, name_reg);
       } else {
         __ Push(holder_reg, name_reg);
@@ -1296,14 +1303,14 @@ void StubCompiler::GenerateLoadInterceptor(Handle<JSObject> object,
       __ bind(&interceptor_failed);
       __ pop(name_reg);
       __ pop(holder_reg);
-      if (lookup->type() == CALLBACKS && !receiver.is(holder_reg)) {
+      if (must_preserve_receiver_reg) {
         __ pop(receiver);
       }
       // Leave the internal frame.
     }
     // Check that the maps from interceptor's holder to lookup's holder
     // haven't changed.  And load lookup's holder into |holder| register.
-    if (*interceptor_holder != lookup->holder()) {
+    if (must_perfrom_prototype_check) {
       holder_reg = CheckPrototypes(interceptor_holder,
                                    holder_reg,
                                    Handle<JSObject>(lookup->holder()),
Index: src/ia32/stub-cache-ia32.cc
diff --git a/src/ia32/stub-cache-ia32.cc b/src/ia32/stub-cache-ia32.cc
index 5a5afdc7474b50944d0d9183c5b2df0f9d41fff0..eb86b2f523c916c74ab3bd496f2b5e63724b1aa6 100644
--- a/src/ia32/stub-cache-ia32.cc
+++ b/src/ia32/stub-cache-ia32.cc
@@ -1124,13 +1124,20 @@ void StubCompiler::GenerateLoadInterceptor(Handle<JSObject> object,
                                           name, miss);
     ASSERT(holder_reg.is(receiver) || holder_reg.is(scratch1));

+ // Preserve the receiver register explicitly whenever it is different from + // the holder and it is needed should the interceptor return without any + // result. The CALLBACKS case needs the receiver to be passed into C++ code,
+    // the FIELD case might cause a miss during the prototype check.
+ bool must_perfrom_prototype_check = *interceptor_holder != lookup->holder();
+    bool must_preserve_receiver_reg = !receiver.is(holder_reg) &&
+        (lookup->type() == CALLBACKS || must_perfrom_prototype_check);
+
     // Save necessary data before invoking an interceptor.
     // Requires a frame to make GC aware of pushed pointers.
     {
       FrameScope frame_scope(masm(), StackFrame::INTERNAL);

-      if (lookup->type() == CALLBACKS && !receiver.is(holder_reg)) {
-        // CALLBACKS case needs a receiver to be passed into C++ callback.
+      if (must_preserve_receiver_reg) {
         __ push(receiver);
       }
       __ push(holder_reg);
@@ -1153,10 +1160,17 @@ void StubCompiler::GenerateLoadInterceptor(Handle<JSObject> object,
       frame_scope.GenerateLeaveFrame();
       __ ret(0);

+      // Clobber registers when generating debug-code to provoke errors.
       __ bind(&interceptor_failed);
+      if (FLAG_debug_code) {
+        __ mov(receiver, Immediate(BitCast<int32_t>(kZapValue)));
+        __ mov(holder_reg, Immediate(BitCast<int32_t>(kZapValue)));
+        __ mov(name_reg, Immediate(BitCast<int32_t>(kZapValue)));
+      }
+
       __ pop(name_reg);
       __ pop(holder_reg);
-      if (lookup->type() == CALLBACKS && !receiver.is(holder_reg)) {
+      if (must_preserve_receiver_reg) {
         __ pop(receiver);
       }

@@ -1165,7 +1179,7 @@ void StubCompiler::GenerateLoadInterceptor(Handle<JSObject> object,

     // Check that the maps from interceptor's holder to lookup's holder
     // haven't changed.  And load lookup's holder into holder_reg.
-    if (*interceptor_holder != lookup->holder()) {
+    if (must_perfrom_prototype_check) {
       holder_reg = CheckPrototypes(interceptor_holder,
                                    holder_reg,
                                    Handle<JSObject>(lookup->holder()),
Index: src/version.cc
diff --git a/src/version.cc b/src/version.cc
index 37c2ec262b9ce1327d6a2f14398bda1e87e961ad..f7c09ed0711109dbe413d4de070bec7c30c7872f 100644
--- a/src/version.cc
+++ b/src/version.cc
@@ -35,7 +35,7 @@
 #define MAJOR_VERSION     3
 #define MINOR_VERSION     9
 #define BUILD_NUMBER      24
-#define PATCH_LEVEL       25
+#define PATCH_LEVEL       26
 // Use 1 for candidates and 0 otherwise.
 // (Boolean macro values are not supported by all preprocessors.)
 #define IS_CANDIDATE_VERSION 0
Index: src/x64/stub-cache-x64.cc
diff --git a/src/x64/stub-cache-x64.cc b/src/x64/stub-cache-x64.cc
index 7ce45688a4db15a1523609087ca6249f48ec0d1d..96ff499354d51627d92c383911d07b64d1289ea9 100644
--- a/src/x64/stub-cache-x64.cc
+++ b/src/x64/stub-cache-x64.cc
@@ -1104,13 +1104,20 @@ void StubCompiler::GenerateLoadInterceptor(Handle<JSObject> object,
                                           name, miss);
     ASSERT(holder_reg.is(receiver) || holder_reg.is(scratch1));

+ // Preserve the receiver register explicitly whenever it is different from + // the holder and it is needed should the interceptor return without any + // result. The CALLBACKS case needs the receiver to be passed into C++ code,
+    // the FIELD case might cause a miss during the prototype check.
+ bool must_perfrom_prototype_check = *interceptor_holder != lookup->holder();
+    bool must_preserve_receiver_reg = !receiver.is(holder_reg) &&
+        (lookup->type() == CALLBACKS || must_perfrom_prototype_check);
+
     // Save necessary data before invoking an interceptor.
     // Requires a frame to make GC aware of pushed pointers.
     {
       FrameScope frame_scope(masm(), StackFrame::INTERNAL);

-      if (lookup->type() == CALLBACKS && !receiver.is(holder_reg)) {
-        // CALLBACKS case needs a receiver to be passed into C++ callback.
+      if (must_preserve_receiver_reg) {
         __ push(receiver);
       }
       __ push(holder_reg);
@@ -1136,7 +1143,7 @@ void StubCompiler::GenerateLoadInterceptor(Handle<JSObject> object,
       __ bind(&interceptor_failed);
       __ pop(name_reg);
       __ pop(holder_reg);
-      if (lookup->type() == CALLBACKS && !receiver.is(holder_reg)) {
+      if (must_preserve_receiver_reg) {
         __ pop(receiver);
       }

@@ -1145,7 +1152,7 @@ void StubCompiler::GenerateLoadInterceptor(Handle<JSObject> object,

     // Check that the maps from interceptor's holder to lookup's holder
     // haven't changed.  And load lookup's holder into |holder| register.
-    if (*interceptor_holder != lookup->holder()) {
+    if (must_perfrom_prototype_check) {
       holder_reg = CheckPrototypes(interceptor_holder,
                                    holder_reg,
                                    Handle<JSObject>(lookup->holder()),
Index: test/cctest/test-api.cc
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index b1a23c1ef7078a24ab166aa1254c76d46ef66838..f4ab9ad0df9b4e1f4260871a1642e435dfc44103 100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -16135,6 +16135,30 @@ THREADED_TEST(Regress93759) {
 }


+THREADED_TEST(Regress125988) {
+  v8::HandleScope scope;
+  Handle<FunctionTemplate> intercept = FunctionTemplate::New();
+ AddInterceptor(intercept, EmptyInterceptorGetter, EmptyInterceptorSetter);
+  LocalContext env;
+  env->Global()->Set(v8_str("Intercept"), intercept->GetFunction());
+  CompileRun("var a = new Object();"
+             "var b = new Intercept();"
+             "var c = new Object();"
+             "c.__proto__ = b;"
+             "b.__proto__ = a;"
+             "a.x = 23;"
+             "for (var i = 0; i < 3; i++) c.x;");
+  ExpectBoolean("c.hasOwnProperty('x')", false);
+  ExpectInt32("c.x", 23);
+  CompileRun("a.y = 42;"
+             "for (var i = 0; i < 3; i++) c.x;");
+  ExpectBoolean("c.hasOwnProperty('x')", false);
+  ExpectInt32("c.x", 23);
+  ExpectBoolean("c.hasOwnProperty('y')", false);
+  ExpectInt32("c.y", 42);
+}
+
+
 static void TestReceiver(Local<Value> expected_result,
                          Local<Value> expected_receiver,
                          const char* code) {


--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to