Skip to content
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

build: update v8 to 12.7.224.18 #409

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
644e5e2
Compile with C++20
mpwarres Aug 18, 2024
fdb5e93
Update googletest to v1.15.2
mpwarres Aug 18, 2024
aab1611
Patch proxy-wasm-cpp-sdk to work around g++ -std=c++20 compilation issue
mpwarres Aug 19, 2024
749ebf8
Add temporary workaround for #412
mpwarres Aug 19, 2024
fc7a80b
Add -fno-sanitize=unsigned-integer-overflow to clang asan options
mpwarres Aug 19, 2024
c2f8102
Disable clang-tidy clang-diagnostic-builtin-macro-redefined check
mpwarres Aug 19, 2024
d7d6af7
Change asan config for NullVM and Wasmtime from clang-asan-strict to …
mpwarres Aug 19, 2024
5092179
v8: update to 12.7.224.18
mpwarres Aug 15, 2024
ce012d6
Update v8.patch to revert v8 commit b26554ec368e9553782012c96aa5e99b1…
mpwarres Aug 16, 2024
4614adf
Bind absl_* build targets referenced by v8 build rules
mpwarres Aug 16, 2024
ef3558a
Fetch fp16 dependency of v8, and patch v8 to find it
mpwarres Aug 16, 2024
7a25b87
Set torque generator path to location where Bazel can find outputs
mpwarres Aug 16, 2024
a65d5e9
Tolerate deprecated declarations in v8
mpwarres Aug 16, 2024
bf4df28
Build with C++20, and take 2 at compiling v8 with -Wno-deprecated-dec…
mpwarres Aug 16, 2024
42d7ee7
Fix tweak to where v8 looks for its fp16 dependency
mpwarres Aug 16, 2024
93358c2
Additionally specify --host_cxxopt=-std=c++20
mpwarres Aug 19, 2024
c1161ce
Merge branch 'use-cpp-20' into v8_v12.7.224.18
mpwarres Aug 19, 2024
394f075
Specify -Wno-invalid-offsetof for gcc builds
mpwarres Aug 19, 2024
e5407f4
Cherrypick V8 commit 35888fee7bbaaaf1f02ccc88a95c9a336fc790bc to addr…
mpwarres Aug 19, 2024
c4500ac
Use --cxxopt instead of --copt to specify -Wno-invalid-offsetof
mpwarres Aug 19, 2024
8f34e8f
Add -Wno-deprecated to gcc cxxopt
mpwarres Aug 19, 2024
4de318c
Merge branch 'main' into use-cpp-20
mpwarres Aug 19, 2024
abc9b10
Merge branch 'use-cpp-20' into v8_v12.7.224.18
mpwarres Aug 19, 2024
d4eec14
Specify -Wno-deprecated for gcc, take 2
mpwarres Aug 19, 2024
06f6411
Set gcc warning flags via --host_cxxopt as well
mpwarres Aug 19, 2024
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
10 changes: 6 additions & 4 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ build:clang-tidy --output_groups=report
build:gcc --action_env=BAZEL_COMPILER=gcc
build:gcc --action_env=CC=gcc
build:gcc --action_env=CXX=g++
build:gcc --cxxopt -Wno-invalid-offsetof --host_cxxopt -Wno-invalid-offsetof
build:gcc --cxxopt -Wno-deprecated --host_cxxopt -Wno-deprecated
Comment on lines +68 to +69
Copy link
Member

Choose a reason for hiding this comment

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

As a general rule, those V8 workarounds should be:

  1. added to v8.patch as patches against V8's bazel/defs.bzl,
  2. upstreamed to V8, so we don't need to maintain those local workarounds forever.

Especially, because those cxxopts won't be propagated to dependent projects (i.e. Envoy).


# Use Zig C/C++ compiler.
build:zig-cc --incompatible_enable_cc_toolchain_resolution
Expand All @@ -80,10 +82,10 @@ build:zig-cc-linux-aarch64 --test_env=QEMU_LD_PREFIX=/usr/aarch64-linux-gnu/

build --enable_platform_specific_config

# Use C++17.
build:linux --cxxopt=-std=c++17
build:macos --cxxopt=-std=c++17
build:windows --cxxopt="/std:c++17"
# Use C++20.
build:linux --cxxopt=-std=c++20 --host_cxxopt=-std=c++20
build:macos --cxxopt=-std=c++20 --host_cxxopt=-std=c++20
build:windows --cxxopt="/std:c++20" --host_cxxopt="/std:c++20"

# Enable symlinks and runfiles on Windows (enabled by default on other platforms).
startup --windows_enable_symlinks
Expand Down
1 change: 1 addition & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@ Checks: clang-*,
-readability-magic-numbers,
-readability-make-member-function-const,
-readability-simplify-boolean-expr,
-clang-diagnostic-builtin-macro-redefined,

WarningsAsErrors: '*'
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ jobs:
os: ubuntu-22.04
arch: x86_64
action: test
flags: --config=clang-asan-strict --define=crypto=system
flags: --config=clang-asan --define=crypto=system
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for that?

- name: 'NullVM on Linux/x86_64 with TSan'
engine: 'null'
os: ubuntu-22.04
Expand Down Expand Up @@ -235,7 +235,7 @@ jobs:
os: ubuntu-22.04
arch: x86_64
action: test
flags: --config=clang-asan-strict --define=crypto=system
flags: --config=clang-asan --define=crypto=system
- name: 'Wasmtime on Linux/aarch64'
engine: 'wasmtime'
repo: 'com_github_bytecodealliance_wasmtime'
Expand Down
13 changes: 0 additions & 13 deletions bazel/external/googletest.patch

This file was deleted.

33 changes: 33 additions & 0 deletions bazel/external/proxy_wasm_cpp_sdk.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Address g++ -std=c++20 variadic template compilation issue:
# https://github.com/proxy-wasm/proxy-wasm-cpp-host/pull/411#issuecomment-2295429152

diff --git a/proxy_wasm_api.h b/proxy_wasm_api.h
index 166b49c..263d2b9 100644
--- a/proxy_wasm_api.h
+++ b/proxy_wasm_api.h
@@ -1207,7 +1207,7 @@ struct SimpleHistogram {
template <typename... Tags> struct Counter : public MetricBase {
static Counter<Tags...> *New(std::string_view name, MetricTagDescriptor<Tags>... fieldnames);

- Counter<Tags...>(std::string_view name, MetricTagDescriptor<Tags>... descriptors)
+ Counter(std::string_view name, MetricTagDescriptor<Tags>... descriptors)
: Counter<Tags...>(std::string(name), std::vector<MetricTag>({toMetricTag(descriptors)...})) {
}

@@ -1256,7 +1256,7 @@ inline Counter<Tags...> *Counter<Tags...>::New(std::string_view name,
template <typename... Tags> struct Gauge : public MetricBase {
static Gauge<Tags...> *New(std::string_view name, MetricTagDescriptor<Tags>... fieldnames);

- Gauge<Tags...>(std::string_view name, MetricTagDescriptor<Tags>... descriptors)
+ Gauge(std::string_view name, MetricTagDescriptor<Tags>... descriptors)
: Gauge<Tags...>(std::string(name), std::vector<MetricTag>({toMetricTag(descriptors)...})) {}

SimpleGauge resolve(Tags... f) {
@@ -1302,6 +1302,6 @@ inline Gauge<Tags...> *Gauge<Tags...>::New(std::string_view name,
template <typename... Tags> struct Histogram : public MetricBase {
static Histogram<Tags...> *New(std::string_view name, MetricTagDescriptor<Tags>... fieldnames);

- Histogram<Tags...>(std::string_view name, MetricTagDescriptor<Tags>... descriptors)
+ Histogram(std::string_view name, MetricTagDescriptor<Tags>... descriptors)
: Histogram<Tags...>(std::string(name),
std::vector<MetricTag>({toMetricTag(descriptors)...})) {}
158 changes: 126 additions & 32 deletions bazel/external/v8.patch
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
# 1. Disable pointer compression (limits the maximum number of WasmVMs).
# 2. Don't expose Wasm C API (only Wasm C++ API).
# 3. Fix gcc build error by disabling nonnull warning.
# 4. Allow compiling v8 on macOS 10.15 to 13.0. TODO(dio): Will remove this patch when https://bugs.chromium.org/p/v8/issues/detail?id=13428 is fixed.
# 3. Revert v8 commit b26554ec368e9553782012c96aa5e99b163eaff2, which removed
# use of _allowlist_function_transition from v8 bazel/defs.bzl, since it is
# still required by the version of Bazel we currently use (6.5.0).
# 4. Tweak where v8 looks for its fp16 dependency, since it isn't downloaded by
# gn.
# 5. Set torque generator path to location where Bazel can find outputs.
# 6. Compile v8 with -Wno-deprecated-declarations
# 7. Cherrypick V8 commit 35888fee7bbaaaf1f02ccc88a95c9a336fc790bc to fix gcc
# build error.

diff --git a/BUILD.bazel b/BUILD.bazel
index 4e89f90e7e..3fcb38b3f3 100644
index 30be47fa333..72de2d422ca 100644
--- a/BUILD.bazel
+++ b/BUILD.bazel
@@ -157,7 +157,7 @@ v8_int(
@@ -220,7 +220,7 @@ v8_int(
# If no explicit value for v8_enable_pointer_compression, we set it to 'none'.
v8_string(
name = "v8_enable_pointer_compression",
Expand All @@ -16,42 +23,129 @@ index 4e89f90e7e..3fcb38b3f3 100644
)

# Default setting for v8_enable_pointer_compression.
@@ -3698,14 +3698,22 @@ filegroup(

v8_library(
name = "lib_fp16",
- srcs = ["third_party/fp16/src/include/fp16.h"],
- hdrs = [
- "third_party/fp16/src/include/fp16/fp16.h",
- "third_party/fp16/src/include/fp16/bitcasts.h",
+ hdrs = ["@fp16//:include/fp16.h"],
+ srcs = [],
+ include_prefix = "third_party/fp16/src",
+ deps = [
+ "lib_fp16_includes",
],
- includes = [
- "third_party/fp16/src/include",
+)
+
+v8_library(
+ name = "lib_fp16_includes",
+ hdrs = [
+ "@fp16//:include/fp16/fp16.h",
+ "@fp16//:include/fp16/bitcasts.h",
],
+ srcs = [],
+ strip_include_prefix = "include",
)

filegroup(
diff --git a/bazel/defs.bzl b/bazel/defs.bzl
index e957c0fad3..063627b72b 100644
index 520a311595e..64b4928abe5 100644
--- a/bazel/defs.bzl
+++ b/bazel/defs.bzl
@@ -131,6 +131,7 @@ def _default_args():
"-Wno-redundant-move",
"-Wno-return-type",
"-Wno-stringop-overflow",
+ "-Wno-nonnull",
# Use GNU dialect, because GCC doesn't allow using
# ##__VA_ARGS__ when in standards-conforming mode.
"-std=gnu++17",
@@ -151,6 +152,18 @@ def _default_args():
"-fno-integrated-as",
@@ -102,7 +102,9 @@ def _default_args():
],
"//conditions:default": [],
+ }) + select({
+ "@v8//bazel/config:is_macos": [
+ # The clang available on macOS catalina has a warning that isn't clean on v8 code.
+ "-Wno-range-loop-analysis",
+
+ # To supress warning on deprecated declaration on v8 code. For example:
+ # external/v8/src/base/platform/platform-darwin.cc:56:22: 'getsectdatafromheader_64'
+ # is deprecated: first deprecated in macOS 13.0.
+ # https://bugs.chromium.org/p/v8/issues/detail?id=13428.
+ "-Wno-deprecated-declarations",
+ ],
+ "//conditions:default": [],
}),
includes = ["include"],
linkopts = select({
- copts = select({
+ copts = [
+ "-Wno-deprecated-declarations",
+ ] + select({
"@v8//bazel/config:is_posix": [
"-fPIC",
"-fno-strict-aliasing",
@@ -131,7 +133,6 @@ def _default_args():
"-Wno-array-bounds",
"-Wno-class-memaccess",
"-Wno-comments",
- "-Wno-deprecated-declarations",
"-Wno-implicit-fallthrough",
"-Wno-int-in-bool-context",
"-Wno-maybe-uninitialized",
@@ -316,7 +317,7 @@ def v8_library(
# split the set of outputs by using OutputGroupInfo, that way we do not need to
# run the torque generator twice.
def _torque_files_impl(ctx):
- v8root = "."
+ v8root = "external/v8"

# Arguments
args = []
@@ -480,6 +481,9 @@ _v8_mksnapshot = rule(
cfg = "exec",
),
"target_os": attr.string(mandatory = True),
+ "_allowlist_function_transition": attr.label(
+ default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
+ ),
"prefix": attr.string(mandatory = True),
"suffix": attr.string(mandatory = True),
},
diff --git a/bazel/v8-non-pointer-compression.bzl b/bazel/v8-non-pointer-compression.bzl
index 8c929454840..57336154cf7 100644
--- a/bazel/v8-non-pointer-compression.bzl
+++ b/bazel/v8-non-pointer-compression.bzl
@@ -47,6 +47,17 @@ v8_binary_non_pointer_compression = rule(
# Note specificaly how it's configured with v8_target_cpu_transition, which
# ensures that setting propagates down the graph.
"binary": attr.label(cfg = v8_disable_pointer_compression),
+ # This is a stock Bazel requirement for any rule that uses Starlark
+ # transitions. It's okay to copy the below verbatim for all such rules.
+ #
+ # The purpose of this requirement is to give the ability to restrict
+ # which packages can invoke these rules, since Starlark transitions
+ # make much larger graphs possible that can have memory and performance
+ # consequences for your build. The whitelist defaults to "everything".
+ # But you can redefine it more strictly if you feel that's prudent.
+ "_allowlist_function_transition": attr.label(
+ default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
+ ),
},
# Making this executable means it works with "$ bazel run".
executable = True,
diff --git a/src/base/macros.h b/src/base/macros.h
index 794505278e1..890365bf3ae 100644
--- a/src/base/macros.h
+++ b/src/base/macros.h
@@ -182,7 +182,7 @@ namespace base {
// base::is_trivially_copyable will differ for these cases.
template <typename T>
struct is_trivially_copyable {
-#if V8_CC_MSVC
+#if V8_CC_MSVC || (__GNUC__ == 12 && __GNUC_MINOR__ <= 2)
// Unfortunately, MSVC 2015 is broken in that std::is_trivially_copyable can
// be false even though it should be true according to the standard.
// (status at 2018-02-26, observed on the msvc waterfall bot).
@@ -190,6 +190,11 @@ struct is_trivially_copyable {
// intended, so we reimplement this according to the standard.
// See also https://developercommunity.visualstudio.com/content/problem/
// 170883/msvc-type-traits-stdis-trivial-is-bugged.html.
+ //
+ // GCC 12.1 and 12.2 are broken too, they are shipped by some stable Linux
+ // distributions, so the same polyfill is also used.
+ // See
+ // https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=aeba3e009b0abfccaf01797556445dbf891cc8dc
static constexpr bool value =
// Copy constructor is trivial or deleted.
(std::is_trivially_copy_constructible<T>::value ||
diff --git a/src/wasm/c-api.cc b/src/wasm/c-api.cc
index 4473e205c0..65a6ec7e1d 100644
index 2d6d0f6c270..61d071acd52 100644
--- a/src/wasm/c-api.cc
+++ b/src/wasm/c-api.cc
@@ -2247,6 +2247,8 @@ auto Instance::exports() const -> ownvec<Extern> {
@@ -2360,6 +2360,8 @@ auto Instance::exports() const -> ownvec<Extern> {

} // namespace wasm

Expand All @@ -60,7 +154,7 @@ index 4473e205c0..65a6ec7e1d 100644
// BEGIN FILE wasm-c.cc

extern "C" {
@@ -3274,3 +3276,5 @@ wasm_instance_t* wasm_frame_instance(const wasm_frame_t* frame) {
@@ -3386,3 +3388,5 @@ wasm_instance_t* wasm_frame_instance(const wasm_frame_t* frame) {
#undef WASM_DEFINE_SHARABLE_REF

} // extern "C"
Expand Down
41 changes: 0 additions & 41 deletions bazel/external/v8_include.patch

This file was deleted.

Loading
Loading