Merge r182058 - Objects with numeric properties intermittently get a …

…phantom 'length' property https://bugs.webkit.org/show_bug.cgi?id=142792 Reviewed by Csaba Osztrogonác. Source/JavaScriptCore: Fixed a > (greater than) that should be a >> (right shift) in the code that disassembles test and branch instructions. This function is used for linking tbz/tbnz branches between two seperately JIT'ed sections of code. Sometime we'd create a bogus tbz instruction in the failure case checks in the GetById array length stub created for "obj.length" access. If the failure case code address was at a negative offset from the stub, we'd look for bit 1 being set when we should have been looking for bit 0. * assembler/ARM64Assembler.h: (JSC::ARM64Assembler::disassembleTestAndBranchImmediate): LayoutTests: New regression test. * js/regress-142792-expected.txt: Added. * js/regress-142792.html: Added. * js/script-tests/regress-142792.js: Added. (isArrayLike): (filter):
WebKit · Apr 13, 2015 · f8611a8 · f8611a8
1 parent 195f59d
commit f8611a8
Show file tree

Hide file tree

Showing 6 changed files with 105 additions and 1 deletion.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,18 @@
+2015-03-27  Michael Saboff  <msaboff@apple.com>
+
+        Objects with numeric properties intermittently get a phantom 'length' property
+        https://bugs.webkit.org/show_bug.cgi?id=142792
+
+        Reviewed by Csaba Osztrogonác.
+
+        New regression test.
+
+        * js/regress-142792-expected.txt: Added.
+        * js/regress-142792.html: Added.
+        * js/script-tests/regress-142792.js: Added.
+        (isArrayLike):
+        (filter):
+
 2015-03-24  Zhuo Li  <zachli@apple.com>
 
         Scripts running in isolated world should not subject to a page's CSP about 'eval'.

diff --git a/LayoutTests/js/regress-142792-expected.txt b/LayoutTests/js/regress-142792-expected.txt
@@ -0,0 +1,10 @@
+Verify that objects with numeric named properties don't set length like an array.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Correct number of iterated keys: 3
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/js/regress-142792.html b/LayoutTests/js/regress-142792.html
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script src="script-tests/regress-142792.js"></script>
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/js/script-tests/regress-142792.js b/LayoutTests/js/script-tests/regress-142792.js
@@ -0,0 +1,52 @@
+description("Verify that objects with numeric named properties don't set length like an array.");
+
+var numOfIterations = 10000;
+var count = 0;
+var obj = {
+    1: 'foo',
+    8: 'bar',
+    50: 'baz'
+};
+
+var expectedCount = Object.keys(obj).length;
+
+function isArrayLike(collection) {
+    var length = collection && collection.length;
+
+    return typeof length == 'number';
+}
+
+function filter(obj, callback, context) {
+    var results = [];
+    var i, length;
+
+    if (isArrayLike(obj)) {
+        for (i = 0, length = obj.length; i < length; i++) {
+            var value = obj[i];
+            if (callback(value))
+                results.push(value);
+        }
+    } else {
+        for (var key in obj) {
+            var value = obj[key];
+            if (callback(value))
+                results.push(value);
+        }
+    }
+
+    return results;
+}
+
+for (var i = 0; i < numOfIterations; i++) {
+    filter([], function() { return true; });
+}
+
+filter(obj, function() { 
+    count++;
+    return true;
+});
+
+if (count !== expectedCount)
+    testFailed("Incorrect number of iterated keys: " + count + ", expected: " + expectedCount);
+else
+    testPassed("Correct number of iterated keys: " + count);
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,20 @@
+2015-03-27  Michael Saboff  <msaboff@apple.com>
+
+        Objects with numeric properties intermittently get a phantom 'length' property
+        https://bugs.webkit.org/show_bug.cgi?id=142792
+
+        Reviewed by Csaba Osztrogonác.
+
+        Fixed a > (greater than) that should be a >> (right shift) in the code that disassembles
+        test and branch instructions.  This function is used for linking tbz/tbnz branches between
+        two seperately JIT'ed sections of code.  Sometime we'd create a bogus tbz instruction in
+        the failure case checks in the GetById array length stub created for "obj.length" access.
+        If the failure case code address was at a negative offset from the stub, we'd look for bit 1
+        being set when we should have been looking for bit 0.
+
+        * assembler/ARM64Assembler.h:
+        (JSC::ARM64Assembler::disassembleTestAndBranchImmediate):
+
 2015-03-25  Mark Lam  <mark.lam@apple.com>
 
         REGRESSION(169139): LLINT intermittently fails JSC testapi tests.

diff --git a/Source/JavaScriptCore/assembler/ARM64Assembler.h b/Source/JavaScriptCore/assembler/ARM64Assembler.h
@@ -3237,7 +3237,7 @@ class ARM64Assembler {
         int insn = *static_cast<int*>(address);
         op = (insn >> 24) & 0x1;
         imm14 = (insn << 13) >> 18;
-        bitNumber = static_cast<unsigned>((((insn >> 26) & 0x20)) | ((insn > 19) & 0x1f));
+        bitNumber = static_cast<unsigned>((((insn >> 26) & 0x20)) | ((insn >> 19) & 0x1f));
         rt = static_cast<RegisterID>(insn & 0x1f);
         return (insn & 0x7e000000) == 0x36000000;