From e3c38c14e7dd9c115da960daafd109d2687f1a08 Mon Sep 17 00:00:00 2001 From: JF Bastien Date: Sun, 10 Jan 2016 10:49:59 -0800 Subject: Add Travis builds with sanitizers This triggers 5 independent build / test runs: - clang, no sanitizer; - clang, UB sanitizer; - clang, address sanitizer (disabled for now); - clang, thread sanitizer (disabled for now); - GCC. Enabling UBSan led to these changes: - Fix a bunch of undefined behavior throughout the code base. - Fix some tests that relied on that undefined behavior. - Make some of the tests easier to debug by printing their command line. - Add ubsan blacklist to work around libstdc++ bug. - Example testcase also needs sanitizer because libsupport.a uses it. --- src/asm2wasm.h | 9 ++-- src/emscripten-optimizer/optimizer-shared.cpp | 21 ++------ src/emscripten-optimizer/optimizer.h | 4 -- src/emscripten-optimizer/parser.h | 18 +++---- src/emscripten-optimizer/simple_ast.h | 29 ++++++----- src/s2wasm.h | 28 ++++++++--- src/support/safe_integer.cpp | 70 +++++++++++++++++++++++++++ src/support/safe_integer.h | 34 +++++++++++++ src/wasm.h | 5 +- src/wasm2asm.h | 4 +- 10 files changed, 165 insertions(+), 57 deletions(-) create mode 100644 src/support/safe_integer.cpp create mode 100644 src/support/safe_integer.h (limited to 'src') diff --git a/src/asm2wasm.h b/src/asm2wasm.h index db7cc1ae0..c72c7e98a 100644 --- a/src/asm2wasm.h +++ b/src/asm2wasm.h @@ -385,7 +385,7 @@ private: } if (ast[1] == MINUS && ast[2][0] == NUM) { double num = -ast[2][1]->getNumber(); - assert(isInteger32(num)); + assert(isSInteger32(num)); return Literal((int32_t)num); } if (ast[1] == PLUS && ast[2][0] == UNARY_PREFIX && ast[2][1] == MINUS && ast[2][2][0] == NUM) { @@ -912,9 +912,12 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) { } else if (what == NUM) { auto ret = allocator.alloc(); double num = ast[1]->getNumber(); - if (isInteger32(num)) { + if (isSInteger32(num)) { ret->value.type = WasmType::i32; - ret->value.i32 = toInteger32(num); + ret->value.i32 = toSInteger32(num); + } else if (isUInteger32(num)) { + ret->value.type = WasmType::i32; + ret->value.i32 = toUInteger32(num); } else { ret->value.type = WasmType::f64; ret->value.f64 = num; diff --git a/src/emscripten-optimizer/optimizer-shared.cpp b/src/emscripten-optimizer/optimizer-shared.cpp index 4466fb0e9..6831d81b0 100644 --- a/src/emscripten-optimizer/optimizer-shared.cpp +++ b/src/emscripten-optimizer/optimizer-shared.cpp @@ -14,7 +14,10 @@ * limitations under the License. */ +#include + #include "optimizer.h" +#include "support/safe_integer.h" using namespace cashew; @@ -26,20 +29,6 @@ IString SIMD_INT8X16_CHECK("SIMD_Int8x16_check"), SIMD_FLOAT32X4_CHECK("SIMD_Float32x4_check"), SIMD_FLOAT64X2_CHECK("SIMD_Float64x2_check"); -bool isInteger(double x) { - return fmod(x, 1) == 0; -} - -bool isInteger32(double x) { - return isInteger(x) && (x == (int32_t)x || x == (uint32_t)x); -} - -int32_t toInteger32(double x) { - if (x == (int32_t)x) return (int32_t)x; - assert(x == (uint32_t)x); - return (uint32_t)x; -} - int parseInt(const char *str) { int ret = *str - '0'; while (*(++str)) { @@ -67,7 +56,7 @@ AsmType detectType(Ref node, AsmData *asmData, bool inVarDef, IString minifiedFr switch (node[0]->getCString()[0]) { case 'n': { if (node[0] == NUM) { - if (!isInteger(node[1]->getNumber())) return ASM_DOUBLE; + if (!wasm::isInteger(node[1]->getNumber())) return ASM_DOUBLE; return ASM_INT; } else if (node[0] == NAME) { if (asmData) { @@ -176,7 +165,7 @@ AsmSign detectSign(Ref node, IString minifiedFround) { double value = node[1]->getNumber(); if (value < 0) return ASM_SIGNED; if (value > uint32_t(-1) || fmod(value, 1) != 0) return ASM_NONSIGNED; - if (value == int32_t(value)) return ASM_FLEXIBLE; + if (wasm::isSInteger32(value)) return ASM_FLEXIBLE; return ASM_UNSIGNED; } else if (type == NAME) { return ASM_FLEXIBLE; diff --git a/src/emscripten-optimizer/optimizer.h b/src/emscripten-optimizer/optimizer.h index 451a9e286..684fc0164 100644 --- a/src/emscripten-optimizer/optimizer.h +++ b/src/emscripten-optimizer/optimizer.h @@ -116,10 +116,6 @@ struct AsmData { } }; -bool isInteger(double x); -bool isInteger32(double x); -int32_t toInteger32(double x); - extern cashew::IString ASM_FLOAT_ZERO; extern cashew::IString SIMD_INT8X16_CHECK, diff --git a/src/emscripten-optimizer/parser.h b/src/emscripten-optimizer/parser.h index c805ca9f6..da3e6f5c7 100644 --- a/src/emscripten-optimizer/parser.h +++ b/src/emscripten-optimizer/parser.h @@ -22,13 +22,14 @@ #ifndef wasm_parser_h #define wasm_parser_h -#include -#include #include - -#include +#include +#include +#include +#include #include "istring.h" +#include "support/safe_integer.h" namespace cashew { @@ -179,10 +180,6 @@ class Parser { static bool hasChar(const char* list, char x) { while (*list) if (*list++ == x) return true; return false; } - static bool is32Bit(double x) { - return x == (int)x || x == (unsigned int)x; - } - // An atomic fragment of something. Stops at a natural boundary. enum FragType { KEYWORD = 0, @@ -249,7 +246,10 @@ class Parser { // for valid asm.js input, the '.' should be enough, and for uglify // in the emscripten optimizer pipeline, we use simple_ast where INT/DOUBLE // is quite the same at this point anyhow - type = (std::find(start, src, '.') == src && is32Bit(num)) ? INT : DOUBLE; + type = (std::find(start, src, '.') == src && + (wasm::isSInteger32(num) || wasm::isUInteger32(num))) + ? INT + : DOUBLE; assert(src > start); } else if (hasChar(OPERATOR_INITS, *src)) { switch (*src) { diff --git a/src/emscripten-optimizer/simple_ast.h b/src/emscripten-optimizer/simple_ast.h index 712845dea..73037815f 100644 --- a/src/emscripten-optimizer/simple_ast.h +++ b/src/emscripten-optimizer/simple_ast.h @@ -17,26 +17,25 @@ #ifndef wasm_simple_ast_h #define wasm_simple_ast_h -#include -#include -#include -#include -#include - -#include -#include -#include +#include +#include +#include +#include +#include +#include +#include #include +#include #include -#include -#include +#include #include -#include #include +#include +#include #include "parser.h" - #include "snprintf.h" +#include "support/safe_integer.h" #define err(str) fprintf(stderr, str "\n"); #define errv(str, ...) fprintf(stderr, str "\n", __VA_ARGS__); @@ -870,8 +869,8 @@ struct JSPrinter { } else { // integer assert(d >= 0); - unsigned long long uu = (unsigned long long)d; - if (uu == d) { + if (wasm::isUInteger64(d)) { + unsigned long long uu = wasm::toUInteger64(d); bool asHex = e && !finalize; snprintf(buffer, BUFFERSIZE-1, asHex ? "0x%llx" : "%llu", uu); if (asHex) { diff --git a/src/s2wasm.h b/src/s2wasm.h index bd3f12e73..d1836b274 100644 --- a/src/s2wasm.h +++ b/src/s2wasm.h @@ -159,16 +159,23 @@ private: return cashew::IString(str.c_str(), false); } - int32_t getInt() { - int32_t ret = 0; + uint32_t getInt() { + uint32_t ret = 0; bool neg = false; if (*s == '-') { neg = true; s++; } while (isdigit(*s)) { + uint32_t digit = *s - '0'; + if (ret > std::numeric_limits::max() / 10) { + abort_on("overflow"); + } ret *= 10; - ret += (*s - '0'); + if (ret > std::numeric_limits::max() - digit) { + abort_on("overflow"); + } + ret += digit; s++; } if (neg) ret = -ret; @@ -197,16 +204,23 @@ private: } } - int64_t getInt64() { - int64_t ret = 0; + uint64_t getInt64() { + uint64_t ret = 0; bool neg = false; if (*s == '-') { neg = true; s++; } while (isdigit(*s)) { + uint64_t digit = *s - '0'; + if (ret > std::numeric_limits::max() / 10) { + abort_on("overflow"); + } ret *= 10; - ret += (*s - '0'); + if (ret > std::numeric_limits::max() - digit) { + abort_on("overflow"); + } + ret += digit; s++; } if (neg) ret = -ret; @@ -933,7 +947,7 @@ private: } else if (match(".int64")) { size_t size = raw->size(); raw->resize(size + 8); - (*(int64_t*)(&(*raw)[size])) = getInt(); + (*(int64_t*)(&(*raw)[size])) = getInt64(); zero = false; } else { break; diff --git a/src/support/safe_integer.cpp b/src/support/safe_integer.cpp new file mode 100644 index 000000000..46057cede --- /dev/null +++ b/src/support/safe_integer.cpp @@ -0,0 +1,70 @@ +/* + * Copyright 2016 WebAssembly Community Group participants + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +#include "support/safe_integer.h" + +using namespace wasm; + +bool wasm::isInteger(double x) { return fmod(x, 1) == 0; } + +bool wasm::isUInteger32(double x) { + return isInteger(x) && x >= 0 && x <= std::numeric_limits::max(); +} + +bool wasm::isSInteger32(double x) { + return isInteger(x) && x >= std::numeric_limits::min() && + x <= std::numeric_limits::max(); +} + +uint32_t wasm::toUInteger32(double x) { + return x < std::numeric_limits::max() + ? x + : std::numeric_limits::max(); +} + +int32_t wasm::toSInteger32(double x) { + return x > std::numeric_limits::min() && + x < std::numeric_limits::max() + ? x + : (x < 0 ? std::numeric_limits::min() + : std::numeric_limits::max()); +} + +bool wasm::isUInteger64(double x) { + return isInteger(x) && x >= 0 && x <= std::numeric_limits::max(); +} + +bool wasm::isSInteger64(double x) { + return isInteger(x) && x >= std::numeric_limits::min() && + x <= std::numeric_limits::max(); +} + +uint64_t wasm::toUInteger64(double x) { + return x < (double)std::numeric_limits::max() + ? (uint64_t)x + : std::numeric_limits::max(); +} + +int64_t wasm::toSInteger64(double x) { + return x > (double)std::numeric_limits::min() && + x < (double)std::numeric_limits::max() + ? (int64_t)x + : (x < 0 ? std::numeric_limits::min() + : std::numeric_limits::max()); +} diff --git a/src/support/safe_integer.h b/src/support/safe_integer.h new file mode 100644 index 000000000..f240644c8 --- /dev/null +++ b/src/support/safe_integer.h @@ -0,0 +1,34 @@ +/* + * Copyright 2016 WebAssembly Community Group participants + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef wasm_safe_integer_h +#define wasm_safe_integer_h + +#include + +namespace wasm { +bool isInteger(double x); +bool isUInteger32(double x); +bool isSInteger32(double x); +uint32_t toUInteger32(double x); +int32_t toSInteger32(double x); +bool isUInteger64(double x); +bool isSInteger64(double x); +uint64_t toUInteger64(double x); +int64_t toSInteger64(double x); +} // namespace wasm + +#endif // wasm_safe_integer_h diff --git a/src/wasm.h b/src/wasm.h index b1b8d84d4..c4654580a 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -45,13 +45,14 @@ #define wasm_wasm_h #include +#include #include #include #include #include #include -#include #include +#include #include "compiler-support.h" #include "emscripten-optimizer/simple_ast.h" @@ -200,7 +201,7 @@ struct Literal { } static void printDouble(std::ostream &o, double d) { - if (d == 0 && 1/d < 0) { + if (d == 0 && std::signbit(d)) { o << "-0"; return; } diff --git a/src/wasm2asm.h b/src/wasm2asm.h index 53cb6df78..1a85bc177 100644 --- a/src/wasm2asm.h +++ b/src/wasm2asm.h @@ -22,6 +22,8 @@ #ifndef wasm_wasm2asm_h #define wasm_wasm2asm_h +#include + #include "wasm.h" #include "emscripten-optimizer/optimizer.h" #include "mixed_arena.h" @@ -914,7 +916,7 @@ Ref Wasm2AsmBuilder::processFunctionBody(Expression* curr, IString result) { } case f64: { double d = curr->value.f64; - if (d == 0 && 1/d < 0) { // negative zero + if (d == 0 && std::signbit(d)) { // negative zero return ValueBuilder::makeUnary(PLUS, ValueBuilder::makeUnary(MINUS, ValueBuilder::makeDouble(0))); } return ValueBuilder::makeUnary(PLUS, ValueBuilder::makeDouble(curr->value.f64)); -- cgit v1.2.3 From 16ed70cb09569b881b6416955000fa7902119264 Mon Sep 17 00:00:00 2001 From: JF Bastien Date: Sun, 10 Jan 2016 19:51:50 -0800 Subject: Asm2WasmBuilder: allow building u/s int32. --- src/asm2wasm.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/asm2wasm.h b/src/asm2wasm.h index c72c7e98a..7992de8af 100644 --- a/src/asm2wasm.h +++ b/src/asm2wasm.h @@ -385,8 +385,9 @@ private: } if (ast[1] == MINUS && ast[2][0] == NUM) { double num = -ast[2][1]->getNumber(); - assert(isSInteger32(num)); - return Literal((int32_t)num); + if (isSInteger32(num)) return Literal((int32_t)num); + if (isUInteger32(num)) return Literal((uint32_t)num); + assert(false && "expected signed or unsigned int32"); } if (ast[1] == PLUS && ast[2][0] == UNARY_PREFIX && ast[2][1] == MINUS && ast[2][2][0] == NUM) { return Literal((double)-ast[2][2][1]->getNumber()); -- cgit v1.2.3 From eb0444e0870052b8ea8021eb50563a9f4d957486 Mon Sep 17 00:00:00 2001 From: JF Bastien Date: Sun, 10 Jan 2016 19:56:36 -0800 Subject: Safe integer: assert before converting double to integer. --- src/support/safe_integer.cpp | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src') diff --git a/src/support/safe_integer.cpp b/src/support/safe_integer.cpp index 46057cede..dbe62ca52 100644 --- a/src/support/safe_integer.cpp +++ b/src/support/safe_integer.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include @@ -33,12 +34,14 @@ bool wasm::isSInteger32(double x) { } uint32_t wasm::toUInteger32(double x) { + assert(isUInteger32(x)); return x < std::numeric_limits::max() ? x : std::numeric_limits::max(); } int32_t wasm::toSInteger32(double x) { + assert(isSInteger32(x)); return x > std::numeric_limits::min() && x < std::numeric_limits::max() ? x @@ -56,12 +59,14 @@ bool wasm::isSInteger64(double x) { } uint64_t wasm::toUInteger64(double x) { + assert(isUInteger64(x)); return x < (double)std::numeric_limits::max() ? (uint64_t)x : std::numeric_limits::max(); } int64_t wasm::toSInteger64(double x) { + assert(isSInteger64(x)); return x > (double)std::numeric_limits::min() && x < (double)std::numeric_limits::max() ? (int64_t)x -- cgit v1.2.3 From 873e5b0500a3edb07a09ed285ba8421be375cd25 Mon Sep 17 00:00:00 2001 From: JF Bastien Date: Mon, 11 Jan 2016 11:32:25 -0800 Subject: Check for negative overflow. --- src/s2wasm.h | 80 +++++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 50 insertions(+), 30 deletions(-) (limited to 'src') diff --git a/src/s2wasm.h b/src/s2wasm.h index d1836b274..a0ecfd710 100644 --- a/src/s2wasm.h +++ b/src/s2wasm.h @@ -159,27 +159,37 @@ private: return cashew::IString(str.c_str(), false); } - uint32_t getInt() { - uint32_t ret = 0; + int32_t getInt() { + const char* loc = s; + uint32_t value = 0; bool neg = false; - if (*s == '-') { + if (*loc == '-') { neg = true; - s++; + loc++; } - while (isdigit(*s)) { - uint32_t digit = *s - '0'; - if (ret > std::numeric_limits::max() / 10) { - abort_on("overflow"); + while (isdigit(*loc)) { + uint32_t digit = *loc - '0'; + if (value > std::numeric_limits::max() / 10) { + abort_on("uint32_t overflow"); } - ret *= 10; - if (ret > std::numeric_limits::max() - digit) { - abort_on("overflow"); + value *= 10; + if (value > std::numeric_limits::max() - digit) { + abort_on("uint32_t overflow"); } - ret += digit; - s++; + value += digit; + loc++; + } + if (neg) { + uint32_t positive_int_min = + (uint32_t) - (1 + std::numeric_limits::min()) + (uint32_t)1; + if (value > positive_int_min) { + abort_on("negative int32_t overflow"); + } + s = loc; + return -value; } - if (neg) ret = -ret; - return ret; + s = loc; + return value; } // gets a constant, which may be a relocation for later. @@ -204,27 +214,37 @@ private: } } - uint64_t getInt64() { - uint64_t ret = 0; + int64_t getInt64() { + const char* loc = s; + uint64_t value = 0; bool neg = false; - if (*s == '-') { + if (*loc == '-') { neg = true; - s++; + loc++; } - while (isdigit(*s)) { - uint64_t digit = *s - '0'; - if (ret > std::numeric_limits::max() / 10) { - abort_on("overflow"); + while (isdigit(*loc)) { + uint64_t digit = *loc - '0'; + if (value > std::numeric_limits::max() / 10) { + abort_on("uint64_t overflow"); } - ret *= 10; - if (ret > std::numeric_limits::max() - digit) { - abort_on("overflow"); + value *= 10; + if (value > std::numeric_limits::max() - digit) { + abort_on("uint64_t overflow"); } - ret += digit; - s++; + value += digit; + loc++; + } + if (neg) { + uint64_t positive_int_min = + (uint64_t) - (1 + std::numeric_limits::min()) + (uint64_t)1; + if (value > positive_int_min) { + abort_on("negative int64_t overflow"); + } + s = loc; + return -value; } - if (neg) ret = -ret; - return ret; + s = loc; + return value; } Name getCommaSeparated() { -- cgit v1.2.3