Revision: 3366
Author: [email protected]
Date: Wed Nov 25 12:29:11 2009
Log: Fixed incorrect instruction usage in KeyedLoadIC for byte and word
external array types. Added regression test based on real-world
failing code and verified that it would have caught this error.

Review URL: http://codereview.chromium.org/437052
http://code.google.com/p/v8/source/detail?r=3366

Modified:
  /branches/bleeding_edge/src/ia32/ic-ia32.cc
  /branches/bleeding_edge/src/x64/ic-x64.cc
  /branches/bleeding_edge/test/cctest/test-api.cc

=======================================
--- /branches/bleeding_edge/src/ia32/ic-ia32.cc Tue Nov 24 06:10:06 2009
+++ /branches/bleeding_edge/src/ia32/ic-ia32.cc Wed Nov 25 12:29:11 2009
@@ -388,13 +388,13 @@
        __ movsx_b(eax, Operand(ecx, eax, times_1, 0));
        break;
      case kExternalUnsignedByteArray:
-      __ mov_b(eax, Operand(ecx, eax, times_1, 0));
+      __ movzx_b(eax, Operand(ecx, eax, times_1, 0));
        break;
      case kExternalShortArray:
        __ movsx_w(eax, Operand(ecx, eax, times_2, 0));
        break;
      case kExternalUnsignedShortArray:
-      __ mov_w(eax, Operand(ecx, eax, times_2, 0));
+      __ movzx_w(eax, Operand(ecx, eax, times_2, 0));
        break;
      case kExternalIntArray:
      case kExternalUnsignedIntArray:
=======================================
--- /branches/bleeding_edge/src/x64/ic-x64.cc   Tue Nov 24 06:10:06 2009
+++ /branches/bleeding_edge/src/x64/ic-x64.cc   Wed Nov 25 12:29:11 2009
@@ -398,7 +398,7 @@
        __ movsxbq(rax, Operand(rcx, rax, times_1, 0));
        break;
      case kExternalUnsignedByteArray:
-      __ movb(rax, Operand(rcx, rax, times_1, 0));
+      __ movzxbq(rax, Operand(rcx, rax, times_1, 0));
        break;
      case kExternalShortArray:
        __ movsxwq(rax, Operand(rcx, rax, times_2, 0));
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc     Wed Nov 25 08:46:56 2009
+++ /branches/bleeding_edge/test/cctest/test-api.cc     Wed Nov 25 12:29:11 2009
@@ -8082,6 +8082,66 @@

    result = CompileRun("ext_array[1] = 23;");
    CHECK_EQ(23, result->Int32Value());
+
+  // Test more complex manipulations which cause eax to contain values
+  // that won't be completely overwritten by loads from the arrays.
+  // This catches bugs in the instructions used for the KeyedLoadIC
+  // for byte and word types.
+  {
+    const int kXSize = 300;
+    const int kYSize = 300;
+    const int kLargeElementCount = kXSize * kYSize * 4;
+    ElementType* large_array_data =
+        static_cast<ElementType*>(malloc(kLargeElementCount *  
element_size));
+    i::Handle<ExternalArrayClass> large_array =
+        i::Handle<ExternalArrayClass>::cast(
+            i::Factory::NewExternalArray(kElementCount, array_type,  
array_data));
+    v8::Handle<v8::Object> large_obj = v8::Object::New();
+    // Set the elements to be the external array.
+    large_obj->SetIndexedPropertiesToExternalArrayData(large_array_data,
+                                                       array_type,
+                                                       kLargeElementCount);
+    context->Global()->Set(v8_str("large_array"), large_obj);
+    result = CompileRun("for (var y = 0; y < 300; y++) {"
+                        "  for (var x = 0; x < 300; x++) {"
+                        "    large_array[4 * 300 * y + 4 * x + 0] = 127;"
+                        "    large_array[4 * 300 * y + 4 * x + 1] = 0;"
+                        "    large_array[4 * 300 * y + 4 * x + 2] = 0;"
+                        "    large_array[4 * 300 * y + 4 * x + 3] = 127;"
+                        "  }"
+                        "}"
+                        "var failed = false;"
+                        "var offset = 0;"
+                        "for (var i = 0; i < 300; i++) {"
+                        "  if (large_array[4 * i] != 127 ||"
+                        "      large_array[4 * i + 1] != 0 ||"
+                        "      large_array[4 * i + 2] != 0 ||"
+                        "      large_array[4 * i + 3] != 127) {"
+                        "    failed = true;"
+                        "  }"
+                        "}"
+                        "offset = 150 * 300 * 4;"
+                        "for (var i = 0; i < 300; i++) {"
+                        "  if (large_array[offset + 4 * i] != 127 ||"
+                        "      large_array[offset + 4 * i + 1] != 0 ||"
+                        "      large_array[offset + 4 * i + 2] != 0 ||"
+                        "      large_array[offset + 4 * i + 3] != 127) {"
+                        "    failed = true;"
+                        "  }"
+                        "}"
+                        "offset = 298 * 300 * 4;"
+                        "for (var i = 0; i < 300; i++) {"
+                        "  if (large_array[offset + 4 * i] != 127 ||"
+                        "      large_array[offset + 4 * i + 1] != 0 ||"
+                        "      large_array[offset + 4 * i + 2] != 0 ||"
+                        "      large_array[offset + 4 * i + 3] != 127) {"
+                        "    failed = true;"
+                        "  }"
+                        "}"
+                        "!failed;");
+    CHECK_EQ(true, result->BooleanValue());
+    free(large_array_data);
+  }

    free(array_data);
  }

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

Reply via email to