Add and use readLE/writeLE helpers

According to godbolt.org these functions optimize to a simple *(T *)ptr
in many cases while ensuring that memory alignment requirements and
endianess does not effect the behavior.

The unroll pragma is needed for GCC to properly optimize the code.

The approach of using bit shifts is also used in multiple other parts
of binaryen (eg. WasmBinaryReader::getInt16) but usage of the helper
function doesn't seem to be that easy there.

Ref #2983

Author: sertonix

src/literal.h

@@ -20,6 +20,7 @@
 #include <array>
 #include <iostream>
 
+#include "support/bits.h"
 #include "support/hash.h"
 #include "support/name.h"
 #include "support/small_vector.h"
@@ -823,7 +824,8 @@ template<> struct hash<wasm::Literal> {
           return digest;
         case wasm::Type::v128:
           uint64_t chunks[2];
-          memcpy(&chunks, a.getv128Ptr(), 16);
+          chunks[0] = wasm::Bits::readLE<uint64_t>(a.getv128Ptr());
+          chunks[1] = wasm::Bits::readLE<uint64_t>(&a.getv128Ptr()[8]);
           wasm::rehash(digest, chunks[0]);
           wasm::rehash(digest, chunks[1]);
           return digest;

src/shell-interface.h

@@ -25,6 +25,7 @@
 #include "interpreter/exception.h"
 #include "ir/module-utils.h"
 #include "shared-constants.h"
+#include "support/bits.h"
 #include "support/name.h"
 #include "support/utilities.h"
 #include "wasm-interpreter.h"
@@ -33,20 +34,9 @@
 namespace wasm {
 
 struct ShellExternalInterface : ModuleRunner::ExternalInterface {
-  // The underlying memory can be accessed through unaligned pointers which
-  // isn't well-behaved in C++. WebAssembly nonetheless expects it to behave
-  // properly. Avoid emitting unaligned load/store by checking for alignment
-  // explicitly, and performing memcpy if unaligned.
-  //
-  // The allocated memory tries to have the same alignment as the memory being
-  // simulated.
   class Memory {
     // Use char because it doesn't run afoul of aliasing rules.
     std::vector<char> memory;
-    template<typename T> static bool aligned(const char* address) {
-      static_assert(!(sizeof(T) & (sizeof(T) - 1)), "must be a power of 2");
-      return 0 == (reinterpret_cast<uintptr_t>(address) & (sizeof(T) - 1));
-    }
 
   public:
     Memory() = default;
@@ -65,20 +55,10 @@ struct ShellExternalInterface : ModuleRunner::ExternalInterface {
       }
     }
     template<typename T> void set(size_t address, T value) {
-      if (aligned<T>(&memory[address])) {
-        *reinterpret_cast<T*>(&memory[address]) = value;
-      } else {
-        std::memcpy(&memory[address], &value, sizeof(T));
-      }
+      Bits::writeLE<T>(value, &memory[address]);
     }
     template<typename T> T get(size_t address) {
-      if (aligned<T>(&memory[address])) {
-        return *reinterpret_cast<T*>(&memory[address]);
-      } else {
-        T loaded;
-        std::memcpy(&loaded, &memory[address], sizeof(T));
-        return loaded;
-      }
+      return Bits::readLE<T>(&memory[address]);
     }
   };
 

src/support/bits.h

@@ -17,8 +17,10 @@
 #ifndef wasm_support_bits_h
 #define wasm_support_bits_h
 
+#include <array>
 #include <climits>
 #include <cstdint>
+#include <cstring>
 #include <type_traits>
 
 /*
@@ -94,6 +96,56 @@ template<typename T, typename U> inline static T rotateRight(T val, U count) {
 uint32_t log2(uint32_t v);
 uint32_t pow2(uint32_t v);
 
+template<
+  typename T,
+  typename std::enable_if<
+    std::is_same<T, typename std::array<uint8_t, std::tuple_size<T>::value>>::
+      value,
+    bool>::type = true>
+void writeLE(T val, void* ptr) {
+  memcpy(ptr, val.data(), sizeof(T));
+}
+
+template<typename T,
+         typename std::enable_if<std::is_integral<T>::value, bool>::type = true>
+void writeLE(T val, void* ptr) {
+  auto v = typename std::conditional<std::is_signed<T>::value,
+                                     typename std::make_unsigned<T>::type,
+                                     T>::type(val);
+  unsigned char* buf = reinterpret_cast<unsigned char*>(ptr);
+#pragma GCC unroll 10
+  for (size_t i = 0; i < sizeof(T); ++i) {
+    buf[i] = v >> (CHAR_BIT * i);
+  }
+}
+
+template<
+  typename T,
+  typename std::enable_if<
+    std::is_same<T, typename std::array<uint8_t, std::tuple_size<T>::value>>::
+      value,
+    bool>::type = true>
+T readLE(const void* ptr) {
+  T v;
+  memcpy(v.data(), ptr, sizeof(T));
+  return v;
+}
+
+template<typename T,
+         typename std::enable_if<std::is_integral<T>::value, bool>::type = true>
+T readLE(const void* ptr) {
+  using TU = typename std::conditional<std::is_signed<T>::value,
+                                       typename std::make_unsigned<T>::type,
+                                       T>::type;
+  TU v = 0;
+  const unsigned char* buf = reinterpret_cast<const unsigned char*>(ptr);
+#pragma GCC unroll 10
+  for (size_t i = 0; i < sizeof(T); ++i) {
+    v += (TU)buf[i] << (CHAR_BIT * i);
+  }
+  return v;
+}
+
 } // namespace wasm::Bits
 
 #endif // wasm_support_bits_h

src/tools/wasm-ctor-eval.cpp

@@ -32,6 +32,7 @@
 #include "ir/memory-utils.h"
 #include "ir/names.h"
 #include "pass.h"
+#include "support/bits.h"
 #include "support/colors.h"
 #include "support/file.h"
 #include "support/insert_ordered.h"
@@ -497,15 +498,11 @@ struct CtorEvalExternalInterface : EvallingModuleRunner::ExternalInterface {
   }
 
   template<typename T> void doStore(Address address, T value, Name memoryName) {
-    // Use memcpy to avoid UB if unaligned.
-    memcpy(getMemory(address, memoryName, sizeof(T)), &value, sizeof(T));
+    Bits::writeLE<T>(value, getMemory(address, memoryName, sizeof(T)));
   }
 
   template<typename T> T doLoad(Address address, Name memoryName) {
-    // Use memcpy to avoid UB if unaligned.
-    T ret;
-    memcpy(&ret, getMemory(address, memoryName, sizeof(T)), sizeof(T));
-    return ret;
+    return Bits::readLE<T>(getMemory(address, memoryName, sizeof(T)));
   }
 
   // Clear the state of the operation of applying the interpreter's runtime

src/tools/wasm-fuzz-lattices.cpp

@@ -36,6 +36,7 @@
 #include "analysis/reaching-definitions-transfer-function.h"
 #include "analysis/transfer-function.h"
 
+#include "support/bits.h"
 #include "support/command-line.h"
 #include "tools/fuzzing.h"
 #include "tools/fuzzing/random.h"
@@ -995,7 +996,7 @@ struct Fuzzer {
     // Fewer bytes are needed to generate three random lattices.
     std::vector<char> funcBytes(128);
     for (size_t i = 0; i < funcBytes.size(); i += sizeof(uint64_t)) {
-      *(uint64_t*)(funcBytes.data() + i) = getFuncRand();
+      Bits::writeLE<uint64_t>(getFuncRand(), funcBytes.data() + i);
     }
 
     Random rand(std::move(funcBytes));
@@ -1030,7 +1031,7 @@ struct Fuzzer {
     // 4kb of random bytes should be enough for anyone!
     std::vector<char> bytes(4096);
     for (size_t i = 0; i < bytes.size(); i += sizeof(uint64_t)) {
-      *(uint64_t*)(bytes.data() + i) = getRand();
+      Bits::writeLE<uint64_t>(getRand(), bytes.data() + i);
     }
 
     Module testModule;

src/tools/wasm-fuzz-types.cpp

@@ -20,6 +20,7 @@
 #include <string>
 #include <variant>
 
+#include "support/bits.h"
 #include "support/command-line.h"
 #include "tools/fuzzing/heap-types.h"
 #include "tools/fuzzing/random.h"
@@ -68,7 +69,7 @@ void Fuzzer::run(uint64_t seed) {
   // 4kb of random bytes should be enough for anyone!
   std::vector<char> bytes(4096);
   for (size_t i = 0; i < bytes.size(); i += sizeof(uint64_t)) {
-    *(uint64_t*)(bytes.data() + i) = getRand();
+    Bits::writeLE<uint64_t>(getRand(), bytes.data() + i);
   }
   rand = Random(std::move(bytes));
 

src/wasm-interpreter.h

@@ -2777,14 +2777,12 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
       case Field::NotPacked:
         return Literal::makeFromMemory(p, field.type);
       case Field::i8: {
-        int8_t i;
-        memcpy(&i, p, sizeof(i));
-        return truncateForPacking(Literal(int32_t(i)), field);
+        return truncateForPacking(Literal(int32_t(Bits::readLE<int8_t>(p))),
+                                  field);
       }
       case Field::i16: {
-        int16_t i;
-        memcpy(&i, p, sizeof(i));
-        return truncateForPacking(Literal(int32_t(i)), field);
+        return truncateForPacking(Literal(int32_t(Bits::readLE<int16_t>(p))),
+                                  field);
       }
       case Field::WaitQueue: {
         WASM_UNREACHABLE("waitqueue not implemented");

src/wasm/literal.cpp

@@ -240,8 +240,7 @@ static void extractBytes(uint8_t (&dest)[16], const LaneArray<Lanes>& lanes) {
   for (size_t lane_index = 0; lane_index < Lanes; ++lane_index) {
     uint8_t bits[16];
     lanes[lane_index].getBits(bits);
-    LaneT lane;
-    memcpy(&lane, bits, sizeof(lane));
+    LaneT lane = Bits::readLE<LaneT>(bits);
     for (size_t offset = 0; offset < lane_width; ++offset) {
       bytes.at(lane_index * lane_width + offset) =
         uint8_t(lane >> (8 * offset));
@@ -316,24 +315,16 @@ Literal Literal::makeFromMemory(void* p, Type type) {
   assert(type.isNumber());
   switch (type.getBasic()) {
     case Type::i32: {
-      int32_t i;
-      memcpy(&i, p, sizeof(i));
-      return Literal(i);
+      return Literal(Bits::readLE<int32_t>(p));
     }
     case Type::i64: {
-      int64_t i;
-      memcpy(&i, p, sizeof(i));
-      return Literal(i);
+      return Literal(Bits::readLE<int64_t>(p));
     }
     case Type::f32: {
-      int32_t i;
-      memcpy(&i, p, sizeof(i));
-      return Literal(bit_cast<float>(i));
+      return Literal(bit_cast<float>(Bits::readLE<int32_t>(p)));
     }
     case Type::f64: {
-      int64_t i;
-      memcpy(&i, p, sizeof(i));
-      return Literal(bit_cast<double>(i));
+      return Literal(bit_cast<double>(Bits::readLE<int64_t>(p)));
     }
     case Type::v128: {
       uint8_t bytes[16];
@@ -460,11 +451,11 @@ void Literal::getBits(uint8_t (&buf)[16]) const {
   switch (type.getBasic()) {
     case Type::i32:
     case Type::f32:
-      memcpy(buf, &i32, sizeof(i32));
+      Bits::writeLE<int32_t>(i32, buf);
       break;
     case Type::i64:
     case Type::f64:
-      memcpy(buf, &i64, sizeof(i64));
+      Bits::writeLE<int64_t>(i64, buf);
       break;
     case Type::v128:
       memcpy(buf, &v128, sizeof(v128));