diff options
| author | Alon Zakai <azakai@google.com> | 2020-07-28 06:58:34 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2020-07-28 06:58:34 -0700 |
| commit | 26f240c72dd62ed8a39f7466df99e51ec34487aa (patch) | |
| tree | 67e7a8bac0b767e421764058f7accb5c3df32688 /src | |
| parent | 32ab8bac04af52121c6985a9a019c0fdec957f03 (diff) | |
| download | binaryen-26f240c72dd62ed8a39f7466df99e51ec34487aa.tar.gz binaryen-26f240c72dd62ed8a39f7466df99e51ec34487aa.tar.bz2 binaryen-26f240c72dd62ed8a39f7466df99e51ec34487aa.zip | |
wasm2js: Don't remove an | 0 or >>> 0 in a boolean context (#2993)
It is usually fine to do if (x | 0) => if (x) since it just cares if the
value is 0 or not. However, if the cast turns it into 0, then that is
incorrect, which the fuzzer found as
-2147483648 + -2147483648 | 0
(the sum is 2^32, which | 0 is 0).
We can maybe look into doing this in a safe way, but for now
just remove it. It doesn't have a big impact on code size as this
is pretty rare (e.g. the minimal runtime code size test is not
broken by this).
Diffstat (limited to 'src')
| -rw-r--r-- | src/tools/wasm2js.cpp | 42 |
1 files changed, 26 insertions, 16 deletions
diff --git a/src/tools/wasm2js.cpp b/src/tools/wasm2js.cpp index fb63627a3..d64f0b0ab 100644 --- a/src/tools/wasm2js.cpp +++ b/src/tools/wasm2js.cpp @@ -260,23 +260,33 @@ static void optimizeJS(Ref ast, Wasm2JSBuilder::Flags flags) { node[0]->setString(UNARY_PREFIX); node[1]->setString(L_NOT); node[3]->setNull(); - } else if (isOrZero(node) || isTrshiftZero(node)) { - // Just being different from 0 is enough, casts don't matter. However, - // in deterministic mode we care about corner cases that would trap in - // wasm, like an integer divide by zero: - // - // if ((1 / 0) | 0) => condition is Infinity | 0 = 0 which is falsey - // - // while - // - // if (1 / 0) => condition is Infinity which is truthy - // - // Thankfully this is not common, and does not occur on % (1 % 0 is a NaN - // which has the right truthiness). - if (!(flags.deterministic && isBinary(node[2], DIV))) { - return node[2]; - } } + // TODO: in some cases it may be possible to turn + // + // if (x | 0) + // + // into + // + // if (x) + // + // In general this is unsafe if e.g. x is -2147483648 + -2147483648 (which + // the | 0 turns into 0, but without it is a truthy value). + // + // Another issue is that in deterministic mode we care about corner cases + // that would trap in wasm, like an integer divide by zero: + // + // if ((1 / 0) | 0) => condition is Infinity | 0 = 0 which is falsey + // + // while + // + // if (1 / 0) => condition is Infinity which is truthy + // + // Thankfully this is not common, and does not occur on % (1 % 0 is a NaN + // which has the right truthiness), so we could perhaps do + // + // if (!(flags.deterministic && isBinary(node[2], DIV))) return node[2]; + // + // (but there is still the first issue). return node; }; |