Skip to content

Add WASM SEGV handler and test. #26

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.

Sign up for GitHub

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

Closed
jplevyak wants to merge 1 commit into from
Closed

Add WASM SEGV handler and test. #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions source/extensions/common/wasm/wasm.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include <memory>
#include <string>
#include <vector>

#include "envoy/common/exception.h"
#include "envoy/config/wasm/v2/wasm.pb.validate.h"
Expand Down Expand Up @@ -440,6 +441,7 @@ uint32_t httpCallHandler(void* raw_context, uint32_t uri_ptr, uint32_t uri_size,

uint32_t getTotalMemoryHandler(void*) { return 0x7FFFFFFF; }
uint32_t _emscripten_get_heap_sizeHandler(void*) { return 0x7FFFFFFF; }
void _llvm_trapHandler(void*) { throw WasmException("emscripten llvm_trap"); }

void setTickPeriodMillisecondsHandler(void* raw_context, uint32_t tick_period_milliseconds) {
WASM_CONTEXT(raw_context)->setTickPeriod(std::chrono::milliseconds(tick_period_milliseconds));
Expand Down Expand Up @@ -938,6 +940,7 @@ Wasm::Wasm(absl::string_view vm, absl::string_view id, absl::string_view initial
#define _REGISTER(_fn) registerCallback(wasm_vm_.get(), #_fn, &_fn##Handler);
_REGISTER(getTotalMemory);
_REGISTER(_emscripten_get_heap_size);
_REGISTER(_llvm_trap);
#undef _REGISTER

// Calls with the "_proxy_" prefix.
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/common/wasm/wasm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ class Wasm : public Envoy::Server::Wasm,
uint32_t next_context_id_ = 0;
std::unique_ptr<WasmVm> wasm_vm_;
std::shared_ptr<Context> general_context_; // Context unrelated to any specific stream.
std::function<void(Context*)> tick_;
std::function<void(Common::Wasm::Context*)> tick_;
std::chrono::milliseconds tick_period_;
Event::TimerPtr timer_;

Expand Down
2 changes: 1 addition & 1 deletion test/extensions/wasm/test_data/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
NO_CONTEXT = true

all: logging.wasm bad_signature.wasm
all: logging.wasm bad_signature.wasm segv.wasm

include ../../../../api/wasm/cpp/Makefile.base
12 changes: 12 additions & 0 deletions test/extensions/wasm/test_data/segv.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// NOLINT(namespace-envoy)
Copy link

@PiotrSikora PiotrSikora Feb 21, 2019

Choose a reason for hiding this comment

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

You need to add segv.wasm and segv.wat to this commit.

Copy link
Author

@jplevyak jplevyak Feb 21, 2019

Choose a reason for hiding this comment

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

done.

Copy link
Author

@jplevyak jplevyak Feb 21, 2019

Choose a reason for hiding this comment

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

How can I reproduce the failure you are seeing? When I run bazel test wasm_test in envoy/test/extensions/wasm I get:

//test/extensions/wasm:wasm_test PASSED in 2.4s

Which is to say I am seeing the Exception caught.

Copy link

@PiotrSikora PiotrSikora Feb 21, 2019

Choose a reason for hiding this comment

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

Yes, it works fine in mocked tests (since you're explicitly catching Exception using EXPECT_THROW_WITH_MESSAGE), but that has nothing to do with "normal" usage.

To reproduce that stacktrace, you can add badptr code to any WASM HTTP filter (e.g. headers.cc), recompile it, and start Envoy with it. Envoy should crash when executing badptr code.

Copy link
Author

@jplevyak jplevyak Feb 21, 2019

Choose a reason for hiding this comment

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

Ah, yes, from the unhandled Exception. I was thinking that generally we would convert all errors of this type to Exceptions so that we can catch them and signal the controller (istio) then unload/disable the filter but I haven't completed that code yet. I was expecting to do it in a later PR. I can capture the stack pointer/trace when I throw the Exception as well. That should generalize to this case as well as those covered by Runtime::catchRuntimeExceptions(). Do you have another design in mind. Let's talk offline.

#include <string>

#include "proxy_wasm_intrinsics.h"

static int* badptr = nullptr;

extern "C" EMSCRIPTEN_KEEPALIVE void proxy_onStart() {
logError("before badptr");
*badptr = 1;
logError("after badptr");
}
Binary file added test/extensions/wasm/test_data/segv.wasm
View file
Open in desktop
Binary file not shown.
Loading