From d61f579dcb227f238890db2a4638211ec56c9cae Mon Sep 17 00:00:00 2001 From: Konstantin Maksimov Date: Wed, 17 Aug 2022 10:52:05 +0200 Subject: [PATCH 1/6] Do not reverse bytes for nullvm 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/pairs_util.h | 9 +++++---- include/proxy-wasm/wasm_vm.h | 6 ++++++ include/proxy-wasm/word.h | 9 +++++---- src/exports.cc | 25 +++++++++++++++---------- src/pairs_util.cc | 17 +++++++++-------- 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/fuzz/pairs_util_fuzzer.cc | 4 ++-- test/null_vm_test.cc | 21 +++++++++++++++++++++ test/wasm_vm_test.cc | 3 ++- 15 files changed, 80 insertions(+), 38 deletions(-) diff --git a/include/proxy-wasm/null_vm.h b/include/proxy-wasm/null_vm.h index 3a38fd99..e1e456cf 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 isWasmByteOrder() override { return false; } std::string plugin_name_; std::unique_ptr plugin_; diff --git a/include/proxy-wasm/pairs_util.h b/include/proxy-wasm/pairs_util.h index 54df2fd8..de1854ff 100644 --- a/include/proxy-wasm/pairs_util.h +++ b/include/proxy-wasm/pairs_util.h @@ -45,11 +45,12 @@ class PairsUtil { * @param size size of the output buffer. * @return indicates whether serialization succeeded or not. */ - static bool marshalPairs(const Pairs &pairs, char *buffer, size_t size); + static bool marshalPairs(const Pairs &pairs, char *buffer, size_t size, bool is_wasm_byte_order); - static bool marshalPairs(const StringPairs &stringpairs, char *buffer, size_t size) { + static bool marshalPairs(const StringPairs &stringpairs, char *buffer, size_t size, + bool is_wasm_byte_order) { Pairs views(stringpairs.begin(), stringpairs.end()); - return marshalPairs(views, buffer, size); + return marshalPairs(views, buffer, size, is_wasm_byte_order); } /** @@ -57,7 +58,7 @@ class PairsUtil { * @param buffer serialized input buffer. * @return deserialized Pairs or an empty instance in case of deserialization failure. */ - static Pairs toPairs(std::string_view buffer); + static Pairs toPairs(std::string_view buffer, bool is_wasm_byte_order); }; } // namespace proxy_wasm diff --git a/include/proxy-wasm/wasm_vm.h b/include/proxy-wasm/wasm_vm.h index 879e200d..88ee42bd 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 isWasmByteOrder() = 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..1af3e4ac 100644 --- a/include/proxy-wasm/word.h +++ b/include/proxy-wasm/word.h @@ -24,11 +24,12 @@ 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 htowasm1111(x, is_null) is_null ? (x) : __builtin_bswap32(x) +#define htowasm(x, is_wasm_byte_order) is_wasm_byte_order ? __builtin_bswap32(x) : (x) +#define wasmtoh(x, is_wasm_byte_order) is_wasm_byte_order ? __builtin_bswap32(x) : (x) #else -#define htowasm(x) (x) -#define wasmtoh(x) (x) +#define htowasm(x, is_wasm_byte_order) (x) +#define wasmtoh(x, is_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..c8ed9e2c 100644 --- a/src/exports.cc +++ b/src/exports.cc @@ -152,7 +152,8 @@ Word send_local_response(Word response_code, Word response_code_details_ptr, if (!details || !body || !additional_response_header_pairs) { return WasmResult::InvalidMemoryAccess; } - auto additional_headers = PairsUtil::toPairs(additional_response_header_pairs.value()); + auto additional_headers = PairsUtil::toPairs(additional_response_header_pairs.value(), + context->wasmVm()->isWasmByteOrder()); context->sendLocalResponse(response_code, body.value(), std::move(additional_headers), grpc_status, details.value()); context->wasm()->stopNextIteration(true); @@ -399,7 +400,7 @@ Word get_header_map_pairs(Word type, Word ptr_ptr, Word size_ptr) { if (buffer == nullptr) { return WasmResult::InvalidMemoryAccess; } - if (!PairsUtil::marshalPairs(pairs, buffer, size)) { + if (!PairsUtil::marshalPairs(pairs, buffer, size, context->wasmVm()->isWasmByteOrder())) { return WasmResult::InvalidMemoryAccess; } if (!context->wasmVm()->setWord(ptr_ptr, Word(ptr))) { @@ -420,8 +421,9 @@ Word set_header_map_pairs(Word type, Word ptr, Word size) { if (!data) { return WasmResult::InvalidMemoryAccess; } - return context->setHeaderMapPairs(static_cast(type.u64_), - PairsUtil::toPairs(data.value())); + return context->setHeaderMapPairs( + static_cast(type.u64_), + PairsUtil::toPairs(data.value(), context->wasmVm()->isWasmByteOrder())); } Word get_header_map_size(Word type, Word result_ptr) { @@ -519,8 +521,8 @@ Word http_call(Word uri_ptr, Word uri_size, Word header_pairs_ptr, Word header_p if (!uri || !body || !header_pairs || !trailer_pairs) { return WasmResult::InvalidMemoryAccess; } - auto headers = PairsUtil::toPairs(header_pairs.value()); - auto trailers = PairsUtil::toPairs(trailer_pairs.value()); + auto headers = PairsUtil::toPairs(header_pairs.value(), context->wasmVm()->isWasmByteOrder()); + auto trailers = PairsUtil::toPairs(trailer_pairs.value(), context->wasmVm()->isWasmByteOrder()); uint32_t token = 0; // NB: try to write the token to verify the memory before starting the async // operation. @@ -589,7 +591,8 @@ Word grpc_call(Word service_ptr, Word service_size, Word service_name_ptr, Word return WasmResult::InvalidMemoryAccess; } uint32_t token = 0; - auto initial_metadata = PairsUtil::toPairs(initial_metadata_pairs.value()); + auto initial_metadata = + PairsUtil::toPairs(initial_metadata_pairs.value(), context->wasmVm()->isWasmByteOrder()); auto result = context->grpcCall(service.value(), service_name.value(), method_name.value(), initial_metadata, request.value(), std::chrono::milliseconds(timeout_milliseconds), &token); @@ -615,7 +618,8 @@ Word grpc_stream(Word service_ptr, Word service_size, Word service_name_ptr, Wor return WasmResult::InvalidMemoryAccess; } uint32_t token = 0; - auto initial_metadata = PairsUtil::toPairs(initial_metadata_pairs.value()); + auto initial_metadata = + PairsUtil::toPairs(initial_metadata_pairs.value(), context->wasmVm()->isWasmByteOrder()); auto result = context->grpcStream(service.value(), service_name.value(), method_name.value(), initial_metadata, &token); if (result != WasmResult::Ok) { @@ -693,8 +697,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 */); + memslice = context->wasmVm()->getMemory( + wasmtoh(iovec[0], context->wasmVm()->isWasmByteOrder()) /* buf */, + wasmtoh(iovec[1], context->wasmVm()->isWasmByteOrder()) /* buf_len */); if (!memslice) { return 21; // __WASI_EFAULT } diff --git a/src/pairs_util.cc b/src/pairs_util.cc index d4925921..a6a75d04 100644 --- a/src/pairs_util.cc +++ b/src/pairs_util.cc @@ -36,7 +36,8 @@ size_t PairsUtil::pairsSize(const Pairs &pairs) { return size; } -bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size) { +bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size, + [[maybe_unused]] bool is_wasm_byte_order) { if (buffer == nullptr) { return false; } @@ -45,7 +46,7 @@ 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(), is_wasm_byte_order); if (pos + sizeof(uint32_t) > end) { return false; } @@ -54,7 +55,7 @@ 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(), is_wasm_byte_order); if (pos + sizeof(uint32_t) > end) { return false; } @@ -62,7 +63,7 @@ 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(), is_wasm_byte_order); if (pos + sizeof(uint32_t) > end) { return false; } @@ -91,7 +92,7 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size) { return pos == end; } -Pairs PairsUtil::toPairs(std::string_view buffer) { +Pairs PairsUtil::toPairs(std::string_view buffer, [[maybe_unused]] bool is_wasm_byte_order) { if (buffer.data() == nullptr || buffer.size() > PROXY_WASM_HOST_PAIRS_MAX_BYTES) { return {}; } @@ -103,7 +104,7 @@ 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), is_wasm_byte_order); pos += sizeof(uint32_t); // Check if we're not going to exceed the limit. @@ -122,14 +123,14 @@ 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), is_wasm_byte_order); 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), is_wasm_byte_order); 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..0d3af504 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 isWasmByteOrder() 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, isWasmByteOrder()); 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(), isWasmByteOrder()); ::memcpy(memory_->data() + pointer, &word32, size); return true; } diff --git a/src/wamr/wamr.cc b/src/wamr/wamr.cc index e193358c..8b14f6d9 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 isWasmByteOrder() 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, isWasmByteOrder()); 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(), isWasmByteOrder()); ::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..cbaddc7c 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 isWasmByteOrder() override { return true; } WasmEdgeLoaderPtr loader_; WasmEdgeValidatorPtr validator_; diff --git a/src/wasmtime/wasmtime.cc b/src/wasmtime/wasmtime.cc index 040dea07..dc52f5c4 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 isWasmByteOrder() 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, isWasmByteOrder()); 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(), isWasmByteOrder()); ::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..bd8936ab 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 isWasmByteOrder() 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, isWasmByteOrder()); return true; } bool Wavm::setWord(uint64_t pointer, Word data) { - uint32_t data32 = htowasm(data.u32()); + uint32_t data32 = htowasm(data.u32(), isWasmByteOrder()); return setMemory(pointer, sizeof(uint32_t), &data32); } diff --git a/test/fuzz/pairs_util_fuzzer.cc b/test/fuzz/pairs_util_fuzzer.cc index aaef0d09..9bda6236 100644 --- a/test/fuzz/pairs_util_fuzzer.cc +++ b/test/fuzz/pairs_util_fuzzer.cc @@ -22,7 +22,7 @@ namespace { extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { auto input = std::string_view(reinterpret_cast(data), size); - auto pairs = PairsUtil::toPairs(input); + auto pairs = PairsUtil::toPairs(input, true); if (!pairs.empty()) { // Verify that non-empty Pairs serializes back to the same bytes. @@ -31,7 +31,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { __builtin_trap(); } std::vector new_data(new_size); - if (!PairsUtil::marshalPairs(pairs, new_data.data(), new_data.size())) { + if (!PairsUtil::marshalPairs(pairs, new_data.data(), new_data.size(), true)) { __builtin_trap(); } if (::memcmp(new_data.data(), data, size) != 0) { diff --git a/test/null_vm_test.cc b/test/null_vm_test.cc index 774c685d..e2db0346 100644 --- a/test/null_vm_test.cc +++ b/test/null_vm_test.cc @@ -16,6 +16,7 @@ #include "include/proxy-wasm/null.h" #include "include/proxy-wasm/null_vm_plugin.h" +#include "include/proxy-wasm/pairs_util.h" #include "gtest/gtest.h" @@ -73,4 +74,24 @@ 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->isWasmByteOrder()); +#if defined(__BYTE_ORDER__) && defined(__ORDER_BIG_ENDIAN__) && \ + __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ + proxy_wasm::Pairs pairs1; + std::string data1("some_data"); + pairs1.push_back({data1.data(), std::to_string(data1.size())}); + std::vector buffer(PairsUtil::pairsSize(pairs1)); + // encode using null_vm byte order + EXPECT_TRUE( + PairsUtil::marshalPairs(pairs1, buffer.data(), buffer.size(), wasm_vm->isWasmByteOrder())); + // decode using host byte order + auto pairs2 = PairsUtil::toPairs(std::string_view(buffer.data(), buffer.size()), false); + EXPECT_EQ(pairs2.size(), pairs1.size()); + EXPECT_EQ(pairs2[0].second, pairs1[0].second); +#endif +} + } // namespace proxy_wasm diff --git a/test/wasm_vm_test.cc b/test/wasm_vm_test.cc index 642ea636..7e3c45b4 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_->isWasmByteOrder()), + htowasm(200, vm_->isWasmByteOrder())}; 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_)); From 6e273eb741392b98f23a6e1ae29488c4ab1f74b9 Mon Sep 17 00:00:00 2001 From: Konstantin Maksimov Date: Fri, 9 Sep 2022 18:36:07 +0200 Subject: [PATCH 2/6] Do not reverse bytes for nullvm additional changes: - moved the call to isWasmByteOrder() into htowasm/wasmtoh macros - added surrounding brackets to htowasm/wasmtoh - removed debug leftover Fixes https://github.com/proxy-wasm/proxy-wasm-cpp-host/issues/294 Signed-off-by: Konstantin Maksimov --- include/proxy-wasm/pairs_util.h | 9 ++++---- include/proxy-wasm/word.h | 5 ++--- src/exports.cc | 20 +++++++----------- src/pairs_util.cc | 37 ++++++++++++++++++++++++--------- test/fuzz/pairs_util_fuzzer.cc | 4 ++-- test/null_vm_test.cc | 7 ++----- 6 files changed, 45 insertions(+), 37 deletions(-) diff --git a/include/proxy-wasm/pairs_util.h b/include/proxy-wasm/pairs_util.h index de1854ff..54df2fd8 100644 --- a/include/proxy-wasm/pairs_util.h +++ b/include/proxy-wasm/pairs_util.h @@ -45,12 +45,11 @@ class PairsUtil { * @param size size of the output buffer. * @return indicates whether serialization succeeded or not. */ - static bool marshalPairs(const Pairs &pairs, char *buffer, size_t size, bool is_wasm_byte_order); + static bool marshalPairs(const Pairs &pairs, char *buffer, size_t size); - static bool marshalPairs(const StringPairs &stringpairs, char *buffer, size_t size, - bool is_wasm_byte_order) { + static bool marshalPairs(const StringPairs &stringpairs, char *buffer, size_t size) { Pairs views(stringpairs.begin(), stringpairs.end()); - return marshalPairs(views, buffer, size, is_wasm_byte_order); + return marshalPairs(views, buffer, size); } /** @@ -58,7 +57,7 @@ class PairsUtil { * @param buffer serialized input buffer. * @return deserialized Pairs or an empty instance in case of deserialization failure. */ - static Pairs toPairs(std::string_view buffer, bool is_wasm_byte_order); + static Pairs toPairs(std::string_view buffer); }; } // namespace proxy_wasm diff --git a/include/proxy-wasm/word.h b/include/proxy-wasm/word.h index 1af3e4ac..f3549d48 100644 --- a/include/proxy-wasm/word.h +++ b/include/proxy-wasm/word.h @@ -24,9 +24,8 @@ 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 htowasm1111(x, is_null) is_null ? (x) : __builtin_bswap32(x) -#define htowasm(x, is_wasm_byte_order) is_wasm_byte_order ? __builtin_bswap32(x) : (x) -#define wasmtoh(x, is_wasm_byte_order) is_wasm_byte_order ? __builtin_bswap32(x) : (x) +#define htowasm(x, is_wasm_byte_order) ((is_wasm_byte_order) ? __builtin_bswap32(x) : (x)) +#define wasmtoh(x, is_wasm_byte_order) ((is_wasm_byte_order) ? __builtin_bswap32(x) : (x)) #else #define htowasm(x, is_wasm_byte_order) (x) #define wasmtoh(x, is_wasm_byte_order) (x) diff --git a/src/exports.cc b/src/exports.cc index c8ed9e2c..fbf63305 100644 --- a/src/exports.cc +++ b/src/exports.cc @@ -152,8 +152,7 @@ Word send_local_response(Word response_code, Word response_code_details_ptr, if (!details || !body || !additional_response_header_pairs) { return WasmResult::InvalidMemoryAccess; } - auto additional_headers = PairsUtil::toPairs(additional_response_header_pairs.value(), - context->wasmVm()->isWasmByteOrder()); + auto additional_headers = PairsUtil::toPairs(additional_response_header_pairs.value()); context->sendLocalResponse(response_code, body.value(), std::move(additional_headers), grpc_status, details.value()); context->wasm()->stopNextIteration(true); @@ -400,7 +399,7 @@ Word get_header_map_pairs(Word type, Word ptr_ptr, Word size_ptr) { if (buffer == nullptr) { return WasmResult::InvalidMemoryAccess; } - if (!PairsUtil::marshalPairs(pairs, buffer, size, context->wasmVm()->isWasmByteOrder())) { + if (!PairsUtil::marshalPairs(pairs, buffer, size)) { return WasmResult::InvalidMemoryAccess; } if (!context->wasmVm()->setWord(ptr_ptr, Word(ptr))) { @@ -421,9 +420,8 @@ Word set_header_map_pairs(Word type, Word ptr, Word size) { if (!data) { return WasmResult::InvalidMemoryAccess; } - return context->setHeaderMapPairs( - static_cast(type.u64_), - PairsUtil::toPairs(data.value(), context->wasmVm()->isWasmByteOrder())); + return context->setHeaderMapPairs(static_cast(type.u64_), + PairsUtil::toPairs(data.value())); } Word get_header_map_size(Word type, Word result_ptr) { @@ -521,8 +519,8 @@ Word http_call(Word uri_ptr, Word uri_size, Word header_pairs_ptr, Word header_p if (!uri || !body || !header_pairs || !trailer_pairs) { return WasmResult::InvalidMemoryAccess; } - auto headers = PairsUtil::toPairs(header_pairs.value(), context->wasmVm()->isWasmByteOrder()); - auto trailers = PairsUtil::toPairs(trailer_pairs.value(), context->wasmVm()->isWasmByteOrder()); + auto headers = PairsUtil::toPairs(header_pairs.value()); + auto trailers = PairsUtil::toPairs(trailer_pairs.value()); uint32_t token = 0; // NB: try to write the token to verify the memory before starting the async // operation. @@ -591,8 +589,7 @@ Word grpc_call(Word service_ptr, Word service_size, Word service_name_ptr, Word return WasmResult::InvalidMemoryAccess; } uint32_t token = 0; - auto initial_metadata = - PairsUtil::toPairs(initial_metadata_pairs.value(), context->wasmVm()->isWasmByteOrder()); + auto initial_metadata = PairsUtil::toPairs(initial_metadata_pairs.value()); auto result = context->grpcCall(service.value(), service_name.value(), method_name.value(), initial_metadata, request.value(), std::chrono::milliseconds(timeout_milliseconds), &token); @@ -618,8 +615,7 @@ Word grpc_stream(Word service_ptr, Word service_size, Word service_name_ptr, Wor return WasmResult::InvalidMemoryAccess; } uint32_t token = 0; - auto initial_metadata = - PairsUtil::toPairs(initial_metadata_pairs.value(), context->wasmVm()->isWasmByteOrder()); + auto initial_metadata = PairsUtil::toPairs(initial_metadata_pairs.value()); auto result = context->grpcStream(service.value(), service_name.value(), method_name.value(), initial_metadata, &token); if (result != WasmResult::Ok) { diff --git a/src/pairs_util.cc b/src/pairs_util.cc index a6a75d04..3b711571 100644 --- a/src/pairs_util.cc +++ b/src/pairs_util.cc @@ -19,8 +19,8 @@ #include #include +#include "include/proxy-wasm/exports.h" #include "include/proxy-wasm/limits.h" -#include "include/proxy-wasm/word.h" namespace proxy_wasm { @@ -36,8 +36,7 @@ size_t PairsUtil::pairsSize(const Pairs &pairs) { return size; } -bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size, - [[maybe_unused]] bool is_wasm_byte_order) { +bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size) { if (buffer == nullptr) { return false; } @@ -46,7 +45,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(), is_wasm_byte_order); + uint32_t num_pairs = + htowasm(pairs.size(), contextOrEffectiveContext() == nullptr + ? false + : contextOrEffectiveContext()->wasmVm()->isWasmByteOrder()); if (pos + sizeof(uint32_t) > end) { return false; } @@ -55,7 +57,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(), is_wasm_byte_order); + uint32_t name_len = + htowasm(p.first.size(), contextOrEffectiveContext() == nullptr + ? false + : contextOrEffectiveContext()->wasmVm()->isWasmByteOrder()); if (pos + sizeof(uint32_t) > end) { return false; } @@ -63,7 +68,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(), is_wasm_byte_order); + uint32_t value_len = + htowasm(p.second.size(), contextOrEffectiveContext() == nullptr + ? false + : contextOrEffectiveContext()->wasmVm()->isWasmByteOrder()); if (pos + sizeof(uint32_t) > end) { return false; } @@ -92,7 +100,7 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size, return pos == end; } -Pairs PairsUtil::toPairs(std::string_view buffer, [[maybe_unused]] bool is_wasm_byte_order) { +Pairs PairsUtil::toPairs(std::string_view buffer) { if (buffer.data() == nullptr || buffer.size() > PROXY_WASM_HOST_PAIRS_MAX_BYTES) { return {}; } @@ -104,7 +112,10 @@ Pairs PairsUtil::toPairs(std::string_view buffer, [[maybe_unused]] bool is_wasm_ if (pos + sizeof(uint32_t) > end) { return {}; } - uint32_t num_pairs = wasmtoh(*reinterpret_cast(pos), is_wasm_byte_order); + uint32_t num_pairs = wasmtoh(*reinterpret_cast(pos), + contextOrEffectiveContext() == nullptr + ? false + : contextOrEffectiveContext()->wasmVm()->isWasmByteOrder()); pos += sizeof(uint32_t); // Check if we're not going to exceed the limit. @@ -123,14 +134,20 @@ Pairs PairsUtil::toPairs(std::string_view buffer, [[maybe_unused]] bool is_wasm_ if (pos + sizeof(uint32_t) > end) { return {}; } - s.first = wasmtoh(*reinterpret_cast(pos), is_wasm_byte_order); + s.first = wasmtoh(*reinterpret_cast(pos), + contextOrEffectiveContext() == nullptr + ? false + : contextOrEffectiveContext()->wasmVm()->isWasmByteOrder()); pos += sizeof(uint32_t); // Read value length. if (pos + sizeof(uint32_t) > end) { return {}; } - s.second = wasmtoh(*reinterpret_cast(pos), is_wasm_byte_order); + s.second = wasmtoh(*reinterpret_cast(pos), + contextOrEffectiveContext() == nullptr + ? false + : contextOrEffectiveContext()->wasmVm()->isWasmByteOrder()); pos += sizeof(uint32_t); } diff --git a/test/fuzz/pairs_util_fuzzer.cc b/test/fuzz/pairs_util_fuzzer.cc index 9bda6236..aaef0d09 100644 --- a/test/fuzz/pairs_util_fuzzer.cc +++ b/test/fuzz/pairs_util_fuzzer.cc @@ -22,7 +22,7 @@ namespace { extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { auto input = std::string_view(reinterpret_cast(data), size); - auto pairs = PairsUtil::toPairs(input, true); + auto pairs = PairsUtil::toPairs(input); if (!pairs.empty()) { // Verify that non-empty Pairs serializes back to the same bytes. @@ -31,7 +31,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { __builtin_trap(); } std::vector new_data(new_size); - if (!PairsUtil::marshalPairs(pairs, new_data.data(), new_data.size(), true)) { + if (!PairsUtil::marshalPairs(pairs, new_data.data(), new_data.size())) { __builtin_trap(); } if (::memcmp(new_data.data(), data, size) != 0) { diff --git a/test/null_vm_test.cc b/test/null_vm_test.cc index e2db0346..6aa20492 100644 --- a/test/null_vm_test.cc +++ b/test/null_vm_test.cc @@ -84,11 +84,8 @@ TEST_F(BaseVmTest, ByteOrder) { std::string data1("some_data"); pairs1.push_back({data1.data(), std::to_string(data1.size())}); std::vector buffer(PairsUtil::pairsSize(pairs1)); - // encode using null_vm byte order - EXPECT_TRUE( - PairsUtil::marshalPairs(pairs1, buffer.data(), buffer.size(), wasm_vm->isWasmByteOrder())); - // decode using host byte order - auto pairs2 = PairsUtil::toPairs(std::string_view(buffer.data(), buffer.size()), false); + 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].second, pairs1[0].second); #endif From ad11d07cb023684ecb6457a2dee410b91d256911 Mon Sep 17 00:00:00 2001 From: Konstantin Maksimov Date: Wed, 14 Sep 2022 18:00:07 +0200 Subject: [PATCH 3/6] Do not reverse bytes for nullvm more changes: - renamed is_wasm_byte_order to vm_uses_wasm_byte_order - renamed isWasmByteOrder() to usesWasmByteOrder() Fixes https://github.com/proxy-wasm/proxy-wasm-cpp-host/issues/294 Signed-off-by: Konstantin Maksimov --- include/proxy-wasm/null_vm.h | 2 +- include/proxy-wasm/wasm_vm.h | 2 +- include/proxy-wasm/word.h | 8 ++++---- src/exports.cc | 4 ++-- src/pairs_util.cc | 12 ++++++------ src/v8/v8.cc | 6 +++--- src/wamr/wamr.cc | 6 +++--- src/wasmedge/wasmedge.cc | 2 +- src/wasmtime/wasmtime.cc | 6 +++--- src/wavm/wavm.cc | 6 +++--- test/null_vm_test.cc | 2 +- test/wasm_vm_test.cc | 4 ++-- 12 files changed, 30 insertions(+), 30 deletions(-) diff --git a/include/proxy-wasm/null_vm.h b/include/proxy-wasm/null_vm.h index e1e456cf..a0a3798f 100644 --- a/include/proxy-wasm/null_vm.h +++ b/include/proxy-wasm/null_vm.h @@ -61,7 +61,7 @@ struct NullVm : public WasmVm { #undef _REGISTER_CALLBACK void terminate() override {} - bool isWasmByteOrder() override { return false; } + 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 88ee42bd..acf0ccf3 100644 --- a/include/proxy-wasm/wasm_vm.h +++ b/include/proxy-wasm/wasm_vm.h @@ -307,7 +307,7 @@ class WasmVm { * Byte order flag (host or wasm). * @return 'false' for a null VM and 'true' for a wasm VM. */ - virtual bool isWasmByteOrder() = 0; + virtual bool usesWasmByteOrder() = 0; bool isFailed() { return failed_ != FailState::Ok; } void fail(FailState fail_state, std::string_view message) { diff --git a/include/proxy-wasm/word.h b/include/proxy-wasm/word.h index f3549d48..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, is_wasm_byte_order) ((is_wasm_byte_order) ? __builtin_bswap32(x) : (x)) -#define wasmtoh(x, is_wasm_byte_order) ((is_wasm_byte_order) ? __builtin_bswap32(x) : (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, is_wasm_byte_order) (x) -#define wasmtoh(x, is_wasm_byte_order) (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 fbf63305..ce011d10 100644 --- a/src/exports.cc +++ b/src/exports.cc @@ -694,8 +694,8 @@ 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], context->wasmVm()->isWasmByteOrder()) /* buf */, - wasmtoh(iovec[1], context->wasmVm()->isWasmByteOrder()) /* buf_len */); + wasmtoh(iovec[0], context->wasmVm()->usesWasmByteOrder()) /* buf */, + wasmtoh(iovec[1], context->wasmVm()->usesWasmByteOrder()) /* buf_len */); if (!memslice) { return 21; // __WASI_EFAULT } diff --git a/src/pairs_util.cc b/src/pairs_util.cc index 3b711571..42cfcc90 100644 --- a/src/pairs_util.cc +++ b/src/pairs_util.cc @@ -48,7 +48,7 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size) { uint32_t num_pairs = htowasm(pairs.size(), contextOrEffectiveContext() == nullptr ? false - : contextOrEffectiveContext()->wasmVm()->isWasmByteOrder()); + : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()); if (pos + sizeof(uint32_t) > end) { return false; } @@ -60,7 +60,7 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size) { uint32_t name_len = htowasm(p.first.size(), contextOrEffectiveContext() == nullptr ? false - : contextOrEffectiveContext()->wasmVm()->isWasmByteOrder()); + : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()); if (pos + sizeof(uint32_t) > end) { return false; } @@ -71,7 +71,7 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size) { uint32_t value_len = htowasm(p.second.size(), contextOrEffectiveContext() == nullptr ? false - : contextOrEffectiveContext()->wasmVm()->isWasmByteOrder()); + : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()); if (pos + sizeof(uint32_t) > end) { return false; } @@ -115,7 +115,7 @@ Pairs PairsUtil::toPairs(std::string_view buffer) { uint32_t num_pairs = wasmtoh(*reinterpret_cast(pos), contextOrEffectiveContext() == nullptr ? false - : contextOrEffectiveContext()->wasmVm()->isWasmByteOrder()); + : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()); pos += sizeof(uint32_t); // Check if we're not going to exceed the limit. @@ -137,7 +137,7 @@ Pairs PairsUtil::toPairs(std::string_view buffer) { s.first = wasmtoh(*reinterpret_cast(pos), contextOrEffectiveContext() == nullptr ? false - : contextOrEffectiveContext()->wasmVm()->isWasmByteOrder()); + : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()); pos += sizeof(uint32_t); // Read value length. @@ -147,7 +147,7 @@ Pairs PairsUtil::toPairs(std::string_view buffer) { s.second = wasmtoh(*reinterpret_cast(pos), contextOrEffectiveContext() == nullptr ? false - : contextOrEffectiveContext()->wasmVm()->isWasmByteOrder()); + : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()); pos += sizeof(uint32_t); } diff --git a/src/v8/v8.cc b/src/v8/v8.cc index 0d3af504..4f8e4f7c 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -105,7 +105,7 @@ class V8 : public WasmVm { #undef _GET_MODULE_FUNCTION void terminate() override; - bool isWasmByteOrder() override { return true; } + bool usesWasmByteOrder() override { return true; } private: wasm::own trap(std::string message); @@ -504,7 +504,7 @@ bool V8::getWord(uint64_t pointer, Word *word) { } uint32_t word32; ::memcpy(&word32, memory_->data() + pointer, size); - word->u64_ = wasmtoh(word32, isWasmByteOrder()); + word->u64_ = wasmtoh(word32, usesWasmByteOrder()); return true; } @@ -517,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(), isWasmByteOrder()); + uint32_t word32 = htowasm(word.u32(), usesWasmByteOrder()); ::memcpy(memory_->data() + pointer, &word32, size); return true; } diff --git a/src/wamr/wamr.cc b/src/wamr/wamr.cc index 8b14f6d9..45ec9e46 100644 --- a/src/wamr/wamr.cc +++ b/src/wamr/wamr.cc @@ -87,7 +87,7 @@ class Wamr : public WasmVm { #undef _GET_MODULE_FUNCTION void terminate() override {} - bool isWasmByteOrder() override { return true; } + bool usesWasmByteOrder() override { return true; } private: template @@ -369,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, isWasmByteOrder()); + word->u64_ = wasmtoh(word32, usesWasmByteOrder()); return true; } @@ -378,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(), isWasmByteOrder()); + uint32_t word32 = htowasm(word.u32(), usesWasmByteOrder()); ::memcpy(wasm_memory_data(memory_.get()) + pointer, &word32, size); return true; } diff --git a/src/wasmedge/wasmedge.cc b/src/wasmedge/wasmedge.cc index cbaddc7c..89813096 100644 --- a/src/wasmedge/wasmedge.cc +++ b/src/wasmedge/wasmedge.cc @@ -281,7 +281,7 @@ class WasmEdge : public WasmVm { std::function *function); void terminate() override {} - bool isWasmByteOrder() override { return true; } + bool usesWasmByteOrder() override { return true; } WasmEdgeLoaderPtr loader_; WasmEdgeValidatorPtr validator_; diff --git a/src/wasmtime/wasmtime.cc b/src/wasmtime/wasmtime.cc index dc52f5c4..1a73d619 100644 --- a/src/wasmtime/wasmtime.cc +++ b/src/wasmtime/wasmtime.cc @@ -98,7 +98,7 @@ class Wasmtime : public WasmVm { std::function *function); void terminate() override {} - bool isWasmByteOrder() override { return true; } + bool usesWasmByteOrder() override { return true; } WasmStorePtr store_; WasmModulePtr module_; @@ -395,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, isWasmByteOrder()); + word->u64_ = wasmtoh(word32, usesWasmByteOrder()); return true; } @@ -404,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(), isWasmByteOrder()); + uint32_t word32 = htowasm(word.u32(), usesWasmByteOrder()); ::memcpy(wasm_memory_data(memory_.get()) + pointer, &word32, size); return true; } diff --git a/src/wavm/wavm.cc b/src/wavm/wavm.cc index bd8936ab..21ca2ced 100644 --- a/src/wavm/wavm.cc +++ b/src/wavm/wavm.cc @@ -230,7 +230,7 @@ struct Wavm : public WasmVm { #undef _REGISTER_CALLBACK void terminate() override {} - bool isWasmByteOrder() override { return true; } + bool usesWasmByteOrder() override { return true; } IR::Module ir_module_; WAVM::Runtime::ModuleRef module_ = nullptr; @@ -390,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, isWasmByteOrder()); + data->u64_ = wasmtoh(data32, usesWasmByteOrder()); return true; } bool Wavm::setWord(uint64_t pointer, Word data) { - uint32_t data32 = htowasm(data.u32(), isWasmByteOrder()); + uint32_t data32 = htowasm(data.u32(), usesWasmByteOrder()); return setMemory(pointer, sizeof(uint32_t), &data32); } diff --git a/test/null_vm_test.cc b/test/null_vm_test.cc index 6aa20492..d7ecb372 100644 --- a/test/null_vm_test.cc +++ b/test/null_vm_test.cc @@ -77,7 +77,7 @@ TEST_F(BaseVmTest, NullVmStartup) { TEST_F(BaseVmTest, ByteOrder) { auto wasm_vm = createNullVm(); EXPECT_TRUE(wasm_vm->load("test_null_vm_plugin", {}, {})); - EXPECT_FALSE(wasm_vm->isWasmByteOrder()); + EXPECT_FALSE(wasm_vm->usesWasmByteOrder()); #if defined(__BYTE_ORDER__) && defined(__ORDER_BIG_ENDIAN__) && \ __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ proxy_wasm::Pairs pairs1; diff --git a/test/wasm_vm_test.cc b/test/wasm_vm_test.cc index 7e3c45b4..a9caac93 100644 --- a/test/wasm_vm_test.cc +++ b/test/wasm_vm_test.cc @@ -53,8 +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), vm_->isWasmByteOrder()), - htowasm(200, vm_->isWasmByteOrder())}; + 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_)); From dec6b94e809f5be19265d70b6276c48c2e9a634c Mon Sep 17 00:00:00 2001 From: Konstantin Maksimov Date: Wed, 28 Sep 2022 18:11:11 +0200 Subject: [PATCH 4/6] Do not reverse bytes for nullvm - use buf and buf_len variables for readability - added back word.h include - hardcoded usesWasmByteOrder() to true Fixes https://github.com/proxy-wasm/proxy-wasm-cpp-host/issues/294 Signed-off-by: Konstantin Maksimov --- src/exports.cc | 6 +++--- src/pairs_util.cc | 1 + src/v8/v8.cc | 4 ++-- src/wamr/wamr.cc | 4 ++-- src/wasmtime/wasmtime.cc | 4 ++-- src/wavm/wavm.cc | 4 ++-- 6 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/exports.cc b/src/exports.cc index ce011d10..0290dcf0 100644 --- a/src/exports.cc +++ b/src/exports.cc @@ -693,9 +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], context->wasmVm()->usesWasmByteOrder()) /* buf */, - wasmtoh(iovec[1], context->wasmVm()->usesWasmByteOrder()) /* 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 42cfcc90..7ef2acb7 100644 --- a/src/pairs_util.cc +++ b/src/pairs_util.cc @@ -21,6 +21,7 @@ #include "include/proxy-wasm/exports.h" #include "include/proxy-wasm/limits.h" +#include "include/proxy-wasm/word.h" namespace proxy_wasm { diff --git a/src/v8/v8.cc b/src/v8/v8.cc index 4f8e4f7c..5718d130 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -504,7 +504,7 @@ bool V8::getWord(uint64_t pointer, Word *word) { } uint32_t word32; ::memcpy(&word32, memory_->data() + pointer, size); - word->u64_ = wasmtoh(word32, usesWasmByteOrder()); + word->u64_ = wasmtoh(word32, true); return true; } @@ -517,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(), usesWasmByteOrder()); + 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 45ec9e46..2ca6863b 100644 --- a/src/wamr/wamr.cc +++ b/src/wamr/wamr.cc @@ -369,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, usesWasmByteOrder()); + word->u64_ = wasmtoh(word32, true); return true; } @@ -378,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(), usesWasmByteOrder()); + uint32_t word32 = htowasm(word.u32(), true); ::memcpy(wasm_memory_data(memory_.get()) + pointer, &word32, size); return true; } diff --git a/src/wasmtime/wasmtime.cc b/src/wasmtime/wasmtime.cc index 1a73d619..2cfc2188 100644 --- a/src/wasmtime/wasmtime.cc +++ b/src/wasmtime/wasmtime.cc @@ -395,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, usesWasmByteOrder()); + word->u64_ = wasmtoh(word32, true); return true; } @@ -404,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(), usesWasmByteOrder()); + 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 21ca2ced..670eb7c4 100644 --- a/src/wavm/wavm.cc +++ b/src/wavm/wavm.cc @@ -390,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, usesWasmByteOrder()); + data->u64_ = wasmtoh(data32, true); return true; } bool Wavm::setWord(uint64_t pointer, Word data) { - uint32_t data32 = htowasm(data.u32(), usesWasmByteOrder()); + uint32_t data32 = htowasm(data.u32(), true); return setMemory(pointer, sizeof(uint32_t), &data32); } From d0a3249bf048894e35f38b7554dbec8300cb0c99 Mon Sep 17 00:00:00 2001 From: Konstantin Maksimov Date: Fri, 30 Sep 2022 21:15:23 +0200 Subject: [PATCH 5/6] Do not reverse bytes for nullvm - moved pairs test into a new file test/pairs_util_test.cc - run pairs test on all platforms - made contextOrEffectiveContext() null check a bit more readable Fixes https://github.com/proxy-wasm/proxy-wasm-cpp-host/issues/294 Signed-off-by: Konstantin Maksimov --- src/pairs_util.cc | 36 ++++++++++++++++++------------------ test/BUILD | 11 +++++++++++ test/null_vm_test.cc | 11 ----------- test/pairs_util_test.cc | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 29 deletions(-) create mode 100644 test/pairs_util_test.cc diff --git a/src/pairs_util.cc b/src/pairs_util.cc index 7ef2acb7..d2113578 100644 --- a/src/pairs_util.cc +++ b/src/pairs_util.cc @@ -47,9 +47,9 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size) { // Write number of pairs. uint32_t num_pairs = - htowasm(pairs.size(), contextOrEffectiveContext() == nullptr - ? false - : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()); + htowasm(pairs.size(), contextOrEffectiveContext() != nullptr + ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder() + : false); if (pos + sizeof(uint32_t) > end) { return false; } @@ -59,9 +59,9 @@ 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(), contextOrEffectiveContext() == nullptr - ? false - : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()); + htowasm(p.first.size(), contextOrEffectiveContext() != nullptr + ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder() + : false); if (pos + sizeof(uint32_t) > end) { return false; } @@ -70,9 +70,9 @@ bool PairsUtil::marshalPairs(const Pairs &pairs, char *buffer, size_t size) { // Write value length. uint32_t value_len = - htowasm(p.second.size(), contextOrEffectiveContext() == nullptr - ? false - : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()); + htowasm(p.second.size(), contextOrEffectiveContext() != nullptr + ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder() + : false); if (pos + sizeof(uint32_t) > end) { return false; } @@ -114,9 +114,9 @@ Pairs PairsUtil::toPairs(std::string_view buffer) { return {}; } uint32_t num_pairs = wasmtoh(*reinterpret_cast(pos), - contextOrEffectiveContext() == nullptr - ? false - : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()); + contextOrEffectiveContext() != nullptr + ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder() + : false); pos += sizeof(uint32_t); // Check if we're not going to exceed the limit. @@ -136,9 +136,9 @@ Pairs PairsUtil::toPairs(std::string_view buffer) { return {}; } s.first = wasmtoh(*reinterpret_cast(pos), - contextOrEffectiveContext() == nullptr - ? false - : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()); + contextOrEffectiveContext() != nullptr + ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder() + : false); pos += sizeof(uint32_t); // Read value length. @@ -146,9 +146,9 @@ Pairs PairsUtil::toPairs(std::string_view buffer) { return {}; } s.second = wasmtoh(*reinterpret_cast(pos), - contextOrEffectiveContext() == nullptr - ? false - : contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder()); + contextOrEffectiveContext() != nullptr + ? contextOrEffectiveContext()->wasmVm()->usesWasmByteOrder() + : false); pos += sizeof(uint32_t); } 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 d7ecb372..8bf7159f 100644 --- a/test/null_vm_test.cc +++ b/test/null_vm_test.cc @@ -78,17 +78,6 @@ TEST_F(BaseVmTest, ByteOrder) { auto wasm_vm = createNullVm(); EXPECT_TRUE(wasm_vm->load("test_null_vm_plugin", {}, {})); EXPECT_FALSE(wasm_vm->usesWasmByteOrder()); -#if defined(__BYTE_ORDER__) && defined(__ORDER_BIG_ENDIAN__) && \ - __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ - proxy_wasm::Pairs pairs1; - std::string data1("some_data"); - pairs1.push_back({data1.data(), std::to_string(data1.size())}); - 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].second, pairs1[0].second); -#endif } } // 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 From a82cc8cd72b7b5b87ac81f9c7a21f32407b13720 Mon Sep 17 00:00:00 2001 From: Konstantin Maksimov Date: Sat, 1 Oct 2022 12:00:41 +0200 Subject: [PATCH 6/6] Do not reverse bytes for nullvm more changes: - removed unnecessary #include from test/null_vm_test.cc Fixes https://github.com/proxy-wasm/proxy-wasm-cpp-host/issues/294 Signed-off-by: Konstantin Maksimov --- test/null_vm_test.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/test/null_vm_test.cc b/test/null_vm_test.cc index 8bf7159f..5bb862f9 100644 --- a/test/null_vm_test.cc +++ b/test/null_vm_test.cc @@ -16,7 +16,6 @@ #include "include/proxy-wasm/null.h" #include "include/proxy-wasm/null_vm_plugin.h" -#include "include/proxy-wasm/pairs_util.h" #include "gtest/gtest.h"