Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not reverse bytes for NullVM. #282

Merged
merged 6 commits into from
Oct 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/proxy-wasm/null_vm.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down
6 changes: 6 additions & 0 deletions include/proxy-wasm/wasm_vm.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions include/proxy-wasm/word.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 3 additions & 2 deletions src/exports.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
31 changes: 25 additions & 6 deletions src/pairs_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
PiotrSikora marked this conversation as resolved.
Show resolved Hide resolved

Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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.
Expand All @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion src/signature_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) + ")";
Expand Down
5 changes: 3 additions & 2 deletions src/v8/v8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}
Expand Down
5 changes: 3 additions & 2 deletions src/wamr/wamr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ class Wamr : public WasmVm {
#undef _GET_MODULE_FUNCTION

void terminate() override {}
bool usesWasmByteOrder() override { return true; }

private:
template <typename... Args>
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions src/wasmedge/wasmedge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down
5 changes: 3 additions & 2 deletions src/wasmtime/wasmtime.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}
Expand Down
5 changes: 3 additions & 2 deletions src/wavm/wavm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down
11 changes: 11 additions & 0 deletions test/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 6 additions & 0 deletions test/null_vm_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
33 changes: 33 additions & 0 deletions test/pairs_util_test.cc
Original file line number Diff line number Diff line change
@@ -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
3 changes: 2 additions & 1 deletion test/wasm_vm_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_));
Expand Down