Commit 067496d0 authored by fpizlo@apple.com's avatar fpizlo@apple.com
Browse files

Incorrect TypedArray#set behavior

https://bugs.webkit.org/show_bug.cgi?id=83818

Source/JavaScriptCore: 

Reviewed by Oliver Hunt and Mark Hahnenberg.
        
This was so much fun! typedArray.set() is like a memmove on steroids, and I'm
not smart enough to figure out optimal versions for *all* of the cases. But I
did come up with optimal implementations for most of the cases, and I wrote
spec-literal code (i.e. copy via a transfer buffer) for the cases I'm not smart
enough to write optimal code for.

* runtime/JSArrayBufferView.h:
(JSC::JSArrayBufferView::hasArrayBuffer):
* runtime/JSArrayBufferViewInlines.h:
(JSC::JSArrayBufferView::buffer):
(JSC::JSArrayBufferView::existingBufferInButterfly):
(JSC::JSArrayBufferView::neuter):
(JSC::JSArrayBufferView::byteOffset):
* runtime/JSGenericTypedArrayView.h:
* runtime/JSGenericTypedArrayViewInlines.h:
(JSC::::setWithSpecificType):
(JSC::::set):
(JSC::::existingBuffer):

LayoutTests: 

Reviewed by Oliver Hunt and Mark Hahnenberg.
        
Made it possible for shouldBe() to compare typed arrays to each other and to any array-like
object.
        
Added a bunch of tests for different kinds of overlapping typedArray.set()'s.
        
For sanity, also added the reduced test case from the bug. Interestingly, though, that test
case already passed on trunk - probably by luck (we had incidentally changed the default
copy direction from one that happened to not work to one that happened to be fine, but only
for this test).

* fast/js/jsc-test-list:
* fast/js/resources/js-test-pre.js:
(isTypedArray):
(isResultCorrect):
(stringify):
(shouldBe):
* fast/js/script-tests/typed-array-copy.js: Added.
* fast/js/script-tests/typedarray-set-destination-smaller-than-source.js: Added.
* fast/js/script-tests/typedarray-set-overlapping-elements-of-same-size.js: Added.
* fast/js/script-tests/typedarray-set-same-type-memmove.js: Added.
(arraysEqual):
* fast/js/script-tests/typedarray-set-source-smaller-than-destination.js: Added.
* fast/js/typed-array-copy-expected.txt: Added.
* fast/js/typed-array-copy.html: Added.
* fast/js/typedarray-set-destination-smaller-than-source-expected.txt: Added.
* fast/js/typedarray-set-destination-smaller-than-source.html: Added.
* fast/js/typedarray-set-overlapping-elements-of-same-size-expected.txt: Added.
* fast/js/typedarray-set-overlapping-elements-of-same-size.html: Added.
* fast/js/typedarray-set-same-type-memmove-expected.txt: Added.
* fast/js/typedarray-set-same-type-memmove.html: Added.
* fast/js/typedarray-set-source-smaller-than-destination-expected.txt: Added.
* fast/js/typedarray-set-source-smaller-than-destination.html: Added.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@154518 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 3dc3498b
2013-08-23 Filip Pizlo <fpizlo@apple.com>
Incorrect TypedArray#set behavior
https://bugs.webkit.org/show_bug.cgi?id=83818
Reviewed by Oliver Hunt and Mark Hahnenberg.
Made it possible for shouldBe() to compare typed arrays to each other and to any array-like
object.
Added a bunch of tests for different kinds of overlapping typedArray.set()'s.
For sanity, also added the reduced test case from the bug. Interestingly, though, that test
case already passed on trunk - probably by luck (we had incidentally changed the default
copy direction from one that happened to not work to one that happened to be fine, but only
for this test).
* fast/js/jsc-test-list:
* fast/js/resources/js-test-pre.js:
(isTypedArray):
(isResultCorrect):
(stringify):
(shouldBe):
* fast/js/script-tests/typed-array-copy.js: Added.
* fast/js/script-tests/typedarray-set-destination-smaller-than-source.js: Added.
* fast/js/script-tests/typedarray-set-overlapping-elements-of-same-size.js: Added.
* fast/js/script-tests/typedarray-set-same-type-memmove.js: Added.
(arraysEqual):
* fast/js/script-tests/typedarray-set-source-smaller-than-destination.js: Added.
* fast/js/typed-array-copy-expected.txt: Added.
* fast/js/typed-array-copy.html: Added.
* fast/js/typedarray-set-destination-smaller-than-source-expected.txt: Added.
* fast/js/typedarray-set-destination-smaller-than-source.html: Added.
* fast/js/typedarray-set-overlapping-elements-of-same-size-expected.txt: Added.
* fast/js/typedarray-set-overlapping-elements-of-same-size.html: Added.
* fast/js/typedarray-set-same-type-memmove-expected.txt: Added.
* fast/js/typedarray-set-same-type-memmove.html: Added.
* fast/js/typedarray-set-source-smaller-than-destination-expected.txt: Added.
* fast/js/typedarray-set-source-smaller-than-destination.html: Added.
2013-08-23 Andre Moreira Magalhaes <andre.magalhaes@collabora.co.uk>
 
LayoutTests/http/tests/media/video-throttled-load.cgi issue on range support
......@@ -437,6 +437,11 @@ fast/js/toString-prefix-postfix-preserve-parens
fast/js/toString-recursion
fast/js/try-catch-try-try-catch-try-finally-return-catch-finally
fast/js/try-try-return-finally-finally
fast/js/typed-array-copy
fast/js/typedarray-set-destination-smaller-than-source
fast/js/typedarray-set-overlapping-elements-of-same-size
fast/js/typedarray-set-same-type-memmove
fast/js/typedarray-set-source-smaller-than-destination
fast/js/typeof-codegen-crash
fast/js/typeof-constant-string
fast/js/unexpected-constant-crash
......
......@@ -133,6 +133,19 @@ function isMinusZero(n)
return n === 0 && 1/n < 0;
}
function isTypedArray(array)
{
return array instanceof Int8Array
|| array instanceof Int16Array
|| array instanceof Int32Array
|| array instanceof Uint8Array
|| array instanceof Uint8ClampedArray
|| array instanceof Uint16Array
|| array instanceof Uint32Array
|| array instanceof Float32Array
|| array instanceof Float64Array;
}
function isResultCorrect(_actual, _expected)
{
if (_expected === 0)
......@@ -141,7 +154,10 @@ function isResultCorrect(_actual, _expected)
return true;
if (typeof(_expected) == "number" && isNaN(_expected))
return typeof(_actual) == "number" && isNaN(_actual);
if (_expected && (Object.prototype.toString.call(_expected) == Object.prototype.toString.call([])))
if (_expected
&& (Object.prototype.toString.call(_expected) ==
Object.prototype.toString.call([])
|| isTypedArray(_expected)))
return areArraysEqual(_actual, _expected);
return false;
}
......@@ -150,7 +166,10 @@ function stringify(v)
{
if (v === 0 && 1/v < 0)
return "-0";
else return "" + v;
else if (isTypedArray(v))
return v.__proto__.constructor.name + ":[" + Array.prototype.join.call(v, ",") + "]";
else
return "" + v;
}
function evalAndLog(_a, _quiet)
......@@ -185,15 +204,15 @@ function shouldBe(_a, _b, quiet)
var _bv = eval(_b);
if (exception)
testFailed(_a + " should be " + _bv + ". Threw exception " + exception);
testFailed(_a + " should be " + stringify(_bv) + ". Threw exception " + exception);
else if (isResultCorrect(_av, _bv)) {
if (!quiet) {
testPassed(_a + " is " + _b);
}
} else if (typeof(_av) == typeof(_bv))
testFailed(_a + " should be " + _bv + ". Was " + stringify(_av) + ".");
testFailed(_a + " should be " + stringify(_bv) + ". Was " + stringify(_av) + ".");
else
testFailed(_a + " should be " + _bv + " (of type " + typeof _bv + "). Was " + _av + " (of type " + typeof _av + ").");
testFailed(_a + " should be " + stringify(_bv) + " (of type " + typeof _bv + "). Was " + _av + " (of type " + typeof _av + ").");
}
// Execute condition every 5 milliseconds until it succeed or failureTime is reached.
......
description(
"Reduced test case for https://bugs.webkit.org/show_bug.cgi?id=83818"
);
a = new Uint16Array(8);
b = new Uint8Array(a.buffer, 0, 2);
b[0] = 0x05;
b[1] = 0x05;
shouldBe("a[0]", "0x0505");
a.set(b);
shouldBe("a[0]", "0x0005");
shouldBe("a[1]", "0x0005");
description(
"Tests the code path in typedArray.set that may have to do a copy via an intermediate buffer because the source and destination overlap and have different size elements (source is larger than destination)."
);
function foo_reference(n) {
var array = new Int8Array(n);
for (var i = 0; i < n; ++i)
array[i] = 42 + i;
var array2 = new Int8Array(array);
array2.set(new Int32Array(array.buffer));
return array2;
}
function foo(n) {
var array = new Int8Array(n);
for (var i = 0; i < n; ++i)
array[i] = 42 + i;
array.set(new Int32Array(array.buffer));
return array;
}
function bar_reference(n) {
var array = new Int8Array(n);
for (var i = 0; i < n; ++i)
array[i] = 42 + i;
var array2 = new Int8Array(array);
array2.set(new Int32Array(array.buffer), n - n / 4);
return array2;
}
function bar(n) {
var array = new Int8Array(n);
for (var i = 0; i < n; ++i)
array[i] = 42 + i;
array.set(new Int32Array(array.buffer), n - n / 4);
return array;
}
function baz_reference(n) {
var array = new Int8Array(n);
for (var i = 0; i < n; ++i)
array[i] = 42 + i;
var array2 = new Int8Array(array);
array2.set(new Int32Array(array.buffer), n / 2 - (n / 4) / 2);
return array2;
}
function baz(n) {
var array = new Int8Array(n);
for (var i = 0; i < n; ++i)
array[i] = 42 + i;
array.set(new Int32Array(array.buffer), n / 2 - (n / 4) / 2);
return array;
}
shouldBe("foo(64)", "foo_reference(64)");
shouldBe("bar(64)", "bar_reference(64)");
shouldBe("baz(64)", "baz_reference(64)");
description(
"Tests the code path of typedArray.set that tries to do a memmove-with-conversion for overlapping arrays."
);
function foo(n) {
var array = new Int32Array(n + 1);
for (var i = 0; i < n; ++i)
array[i] = 42 + i;
array.set(new Uint32Array(array.buffer, 0, n), 1);
return array;
}
function bar(n) {
var array = new Int32Array(n + 1);
for (var i = 0; i < n; ++i)
array[i + 1] = 42 + i;
array.set(new Uint32Array(array.buffer, 4), 0);
return array;
}
shouldBe("foo(10)", "[42,42,42,42,42,42,42,42,42,42,42]");
shouldBe("bar(10)", "[42,43,44,45,46,47,48,49,50,51,51]");
description(
"Tests the code path of typedArray.set that bottoms out in memmove."
);
function foo(n) {
var array = new Int32Array(n + 1);
for (var i = 0; i < n; ++i)
array[i] = 42 + i;
array.set(array.subarray(0, n), 1);
return array;
}
function bar(n) {
var array = new Int32Array(n + 1);
for (var i = 0; i < n; ++i)
array[i + 1] = 42 + i;
array.set(array.subarray(1, n + 1), 0);
return array;
}
function arraysEqual(a, b) {
if (a.length != b.length)
return false;
}
shouldBe("foo(10)", "[42,42,43,44,45,46,47,48,49,50,51]");
shouldBe("bar(10)", "[42,43,44,45,46,47,48,49,50,51,51]");
description(
"Tests the code path in typedArray.set that may have to do a copy via an intermediate buffer because the source and destination overlap and have different size elements (source is smaller than destination)."
);
function foo_reference(n) {
var array = new Int32Array(n + 1);
for (var i = 0; i < n; ++i)
array[i] = 42 + i;
var array2 = new Int32Array(array);
array2.set(new Uint8Array(array.buffer, 0, n), 1);
return array2;
}
function foo(n) {
var array = new Int32Array(n + 1);
for (var i = 0; i < n; ++i)
array[i] = 42 + i;
array.set(new Uint8Array(array.buffer, 0, n), 1);
return array;
}
function bar_reference(n) {
var array = new Int32Array(n + 1);
for (var i = 0; i < n; ++i)
array[i + 1] = 42 + i;
var array2 = new Int32Array(array);
array2.set(new Uint8Array(array.buffer, (n + 1) * 4 - n), 0);
return array2;
}
function bar(n) {
var array = new Int32Array(n + 1);
for (var i = 0; i < n; ++i)
array[i + 1] = 42 + i;
array.set(new Uint8Array(array.buffer, (n + 1) * 4 - n), 0);
return array;
}
function baz_reference(n) {
var array = new Int32Array(n);
for (var i = 0; i < n; ++i)
array[i] = 42 + i;
var array2 = new Int32Array(array);
array2.set(new Uint8Array(array.buffer, 0, n));
return array2;
}
function baz(n) {
var array = new Int32Array(n);
for (var i = 0; i < n; ++i)
array[i] = 42 + i;
array.set(new Uint8Array(array.buffer, 0, n));
return array;
}
function fuz_reference(n) {
var array = new Int32Array(n);
for (var i = 0; i < n; ++i)
array[i] = 42 + i;
var array2 = new Int32Array(array);
array2.set(new Uint8Array(array.buffer, n * 4 - n));
return array2;
}
function fuz(n) {
var array = new Int32Array(n);
for (var i = 0; i < n; ++i)
array[i] = 42 + i;
array.set(new Uint8Array(array.buffer, n * 4 - n));
return array;
}
function thingy_reference(n) {
var array = new Int32Array(n);
for (var i = 0; i < n; ++i)
array[i] = 42 + i;
var array2 = new Int32Array(array);
array2.set(new Uint8Array(array.buffer, 4, n));
return array2;
}
function thingy(n) {
var array = new Int32Array(n);
for (var i = 0; i < n; ++i)
array[i] = 42 + i;
array.set(new Uint8Array(array.buffer, 4, n));
return array;
}
shouldBe("foo(10)", "foo_reference(10)");
shouldBe("bar(10)", "bar_reference(10)");
shouldBe("baz(10)", "baz_reference(10)");
shouldBe("fuz(10)", "fuz_reference(10)");
shouldBe("thingy(10)", "thingy_reference(10)");
Reduced test case for https://bugs.webkit.org/show_bug.cgi?id=83818
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS a[0] is 0x0505
PASS a[0] is 0x0005
PASS a[1] is 0x0005
PASS successfullyParsed is true
TEST COMPLETE
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
<html>
<head>
<script src="resources/js-test-pre.js"></script>
</head>
<body>
<script src="script-tests/typed-array-copy.js"></script>
<script src="resources/js-test-post.js"></script>
</body>
</html>
Tests the code path in typedArray.set that may have to do a copy via an intermediate buffer because the source and destination overlap and have different size elements (source is larger than destination).
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS foo(64) is foo_reference(64)
PASS bar(64) is bar_reference(64)
PASS baz(64) is baz_reference(64)
PASS successfullyParsed is true
TEST COMPLETE
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
<html>
<head>
<script src="resources/js-test-pre.js"></script>
</head>
<body>
<script src="script-tests/typedarray-set-destination-smaller-than-source.js"></script>
<script src="resources/js-test-post.js"></script>
</body>
</html>
Tests the code path of typedArray.set that tries to do a memmove-with-conversion for overlapping arrays.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS foo(10) is [42,42,42,42,42,42,42,42,42,42,42]
PASS bar(10) is [42,43,44,45,46,47,48,49,50,51,51]
PASS successfullyParsed is true
TEST COMPLETE
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
<html>
<head>
<script src="resources/js-test-pre.js"></script>
</head>
<body>
<script src="script-tests/typedarray-set-overlapping-elements-of-same-size.js"></script>
<script src="resources/js-test-post.js"></script>
</body>
</html>
Tests the code path of typedArray.set that bottoms out in memmove.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS foo(10) is [42,42,43,44,45,46,47,48,49,50,51]
PASS bar(10) is [42,43,44,45,46,47,48,49,50,51,51]
PASS successfullyParsed is true
TEST COMPLETE
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
<html>
<head>
<script src="resources/js-test-pre.js"></script>
</head>
<body>
<script src="script-tests/typedarray-set-same-type-memmove.js"></script>
<script src="resources/js-test-post.js"></script>
</body>
</html>
Tests the code path in typedArray.set that may have to do a copy via an intermediate buffer because the source and destination overlap and have different size elements (source is smaller than destination).
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS foo(10) is foo_reference(10)
PASS bar(10) is bar_reference(10)
PASS baz(10) is baz_reference(10)
PASS fuz(10) is fuz_reference(10)
PASS thingy(10) is thingy_reference(10)
PASS successfullyParsed is true
TEST COMPLETE
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
<html>
<head>
<script src="resources/js-test-pre.js"></script>
</head>
<body>
<script src="script-tests/typedarray-set-source-smaller-than-destination.js"></script>
<script src="resources/js-test-post.js"></script>
</body>
</html>
2013-08-23 Filip Pizlo <fpizlo@apple.com>
Incorrect TypedArray#set behavior
https://bugs.webkit.org/show_bug.cgi?id=83818
Reviewed by Oliver Hunt and Mark Hahnenberg.
This was so much fun! typedArray.set() is like a memmove on steroids, and I'm
not smart enough to figure out optimal versions for *all* of the cases. But I
did come up with optimal implementations for most of the cases, and I wrote
spec-literal code (i.e. copy via a transfer buffer) for the cases I'm not smart
enough to write optimal code for.
* runtime/JSArrayBufferView.h:
(JSC::JSArrayBufferView::hasArrayBuffer):
* runtime/JSArrayBufferViewInlines.h:
(JSC::JSArrayBufferView::buffer):
(JSC::JSArrayBufferView::existingBufferInButterfly):
(JSC::JSArrayBufferView::neuter):
(JSC::JSArrayBufferView::byteOffset):
* runtime/JSGenericTypedArrayView.h:
* runtime/JSGenericTypedArrayViewInlines.h:
(JSC::::setWithSpecificType):
(JSC::::set):
(JSC::::existingBuffer):
2013-08-23 Alex Christensen <achristensen@apple.com>
 
Re-separating Win32 and Win64 builds.
......
......@@ -154,6 +154,8 @@ protected:
public:
TypedArrayMode mode() const { return m_mode; }
bool hasArrayBuffer() const { return JSC::hasArrayBuffer(mode()); }
ArrayBuffer* buffer();
PassRefPtr<ArrayBufferView> impl();
void neuter();
......@@ -173,6 +175,8 @@ private:
protected:
static const unsigned StructureFlags = OverridesGetPropertyNames | OverridesGetOwnPropertySlot | Base::StructureFlags;
ArrayBuffer* existingBufferInButterfly();
void* m_vector;
uint32_t m_length;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment