From 69395dc925e2e4f563a834135b9b755126b116be Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Thu, 22 Aug 2019 17:57:18 -0700 Subject: [PATCH] build: use common source and local toolchain when building wee8. (#143) Signed-off-by: Piotr Sikora --- .circleci/config.yml | 4 ++- bazel/BUILD | 10 ------ bazel/external/wee8.genrule_cmd | 2 +- bazel/external/wee8.patch | 55 ++++++++++++++++++++++++++++++++- bazel/repositories.bzl | 31 +++++-------------- bazel/repository_locations.bzl | 16 +++------- test/test_common/environment.cc | 8 ++--- 7 files changed, 74 insertions(+), 52 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 401c6ca19c34..95d19d3dbf23 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -73,10 +73,12 @@ jobs: macos: macos: - xcode: "10.2.1" + xcode: "11.0.0" environment: BAZEL_BUILD_EXTRA_OPTIONS: "--define wasm=v8" # v8 only, WAVM segfaults in tests. BAZEL_TEST_TARGETS: "//test/extensions/access_loggers/wasm/... //test/extensions/filters/http/wasm/... //test/extensions/wasm/..." + CC: clang + CXX: clang++ steps: - run: sudo sntp -sS time.apple.com - run: rm -rf /home/circleci/project/.git # CircleCI git caching is likely broken diff --git a/bazel/BUILD b/bazel/BUILD index a3ceca307f01..3ce73e899b22 100755 --- a/bazel/BUILD +++ b/bazel/BUILD @@ -214,16 +214,6 @@ alias( }), ) -# Alias pointing to a platform-specific version of wee8. -alias( - name = "wee8", - actual = select({ - ":darwin": "@wee8_macos//:wee8", - ":darwin_x86_64": "@wee8_macos//:wee8", - "//conditions:default": "@wee8_linux//:wee8", - }), -) - config_setting( name = "linux_x86_64", values = {"cpu": "k8"}, diff --git a/bazel/external/wee8.genrule_cmd b/bazel/external/wee8.genrule_cmd index 1b5f37d14a76..e93bc8d685cc 100644 --- a/bazel/external/wee8.genrule_cmd +++ b/bazel/external/wee8.genrule_cmd @@ -16,7 +16,7 @@ pushd $$ROOT/wee8 rm -rf out/wee8 # Build wee8. -third_party/depot_tools/gn gen out/wee8 --args="v8_use_external_startup_data=false v8_enable_i18n_support=false v8_enable_gdbjit=false v8_expose_symbols=true is_component_build=false is_debug=false use_sysroot=false use_custom_libcxx=false" +AR=ar NM=nm third_party/depot_tools/gn gen out/wee8 --args='v8_use_external_startup_data=false v8_enable_i18n_support=false v8_enable_gdbjit=false v8_expose_symbols=true is_component_build=false is_debug=false clang_use_chrome_plugins=false use_sysroot=false use_custom_libcxx=false custom_toolchain="//build/toolchain/linux/unbundle:default"' third_party/depot_tools/ninja -C out/wee8 wee8 # Move compiled library to the expected destinations. diff --git a/bazel/external/wee8.patch b/bazel/external/wee8.patch index b64ec9738883..152aad34b727 100644 --- a/bazel/external/wee8.patch +++ b/bazel/external/wee8.patch @@ -1,6 +1,28 @@ # 1. Fix handling of f64 globals. # 2. Force full GC when destroying VMs. # 3. Fix build with -DDEBUG. +# 4. Fix for VMs with overlapping lifetimes (https://crrev.com/c/1698387). +# 5. Fix linking with unbundled toolchain on macOS. +--- a/wee8/build/toolchain/gcc_toolchain.gni ++++ b/wee8/build/toolchain/gcc_toolchain.gni +@@ -355,6 +355,8 @@ + # AIX does not support either -D (deterministic output) or response + # files. + command = "$ar -X64 {{arflags}} -r -c -s {{output}} {{inputs}}" ++ } else if (current_os == "mac") { ++ command = "\"$ar\" {{arflags}} -r -c -s {{output}} {{inputs}}" + } else { + rspfile = "{{output}}.rsp" + rspfile_content = "{{inputs}}" +@@ -546,7 +548,7 @@ + + start_group_flag = "" + end_group_flag = "" +- if (current_os != "aix") { ++ if (current_os != "aix" && current_os != "mac") { + # the "--start-group .. --end-group" feature isn't available on the aix ld. + start_group_flag = "-Wl,--start-group" + end_group_flag = "-Wl,--end-group " --- a/wee8/src/wasm/c-api.cc +++ b/wee8/src/wasm/c-api.cc @@ -825,7 +825,7 @@ void global_set_f32(v8::Local global, float val) { @@ -12,7 +34,16 @@ } // Tables -@@ -1107,7 +1107,7 @@ class StoreImpl { +@@ -985,7 +985,7 @@ auto seal(const typename implement::type* x) -> const C* { + return reinterpret_cast(x); + } + +-#ifdef DEBUG ++#if 0 + template + void vec::make_data() {} + +@@ -1107,13 +1107,12 @@ class StoreImpl { StoreImpl() {} ~StoreImpl() { @@ -21,6 +52,28 @@ reinterpret_cast(isolate_)->heap()->PreciseCollectAllGarbage( i::Heap::kNoGCFlags, i::GarbageCollectionReason::kTesting, v8::kGCCallbackFlagForced); + #endif + context()->Exit(); +- isolate_->Exit(); + isolate_->Dispose(); + delete create_params_.array_buffer_allocator; + } +@@ -1163,7 +1162,6 @@ auto Store::make(Engine*) -> own { + if (!isolate) return own(); + + { +- v8::Isolate::Scope isolate_scope(isolate); + v8::HandleScope handle_scope(isolate); + + // Create context. +@@ -1246,7 +1244,6 @@ auto Store::make(Engine*) -> own { + store->host_data_map_ = v8::Eternal(isolate, map); + } + +- store->isolate()->Enter(); + store->context()->Enter(); + isolate->SetData(0, store.get()); + --- a/wee8/third_party/wasm-api/wasm.hh +++ b/wee8/third_party/wasm-api/wasm.hh @@ -111,7 +111,7 @@ class vec { diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 725c4e1ce3b7..8b53b129a7ec 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -120,14 +120,6 @@ def envoy_dependencies(skip_targets = []): actual = "@envoy//bazel:boringssl", ) - # Binding to an alias pointing to a platform-specific version of wee8. - _wee8_linux() - _wee8_macos() - native.bind( - name = "wee8", - actual = "@envoy//bazel:wee8", - ) - # The long repo names (`com_github_fmtlib_fmt` instead of `fmtlib`) are # semi-standard in the Bazel community, intended to avoid both duplicate # dependencies and name conflicts. @@ -158,6 +150,7 @@ def envoy_dependencies(skip_targets = []): _com_github_curl() _com_github_envoyproxy_sqlparser() _com_googlesource_quiche() + _com_googlesource_chromium_v8() _org_llvm_llvm() _com_github_wavm_wavm() _com_lightstep_tracer_cpp() @@ -731,26 +724,18 @@ def _com_github_wavm_wavm(): actual = "@envoy//bazel/foreign_cc:wavm", ) -def _wee8_linux(): - location = REPOSITORY_LOCATIONS["wee8_linux"] +def _com_googlesource_chromium_v8(): + location = REPOSITORY_LOCATIONS["com_googlesource_chromium_v8"] genrule_repository( - name = "wee8_linux", - urls = location["urls"], - sha256 = location["sha256"], + name = "com_googlesource_chromium_v8", genrule_cmd_file = "@envoy//bazel/external:wee8.genrule_cmd", build_file = "@envoy//bazel/external:wee8.BUILD", patches = ["@envoy//bazel/external:wee8.patch"], + **location ) - -def _wee8_macos(): - location = REPOSITORY_LOCATIONS["wee8_macos"] - genrule_repository( - name = "wee8_macos", - urls = location["urls"], - sha256 = location["sha256"], - genrule_cmd_file = "@envoy//bazel/external:wee8.genrule_cmd", - build_file = "@envoy//bazel/external:wee8.BUILD", - patches = ["@envoy//bazel/external:wee8.patch"], + native.bind( + name = "wee8", + actual = "@com_googlesource_chromium_v8//:wee8", ) def _foreign_cc_dependencies(): diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 637c2e1bd7ad..0436eacff131 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -241,19 +241,11 @@ REPOSITORY_LOCATIONS = dict( strip_prefix = "WAVM-95dbf08c8695b8941e7020c557d8612f9d2af895", urls = ["https://github.com/WAVM/WAVM/archive/95dbf08c8695b8941e7020c557d8612f9d2af895.tar.gz"], ), - wee8_linux = dict( + com_googlesource_chromium_v8 = dict( # This archive was created using https://storage.googleapis.com/envoyproxy-wee8/wee8-archive.sh - # and contains complete checkout of v8 with all dependencies necessary to build wee8 on Linux-x86_64. - # TODO(PiotrSikora): switch to local compiler and provide single platform-agnostic archive. - sha256 = "1caebced30cb9d3531be4720b70c9132c988b362160f7721bc01caeb572c0eb7", - urls = ["https://storage.googleapis.com/envoyproxy-wee8/wee8-7.5.288.22-linux-x86_64.tar.gz"], - ), - wee8_macos = dict( - # This archive was created using https://storage.googleapis.com/envoyproxy-wee8/wee8-archive.sh - # and contains complete checkout of v8 with all dependencies necessary to build wee8 on macOS-x86_64. - # TODO(PiotrSikora): switch to local compiler and provide single platform-agnostic archive. - sha256 = "f84e423417db0b03b96e853b7d92f69be7d4936a33d1e8e05848fb146925ff68", - urls = ["https://storage.googleapis.com/envoyproxy-wee8/wee8-7.5.288.22-macos-x86_64.tar.gz"], + # and contains complete checkout of v8 with all dependencies necessary to build wee8. + sha256 = "d59c26ce5d7a2b23fb6251823df700cda0dc9a93086c932be5dfd9048a763d03", + urls = ["https://storage.googleapis.com/envoyproxy-wee8/wee8-7.5.288.22.tar.gz"], ), io_opencensus_cpp = dict( sha256 = "8d6016e47c2e19e7acbadb6f905b8c422748c64299d71101ac8f28151677e195", diff --git a/test/test_common/environment.cc b/test/test_common/environment.cc index 86b0a0ec3bdd..d0f0577f4763 100644 --- a/test/test_common/environment.cc +++ b/test/test_common/environment.cc @@ -8,7 +8,7 @@ #if defined(_LIBCPP_VERSION) && !defined(__APPLE__) #include #elif defined __has_include -#if __has_include() +#if __has_include() && !defined(__APPLE__) #include #endif #endif @@ -44,7 +44,7 @@ std::string makeTempDir(char* name_template) { name_template, strerror(errno))); #if defined(_LIBCPP_VERSION) && _LIBCPP_VERSION >= 9000 && !defined(__APPLE__) std::__fs::filesystem::create_directories(dirname); -#elif defined __cpp_lib_experimental_filesystem +#elif defined __cpp_lib_experimental_filesystem && !defined(__APPLE__) std::experimental::filesystem::create_directories(dirname); #endif #else @@ -102,7 +102,7 @@ void TestEnvironment::createParentPath(const std::string& path) { // We don't want to rely on mkdir etc. if we can avoid it, since it might not // exist in some environments such as ClusterFuzz. std::__fs::filesystem::create_directories(std::__fs::filesystem::path(path).parent_path()); -#elif defined __cpp_lib_experimental_filesystem +#elif defined __cpp_lib_experimental_filesystem && !defined(__APPLE__) std::experimental::filesystem::create_directories( std::experimental::filesystem::path(path).parent_path()); #else @@ -120,7 +120,7 @@ void TestEnvironment::removePath(const std::string& path) { return; } std::__fs::filesystem::remove_all(std::__fs::filesystem::path(path)); -#elif defined __cpp_lib_experimental_filesystem +#elif defined __cpp_lib_experimental_filesystem && !defined(__APPLE__) if (!std::experimental::filesystem::exists(path)) { return; }