[wasm] Track in the interpreter if a NaN could have been produced.

[wasm] Track in the interpreter if a NaN could have been produced.

The wasm specification does not fully specify the binary representation
of NaN: the sign bit can be non-deterministic. The wasm-code fuzzer
found a test case where the wasm interpreter and the compiled code
produce a different sign bit for a NaN, and as a consequence they
produce different results.

With this CL the interpreter tracks whether it executed an instruction
which can produce a NaN, which are div and sqrt instructions. The
fuzzer uses this information and compares the result of the interpreter
with the result of the compiled code only if there was no instruction
which could have produced a NaN.

R=titzer@chromium.org

TEST=cctest/test-run-wasm-interpreter/TestMayProduceNaN
BUG=chromium:657481
Committed: https://crrev.com/57b14b0606e43d0ab023caf0514d0a252f72cae1
Cr-Commit-Position: refs/heads/master@{#40474}

[ahaas, 2016-10-20 10:49:27]

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

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

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-20 10:53:24]

General approach looks good, see comments

https://chromiumcodereview.appspot.com/2438603003/diff/1/src/wasm/wasm-interp...
File src/wasm/wasm-interpreter.cc (right):

https://chromiumcodereview.appspot.com/2438603003/diff/1/src/wasm/wasm-interp...
src/wasm/wasm-interpreter.cc:1616: may_produced_nan_ |= true;                   
        \
Don't we want to set this flag only if the result value is a nan?

https://chromiumcodereview.appspot.com/2438603003/diff/1/src/wasm/wasm-interp...
File src/wasm/wasm-interpreter.h (right):

https://chromiumcodereview.appspot.com/2438603003/diff/1/src/wasm/wasm-interp...
src/wasm/wasm-interpreter.h:128: virtual bool MayProducedNaN() = 0;
Call this possibly nondeterministic? And add a comment to explain what it's for,
how it may be set, etc.

https://chromiumcodereview.appspot.com/2438603003/diff/1/test/common/wasm/was...
File test/common/wasm/wasm-module-runner.cc (right):

https://chromiumcodereview.appspot.com/2438603003/diff/1/test/common/wasm/was...
test/common/wasm/wasm-module-runner.cc:117: WasmVal* args, bool*
may_produced_nan) {
Same here. s/may_produced_nan/possible_nondeterminism/g

[commit-bot: I haz the power, 2016-10-20 11:18:08]

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

[commit-bot: I haz the power, 2016-10-20 11:18:09]

Dry run: This issue passed the CQ dry run.

[ahaas, 2016-10-20 13:42:05]

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

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

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-20 13:43:23]

https://chromiumcodereview.appspot.com/2438603003/diff/1/src/wasm/wasm-interp...
File src/wasm/wasm-interpreter.cc (right):

https://chromiumcodereview.appspot.com/2438603003/diff/1/src/wasm/wasm-interp...
src/wasm/wasm-interpreter.cc:1616: may_produced_nan_ |= true;                   
        \
On 2016/10/20 at 10:53:24, titzer wrote:
> Don't we want to set this flag only if the result value is a nan?

Done.

https://chromiumcodereview.appspot.com/2438603003/diff/1/src/wasm/wasm-interp...
File src/wasm/wasm-interpreter.h (right):

https://chromiumcodereview.appspot.com/2438603003/diff/1/src/wasm/wasm-interp...
src/wasm/wasm-interpreter.h:128: virtual bool MayProducedNaN() = 0;
On 2016/10/20 at 10:53:24, titzer wrote:
> Call this possibly nondeterministic? And add a comment to explain what it's
for, how it may be set, etc.

Done.

https://chromiumcodereview.appspot.com/2438603003/diff/1/test/common/wasm/was...
File test/common/wasm/wasm-module-runner.cc (right):

https://chromiumcodereview.appspot.com/2438603003/diff/1/test/common/wasm/was...
test/common/wasm/wasm-module-runner.cc:117: WasmVal* args, bool*
may_produced_nan) {
On 2016/10/20 at 10:53:24, titzer wrote:
> Same here. s/may_produced_nan/possible_nondeterminism/g

Done.

[titzer, 2016-10-20 13:45:48]

lgtm with nit

https://chromiumcodereview.appspot.com/2438603003/diff/20001/test/common/wasm...
File test/common/wasm/wasm-module-runner.cc (right):

https://chromiumcodereview.appspot.com/2438603003/diff/20001/test/common/wasm...
test/common/wasm/wasm-module-runner.cc:117: WasmVal* args, bool*
may_produced_nan) {
name this also possible_nondeterminism?

[ahaas, 2016-10-20 13:48:35]

https://codereview.chromium.org/2438603003/diff/20001/test/common/wasm/wasm-m...
File test/common/wasm/wasm-module-runner.cc (right):

https://codereview.chromium.org/2438603003/diff/20001/test/common/wasm/wasm-m...
test/common/wasm/wasm-module-runner.cc:117: WasmVal* args, bool*
may_produced_nan) {
On 2016/10/20 at 13:45:48, titzer wrote:
> name this also possible_nondeterminism?

Done.

[ahaas, 2016-10-20 13:48:39]

The CQ bit was checked by ahaas@chromium.org

[ahaas, 2016-10-20 13:48:39]

The patchset sent to the CQ was uploaded after l-g-t-m from titzer@chromium.org
Link to the patchset: https://codereview.chromium.org/2438603003/#ps40001
(title: "Fixed nits.")

[commit-bot: I haz the power, 2016-10-20 13:48: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-20 14:27:30]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-17 22:08:21]

Description was changed from

==========
[wasm] Track in the interpreter if a NaN could have been produced.

The wasm specification does not fully specify the binary representation
of NaN: the sign bit can be non-deterministic. The wasm-code fuzzer
found a test case where the wasm interpreter and the compiled code
produce a different sign bit for a NaN, and as a consequence they
produce different results.

With this CL the interpreter tracks whether it executed an instruction
which can produce a NaN, which are div and sqrt instructions. The
fuzzer uses this information and compares the result of the interpreter
with the result of the compiled code only if there was no instruction
which could have produced a NaN.

R=titzer@chromium.org

TEST=cctest/test-run-wasm-interpreter/TestMayProduceNaN
BUG=chromium:657481
==========

to

==========
[wasm] Track in the interpreter if a NaN could have been produced.

The wasm specification does not fully specify the binary representation
of NaN: the sign bit can be non-deterministic. The wasm-code fuzzer
found a test case where the wasm interpreter and the compiled code
produce a different sign bit for a NaN, and as a consequence they
produce different results.

With this CL the interpreter tracks whether it executed an instruction
which can produce a NaN, which are div and sqrt instructions. The
fuzzer uses this information and compares the result of the interpreter
with the result of the compiled code only if there was no instruction
which could have produced a NaN.

R=titzer@chromium.org

TEST=cctest/test-run-wasm-interpreter/TestMayProduceNaN
BUG=chromium:657481

Committed: https://crrev.com/57b14b0606e43d0ab023caf0514d0a252f72cae1
Cr-Commit-Position: refs/heads/master@{#40474}
==========

[commit-bot: I haz the power, 2016-11-17 22:08:22]

Patchset 3 (id:??) landed as
https://crrev.com/57b14b0606e43d0ab023caf0514d0a252f72cae1
Cr-Commit-Position: refs/heads/master@{#40474}