Skip to content

v8: emit backtrace when trap happens. #66

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

Merged
merged 25 commits into from
Dec 15, 2020
Merged
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
5cb8386
v8: emit backtrace when trap happens
mathetake Oct 13, 2020
73e14cb
tidy up get_fail_message
mathetake Oct 13, 2020
1b47c0e
review: rename, refactor
mathetake Oct 27, 2020
0a8a65f
review: simplify message emission
mathetake Oct 27, 2020
256b844
review: build function name index in load()
mathetake Oct 27, 2020
5cb4a07
align leading spaces like wavm
mathetake Nov 4, 2020
f0e7a1a
Merge branch 'master' into trace-print
mathetake Nov 12, 2020
155e5c7
Tidy up trace message
mathetake Nov 17, 2020
0ee805f
Handle the case of having no name custom section
mathetake Nov 17, 2020
59d816c
Merge branch 'master' into trace-print
mathetake Nov 18, 2020
6fbdbc6
Merge branch 'master' into trace-print
mathetake Dec 7, 2020
e2facc1
Merge branch 'master' into trace-print
mathetake Dec 7, 2020
9a555a0
Review: remove unnecessary 'this->'
mathetake Dec 7, 2020
5c28a0b
Merge branch 'trace-print' of github.com:mathetake/proxy-wasm-cpp-hos…
mathetake Dec 7, 2020
51fc718
Review: mv \n to the beginning of of trace
mathetake Dec 10, 2020
bb8fd80
Review: remove the trailing newline in msg & fix style
mathetake Dec 12, 2020
a24e213
Review: add buildFunctionNameIndex member
mathetake Dec 12, 2020
4f815f5
review: fix typo
mathetake Dec 14, 2020
07c1351
Merge branch 'master' into trace-print
PiotrSikora Dec 15, 2020
a31cfdf
Break loop when name section is malformed
mathetake Dec 15, 2020
b83089f
Merge branch 'trace-print' of github.com:mathetake/proxy-wasm-cpp-hos…
mathetake Dec 15, 2020
bcf2182
Review: check the size of subsection
mathetake Dec 15, 2020
6ea1604
Return if malformed
mathetake Dec 15, 2020
63312af
Review: check all the parsed value
mathetake Dec 15, 2020
afa45f0
Remove unnecessary boundary check for func_index
mathetake Dec 15, 2020
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
95 changes: 90 additions & 5 deletions src/v8/v8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@
#include "include/proxy-wasm/v8.h"

#include <cassert>

#include <iomanip>
#include <memory>
#include <optional>
#include <sstream>
#include <utility>
#include <vector>

Expand Down Expand Up @@ -84,7 +85,9 @@ class V8 : public WasmVm {
#undef _GET_MODULE_FUNCTION

private:
void buildFunctionNameIndex();
wasm::vec<byte_t> getStrippedSource();
std::string getFailMessage(std::string_view function_name, wasm::own<wasm::Trap> trap);

template <typename... Args>
void registerHostFunctionImpl(std::string_view module_name, std::string_view function_name,
Expand Down Expand Up @@ -112,6 +115,8 @@ class V8 : public WasmVm {

absl::flat_hash_map<std::string, FuncDataPtr> host_functions_;
absl::flat_hash_map<std::string, wasm::own<wasm::Func>> module_functions_;

absl::flat_hash_map<uint32_t, std::string> function_names_index_;
};

// Helper functions.
Expand Down Expand Up @@ -263,9 +268,58 @@ bool V8::load(const std::string &code, bool allow_precompiled) {
assert((shared_module_ != nullptr));
}

buildFunctionNameIndex();

return module_ != nullptr;
}

void V8::buildFunctionNameIndex() {
// build function index -> function name map for backtrace
// https://webassembly.github.io/spec/core/appendix/custom.html#binary-namesubsection
auto name_section = getCustomSection("name");
if (name_section.size()) {
const byte_t *pos = name_section.data();
const byte_t *end = name_section.data() + name_section.size();
while (pos < end) {
if (*pos++ != 1) {
pos += parseVarint(pos, end);
} else {
const auto size = parseVarint(pos, end);
if (size == static_cast<uint32_t>(-1) || pos + size > end) {
function_names_index_ = {};
return;
}
const auto start = pos;
const auto namemap_vector_size = parseVarint(pos, end);
if (namemap_vector_size == static_cast<uint32_t>(-1) || pos + namemap_vector_size > end) {
function_names_index_ = {};
return;
}
for (auto i = 0; i < namemap_vector_size; i++) {
const auto func_index = parseVarint(pos, end);
if (func_index == static_cast<uint32_t>(-1)) {
function_names_index_ = {};
return;
}

const auto func_name_size = parseVarint(pos, end);
if (func_name_size == static_cast<uint32_t>(-1) || pos + func_name_size > end) {
function_names_index_ = {};
return;
}
function_names_index_.insert({func_index, std::string(pos, func_name_size)});
pos += func_name_size;
}

if (start + size != pos) {
function_names_index_ = {};
return;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: if this fails then we should probably break from the loop, since we can't trust the pos anyway... But I just realized that we don't do this type of verification when parsing bytecode in other places, so feel free to drop this whole if block if you prefer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, we trust given binaries anywhere while parsing so far 😄 maybe we should decide and stick to whether or not validate binary or not.

will add break for now.

}
}
}
}

std::unique_ptr<WasmVm> V8::clone() {
assert(shared_module_ != nullptr);

Expand All @@ -274,6 +328,7 @@ std::unique_ptr<WasmVm> V8::clone() {
clone->store_ = wasm::Store::make(engine());

clone->module_ = wasm::Module::obtain(clone->store_.get(), shared_module_.get());
clone->function_names_index_ = function_names_index_;

return clone;
}
Expand Down Expand Up @@ -500,6 +555,7 @@ bool V8::link(std::string_view debug_name) {
} break;
}
}

return !isFailed();
}

Expand Down Expand Up @@ -618,8 +674,7 @@ void V8::getModuleFunctionImpl(std::string_view function_name,
SaveRestoreContext saved_context(context);
auto trap = func->call(params, nullptr);
if (trap) {
fail(FailState::RuntimeError, "Function: " + std::string(function_name) + " failed: " +
std::string(trap->message().get(), trap->message().size()));
fail(FailState::RuntimeError, getFailMessage(std::string(function_name), std::move(trap)));
}
};
}
Expand Down Expand Up @@ -651,15 +706,45 @@ void V8::getModuleFunctionImpl(std::string_view function_name,
SaveRestoreContext saved_context(context);
auto trap = func->call(params, results);
if (trap) {
fail(FailState::RuntimeError, "Function: " + std::string(function_name) + " failed: " +
std::string(trap->message().get(), trap->message().size()));
fail(FailState::RuntimeError, getFailMessage(std::string(function_name), std::move(trap)));
return R{};
}
R rvalue = results[0].get<typename ConvertWordTypeToUint32<R>::type>();
return rvalue;
};
}

std::string V8::getFailMessage(std::string_view function_name, wasm::own<wasm::Trap> trap) {
auto message = "Function: " + std::string(function_name) + " failed: ";
message += std::string(trap->message().get(), trap->message().size());

if (function_names_index_.empty()) {
return message;
}

auto trace = trap->trace();
message += "\nProxy-Wasm plugin in-VM backtrace:";
for (size_t i = 0; i < trace.size(); ++i) {
auto frame = trace[i].get();
std::ostringstream oss;
oss << std::setw(3) << std::setfill(' ') << std::to_string(i);
message += "\n" + oss.str() + ": ";

std::stringstream address;
address << std::hex << frame->module_offset();
message += " 0x" + address.str() + " - ";

auto func_index = frame->func_index();
auto it = function_names_index_.find(func_index);
if (it != function_names_index_.end()) {
message += it->second;
} else {
message += "unknown(function index:" + std::to_string(func_index) + ")";
}
}
return message;
}

} // namespace

std::unique_ptr<WasmVm> createV8Vm() { return std::make_unique<V8>(); }
Expand Down