[wasm] Do not generate a loop stack check upon a decoder error.

[wasm] Do not generate a loop stack check upon a decoder error.

A decoder error sets builder_ to null, which causes builder_->StackCheck
to segfault.

R=titzer@chromium.org

TEST=mjsunit/regress/wasm/loop-stack-check

Committed: https://crrev.com/fa1f9c37d11af2e898fd8b0b708ac980cac53cfc
Cr-Commit-Position: refs/heads/master@{#40271}

[ahaas, 2016-10-13 13:20:26]

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

[commit-bot: I haz the power, 2016-10-13 13:20:35]

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

[titzer, 2016-10-13 13:24:42]

https://codereview.chromium.org/2416873002/diff/1/src/wasm/ast-decoder.cc
File src/wasm/ast-decoder.cc (right):

https://codereview.chromium.org/2416873002/diff/1/src/wasm/ast-decoder.cc#new...
src/wasm/ast-decoder.cc:1638: if (failed()) return loop_body_env;
I think you should move this up to just after the loop assignment analysis.

https://codereview.chromium.org/2416873002/diff/1/src/wasm/ast-decoder.cc#new...
src/wasm/ast-decoder.cc:1652: if (failed()) return loop_body_env;
I don't think this check is necessary, since we have one at the beginning of the
function, and the only thing that can make decoding fail until then is the loop
assignment analysis.

[ahaas, 2016-10-13 13:31:05]

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

[commit-bot: I haz the power, 2016-10-13 13:31:20]

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

[ahaas, 2016-10-13 13:31:55]

https://codereview.chromium.org/2416873002/diff/1/src/wasm/ast-decoder.cc
File src/wasm/ast-decoder.cc (right):

https://codereview.chromium.org/2416873002/diff/1/src/wasm/ast-decoder.cc#new...
src/wasm/ast-decoder.cc:1638: if (failed()) return loop_body_env;
On 2016/10/13 at 13:24:42, titzer wrote:
> I think you should move this up to just after the loop assignment analysis.

Done.

https://codereview.chromium.org/2416873002/diff/1/src/wasm/ast-decoder.cc#new...
src/wasm/ast-decoder.cc:1652: if (failed()) return loop_body_env;
On 2016/10/13 at 13:24:42, titzer wrote:
> I don't think this check is necessary, since we have one at the beginning of
the function, and the only thing that can make decoding fail until then is the
loop assignment analysis.

Done.

[titzer, 2016-10-13 13:42:58]

lgtm

[commit-bot: I haz the power, 2016-10-13 14:13:40]

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

[commit-bot: I haz the power, 2016-10-13 14:13:41]

Dry run: This issue passed the CQ dry run.

[ahaas, 2016-10-13 14:16:42]

The CQ bit was checked by ahaas@chromium.org

[commit-bot: I haz the power, 2016-10-13 14:16:51]

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 14:32:51]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-13 14:33:15]

Description was changed from

==========
[wasm] Do not generate a loop stack check upon a decoder error.

A decoder error sets builder_ to null, which causes builder_->StackCheck
to segfault.

R=titzer@chromium.org

TEST=mjsunit/regress/wasm/loop-stack-check
==========

to

==========
[wasm] Do not generate a loop stack check upon a decoder error.

A decoder error sets builder_ to null, which causes builder_->StackCheck
to segfault.

R=titzer@chromium.org

TEST=mjsunit/regress/wasm/loop-stack-check

Committed: https://crrev.com/fa1f9c37d11af2e898fd8b0b708ac980cac53cfc
Cr-Commit-Position: refs/heads/master@{#40271}
==========

[commit-bot: I haz the power, 2016-10-13 14:33:17]

Patchset 2 (id:??) landed as
https://crrev.com/fa1f9c37d11af2e898fd8b0b708ac980cac53cfc
Cr-Commit-Position: refs/heads/master@{#40271}