From c135f2de07094fa6a9e0d7bfd7ebadd40f201eb9 Mon Sep 17 00:00:00 2001
From: "kbr@chromium.org"
<kbr@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Wed, 25 Nov 2009 20:29:11 +0000
Subject: [PATCH] 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
git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3366 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
src/ia32/ic-ia32.cc | 4 +--
src/x64/ic-x64.cc | 2 +-
test/cctest/test-api.cc | 60 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 63 insertions(+), 3 deletions(-)
diff --git a/src/ia32/ic-ia32.cc b/src/ia32/ic-ia32.cc
index d209fbead40..6988fe09f67 100644
--- a/src/ia32/ic-ia32.cc
+++ b/src/ia32/ic-ia32.cc
@@ -388,13 +388,13 @@ void KeyedLoadIC::GenerateExternalArray(MacroAssembler* masm,
__ 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:
diff --git a/src/x64/ic-x64.cc b/src/x64/ic-x64.cc
index b5cc26a5932..b241573a380 100644
--- a/src/x64/ic-x64.cc
+++ b/src/x64/ic-x64.cc
@@ -398,7 +398,7 @@ void KeyedLoadIC::GenerateExternalArray(MacroAssembler* masm,
__ 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));
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 9b6a9cd4b15..c650dce9e82 100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -8083,6 +8083,66 @@ static void ExternalArrayTestHelper(v8::ExternalArrayType array_type,
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);
}
--
2.18.1