[wasm] test deserialization when header is invalid

[wasm] test deserialization when header is invalid

A test where the deserialization data has a header, but the
header is invalid. This is in addition to the current test
where we have empty deserialization data.

BUG=

Committed: https://crrev.com/80caaac31bd05649ab81c0b84b320833c96b4ed7
Cr-Commit-Position: refs/heads/master@{#40321}

[Mircea Trofin, 2016-10-12 05:02:31]

The CQ bit was checked by mtrofin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-12 05:02:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mircea Trofin, 2016-10-12 05:10:39]

Description was changed from

==========
[wasm] test deserialization when header is invalid

A test where the deserialization data has a header, but the
header is invalid.

BUG=
==========

to

==========
[wasm] test deserialization when header is invalid

A test where the deserialization data has a header, but the
header is invalid. This is in addition to the current test
where we have empty deserialization data.

BUG=
==========

[Mircea Trofin, 2016-10-12 05:10:40]

mtrofin@chromium.org changed reviewers:
+ bradnelson@chromium.org

[commit-bot: I haz the power, 2016-10-12 05:26:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-12 05:26:49]

Dry run: This issue passed the CQ dry run.

[bradnelson, 2016-10-12 19:10:25]

https://codereview.chromium.org/2418483002/diff/20001/test/cctest/wasm/test-r...
File test/cctest/wasm/test-run-wasm-module.cc (right):

https://codereview.chromium.org/2418483002/diff/20001/test/cctest/wasm/test-r...
test/cctest/wasm/test-run-wasm-module.cc:233: // Mess the header.
Mess with?

https://codereview.chromium.org/2418483002/diff/20001/test/cctest/wasm/test-r...
test/cctest/wasm/test-run-wasm-module.cc:235:
const_cast<uint8_t*>(data.first.get())[0] = 0;
Kinda bad. Maybe create an accessor that's a friend on a test fixture?

[Mircea Trofin, 2016-10-12 20:30:36]

The CQ bit was checked by mtrofin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-12 20:30:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mircea Trofin, 2016-10-12 20:31:46]

mtrofin@chromium.org changed reviewers:
+ yangguo@chromium.org

[Mircea Trofin, 2016-10-12 20:32:06]

+Yang, for the "ForTest" API on the snapshot side.

https://codereview.chromium.org/2418483002/diff/20001/test/cctest/wasm/test-r...
File test/cctest/wasm/test-run-wasm-module.cc (right):

https://codereview.chromium.org/2418483002/diff/20001/test/cctest/wasm/test-r...
test/cctest/wasm/test-run-wasm-module.cc:233: // Mess the header.
On 2016/10/12 19:10:25, bradnelson wrote:
> Mess with?

Done.

https://codereview.chromium.org/2418483002/diff/20001/test/cctest/wasm/test-r...
test/cctest/wasm/test-run-wasm-module.cc:235:
const_cast<uint8_t*>(data.first.get())[0] = 0;
On 2016/10/12 19:10:25, bradnelson wrote:
> Kinda bad. Maybe create an accessor that's a friend on a test fixture?

Done.

[commit-bot: I haz the power, 2016-10-12 20:36:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-12 20:36:59]

Dry run: Try jobs failed on following builders:
  v8_win_compile_dbg on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/2...)

[Mircea Trofin, 2016-10-12 20:39:17]

The CQ bit was checked by mtrofin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-12 20:39:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mircea Trofin, 2016-10-12 20:49:21]

Patchset #3 (id:40001) has been deleted

[commit-bot: I haz the power, 2016-10-12 21:06:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-12 21:06:22]

Dry run: This issue passed the CQ dry run.

[bradnelson, 2016-10-13 05:28:16]

lgtm

[Yang, 2016-10-13 06:52:59]

https://codereview.chromium.org/2418483002/diff/60001/src/snapshot/code-seria...
File src/snapshot/code-serializer.cc (right):

https://codereview.chromium.org/2418483002/diff/60001/src/snapshot/code-seria...
src/snapshot/code-serializer.cc:418: void
SerializedCodeData::InvalidateVersionForTest(byte* buffer,
I'm not really happy with introducing test APIs. Can we either extend
SerializedCodeData in test-run-wasm-module.cc to include a test method, or
simply flip some bits in the first bytes in the serialized data?

[Mircea Trofin, 2016-10-13 16:09:10]

https://codereview.chromium.org/2418483002/diff/60001/src/snapshot/code-seria...
File src/snapshot/code-serializer.cc (right):

https://codereview.chromium.org/2418483002/diff/60001/src/snapshot/code-seria...
src/snapshot/code-serializer.cc:418: void
SerializedCodeData::InvalidateVersionForTest(byte* buffer,
On 2016/10/13 06:52:59, Yang wrote:
> I'm not really happy with introducing test APIs. Can we either extend
> SerializedCodeData in test-run-wasm-module.cc to include a test method, or
> simply flip some bits in the first bytes in the serialized data?

The former is more complicated, because the field definitions
(kVersionHashOffset, etc) are private. I'd rather make those public, together
with the general-purpose SetHeaderValueInBuffer static method I just added.
Feels more appropriate for the scope of the change.

Simply flipping the bits - directly - was what I originally did, but received
pushback in the previous review :)

[Mircea Trofin, 2016-10-13 22:04:26]

The CQ bit was checked by mtrofin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-13 22:04:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-13 22:29:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-13 22:29:27]

Dry run: This issue passed the CQ dry run.

[Yang, 2016-10-14 13:01:54]

On 2016/10/13 22:12:45, Mircea Trofin wrote:

lgtm.

[Mircea Trofin, 2016-10-14 14:21:57]

The CQ bit was checked by mtrofin@chromium.org

[Mircea Trofin, 2016-10-14 14:21:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
bradnelson@chromium.org
Link to the patchset: https://codereview.chromium.org/2418483002/#ps80001
(title: "feedback")

[commit-bot: I haz the power, 2016-10-14 14:22:12]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-14 14:25:02]

Description was changed from

==========
[wasm] test deserialization when header is invalid

A test where the deserialization data has a header, but the
header is invalid. This is in addition to the current test
where we have empty deserialization data.

BUG=
==========

to

==========
[wasm] test deserialization when header is invalid

A test where the deserialization data has a header, but the
header is invalid. This is in addition to the current test
where we have empty deserialization data.

BUG=
==========

[commit-bot: I haz the power, 2016-10-14 14:25:03]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-10-14 14:25:34]

Description was changed from

==========
[wasm] test deserialization when header is invalid

A test where the deserialization data has a header, but the
header is invalid. This is in addition to the current test
where we have empty deserialization data.

BUG=
==========

to

==========
[wasm] test deserialization when header is invalid

A test where the deserialization data has a header, but the
header is invalid. This is in addition to the current test
where we have empty deserialization data.

BUG=

Committed: https://crrev.com/80caaac31bd05649ab81c0b84b320833c96b4ed7
Cr-Commit-Position: refs/heads/master@{#40321}
==========

[commit-bot: I haz the power, 2016-10-14 14:25:35]

Patchset 4 (id:??) landed as
https://crrev.com/80caaac31bd05649ab81c0b84b320833c96b4ed7
Cr-Commit-Position: refs/heads/master@{#40321}