Apply numeric casts correctly in typed arrays and related code.

Apply numeric casts correctly in typed arrays and related code.

R=jkummerow@chromium.org
BUG=353004
LOG=Y

Committed: https://code.google.com/p/v8/source/detail?r=20020

Committed: https://code.google.com/p/v8/source/detail?r=20022

[Dmitry Lomov (no reviews), 2014-03-17 16:00:26]

PTAL

[Jakob Kummerow, 2014-03-17 17:29:04]

LGTM with some comments.

https://codereview.chromium.org/201873005/diff/10001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/201873005/diff/10001/src/runtime.cc#newcode866
src/runtime.cc:866: CHECK(start <= source_byte_length);
Why CHECK rather than RUNTIME_ASSERT? Is a crash better than throwing an
exception?

https://codereview.chromium.org/201873005/diff/10001/src/typedarray.js
File src/typedarray.js (right):

https://codereview.chromium.org/201873005/diff/10001/src/typedarray.js#newcod...
src/typedarray.js:409: ToPositiveDataViewOffset(offset),
Is this safe?

https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/reg...
File test/mjsunit/regress/regress-353004.js (right):

https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/reg...
test/mjsunit/regress/regress-353004.js:2: // Redistribution and use in source
and binary forms, with or without
nit: you can use the new, short licence header.

https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/reg...
test/mjsunit/regress/regress-353004.js:30: buffer1 = new ArrayBuffer(100 * 1024
* 1024);
Wouldn't 100K be enough for each bufferX? I fear that working with big arrays
might lead to OOM crashes on memory-constrained devices/buildbots.

https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/reg...
test/mjsunit/regress/regress-353004.js:32: array1 = new Uint8Array(buffer1,
{valueOf : function() {
nit: how about some "var" keywords, here and for every other variable in this
test?

https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/reg...
test/mjsunit/regress/regress-353004.js:38: for (i = 0; i < array1.length; i++) {
This loop is pointless; if the assertEquals right above passed, it runs for zero
iterations.

https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/reg...
test/mjsunit/regress/regress-353004.js:59: for (i = 0; i < dataView1.byteLength;
i++) {
another pointless loop

https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/reg...
test/mjsunit/regress/regress-353004.js:91: %ArrayBufferNeuter(buffer9);
nit: funky indentation

[Dmitry Lomov (no reviews), 2014-03-18 10:22:31]

Commends addressed, landing.

https://codereview.chromium.org/201873005/diff/10001/src/typedarray.js
File src/typedarray.js (right):

https://codereview.chromium.org/201873005/diff/10001/src/typedarray.js#newcod...
src/typedarray.js:409: ToPositiveDataViewOffset(offset),
On 2014/03/17 17:29:05, Jakob wrote:
> Is this safe?
Looks safe to me.

https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/reg...
File test/mjsunit/regress/regress-353004.js (right):

https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/reg...
test/mjsunit/regress/regress-353004.js:2: // Redistribution and use in source
and binary forms, with or without
On 2014/03/17 17:29:05, Jakob wrote:
> nit: you can use the new, short licence header.

Done.

https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/reg...
test/mjsunit/regress/regress-353004.js:30: buffer1 = new ArrayBuffer(100 * 1024
* 1024);
On 2014/03/17 17:29:05, Jakob wrote:
> Wouldn't 100K be enough for each bufferX? I fear that working with big arrays
> might lead to OOM crashes on memory-constrained devices/buildbots.

Done.

https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/reg...
test/mjsunit/regress/regress-353004.js:32: array1 = new Uint8Array(buffer1,
{valueOf : function() {
On 2014/03/17 17:29:05, Jakob wrote:
> nit: how about some "var" keywords, here and for every other variable in this
> test?

Done.

https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/reg...
test/mjsunit/regress/regress-353004.js:38: for (i = 0; i < array1.length; i++) {
On 2014/03/17 17:29:05, Jakob wrote:
> This loop is pointless; if the assertEquals right above passed, it runs for
zero
> iterations.

Done.

https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/reg...
test/mjsunit/regress/regress-353004.js:59: for (i = 0; i < dataView1.byteLength;
i++) {
On 2014/03/17 17:29:05, Jakob wrote:
> another pointless loop

Done.

https://codereview.chromium.org/201873005/diff/10001/test/mjsunit/regress/reg...
test/mjsunit/regress/regress-353004.js:91: %ArrayBufferNeuter(buffer9);
On 2014/03/17 17:29:05, Jakob wrote:
> nit: funky indentation

Done.

[Dmitry Lomov (no reviews), 2014-03-18 10:23:58]

Committed patchset #3 manually as r20020 (presubmit successful).

[Dmitry Lomov (no reviews), 2014-03-18 10:55:07]

Relanding with Win64 build fix.

[Dmitry Lomov (no reviews), 2014-03-18 10:55:36]

Committed patchset #4 manually as r20022 (presubmit successful).