From 537b944de526aaeda803dfa3bbcffe1f408b986c Mon Sep 17 00:00:00 2001 From: Konstantin Maksimov <18444445+knm3000@users.noreply.github.com> Date: Sun, 2 Oct 2022 04:08:53 +0200 Subject: [PATCH] Do not reverse bytes for NullVM. (#282) Fixes https://github.com/proxy-wasm/proxy-wasm-cpp-host/issues/294. Signed-off-by: Konstantin Maksimov --- include/proxy-wasm/null_vm.h | 1 + include/proxy-wasm/wasm_vm.h | 6 ++++++ include/proxy-wasm/word.h | 8 ++++---- src/exports.cc | 5 +++-- src/pairs_util.cc | 31 +++++++++++++++++++++++++------ src/signature_util.cc | 2 +- src/v8/v8.cc | 5 +++-- src/wamr/wamr.cc | 5 +++-- src/wasmedge/wasmedge.cc | 1 + src/wasmtime/wasmtime.cc | 5 +++-- src/wavm/wavm.cc | 5 +++-- test/BUILD | 11 +++++++++++ test/null_vm_test.cc | 6 ++++++ test/pairs_util_test.cc | 33 +++++++++++++++++++++++++++++++++ test/wasm_vm_test.cc | 3 ++- 15 files changed, 105 insertions(+), 22 deletions(-) create mode 100644 test/pairs_util_test.cc diff --git a/include/proxy-wasm/null_vm.h b/include/proxy-wasm/null_vm.h index 3a38fd99..a0a3798f 100644 --- 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 plugin_; diff --git a/include/proxy-wasm/wasm_vm.h b/include/proxy-wasm/wasm_vm.h index 879e200d..acf0ccf3 100644 --- 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 index 559471eb..90ea932d 100644 --- 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 index b06c4043..0290dcf0 100644 --- 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(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 index d4925921..d2113578 100644 --- a/src/pairs_util.cc +++ b/src/pairs_util.cc @@ -19,6 +19,7 @@ #include #include +#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,7 +58,10 @@ 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; } @@ -62,7 +69,10 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size) { 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(pos)); + uint32_t num_pairs = wasmtoh(*reinterpret_cast(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(pos)); + s.first = wasmtoh(*reinterpret_cast(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(pos)); + s.second = wasmtoh(*reinterpret_cast(pos), + contextOrEffectiveContext() != nullptr + ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder() + : false); pos += sizeof(uint32_t); } diff --git a/src/signature_util.cc b/src/signature_util.cc index a8f29363..46b9d333 100644 --- 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 index 2d8660bc..5718d130 100644 --- 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 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 index e193358c..2ca6863b 100644 --- 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 @@ -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 index f22d588a..89813096 100644 --- a/src/wasmedge/wasmedge.cc +++ b/src/wasmedge/wasmedge.cc @@ -281,6 +281,7 @@ class WasmEdge : public WasmVm { std::function *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 index 040dea07..2cfc2188 100644 --- a/src/wasmtime/wasmtime.cc +++ b/src/wasmtime/wasmtime.cc @@ -98,6 +98,7 @@ class Wasmtime : public WasmVm { std::function *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 index 1041b7fa..670eb7c4 100644 --- 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(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 index 42748b92..71b30296 100644 --- 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 index 774c685d..5bb862f9 100644 --- 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 new file mode 100644 index 00000000..3f9fcd96 --- /dev/null +++ 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 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 index 642ea636..a9caac93 100644 --- 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(-1)), htowasm(200)}; + uint32_t data[2] = {htowasm(static_cast(-1), vm_->usesWasmByteOrder()), + htowasm(200, vm_->usesWasmByteOrder())}; ASSERT_TRUE(vm_->setMemory(0x200, sizeof(int32_t) * 2, static_cast(data))); ASSERT_TRUE(vm_->getWord(0x200, &word)); ASSERT_EQ(-1, static_cast(word.u64_));