1244571 - wasm: Implement more operators

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Dan Gohman [:sunfish]]

Bug 1244272 implemented parsing, encoding, and decoding for several operators. There are more!

Comment 1

[Dan Gohman [:sunfish]]

Attachment: wasm-unary-ops.patch

The unary operators.

Comment 2

[Dan Gohman [:sunfish]]

Attachment: wasm-comparisons.patch

The comparisons.

Comment 3

[Dan Gohman [:sunfish]]

Attachment: wasm-unary-binary-typecheck.patch

While implementing the comparisons, I noticed that the earlier binary and unary operator patches didn't do full type checking. This patch rectifies that.

Comment 4

[Dan Gohman [:sunfish]]

Attachment: wasm-conversions.patch

The conversion operators.

Comment 5

[Dan Gohman [:sunfish]]

Attachment: wasm-no-i8const.patch

This patch just removes an unused opcode.

Comment 6

[Dan Gohman [:sunfish]]

Attachment: wasm-i64.patch

This patch adds i64 support to parsing, encoding, decoding, and type-checking. i64 isn't supported in WasmIonCompile.cpp yet, but this patch sets up the front-end support it'll need.

Comment 7

[Dan Gohman [:sunfish]]

Attachment: wasm-parser-cleanup.patch

As suggested in bug 1244272 comment 4, this patch tidies up parsing to have the full names of the opcodes in the code, at the cost of some mild inefficiency (but it's the s-expression parser, so it's fine).

Comment 8

[Luke Wagner [:luke]]

Comment on attachment 8714142 [details] [diff] [review]
wasm-i64.patch

Review of attachment 8714142 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/asmjs/Wasm.cpp
@@ +325,5 @@
>          return DecodeUnaryOperator(f, expected, ExprType::I32);
> +      case Expr::I64Clz:
> +      case Expr::I64Ctz:
> +      case Expr::I64Popcnt:
> +        return DecodeUnaryOperator(f, expected, ExprType::I64);

Unless I've missed it, I think this means that it will be possible for the fuzzer to generate i64 ops which will crash.  While keeping the code structure the same, can you have these fail to validate (see e.g., I64 case in CheckValType).

Comment 9

[Luke Wagner [:luke]]

Comment on attachment 8714143 [details] [diff] [review]
wasm-parser-cleanup.patch

Review of attachment 8714143 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 10

[Dan Gohman [:sunfish]]

(In reply to Luke Wagner [:luke] from comment #8)
> Unless I've missed it, I think this means that it will be possible for the
> fuzzer to generate i64 ops which will crash.  While keeping the code
> structure the same, can you have these fail to validate (see e.g., I64 case
> in CheckValType).

The patches here do this in several areas: I32Ctz, F32Copysign, and so on. Should I disable parsing and decoding of such operators until Ion implementations are in place?

Comment 11

[Dan Gohman [:sunfish]]

Attachment: wasm-nyi-fail-decode.patch

This patches fixes decoding to fail on operators that WasmIonCompile doesn't support yet, so that fuzzers don't see MOZ_CRASH on them.

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/89f9764ae65e
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5095d17bd44
https://hg.mozilla.org/integration/mozilla-inbound/rev/594a1d126773
https://hg.mozilla.org/integration/mozilla-inbound/rev/f3f5fc7cb841
https://hg.mozilla.org/integration/mozilla-inbound/rev/8aed04de4330
https://hg.mozilla.org/integration/mozilla-inbound/rev/6012d29342bd
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ae60a91cfe4
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7c161576532

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/89f9764ae65e
https://hg.mozilla.org/mozilla-central/rev/f5095d17bd44
https://hg.mozilla.org/mozilla-central/rev/594a1d126773
https://hg.mozilla.org/mozilla-central/rev/f3f5fc7cb841
https://hg.mozilla.org/mozilla-central/rev/8aed04de4330
https://hg.mozilla.org/mozilla-central/rev/6012d29342bd
https://hg.mozilla.org/mozilla-central/rev/3ae60a91cfe4
https://hg.mozilla.org/mozilla-central/rev/d7c161576532