From 5cb8386d2428e579ad5cf5d6b8029bc4ec9c733a Mon Sep 17 00:00:00 2001 From: mathetake Date: Tue, 13 Oct 2020 17:45:34 +0900 Subject: [PATCH 01/17] v8: emit backtrace when trap happens Signed-off-by: mathetake --- src/v8/v8.cc | 80 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 74 insertions(+), 6 deletions(-) diff --git a/src/v8/v8.cc b/src/v8/v8.cc index 648f277e..d252a487 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -112,6 +113,9 @@ class V8 : public WasmVm { absl::flat_hash_map host_functions_; absl::flat_hash_map> module_functions_; + + std::string_view name_section; + std::string get_fail_message(std::string function_name, wasm::own trap); }; // Helper functions. @@ -263,6 +267,7 @@ bool V8::load(const std::string &code, bool allow_precompiled) { assert((shared_module_ != nullptr)); } + name_section = getCustomSection("name"); return module_ != nullptr; } @@ -274,7 +279,7 @@ std::unique_ptr V8::clone() { clone->store_ = wasm::Store::make(engine()); clone->module_ = wasm::Module::obtain(clone->store_.get(), shared_module_.get()); - + clone->name_section = name_section; return clone; } @@ -591,7 +596,6 @@ void V8::registerHostFunctionImpl(std::string_view module_name, std::string_view host_functions_.insert_or_assign(std::string(module_name) + "." + std::string(function_name), std::move(data)); } - template void V8::getModuleFunctionImpl(std::string_view function_name, std::function *function) { @@ -618,8 +622,8 @@ 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())); + auto message = this->get_fail_message(std::string(function_name), std::move(trap)); + fail(FailState::RuntimeError, message); } }; } @@ -651,8 +655,8 @@ 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())); + auto message = this->get_fail_message(std::string(function_name), std::move(trap)); + fail(FailState::RuntimeError, message); return R{}; } R rvalue = results[0].get::type>(); @@ -660,6 +664,70 @@ void V8::getModuleFunctionImpl(std::string_view function_name, }; } +std::string V8::get_fail_message(std::string function_name, wasm::own trap) { + auto message = "Function: " + function_name + " failed:\n"; + message += + "message from V8: " + std::string(trap->message().get(), trap->message().size()) + "\n"; + + auto trace = trap->trace(); + + if (!trace.size() || !name_section.size()) { + return message; + } + + const byte_t *pos = name_section.data(); + const byte_t *end = name_section.data() + name_section.size(); + + std::unordered_map function_names{}; + // https://webassembly.github.io/spec/core/appendix/custom.html#binary-namesubsection + + if (*pos == 0) { // module name section + pos++; + parseVarint(pos, end); // skip subsubsection size + const uint32_t module_name_size = parseVarint(pos, end); + message += "module name: " + std::string(pos, module_name_size) + "\n"; + pos += module_name_size; + } + + if (*pos == 1) { // function name section + pos++; + parseVarint(pos, end); // skip subsection size + const uint32_t namemap_vector_size = parseVarint(pos, end); + for (auto i = 0; i < namemap_vector_size; i++) { + const uint32_t func_index = parseVarint(pos, end); + const uint32_t func_name_size = parseVarint(pos, end); + function_names.insert({func_index, std::string(pos, func_name_size)}); + pos += func_name_size; + } + } + + if (function_names.empty()) { + return message; + } + + message += "wasm backtrace:\n"; + + for (size_t i = 0; i < trace.size(); ++i) { + auto frame = trace[i].get(); + message += " " + std::to_string(i) + ": "; + + std::stringstream address; + address << std::hex << frame->module_offset(); + message += " 0x" + address.str() + " - "; + + auto func_index = frame->func_index(); + auto it = function_names.find(func_index); + std::string function_name; + if (it != function_names.end()) { + function_name = it->second; + } else { + function_name = "unknown(function index:" + std::to_string(func_index) + ")"; + } + message += function_name + "\n"; + } + return message; +} + } // namespace std::unique_ptr createV8Vm() { return std::make_unique(); } From 73e14cbdbe2e3963bbdf9dc8557d036abdeceeea Mon Sep 17 00:00:00 2001 From: mathetake Date: Tue, 13 Oct 2020 18:50:44 +0900 Subject: [PATCH 02/17] tidy up get_fail_message Signed-off-by: mathetake --- src/v8/v8.cc | 47 ++++++++++++++++++++--------------------------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/src/v8/v8.cc b/src/v8/v8.cc index d252a487..a5477d8e 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -666,8 +666,7 @@ void V8::getModuleFunctionImpl(std::string_view function_name, std::string V8::get_fail_message(std::string function_name, wasm::own trap) { auto message = "Function: " + function_name + " failed:\n"; - message += - "message from V8: " + std::string(trap->message().get(), trap->message().size()) + "\n"; + message += "V8 message: " + std::string(trap->message().get(), trap->message().size()) + "\n"; auto trace = trap->trace(); @@ -677,8 +676,6 @@ std::string V8::get_fail_message(std::string function_name, wasm::own function_names{}; // https://webassembly.github.io/spec/core/appendix/custom.html#binary-namesubsection if (*pos == 0) { // module name section @@ -690,6 +687,7 @@ std::string V8::get_fail_message(std::string function_name, wasm::own function_names{}; pos++; parseVarint(pos, end); // skip subsection size const uint32_t namemap_vector_size = parseVarint(pos, end); @@ -699,31 +697,26 @@ std::string V8::get_fail_message(std::string function_name, wasm::ownmodule_offset(); - message += " 0x" + address.str() + " - "; - - auto func_index = frame->func_index(); - auto it = function_names.find(func_index); - std::string function_name; - if (it != function_names.end()) { - function_name = it->second; - } else { - function_name = "unknown(function index:" + std::to_string(func_index) + ")"; + message += "wasm backtrace:\n"; + for (size_t i = 0; i < trace.size(); ++i) { + auto frame = trace[i].get(); + message += " " + std::to_string(i) + ": "; + + std::stringstream address; + address << std::hex << frame->module_offset(); + message += " 0x" + address.str() + " - "; + + auto func_index = frame->func_index(); + auto it = function_names.find(func_index); + std::string function_name; + if (it != function_names.end()) { + function_name = it->second; + } else { + function_name = "unknown(function index:" + std::to_string(func_index) + ")"; + } + message += function_name + "\n"; } - message += function_name + "\n"; } return message; } From 1b47c0e5b785e8496dc55478cf429ad4291a59cc Mon Sep 17 00:00:00 2001 From: mathetake Date: Tue, 27 Oct 2020 14:46:05 +0900 Subject: [PATCH 03/17] review: rename, refactor Signed-off-by: mathetake --- src/v8/v8.cc | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/v8/v8.cc b/src/v8/v8.cc index a5477d8e..8d5b98bf 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -85,6 +85,7 @@ class V8 : public WasmVm { #undef _GET_MODULE_FUNCTION private: + std::string getFailMessage(std::string function_name, wasm::own trap); wasm::vec getStrippedSource(); template @@ -114,8 +115,7 @@ class V8 : public WasmVm { absl::flat_hash_map host_functions_; absl::flat_hash_map> module_functions_; - std::string_view name_section; - std::string get_fail_message(std::string function_name, wasm::own trap); + std::string_view name_section_; }; // Helper functions. @@ -267,7 +267,7 @@ bool V8::load(const std::string &code, bool allow_precompiled) { assert((shared_module_ != nullptr)); } - name_section = getCustomSection("name"); + name_section_ = getCustomSection("name"); return module_ != nullptr; } @@ -279,7 +279,7 @@ std::unique_ptr V8::clone() { clone->store_ = wasm::Store::make(engine()); clone->module_ = wasm::Module::obtain(clone->store_.get(), shared_module_.get()); - clone->name_section = name_section; + clone->name_section_ = name_section; return clone; } @@ -596,6 +596,7 @@ void V8::registerHostFunctionImpl(std::string_view module_name, std::string_view host_functions_.insert_or_assign(std::string(module_name) + "." + std::string(function_name), std::move(data)); } + template void V8::getModuleFunctionImpl(std::string_view function_name, std::function *function) { @@ -622,7 +623,7 @@ void V8::getModuleFunctionImpl(std::string_view function_name, SaveRestoreContext saved_context(context); auto trap = func->call(params, nullptr); if (trap) { - auto message = this->get_fail_message(std::string(function_name), std::move(trap)); + auto message = this->getFailMessage(std::string(function_name), std::move(trap)); fail(FailState::RuntimeError, message); } }; @@ -655,7 +656,7 @@ void V8::getModuleFunctionImpl(std::string_view function_name, SaveRestoreContext saved_context(context); auto trap = func->call(params, results); if (trap) { - auto message = this->get_fail_message(std::string(function_name), std::move(trap)); + auto message = this->getFailMessage(std::string(function_name), std::move(trap)); fail(FailState::RuntimeError, message); return R{}; } @@ -664,18 +665,18 @@ void V8::getModuleFunctionImpl(std::string_view function_name, }; } -std::string V8::get_fail_message(std::string function_name, wasm::own trap) { +std::string V8::getFailMessage(std::string_view function_name, wasm::own trap) { auto message = "Function: " + function_name + " failed:\n"; message += "V8 message: " + std::string(trap->message().get(), trap->message().size()) + "\n"; auto trace = trap->trace(); - if (!trace.size() || !name_section.size()) { + if (!trace.size() || !name_section_.size()) { return message; } - const byte_t *pos = name_section.data(); - const byte_t *end = name_section.data() + name_section.size(); + const byte_t *pos = name_section_.data(); + const byte_t *end = name_section_.data() + name_section_.size(); // https://webassembly.github.io/spec/core/appendix/custom.html#binary-namesubsection if (*pos == 0) { // module name section From 0a8a65fdd2e1b987e2a61d92b9a1fd44e98e1d9d Mon Sep 17 00:00:00 2001 From: mathetake Date: Tue, 27 Oct 2020 14:48:46 +0900 Subject: [PATCH 04/17] review: simplify message emission Signed-off-by: mathetake --- src/v8/v8.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/v8/v8.cc b/src/v8/v8.cc index 8d5b98bf..aa924652 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -623,8 +623,8 @@ void V8::getModuleFunctionImpl(std::string_view function_name, SaveRestoreContext saved_context(context); auto trap = func->call(params, nullptr); if (trap) { - auto message = this->getFailMessage(std::string(function_name), std::move(trap)); - fail(FailState::RuntimeError, message); + fail(FailState::RuntimeError, + this->getFailMessage(std::string(function_name), std::move(trap))); } }; } @@ -656,8 +656,8 @@ void V8::getModuleFunctionImpl(std::string_view function_name, SaveRestoreContext saved_context(context); auto trap = func->call(params, results); if (trap) { - auto message = this->getFailMessage(std::string(function_name), std::move(trap)); - fail(FailState::RuntimeError, message); + fail(FailState::RuntimeError, + this->getFailMessage(std::string(function_name), std::move(trap))); return R{}; } R rvalue = results[0].get::type>(); From 256b844d7a570d4c7f3335f82d284bf86c06d551 Mon Sep 17 00:00:00 2001 From: mathetake Date: Tue, 27 Oct 2020 15:19:12 +0900 Subject: [PATCH 05/17] review: build function name index in load() Signed-off-by: mathetake --- src/v8/v8.cc | 93 +++++++++++++++++++++++++--------------------------- 1 file changed, 44 insertions(+), 49 deletions(-) diff --git a/src/v8/v8.cc b/src/v8/v8.cc index aa924652..f2fb5b0e 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -85,7 +85,7 @@ class V8 : public WasmVm { #undef _GET_MODULE_FUNCTION private: - std::string getFailMessage(std::string function_name, wasm::own trap); + std::string getFailMessage(std::string_view function_name, wasm::own trap); wasm::vec getStrippedSource(); template @@ -115,7 +115,7 @@ class V8 : public WasmVm { absl::flat_hash_map host_functions_; absl::flat_hash_map> module_functions_; - std::string_view name_section_; + absl::flat_hash_map function_names_index_; }; // Helper functions. @@ -267,7 +267,30 @@ bool V8::load(const std::string &code, bool allow_precompiled) { assert((shared_module_ != nullptr)); } - name_section_ = getCustomSection("name"); + // build function index -> function name map for backtrace + // https://webassembly.github.io/spec/core/appendix/custom.html#binary-namesubsection + auto name_section = getCustomSection("name"); + const byte_t *pos = name_section.data(); + const byte_t *end = name_section.data() + name_section.size(); + + // module name subsection (id=0) is currently unimplemented in LLVM but we handle the + // case just in case + if (*pos == 0) { + pos++; + pos += parseVarint(pos, end); + } + + if (*pos == 1) { + pos++; + parseVarint(pos, end); // skip subsection size + const uint32_t namemap_vector_size = parseVarint(pos, end); + for (auto i = 0; i < namemap_vector_size; i++) { + const uint32_t func_index = parseVarint(pos, end); + const uint32_t func_name_size = parseVarint(pos, end); + function_names_index_.insert({func_index, std::string(pos, func_name_size)}); + pos += func_name_size; + } + } return module_ != nullptr; } @@ -279,7 +302,7 @@ std::unique_ptr V8::clone() { clone->store_ = wasm::Store::make(engine()); clone->module_ = wasm::Module::obtain(clone->store_.get(), shared_module_.get()); - clone->name_section_ = name_section; + clone->function_names_index_ = function_names_index_; return clone; } @@ -505,6 +528,7 @@ bool V8::link(std::string_view debug_name) { } break; } } + return !isFailed(); } @@ -666,58 +690,29 @@ void V8::getModuleFunctionImpl(std::string_view function_name, } std::string V8::getFailMessage(std::string_view function_name, wasm::own trap) { - auto message = "Function: " + function_name + " failed:\n"; + auto message = "Function: " + std::string(function_name) + " failed:\n"; message += "V8 message: " + std::string(trap->message().get(), trap->message().size()) + "\n"; auto trace = trap->trace(); - if (!trace.size() || !name_section_.size()) { - return message; - } + message += "wasm backtrace:\n"; + for (size_t i = 0; i < trace.size(); ++i) { + auto frame = trace[i].get(); + message += " " + std::to_string(i) + ": "; - const byte_t *pos = name_section_.data(); - const byte_t *end = name_section_.data() + name_section_.size(); - // https://webassembly.github.io/spec/core/appendix/custom.html#binary-namesubsection - - if (*pos == 0) { // module name section - pos++; - parseVarint(pos, end); // skip subsubsection size - const uint32_t module_name_size = parseVarint(pos, end); - message += "module name: " + std::string(pos, module_name_size) + "\n"; - pos += module_name_size; - } + std::stringstream address; + address << std::hex << frame->module_offset(); + message += " 0x" + address.str() + " - "; - if (*pos == 1) { // function name section - std::unordered_map function_names{}; - pos++; - parseVarint(pos, end); // skip subsection size - const uint32_t namemap_vector_size = parseVarint(pos, end); - for (auto i = 0; i < namemap_vector_size; i++) { - const uint32_t func_index = parseVarint(pos, end); - const uint32_t func_name_size = parseVarint(pos, end); - function_names.insert({func_index, std::string(pos, func_name_size)}); - pos += func_name_size; - } - - message += "wasm backtrace:\n"; - for (size_t i = 0; i < trace.size(); ++i) { - auto frame = trace[i].get(); - message += " " + std::to_string(i) + ": "; - - std::stringstream address; - address << std::hex << frame->module_offset(); - message += " 0x" + address.str() + " - "; - - auto func_index = frame->func_index(); - auto it = function_names.find(func_index); - std::string function_name; - if (it != function_names.end()) { - function_name = it->second; - } else { - function_name = "unknown(function index:" + std::to_string(func_index) + ")"; - } - message += function_name + "\n"; + auto func_index = frame->func_index(); + auto it = function_names_index_.find(func_index); + std::string function_name; + if (it != function_names_index_.end()) { + function_name = it->second; + } else { + function_name = "unknown(function index:" + std::to_string(func_index) + ")"; } + message += function_name + "\n"; } return message; } From 5cb4a07abccbf8a8245ec782470371030f1a2409 Mon Sep 17 00:00:00 2001 From: mathetake Date: Wed, 4 Nov 2020 11:24:56 +0900 Subject: [PATCH 06/17] align leading spaces like wavm Signed-off-by: mathetake --- src/v8/v8.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/v8/v8.cc b/src/v8/v8.cc index f2fb5b0e..5c5552d1 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -16,7 +16,7 @@ #include "include/proxy-wasm/v8.h" #include - +#include #include #include #include @@ -698,7 +698,9 @@ std::string V8::getFailMessage(std::string_view function_name, wasm::ownmodule_offset(); From 155e5c7d08fa032cfc59ca6c72b05cf69af82b08 Mon Sep 17 00:00:00 2001 From: mathetake Date: Tue, 17 Nov 2020 11:54:54 +0900 Subject: [PATCH 07/17] Tidy up trace message Signed-off-by: mathetake --- src/v8/v8.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/v8/v8.cc b/src/v8/v8.cc index 5c5552d1..5aa3e7e6 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -690,12 +690,12 @@ void V8::getModuleFunctionImpl(std::string_view function_name, } std::string V8::getFailMessage(std::string_view function_name, wasm::own trap) { - auto message = "Function: " + std::string(function_name) + " failed:\n"; - message += "V8 message: " + std::string(trap->message().get(), trap->message().size()) + "\n"; + auto message = "Function: " + std::string(function_name) + " failed: "; + message += std::string(trap->message().get(), trap->message().size()) + "\n"; auto trace = trap->trace(); - message += "wasm backtrace:\n"; + message += "Proxy-Wasm plugin in-VM backtrace:\n"; for (size_t i = 0; i < trace.size(); ++i) { auto frame = trace[i].get(); std::ostringstream oss; From 0ee805f17faed66168235b4bde317b97fdd485ff Mon Sep 17 00:00:00 2001 From: mathetake Date: Tue, 17 Nov 2020 12:17:25 +0900 Subject: [PATCH 08/17] Handle the case of having no name custom section Signed-off-by: mathetake --- src/v8/v8.cc | 43 ++++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/src/v8/v8.cc b/src/v8/v8.cc index 5aa3e7e6..b3336d71 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -270,25 +270,27 @@ bool V8::load(const std::string &code, bool allow_precompiled) { // build function index -> function name map for backtrace // https://webassembly.github.io/spec/core/appendix/custom.html#binary-namesubsection auto name_section = getCustomSection("name"); - const byte_t *pos = name_section.data(); - const byte_t *end = name_section.data() + name_section.size(); - - // module name subsection (id=0) is currently unimplemented in LLVM but we handle the - // case just in case - if (*pos == 0) { - pos++; - pos += parseVarint(pos, end); - } + if (name_section.size()) { + const byte_t *pos = name_section.data(); + const byte_t *end = name_section.data() + name_section.size(); + + // module name subsection (id=0) is currently unimplemented in LLVM but we handle the + // case just in case + if (*pos == 0) { + pos++; + pos += parseVarint(pos, end); + } - if (*pos == 1) { - pos++; - parseVarint(pos, end); // skip subsection size - const uint32_t namemap_vector_size = parseVarint(pos, end); - for (auto i = 0; i < namemap_vector_size; i++) { - const uint32_t func_index = parseVarint(pos, end); - const uint32_t func_name_size = parseVarint(pos, end); - function_names_index_.insert({func_index, std::string(pos, func_name_size)}); - pos += func_name_size; + if (*pos == 1) { + pos++; + parseVarint(pos, end); // skip subsection size + const uint32_t namemap_vector_size = parseVarint(pos, end); + for (auto i = 0; i < namemap_vector_size; i++) { + const uint32_t func_index = parseVarint(pos, end); + const uint32_t func_name_size = parseVarint(pos, end); + function_names_index_.insert({func_index, std::string(pos, func_name_size)}); + pos += func_name_size; + } } } return module_ != nullptr; @@ -693,8 +695,11 @@ std::string V8::getFailMessage(std::string_view function_name, wasm::ownmessage().get(), trap->message().size()) + "\n"; - auto trace = trap->trace(); + if (function_names_index_.empty()) { + return message; + } + auto trace = trap->trace(); message += "Proxy-Wasm plugin in-VM backtrace:\n"; for (size_t i = 0; i < trace.size(); ++i) { auto frame = trace[i].get(); From 9a555a0846059c52ca11f8068db004c29edde0a8 Mon Sep 17 00:00:00 2001 From: mathetake Date: Mon, 7 Dec 2020 20:02:35 +0900 Subject: [PATCH 09/17] Review: remove unnecessary 'this->' Signed-off-by: mathetake --- src/v8/v8.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/v8/v8.cc b/src/v8/v8.cc index b3336d71..cff5cd52 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -649,8 +649,7 @@ void V8::getModuleFunctionImpl(std::string_view function_name, SaveRestoreContext saved_context(context); auto trap = func->call(params, nullptr); if (trap) { - fail(FailState::RuntimeError, - this->getFailMessage(std::string(function_name), std::move(trap))); + fail(FailState::RuntimeError, getFailMessage(std::string(function_name), std::move(trap))); } }; } @@ -682,8 +681,7 @@ void V8::getModuleFunctionImpl(std::string_view function_name, SaveRestoreContext saved_context(context); auto trap = func->call(params, results); if (trap) { - fail(FailState::RuntimeError, - this->getFailMessage(std::string(function_name), std::move(trap))); + fail(FailState::RuntimeError, getFailMessage(std::string(function_name), std::move(trap))); return R{}; } R rvalue = results[0].get::type>(); From bb8fd80dad698cf400e1801179043c059ec9f6bf Mon Sep 17 00:00:00 2001 From: mathetake Date: Sat, 12 Dec 2020 14:26:27 +0900 Subject: [PATCH 10/17] Review: remove the trailing newline in msg & fix style Signed-off-by: mathetake --- src/v8/v8.cc | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/v8/v8.cc b/src/v8/v8.cc index bcf3c0f0..5f537687 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -85,8 +85,8 @@ class V8 : public WasmVm { #undef _GET_MODULE_FUNCTION private: - std::string getFailMessage(std::string_view function_name, wasm::own trap); wasm::vec getStrippedSource(); + std::string getFailMessage(std::string_view function_name, wasm::own trap); template void registerHostFunctionImpl(std::string_view module_name, std::string_view function_name, @@ -305,6 +305,7 @@ std::unique_ptr V8::clone() { clone->module_ = wasm::Module::obtain(clone->store_.get(), shared_module_.get()); clone->function_names_index_ = function_names_index_; + return clone; } @@ -698,12 +699,12 @@ std::string V8::getFailMessage(std::string_view function_name, wasm::owntrace(); - message += "\nProxy-Wasm plugin in-VM backtrace:\n"; + 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 += oss.str() + ": "; + message += "\n" + oss.str() + ": "; std::stringstream address; address << std::hex << frame->module_offset(); @@ -711,13 +712,11 @@ std::string V8::getFailMessage(std::string_view function_name, wasm::ownfunc_index(); auto it = function_names_index_.find(func_index); - std::string function_name; if (it != function_names_index_.end()) { - function_name = it->second; + message += it->second; } else { - function_name = "unknown(function index:" + std::to_string(func_index) + ")"; + message += "unknown(function index:" + std::to_string(func_index) + ")"; } - message += function_name + "\n"; } return message; } From a24e21325f14c077146ce09dd162b38a94429fba Mon Sep 17 00:00:00 2001 From: mathetake Date: Sat, 12 Dec 2020 14:55:23 +0900 Subject: [PATCH 11/17] Review: add buildFunctionNameIndex member Signed-off-by: mathetake --- src/v8/v8.cc | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/src/v8/v8.cc b/src/v8/v8.cc index 5f537687..83a83844 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -85,6 +85,7 @@ class V8 : public WasmVm { #undef _GET_MODULE_FUNCTION private: + void buildFunctionNameIndex(); wasm::vec getStrippedSource(); std::string getFailMessage(std::string_view function_name, wasm::own trap); @@ -267,33 +268,39 @@ 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 { + auto size = parseVarint(pos, end); + auto start = pos; + const uint32_t namemap_vector_size = parseVarint(pos, end); + for (auto i = 0; i < namemap_vector_size; i++) { + const uint32_t func_index = parseVarint(pos, end); + const uint32_t func_name_size = parseVarint(pos, end); + function_names_index_.insert({func_index, std::string(pos, func_name_size)}); + pos += func_name_size; + } - // module name subsection (id=0) is currently unimplemented in LLVM but we handle the - // case just in case - if (*pos == 0) { - pos++; - pos += parseVarint(pos, end); - } - - if (*pos == 1) { - pos++; - parseVarint(pos, end); // skip subsection size - const uint32_t namemap_vector_size = parseVarint(pos, end); - for (auto i = 0; i < namemap_vector_size; i++) { - const uint32_t func_index = parseVarint(pos, end); - const uint32_t func_name_size = parseVarint(pos, end); - function_names_index_.insert({func_index, std::string(pos, func_name_size)}); - pos += func_name_size; + if (start + size != pos) { + // indicates the mulformed function name subsection, so clear the stored indexes. + function_names_index_ = {}; + } } } } - return module_ != nullptr; } std::unique_ptr V8::clone() { From 4f815f5224ecc9aba98c863d5967caae3be5ce2c Mon Sep 17 00:00:00 2001 From: mathetake Date: Mon, 14 Dec 2020 18:36:13 +0900 Subject: [PATCH 12/17] review: fix typo Signed-off-by: mathetake --- src/v8/v8.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/v8/v8.cc b/src/v8/v8.cc index 83a83844..f05cd051 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -295,7 +295,7 @@ void V8::buildFunctionNameIndex() { } if (start + size != pos) { - // indicates the mulformed function name subsection, so clear the stored indexes. + // indicates the malformed function name subsection, so clear the stored indexes. function_names_index_ = {}; } } From a31cfdf4f04d84abae7fa9c524b2d193afc3ecbb Mon Sep 17 00:00:00 2001 From: mathetake Date: Tue, 15 Dec 2020 16:39:32 +0900 Subject: [PATCH 13/17] Break loop when name section is malformed Signed-off-by: mathetake --- src/v8/v8.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/v8/v8.cc b/src/v8/v8.cc index f05cd051..6d3b445a 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -297,6 +297,7 @@ void V8::buildFunctionNameIndex() { if (start + size != pos) { // indicates the malformed function name subsection, so clear the stored indexes. function_names_index_ = {}; + break; } } } From bcf218224458a23ffa9e8748ad2501eecf95ee80 Mon Sep 17 00:00:00 2001 From: mathetake Date: Tue, 15 Dec 2020 16:51:06 +0900 Subject: [PATCH 14/17] Review: check the size of subsection Signed-off-by: mathetake --- src/v8/v8.cc | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/v8/v8.cc b/src/v8/v8.cc index 6d3b445a..51c04877 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -284,18 +284,21 @@ void V8::buildFunctionNameIndex() { if (*pos++ != 1) { pos += parseVarint(pos, end); } else { - auto size = parseVarint(pos, end); - auto start = pos; - const uint32_t namemap_vector_size = parseVarint(pos, end); + const auto size = parseVarint(pos, end); + if (size == static_cast(-1) || pos + size > end) { + function_names_index_ = {}; + break; + } + const auto start = pos; + const auto namemap_vector_size = parseVarint(pos, end); for (auto i = 0; i < namemap_vector_size; i++) { - const uint32_t func_index = parseVarint(pos, end); - const uint32_t func_name_size = parseVarint(pos, end); + const auto func_index = parseVarint(pos, end); + const auto func_name_size = parseVarint(pos, end); function_names_index_.insert({func_index, std::string(pos, func_name_size)}); pos += func_name_size; } if (start + size != pos) { - // indicates the malformed function name subsection, so clear the stored indexes. function_names_index_ = {}; break; } From 6ea1604ebeb17edaf2cb2a06ff257cc3234efb9d Mon Sep 17 00:00:00 2001 From: mathetake Date: Tue, 15 Dec 2020 16:52:22 +0900 Subject: [PATCH 15/17] Return if malformed Signed-off-by: mathetake --- src/v8/v8.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/v8/v8.cc b/src/v8/v8.cc index 51c04877..8b757ecf 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -287,7 +287,7 @@ void V8::buildFunctionNameIndex() { const auto size = parseVarint(pos, end); if (size == static_cast(-1) || pos + size > end) { function_names_index_ = {}; - break; + return; } const auto start = pos; const auto namemap_vector_size = parseVarint(pos, end); @@ -300,7 +300,7 @@ void V8::buildFunctionNameIndex() { if (start + size != pos) { function_names_index_ = {}; - break; + return; } } } From 63312af41fecbb2677a334ad38d01f068b1b9922 Mon Sep 17 00:00:00 2001 From: mathetake Date: Tue, 15 Dec 2020 16:59:35 +0900 Subject: [PATCH 16/17] Review: check all the parsed value Signed-off-by: mathetake --- src/v8/v8.cc | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/v8/v8.cc b/src/v8/v8.cc index 8b757ecf..e3efec3a 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -291,9 +291,22 @@ void V8::buildFunctionNameIndex() { } const auto start = pos; const auto namemap_vector_size = parseVarint(pos, end); + if (namemap_vector_size == static_cast(-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(-1) || pos + func_index > end) { + function_names_index_ = {}; + return; + } + const auto func_name_size = parseVarint(pos, end); + if (func_name_size == static_cast(-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; } From afa45f07dcb0ec959d56a6ce72f8b5ef8fd136d1 Mon Sep 17 00:00:00 2001 From: mathetake Date: Tue, 15 Dec 2020 17:21:48 +0900 Subject: [PATCH 17/17] Remove unnecessary boundary check for func_index Signed-off-by: mathetake --- src/v8/v8.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/v8/v8.cc b/src/v8/v8.cc index e3efec3a..537dde50 100644 --- a/src/v8/v8.cc +++ b/src/v8/v8.cc @@ -297,7 +297,7 @@ void V8::buildFunctionNameIndex() { } for (auto i = 0; i < namemap_vector_size; i++) { const auto func_index = parseVarint(pos, end); - if (func_index == static_cast(-1) || pos + func_index > end) { + if (func_index == static_cast(-1)) { function_names_index_ = {}; return; }