Skip to content
This repository has been archived by the owner on Dec 16, 2020. It is now read-only.

Compile with protobuf full does not work with V8 #32

Closed
bianpengyuan opened this issue Jun 3, 2019 · 4 comments · Fixed by #29
Closed

Compile with protobuf full does not work with V8 #32

bianpengyuan opened this issue Jun 3, 2019 · 4 comments · Fixed by #29

Comments

@bianpengyuan
Copy link
Contributor

I tried to recompile the example in example/wasm/ wasm module with intrinsics.proto and libprotobuf instead of libprotobuf_lite, but v8 failed to run it.

diff --git a/examples/wasm/Makefile b/examples/wasm/Makefile
index 9a71c6f23..650ce9393 100644
--- a/examples/wasm/Makefile
+++ b/examples/wasm/Makefile
@@ -1,3 +1,3 @@
 all: envoy_filter_http_wasm_example.wasm
 
-include ../../api/wasm/cpp/Makefile.base_lite
+include ../../api/wasm/cpp/Makefile.base
diff --git a/examples/wasm/envoy.yaml b/examples/wasm/envoy.yaml
index f89990a7f..61b4a2097 100644
--- a/examples/wasm/envoy.yaml
+++ b/examples/wasm/envoy.yaml
@@ -26,7 +26,7 @@ static_resources:
           - name: envoy.wasm
             config:
               vm_config:
-                vm: "envoy.wasm.vm.wavm"
+                vm: "envoy.wasm.vm.v8"
                 code:
                   filename: "/etc/envoy_filter_http_wasm_example.wasm"
                 allow_precompiled: true

Here is the error output:

[libprotobuf  google/protobuf/descriptor_database.cc:318] Invalid file descriptor data passed to EncodedDescriptorDatabase::Add().

We've seen this before when running wasm-sd module and we thought the problem might be from generated code there. But seems like it is more fundamental than that.

@PiotrSikora @jplevyak

@PiotrSikora
Copy link
Contributor

There is actually one more error line:

[libprotobuf ERROR google/protobuf/descriptor_database.cc:318] Invalid file descriptor data passed to EncodedDescriptorDatabase::Add().
[libprotobuf FATAL google/protobuf/descriptor.cc:1358] CHECK failed: GeneratedDatabase()->Add(encoded_file_descriptor, size):

I've spent some time last week and over the weekend digging into this, but I didn't find the reason for this failure yet. I'm going to add support for the cxa_allocate_exception() to get the stacktrace.

I looked at the module<>host traces between WAVM and V8 and they are pretty much the same. The calls are limited to emscripten_get_heap_size() and pthread_equal(), and in neither case the module tries to access memory, so this is pretty puzzling.

@bianpengyuan @mandarjog any chance one of you could try to see if it loads under node.js?

@bianpengyuan
Copy link
Contributor Author

bianpengyuan commented Jun 3, 2019

Btw, by code reading I found it seems like v8 and wavm behave differently in start() function, where v8 calls all module functions with __Global__ prefix and that does not happen in wavm. If you look at generated module code, bunch of protobuf related functions have that kind of pattern:

  (export "__GLOBAL__sub_I_descriptor_pb_cc" (func $__GLOBAL__sub_I_descriptor_pb_cc))
  (export "__GLOBAL__sub_I_message_cc" (func $__GLOBAL__sub_I_message_cc))
  (export "__GLOBAL__sub_I_proxy_wasm_intrinsics_cc" (func $__GLOBAL__sub_I_proxy_wasm_intrinsics_cc))
  (export "__GLOBAL__sub_I_proxy_wasm_intrinsics_pb_cc" (func $__GLOBAL__sub_I_proxy_wasm_intrinsics_pb_cc))
  (export "__GLOBAL__sub_I_status_cc" (func $__GLOBAL__sub_I_status_cc))
  (export "__GLOBAL__sub_I_struct_pb_cc" (func $__GLOBAL__sub_I_struct_pb_cc))

Could wavm successfully run just because those functions calls are not triggered? Why do we need to call those __Global__ prefixed functions at start in v8?

@PiotrSikora
Copy link
Contributor

@bianpengyuan those functions are called in WAVM, but the library is handling it internally, so you don't see them being explicitly called in Envoy's codebase.

@PiotrSikora
Copy link
Contributor

OK, I figured it out... DYNAMICTOP wasn't properly initialized in memory. Fixed in #29.

PiotrSikora added a commit that referenced this issue Jun 4, 2019
This allows us to use more recent versions of Emscripten.

Fixes #32.

Signed-off-by: Piotr Sikora <[email protected]>
PiotrSikora pushed a commit that referenced this issue Aug 13, 2019
yxue pushed a commit to yxue/envoy-wasm that referenced this issue Oct 8, 2019
…cripten support). STACKED on Emscripten version PR. (envoyproxy#32)

* Add support for many intrinsic modules (e.g. for Emscripten support).
brian-avery added a commit to brian-avery/envoy-wasm that referenced this issue Sep 29, 2020
* Update SHA for CVE 2020-25017

* add bazel build args into test framework and do not fail silently

* allow empty args and remove spaces

Co-authored-by: Pengyuan Bian <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants