Fix UB in SIMD dot product arithmetic (#8234)

* Use Literal::add and Literal::mul to avoid overflow of signed
arithmetic. These do the right thing by casting to unsigned first.
* Use bit_cast instead of a C-style cast to convey the intent better
when converting between signed and unsigned.
* Also add a unit test for i8x16.avgr_u to demonstrate that it doesn't
overflow.

From reading the code, most other arithmetic in Literal.cpp shouldn't
invoke UB. There are some spec tests that are disabled due to float/nan
behavior that I'll look at in future PRs.

Author: stevenfontanella

scripts/test/shared.py

@@ -462,9 +462,6 @@ def get_tests(test_dir, extensions=[], recursive=False):
     'simd_f64x2.wast',            # Min of 0 and NaN should give a canonical NaN
     'simd_f64x2_arith.wast',      # Adding inf and -inf should give a canonical NaN
     'simd_f64x2_rounding.wast',   # Ceil of NaN should give a canonical NaN
-    'simd_i32x4_cmp.wast',        # UBSan error on integer overflow
-    'simd_i32x4_arith2.wast',     # UBSan error on integer overflow
-    'simd_i32x4_dot_i16x8.wast',  # UBSan error on integer overflow
     'token.wast',                 # Lexer should require spaces between strings and non-paren tokens
 ]
 

src/wasm/literal.cpp

@@ -1161,9 +1161,9 @@ Literal Literal::demote() const {
 Literal Literal::add(const Literal& other) const {
   switch (type.getBasic()) {
     case Type::i32:
-      return Literal(uint32_t(i32) + uint32_t(other.i32));
+      return Literal(bit_cast<uint32_t>(i32) + bit_cast<uint32_t>(other.i32));
     case Type::i64:
-      return Literal(uint64_t(i64) + uint64_t(other.i64));
+      return Literal(bit_cast<uint64_t>(i64) + bit_cast<uint64_t>(other.i64));
     case Type::f32:
       return standardizeNaN(Literal(getf32() + other.getf32()));
     case Type::f64:
@@ -1408,6 +1408,8 @@ Literal Literal::maxUInt(const Literal& other) const {
 }
 
 Literal Literal::avgrUInt(const Literal& other) const {
+  // This looks like it could overflow, but these are promoted from uint8 in the
+  // caller (`binary`).
   return Literal((geti32() + other.geti32() + 1) / 2);
 }
 
@@ -2652,8 +2654,7 @@ static Literal dot(const Literal& left, const Literal& right) {
   for (size_t i = 0; i < Lanes; ++i) {
     result[i] = Literal(int32_t(0));
     for (size_t j = 0; j < Factor; ++j) {
-      result[i] = Literal(result[i].geti32() + lhs[i * Factor + j].geti32() *
-                                                 rhs[i * Factor + j].geti32());
+      result[i] = result[i].add(lhs[i * Factor + j].mul(rhs[i * Factor + j]));
     }
   }
   return Literal(result);

test/lit/exec/simd.wast

@@ -23,12 +23,25 @@
    (i64.const -4611686018427387904)
   )
  )
+
+ ;; CHECK:      [fuzz-exec] calling i8x16.avgr_u
+ ;; CHECK-NEXT: [fuzz-exec] note result: i8x16.avgr_u => i32x4 0x80808080 0x80808080 0x80808080 0x80808080
+ (func $i8x16.avgr_u (export "i8x16.avgr_u") (result v128)
+  (i8x16.avgr_u
+   (v128.const i8x16 -128 -128 -128 -128 -128 -128 -128 -128 -128 -128 -128 -128 -128 -128 -128 -128)
+   (v128.const i8x16 -128 -128 -128 -128 -128 -128 -128 -128 -128 -128 -128 -128 -128 -128 -128 -128)
+  )
+ )
 )
 
 ;; CHECK:      [fuzz-exec] calling load8x8_s
 ;; CHECK-NEXT: [fuzz-exec] note result: load8x8_s => i32x4 0x00620061 0x00640063 0x00660065 0x00000067
 
 ;; CHECK:      [fuzz-exec] calling load32x2_u
 ;; CHECK-NEXT: [trap final > memory: 13835058055282163712 > 1048576]
+
+;; CHECK:      [fuzz-exec] calling i8x16.avgr_u
+;; CHECK-NEXT: [fuzz-exec] note result: i8x16.avgr_u => i32x4 0x80808080 0x80808080 0x80808080 0x80808080
+;; CHECK-NEXT: [fuzz-exec] comparing i8x16.avgr_u
 ;; CHECK-NEXT: [fuzz-exec] comparing load32x2_u
 ;; CHECK-NEXT: [fuzz-exec] comparing load8x8_s