Do not reverse bytes for NullVM. (#282)

Fixes #294. Signed-off-by: Konstantin Maksimov <konstantin.maksimov@ibm.com>
proxy-wasm · Oct 2, 2022 · 537b944 · 537b944
1 parent 4fcf895
commit 537b944
Show file tree

Hide file tree

Showing 15 changed files with 105 additions and 22 deletions.
diff --git a/include/proxy-wasm/null_vm.h b/include/proxy-wasm/null_vm.h
@@ -61,6 +61,7 @@ struct NullVm : public WasmVm {
 #undef _REGISTER_CALLBACK
 
   void terminate() override {}
+  bool usesWasmByteOrder() override { return false; }
 
   std::string plugin_name_;
   std::unique_ptr<NullVmPlugin> plugin_;

diff --git a/include/proxy-wasm/wasm_vm.h b/include/proxy-wasm/wasm_vm.h
@@ -303,6 +303,12 @@ class WasmVm {
    */
   virtual void terminate() = 0;
 
+  /**
+   * Byte order flag (host or wasm).
+   * @return 'false' for a null VM and 'true' for a wasm VM.
+   */
+  virtual bool usesWasmByteOrder() = 0;
+
   bool isFailed() { return failed_ != FailState::Ok; }
   void fail(FailState fail_state, std::string_view message) {
     integration()->error(message);

diff --git a/include/proxy-wasm/word.h b/include/proxy-wasm/word.h
@@ -24,11 +24,11 @@ namespace proxy_wasm {
 // Use byteswap functions only when compiling for big-endian platforms.
 #if defined(__BYTE_ORDER__) && defined(__ORDER_BIG_ENDIAN__) &&                                    \
     __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
-#define htowasm(x) __builtin_bswap32(x)
-#define wasmtoh(x) __builtin_bswap32(x)
+#define htowasm(x, vm_uses_wasm_byte_order) ((vm_uses_wasm_byte_order) ? __builtin_bswap32(x) : (x))
+#define wasmtoh(x, vm_uses_wasm_byte_order) ((vm_uses_wasm_byte_order) ? __builtin_bswap32(x) : (x))
 #else
-#define htowasm(x) (x)
-#define wasmtoh(x) (x)
+#define htowasm(x, vm_uses_wasm_byte_order) (x)
+#define wasmtoh(x, vm_uses_wasm_byte_order) (x)
 #endif
 
 // Represents a Wasm-native word-sized datum. On 32-bit VMs, the high bits are always zero.

diff --git a/src/exports.cc b/src/exports.cc
@@ -693,8 +693,9 @@ Word writevImpl(Word fd, Word iovs, Word iovs_len, Word *nwritten_ptr) {
     }
     const auto *iovec = reinterpret_cast<const uint32_t *>(memslice.value().data());
     if (iovec[1] != 0U /* buf_len */) {
-      memslice = context->wasmVm()->getMemory(wasmtoh(iovec[0]) /* buf */,
-                                              wasmtoh(iovec[1]) /* buf_len */);
+      const auto buf = wasmtoh(iovec[0], context->wasmVm()->usesWasmByteOrder());
+      const auto buf_len = wasmtoh(iovec[1], context->wasmVm()->usesWasmByteOrder());
+      memslice = context->wasmVm()->getMemory(buf, buf_len);
       if (!memslice) {
         return 21; // __WASI_EFAULT
       }

diff --git a/src/pairs_util.cc b/src/pairs_util.cc
@@ -19,6 +19,7 @@
 #include <string_view>
 #include <vector>
 
+#include "include/proxy-wasm/exports.h"
 #include "include/proxy-wasm/limits.h"
 #include "include/proxy-wasm/word.h"
 
@@ -45,7 +46,10 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size) {
   const char *end = buffer + size;
 
   // Write number of pairs.
-  uint32_t num_pairs = htowasm(pairs.size());
+  uint32_t num_pairs =
+      htowasm(pairs.size(), contextOrEffectiveContext() != nullptr
+                                ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()
+                                : false);
   if (pos + sizeof(uint32_t) > end) {
     return false;
   }
@@ -54,15 +58,21 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size) {
 
   for (const auto &p : pairs) {
     // Write name length.
-    uint32_t name_len = htowasm(p.first.size());
+    uint32_t name_len =
+        htowasm(p.first.size(), contextOrEffectiveContext() != nullptr
+                                    ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()
+                                    : false);
     if (pos + sizeof(uint32_t) > end) {
       return false;
     }
     ::memcpy(pos, &name_len, sizeof(uint32_t));
     pos += sizeof(uint32_t);
 
     // Write value length.
-    uint32_t value_len = htowasm(p.second.size());
+    uint32_t value_len =
+        htowasm(p.second.size(), contextOrEffectiveContext() != nullptr
+                                     ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()
+                                     : false);
     if (pos + sizeof(uint32_t) > end) {
       return false;
     }
@@ -103,7 +113,10 @@ Pairs PairsUtil::toPairs(std::string_view buffer) {
   if (pos + sizeof(uint32_t) > end) {
     return {};
   }
-  uint32_t num_pairs = wasmtoh(*reinterpret_cast<const uint32_t *>(pos));
+  uint32_t num_pairs = wasmtoh(*reinterpret_cast<const uint32_t *>(pos),
+                               contextOrEffectiveContext() != nullptr
+                                   ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()
+                                   : false);
   pos += sizeof(uint32_t);
 
   // Check if we're not going to exceed the limit.
@@ -122,14 +135,20 @@ Pairs PairsUtil::toPairs(std::string_view buffer) {
     if (pos + sizeof(uint32_t) > end) {
       return {};
     }
-    s.first = wasmtoh(*reinterpret_cast<const uint32_t *>(pos));
+    s.first = wasmtoh(*reinterpret_cast<const uint32_t *>(pos),
+                      contextOrEffectiveContext() != nullptr
+                          ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()
+                          : false);
     pos += sizeof(uint32_t);
 
     // Read value length.
     if (pos + sizeof(uint32_t) > end) {
       return {};
     }
-    s.second = wasmtoh(*reinterpret_cast<const uint32_t *>(pos));
+    s.second = wasmtoh(*reinterpret_cast<const uint32_t *>(pos),
+                       contextOrEffectiveContext() != nullptr
+                           ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()
+                           : false);
     pos += sizeof(uint32_t);
   }
 

diff --git a/src/signature_util.cc b/src/signature_util.cc
@@ -86,7 +86,7 @@ bool SignatureUtil::verifySignature(std::string_view bytecode, std::string &mess
 
   uint32_t alg_id;
   std::memcpy(&alg_id, payload.data(), sizeof(uint32_t));
-  alg_id = wasmtoh(alg_id);
+  alg_id = wasmtoh(alg_id, true);
 
   if (alg_id != 2) {
     message = "Signature has a wrong alg_id (want: 2, is: " + std::to_string(alg_id) + ")";

diff --git a/src/v8/v8.cc b/src/v8/v8.cc
@@ -105,6 +105,7 @@ class V8 : public WasmVm {
 #undef _GET_MODULE_FUNCTION
 
   void terminate() override;
+  bool usesWasmByteOrder() override { return true; }
 
 private:
   wasm::own<wasm::Trap> trap(std::string message);
@@ -503,7 +504,7 @@ bool V8::getWord(uint64_t pointer, Word *word) {
   }
   uint32_t word32;
   ::memcpy(&word32, memory_->data() + pointer, size);
-  word->u64_ = wasmtoh(word32);
+  word->u64_ = wasmtoh(word32, true);
   return true;
 }
 
@@ -516,7 +517,7 @@ bool V8::setWord(uint64_t pointer, Word word) {
   if (pointer + size > memory_->data_size()) {
     return false;
   }
-  uint32_t word32 = htowasm(word.u32());
+  uint32_t word32 = htowasm(word.u32(), true);
   ::memcpy(memory_->data() + pointer, &word32, size);
   return true;
 }

diff --git a/src/wamr/wamr.cc b/src/wamr/wamr.cc
@@ -87,6 +87,7 @@ class Wamr : public WasmVm {
 #undef _GET_MODULE_FUNCTION
 
   void terminate() override {}
+  bool usesWasmByteOrder() override { return true; }
 
 private:
   template <typename... Args>
@@ -368,7 +369,7 @@ bool Wamr::getWord(uint64_t pointer, Word *word) {
 
   uint32_t word32;
   ::memcpy(&word32, wasm_memory_data(memory_.get()) + pointer, size);
-  word->u64_ = wasmtoh(word32);
+  word->u64_ = wasmtoh(word32, true);
   return true;
 }
 
@@ -377,7 +378,7 @@ bool Wamr::setWord(uint64_t pointer, Word word) {
   if (pointer + size > wasm_memory_data_size(memory_.get())) {
     return false;
   }
-  uint32_t word32 = htowasm(word.u32());
+  uint32_t word32 = htowasm(word.u32(), true);
   ::memcpy(wasm_memory_data(memory_.get()) + pointer, &word32, size);
   return true;
 }

diff --git a/src/wasmedge/wasmedge.cc b/src/wasmedge/wasmedge.cc
@@ -281,6 +281,7 @@ class WasmEdge : public WasmVm {
                              std::function<R(ContextBase *, Args...)> *function);
 
   void terminate() override {}
+  bool usesWasmByteOrder() override { return true; }
 
   WasmEdgeLoaderPtr loader_;
   WasmEdgeValidatorPtr validator_;

diff --git a/src/wasmtime/wasmtime.cc b/src/wasmtime/wasmtime.cc
@@ -98,6 +98,7 @@ class Wasmtime : public WasmVm {
                              std::function<R(ContextBase *, Args...)> *function);
 
   void terminate() override {}
+  bool usesWasmByteOrder() override { return true; }
 
   WasmStorePtr store_;
   WasmModulePtr module_;
@@ -394,7 +395,7 @@ bool Wasmtime::getWord(uint64_t pointer, Word *word) {
 
   uint32_t word32;
   ::memcpy(&word32, wasm_memory_data(memory_.get()) + pointer, size);
-  word->u64_ = wasmtoh(word32);
+  word->u64_ = wasmtoh(word32, true);
   return true;
 }
 
@@ -403,7 +404,7 @@ bool Wasmtime::setWord(uint64_t pointer, Word word) {
   if (pointer + size > wasm_memory_data_size(memory_.get())) {
     return false;
   }
-  uint32_t word32 = htowasm(word.u32());
+  uint32_t word32 = htowasm(word.u32(), true);
   ::memcpy(wasm_memory_data(memory_.get()) + pointer, &word32, size);
   return true;
 }

diff --git a/src/wavm/wavm.cc b/src/wavm/wavm.cc
@@ -230,6 +230,7 @@ struct Wavm : public WasmVm {
 #undef _REGISTER_CALLBACK
 
   void terminate() override {}
+  bool usesWasmByteOrder() override { return true; }
 
   IR::Module ir_module_;
   WAVM::Runtime::ModuleRef module_ = nullptr;
@@ -389,12 +390,12 @@ bool Wavm::getWord(uint64_t pointer, Word *data) {
   auto *p = reinterpret_cast<char *>(memory_base_ + pointer);
   uint32_t data32;
   memcpy(&data32, p, sizeof(uint32_t));
-  data->u64_ = wasmtoh(data32);
+  data->u64_ = wasmtoh(data32, true);
   return true;
 }
 
 bool Wavm::setWord(uint64_t pointer, Word data) {
-  uint32_t data32 = htowasm(data.u32());
+  uint32_t data32 = htowasm(data.u32(), true);
   return setMemory(pointer, sizeof(uint32_t), &data32);
 }
 

diff --git a/test/BUILD b/test/BUILD
@@ -165,6 +165,17 @@ cc_test(
     ],
 )
 
+cc_test(
+    name = "pairs_util_test",
+    srcs = ["pairs_util_test.cc"],
+    linkstatic = 1,
+    deps = [
+        "//:lib",
+        "@com_google_googletest//:gtest",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
+
 cc_library(
     name = "utility_lib",
     testonly = True,

diff --git a/test/null_vm_test.cc b/test/null_vm_test.cc
@@ -73,4 +73,10 @@ TEST_F(BaseVmTest, NullVmStartup) {
   EXPECT_NE(test_null_vm_plugin, nullptr);
 }
 
+TEST_F(BaseVmTest, ByteOrder) {
+  auto wasm_vm = createNullVm();
+  EXPECT_TRUE(wasm_vm->load("test_null_vm_plugin", {}, {}));
+  EXPECT_FALSE(wasm_vm->usesWasmByteOrder());
+}
+
 } // namespace proxy_wasm
diff --git a/test/pairs_util_test.cc b/test/pairs_util_test.cc
@@ -0,0 +1,33 @@
+// Copyright 2022 Google LLC
+//
+// 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/proxy-wasm/pairs_util.h"
+#include "gtest/gtest.h"
+
+namespace proxy_wasm {
+
+TEST(PairsUtilTest, EncodeDecode) {
+  proxy_wasm::Pairs pairs1;
+  std::string data1("some_data");
+  auto size_str = std::to_string(data1.size());
+  pairs1.push_back({data1, size_str});
+  std::vector<char> buffer(PairsUtil::pairsSize(pairs1));
+  EXPECT_TRUE(PairsUtil::marshalPairs(pairs1, buffer.data(), buffer.size()));
+  auto pairs2 = PairsUtil::toPairs(std::string_view(buffer.data(), buffer.size()));
+  EXPECT_EQ(pairs2.size(), pairs1.size());
+  EXPECT_EQ(pairs2[0].first, pairs1[0].first);
+  EXPECT_EQ(pairs2[0].second, pairs1[0].second);
+}
+
+} // namespace proxy_wasm
diff --git a/test/wasm_vm_test.cc b/test/wasm_vm_test.cc
@@ -53,7 +53,8 @@ TEST_P(TestVm, Memory) {
   ASSERT_TRUE(vm_->getWord(0x2000, &word));
   ASSERT_EQ(100, word.u64_);
 
-  uint32_t data[2] = {htowasm(static_cast<uint32_t>(-1)), htowasm(200)};
+  uint32_t data[2] = {htowasm(static_cast<uint32_t>(-1), vm_->usesWasmByteOrder()),
+                      htowasm(200, vm_->usesWasmByteOrder())};
   ASSERT_TRUE(vm_->setMemory(0x200, sizeof(int32_t) * 2, static_cast<void *>(data)));
   ASSERT_TRUE(vm_->getWord(0x200, &word));
   ASSERT_EQ(-1, static_cast<int32_t>(word.u64_));