From 7a976a59ca37719ab7790c675f794004aa27f03e Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Fri, 31 Aug 2018 10:26:09 -0700 Subject: [PATCH 1/5] change the Literal's operator== to be a bitwise comparison. this makes Literals work properly in hashtables, fixing an infinite loop fuzz bug in the rse pass --- src/ir/ExpressionAnalyzer.cpp | 2 +- src/literal.h | 6 +++++- src/tools/execution-results.h | 2 +- src/tools/wasm-shell.cpp | 4 ++-- src/wasm/literal.cpp | 20 ++++++++++---------- test/passes/rse.wast | 29 +++++++++++++++++++++++++++++ 6 files changed, 48 insertions(+), 15 deletions(-) diff --git a/src/ir/ExpressionAnalyzer.cpp b/src/ir/ExpressionAnalyzer.cpp index 64b1ce24b19..d91bc370c4c 100644 --- a/src/ir/ExpressionAnalyzer.cpp +++ b/src/ir/ExpressionAnalyzer.cpp @@ -257,7 +257,7 @@ bool ExpressionAnalyzer::flexibleEqual(Expression* left, Expression* right, Expr break; } case Expression::Id::ConstId: { - if (!left->cast()->value.bitwiseEqual(right->cast()->value)) { + if (left->cast()->value != right->cast()->value) { return false; } break; diff --git a/src/literal.h b/src/literal.h index ae628149ec1..137a1c945b6 100644 --- a/src/literal.h +++ b/src/literal.h @@ -71,9 +71,13 @@ class Literal { int64_t getInteger() const; double getFloat() const; int64_t getBits() const; + // Equality checks for the type and the bits, so a nan float would + // be compared bitwise. bool operator==(const Literal& other) const; bool operator!=(const Literal& other) const; - bool bitwiseEqual(const Literal& other) const; + // Compare in a non-bitwise manner, which means for a nan a we have + // a != a + bool nonBitwiseEqual(const Literal& other) const; static uint32_t NaNPayload(float f); static uint64_t NaNPayload(double f); diff --git a/src/tools/execution-results.h b/src/tools/execution-results.h index fe82c024aef..1e8fba4ff6f 100644 --- a/src/tools/execution-results.h +++ b/src/tools/execution-results.h @@ -78,7 +78,7 @@ struct ExecutionResults { abort(); } std::cout << "[fuzz-exec] comparing " << name << '\n'; - if (!results[name].bitwiseEqual(other.results[name])) { + if (results[name] != other.results[name]) { std::cout << "not identical!\n"; abort(); } diff --git a/src/tools/wasm-shell.cpp b/src/tools/wasm-shell.cpp index 613fc8040de..2a59be167bd 100644 --- a/src/tools/wasm-shell.cpp +++ b/src/tools/wasm-shell.cpp @@ -201,14 +201,14 @@ static void run_asserts(Name moduleName, size_t* i, bool* checked, Module* wasm, ->dynCast() ->value; std::cerr << "seen " << result << ", expected " << expected << '\n'; - if (!expected.bitwiseEqual(result)) { + if (expected != result) { std::cout << "unexpected, should be identical\n"; abort(); } } else { Literal expected; std::cerr << "seen " << result << ", expected " << expected << '\n'; - if (!expected.bitwiseEqual(result)) { + if (expected != result) { std::cout << "unexpected, should be identical\n"; abort(); } diff --git a/src/wasm/literal.cpp b/src/wasm/literal.cpp index 032b3f7f8a7..622dd8d0d03 100644 --- a/src/wasm/literal.cpp +++ b/src/wasm/literal.cpp @@ -80,6 +80,16 @@ int64_t Literal::getBits() const { } bool Literal::operator==(const Literal& other) const { + if (type != other.type) return false; + if (type == none) return true; + return getBits() == other.getBits(); +} + +bool Literal::operator!=(const Literal& other) const { + return !(*this == other); +} + +bool Literal::nonBitwiseEqual(const Literal& other) const { if (type != other.type) return false; switch (type) { case Type::none: return true; @@ -91,16 +101,6 @@ bool Literal::operator==(const Literal& other) const { } } -bool Literal::operator!=(const Literal& other) const { - return !(*this == other); -} - -bool Literal::bitwiseEqual(const Literal& other) const { - if (type != other.type) return false; - if (type == none) return true; - return getBits() == other.getBits(); -} - uint32_t Literal::NaNPayload(float f) { assert(std::isnan(f) && "expected a NaN"); // SEEEEEEE EFFFFFFF FFFFFFFF FFFFFFFF diff --git a/test/passes/rse.wast b/test/passes/rse.wast index 6a0ada01889..3b3d38ec186 100644 --- a/test/passes/rse.wast +++ b/test/passes/rse.wast @@ -248,5 +248,34 @@ ) ) ) + (func $fuzz-nan + (local $0 f64) + (local $1 f64) + (block $block + (br_if $block + (i32.const 0) + ) + (loop $loop + (set_local $1 + (get_local $0) + ) + (set_local $0 + (f64.const -nan:0xfffffffffff87) + ) + (br_if $loop + (i32.const 1) + ) + ) + ) + (set_local $0 ;; make them equal + (get_local $1) + ) + (if + (i32.const 0) + (set_local $1 ;; we can drop this + (get_local $0) + ) + ) + ) ) From 76a32629e5d0daabff144da99c8f19744d551f93 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Fri, 31 Aug 2018 10:43:01 -0700 Subject: [PATCH 2/5] fix --- src/passes/Precompute.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/passes/Precompute.cpp b/src/passes/Precompute.cpp index b902c423170..9735b91d7ee 100644 --- a/src/passes/Precompute.cpp +++ b/src/passes/Precompute.cpp @@ -316,7 +316,7 @@ struct Precompute : public WalkerPass Date: Fri, 31 Aug 2018 10:47:21 -0700 Subject: [PATCH 3/5] update test output --- test/passes/rse.txt | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/passes/rse.txt b/test/passes/rse.txt index ba31b58ccf3..7ca2894e19b 100644 --- a/test/passes/rse.txt +++ b/test/passes/rse.txt @@ -430,4 +430,33 @@ ) ) ) + (func $fuzz-nan (; 18 ;) (type $2) + (local $0 f64) + (local $1 f64) + (block $block + (br_if $block + (i32.const 0) + ) + (loop $loop + (set_local $1 + (get_local $0) + ) + (set_local $0 + (f64.const -nan:0xfffffffffff87) + ) + (br_if $loop + (i32.const 1) + ) + ) + ) + (set_local $0 + (get_local $1) + ) + (if + (i32.const 0) + (drop + (get_local $0) + ) + ) + ) ) From 3a6086e682a0b347f5b3cb1faf2a8e8abe636925 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Fri, 31 Aug 2018 12:20:18 -0700 Subject: [PATCH 4/5] remove an unneeded function, and add comments --- src/literal.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/literal.h b/src/literal.h index 137a1c945b6..aa9fd970586 100644 --- a/src/literal.h +++ b/src/literal.h @@ -72,12 +72,10 @@ class Literal { double getFloat() const; int64_t getBits() const; // Equality checks for the type and the bits, so a nan float would - // be compared bitwise. + // be compared bitwise (which means that a Literal containing a nan + // would be equal to itself, if the bits are equal). bool operator==(const Literal& other) const; bool operator!=(const Literal& other) const; - // Compare in a non-bitwise manner, which means for a nan a we have - // a != a - bool nonBitwiseEqual(const Literal& other) const; static uint32_t NaNPayload(float f); static uint64_t NaNPayload(double f); @@ -134,6 +132,9 @@ class Literal { Literal rotL(const Literal& other) const; Literal rotR(const Literal& other) const; + // Note that these functions perform equality checks based + // on the type of the literal, so that (unlike the == operator) + // a float nan would not be identical to itself. Literal eq(const Literal& other) const; Literal ne(const Literal& other) const; Literal ltS(const Literal& other) const; From 87336c3b0bd5fc6168cc3b94883b8ae333d79c39 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Fri, 31 Aug 2018 13:56:07 -0700 Subject: [PATCH 5/5] remove unused function --- src/wasm/literal.cpp | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/wasm/literal.cpp b/src/wasm/literal.cpp index 622dd8d0d03..b5c575b1c9a 100644 --- a/src/wasm/literal.cpp +++ b/src/wasm/literal.cpp @@ -89,18 +89,6 @@ bool Literal::operator!=(const Literal& other) const { return !(*this == other); } -bool Literal::nonBitwiseEqual(const Literal& other) const { - if (type != other.type) return false; - switch (type) { - case Type::none: return true; - case Type::i32: return i32 == other.i32; - case Type::f32: return getf32() == other.getf32(); - case Type::i64: return i64 == other.i64; - case Type::f64: return getf64() == other.getf64(); - default: abort(); - } -} - uint32_t Literal::NaNPayload(float f) { assert(std::isnan(f) && "expected a NaN"); // SEEEEEEE EFFFFFFF FFFFFFFF FFFFFFFF