From abb71f53074dbb23f671c44a70118daeed61e240 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 29 Nov 2023 12:46:27 +0900 Subject: [PATCH] chore: make use of the v8_expose_public_symbols flag (#40624) * chore: make use of the v8_expose_public_symbols flag Use the newly added v8_expose_public_symbols flag to expose V8 symbols, instead of relying on custom patches. * chore: update patches --------- Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com> --- build/args/all.gn | 3 + patches/v8/.patches | 4 +- ...ymbols_with_v8_expose_public_symbols.patch | 220 ++++++++++++++++++ ...build_expose_mksnapshot_to_embedders.patch | 20 ++ patches/v8/build_gn.patch | 40 ---- ...export_private_v8_symbols_on_windows.patch | 52 ----- 6 files changed, 245 insertions(+), 94 deletions(-) create mode 100644 patches/v8/build_correctly_expose_public_symbols_with_v8_expose_public_symbols.patch create mode 100644 patches/v8/build_expose_mksnapshot_to_embedders.patch delete mode 100644 patches/v8/build_gn.patch delete mode 100644 patches/v8/do_not_export_private_v8_symbols_on_windows.patch diff --git a/build/args/all.gn b/build/args/all.gn index c93c2d994aff..eb0549da3a90 100644 --- a/build/args/all.gn +++ b/build/args/all.gn @@ -60,3 +60,6 @@ enable_dangling_raw_ptr_checks = false # This flag speeds up the performance of fork/execve on linux systems. # Ref: https://chromium-review.googlesource.com/c/v8/v8/+/4602858 v8_enable_private_mapping_fork_optimization = true + +# Expose public V8 symbols for native modules. +v8_expose_public_symbols = true diff --git a/patches/v8/.patches b/patches/v8/.patches index a5c050d70cc2..8946c10af7f3 100644 --- a/patches/v8/.patches +++ b/patches/v8/.patches @@ -1,3 +1,3 @@ -build_gn.patch -do_not_export_private_v8_symbols_on_windows.patch chore_allow_customizing_microtask_policy_per_context.patch +build_expose_mksnapshot_to_embedders.patch +build_correctly_expose_public_symbols_with_v8_expose_public_symbols.patch diff --git a/patches/v8/build_correctly_expose_public_symbols_with_v8_expose_public_symbols.patch b/patches/v8/build_correctly_expose_public_symbols_with_v8_expose_public_symbols.patch new file mode 100644 index 000000000000..2aff9090f9eb --- /dev/null +++ b/patches/v8/build_correctly_expose_public_symbols_with_v8_expose_public_symbols.patch @@ -0,0 +1,220 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Cheng Zhao +Date: Mon, 27 Nov 2023 20:02:16 +0900 +Subject: build: correctly expose public symbols with v8_expose_public_symbols + +Backport: https://chromium-review.googlesource.com/c/v8/v8/+/5052302 + +diff --git a/BUILD.gn b/BUILD.gn +index 206f1084252a264b060239e7218331d88a858764..68633c7cad5d6c6d64eeae51bad44b6bf803c5ea 100644 +--- a/BUILD.gn ++++ b/BUILD.gn +@@ -723,6 +723,10 @@ if (v8_enable_single_generation == true) { + assert(!v8_enable_snapshot_compression || v8_use_zlib, + "Snapshot compression requires zlib") + ++if (v8_expose_public_symbols == "") { ++ v8_expose_public_symbols = v8_expose_symbols ++} ++ + v8_random_seed = "314159265" + v8_toolset_for_shell = "host" + +@@ -757,6 +761,8 @@ config("internal_config") { + ] + + if (is_component_build) { ++ defines += [ "BUILDING_V8_SHARED_PRIVATE" ] ++ } else if (v8_expose_public_symbols) { + defines += [ "BUILDING_V8_SHARED" ] + } + +@@ -827,7 +833,10 @@ config("external_config") { + configs = [ ":headers_config" ] + defines = [] + if (is_component_build) { +- defines += [ "USING_V8_SHARED" ] ++ defines += [ ++ "USING_V8_SHARED", ++ "USING_V8_SHARED_PRIVATE", ++ ] + } + + if (current_cpu == "riscv64" || current_cpu == "riscv32") { +diff --git a/gni/v8.gni b/gni/v8.gni +index b2cc02ec80d468d5fb6ae55006aa6776901fe099..a72edb4ca320b933525ccaaf7f6bbfe805d509f9 100644 +--- a/gni/v8.gni ++++ b/gni/v8.gni +@@ -48,7 +48,11 @@ declare_args() { + # Enable monolithic static library for embedders. + v8_monolithic = false + +- # Expose symbols for dynamic linking. ++ # Expose public symbols for native modules of Node.js and Electron. Default ++ # is false. ++ v8_expose_public_symbols = "" ++ ++ # Deprecated for v8_expose_public_symbols. + v8_expose_symbols = false + + # Implement tracing using Perfetto (https://perfetto.dev). +@@ -86,7 +90,8 @@ declare_args() { + # Enable runtime call stats. + # TODO(liviurau): Remove old name after Chromium config update + # https://crbug.com/1476977. +- v8_enable_runtime_call_stats = !(is_on_release_branch || v8_is_on_release_branch) ++ v8_enable_runtime_call_stats = ++ !(is_on_release_branch || v8_is_on_release_branch) + + # Add fuzzilli fuzzer support. + v8_fuzzilli = false +@@ -257,8 +262,7 @@ if (v8_symbol_level != symbol_level) { + } + } + +-if ((is_posix || is_fuchsia) && +- (v8_enable_backtrace || v8_monolithic || v8_expose_symbols)) { ++if ((is_posix || is_fuchsia) && (v8_enable_backtrace || v8_monolithic)) { + v8_remove_configs += [ "//build/config/gcc:symbol_visibility_hidden" ] + v8_add_configs += [ "//build/config/gcc:symbol_visibility_default" ] + } +diff --git a/include/v8config.h b/include/v8config.h +index f407c289b1bd984d883a9b2a8670d21ba1eba7aa..ae42abde643f44349d3aecc1ab4e207982aa0d0f 100644 +--- a/include/v8config.h ++++ b/include/v8config.h +@@ -700,6 +700,11 @@ path. Add it with -I to the command line + #define V8_CLANG_NO_SANITIZE(what) + #endif + ++// Exposing private symbols requires exposing public symbols too. ++#ifdef BUILDING_V8_SHARED_PRIVATE ++#define BUILDING_V8_SHARED ++#endif ++ + #if defined(BUILDING_V8_SHARED) && defined(USING_V8_SHARED) + #error Inconsistent build configuration: To build the V8 shared library \ + set BUILDING_V8_SHARED, to include its headers for linking against the \ +diff --git a/src/base/macros.h b/src/base/macros.h +index 56ff8736b3000644905f128fd365797424761798..8994d888b508f41f1141c9d9b2f0bbe78919fea7 100644 +--- a/src/base/macros.h ++++ b/src/base/macros.h +@@ -388,9 +388,9 @@ bool is_inbounds(float_t v) { + + // Setup for Windows shared library export. + #define V8_EXPORT_ENUM +-#ifdef BUILDING_V8_SHARED ++#ifdef BUILDING_V8_SHARED_PRIVATE + #define V8_EXPORT_PRIVATE __declspec(dllexport) +-#elif USING_V8_SHARED ++#elif USING_V8_SHARED_PRIVATE + #define V8_EXPORT_PRIVATE __declspec(dllimport) + #else + #define V8_EXPORT_PRIVATE +@@ -400,7 +400,7 @@ bool is_inbounds(float_t v) { + + // Setup for Linux shared library export. + #if V8_HAS_ATTRIBUTE_VISIBILITY +-#ifdef BUILDING_V8_SHARED ++#ifdef BUILDING_V8_SHARED_PRIVATE + #define V8_EXPORT_PRIVATE __attribute__((visibility("default"))) + #define V8_EXPORT_ENUM V8_EXPORT_PRIVATE + #else +diff --git a/src/common/ptr-compr-inl.h b/src/common/ptr-compr-inl.h +index d5a3d5a55bfa860e19765e62c1b1c77285859d85..4a9ec6f448496daa103bd996729e9db725d68a64 100644 +--- a/src/common/ptr-compr-inl.h ++++ b/src/common/ptr-compr-inl.h +@@ -44,7 +44,8 @@ Address V8HeapCompressionScheme::GetPtrComprCageBaseAddress( + // static + void V8HeapCompressionScheme::InitBase(Address base) { + CHECK_EQ(base, GetPtrComprCageBaseAddress(base)); +-#if defined(USING_V8_SHARED) && defined(V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE) ++#if defined(USING_V8_SHARED_PRIVATE) && \ ++ defined(V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE) + set_base_non_inlined(base); + #else + base_ = base; +@@ -53,7 +54,8 @@ void V8HeapCompressionScheme::InitBase(Address base) { + + // static + V8_CONST Address V8HeapCompressionScheme::base() { +-#if defined(USING_V8_SHARED) && defined(V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE) ++#if defined(USING_V8_SHARED_PRIVATE) && \ ++ defined(V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE) + Address base = base_non_inlined(); + #else + Address base = base_; +@@ -148,7 +150,8 @@ Address ExternalCodeCompressionScheme::GetPtrComprCageBaseAddress( + // static + void ExternalCodeCompressionScheme::InitBase(Address base) { + CHECK_EQ(base, PrepareCageBaseAddress(base)); +-#if defined(USING_V8_SHARED) && defined(V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE) ++#if defined(USING_V8_SHARED_PRIVATE) && \ ++ defined(V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE) + set_base_non_inlined(base); + #else + base_ = base; +@@ -157,7 +160,8 @@ void ExternalCodeCompressionScheme::InitBase(Address base) { + + // static + V8_CONST Address ExternalCodeCompressionScheme::base() { +-#if defined(USING_V8_SHARED) && defined(V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE) ++#if defined(USING_V8_SHARED_PRIVATE) && \ ++ defined(V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE) + Address base = base_non_inlined(); + #else + Address base = base_; +diff --git a/src/trap-handler/trap-handler.h b/src/trap-handler/trap-handler.h +index 289a755d3b119cd90b88ee9d76ea13b709fe3a80..674706220c64d63727b8bd461903abd5412fc137 100644 +--- a/src/trap-handler/trap-handler.h ++++ b/src/trap-handler/trap-handler.h +@@ -58,11 +58,11 @@ namespace trap_handler { + #endif + + // Setup for shared library export. +-#if defined(BUILDING_V8_SHARED) && defined(V8_OS_WIN) ++#if defined(BUILDING_V8_SHARED_PRIVATE) && defined(V8_OS_WIN) + #define TH_EXPORT_PRIVATE __declspec(dllexport) +-#elif defined(BUILDING_V8_SHARED) ++#elif defined(BUILDING_V8_SHARED_PRIVATE) + #define TH_EXPORT_PRIVATE __attribute__((visibility("default"))) +-#elif defined(USING_V8_SHARED) && defined(V8_OS_WIN) ++#elif defined(USING_V8_SHARED_PRIVATE) && defined(V8_OS_WIN) + #define TH_EXPORT_PRIVATE __declspec(dllimport) + #else + #define TH_EXPORT_PRIVATE +diff --git a/src/utils/v8dll-main.cc b/src/utils/v8dll-main.cc +index 9bdd97f365a87994b2648456eb2b9759e58f1109..c153431a7321129c8b52288a65280deac6aad33b 100644 +--- a/src/utils/v8dll-main.cc ++++ b/src/utils/v8dll-main.cc +@@ -5,6 +5,7 @@ + // The GYP based build ends up defining USING_V8_SHARED when compiling this + // file. + #undef USING_V8_SHARED ++#undef USING_V8_SHARED_PRIVATE + #include "include/v8config.h" + + #if V8_OS_WIN +diff --git a/third_party/googletest/BUILD.gn b/third_party/googletest/BUILD.gn +index bc82c635da35c2a98162acad470d8fcc56019255..1cf84b3d8f495128d4d008823b1a7028630cb902 100644 +--- a/third_party/googletest/BUILD.gn ++++ b/third_party/googletest/BUILD.gn +@@ -94,8 +94,7 @@ source_set("gtest") { + # V8-only workaround for http://crbug.com/chromium/1191946. Ensures that + # googletest is compiled with the same visibility such as the rest of V8, see + # https://source.chromium.org/chromium/chromium/src/+/master:v8/gni/v8.gni +- if ((is_posix || is_fuchsia) && +- (v8_enable_backtrace || v8_monolithic || v8_expose_symbols)) { ++ if ((is_posix || is_fuchsia) && (v8_enable_backtrace || v8_monolithic)) { + configs -= [ "//build/config/gcc:symbol_visibility_hidden" ] + configs += [ "//build/config/gcc:symbol_visibility_default" ] + } +@@ -147,8 +146,7 @@ source_set("gmock") { + # V8-only workaround for http://crbug.com/chromium/1191946. Ensures that + # googletest is compiled with the same visibility such as the rest of V8, see + # https://source.chromium.org/chromium/chromium/src/+/master:v8/gni/v8.gni +- if ((is_posix || is_fuchsia) && +- (v8_enable_backtrace || v8_monolithic || v8_expose_symbols)) { ++ if ((is_posix || is_fuchsia) && (v8_enable_backtrace || v8_monolithic)) { + configs -= [ "//build/config/gcc:symbol_visibility_hidden" ] + configs += [ "//build/config/gcc:symbol_visibility_default" ] + } diff --git a/patches/v8/build_expose_mksnapshot_to_embedders.patch b/patches/v8/build_expose_mksnapshot_to_embedders.patch new file mode 100644 index 000000000000..191e301a1a8d --- /dev/null +++ b/patches/v8/build_expose_mksnapshot_to_embedders.patch @@ -0,0 +1,20 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Cheng Zhao +Date: Wed, 22 Nov 2023 17:21:29 +0900 +Subject: build: expose mksnapshot to embedders + +Backport: https://chromium-review.googlesource.com/c/v8/v8/+/5052522 + +diff --git a/BUILD.gn b/BUILD.gn +index 17a0e5dd1a693b0f461396d9dcfb9ea812885472..206f1084252a264b060239e7218331d88a858764 100644 +--- a/BUILD.gn ++++ b/BUILD.gn +@@ -6913,8 +6913,6 @@ if (current_toolchain == v8_generator_toolchain) { + + if (current_toolchain == v8_snapshot_toolchain) { + v8_executable("mksnapshot") { +- visibility = [ ":*" ] # Only targets in this file can depend on this. +- + sources = [ + "src/snapshot/embedded/embedded-empty.cc", + "src/snapshot/embedded/embedded-file-writer.cc", diff --git a/patches/v8/build_gn.patch b/patches/v8/build_gn.patch deleted file mode 100644 index 59a1e94a0e5b..000000000000 --- a/patches/v8/build_gn.patch +++ /dev/null @@ -1,40 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Jeremy Apthorp -Date: Tue, 16 Apr 2019 10:43:04 -0700 -Subject: build_gn.patch - -We force V8 into 'shared library' mode so that it exports its symbols, which is -necessary for native modules to load. - -Also change visibility on mksnapshot in order to target mksnapshot for mksnapshot zip. - -diff --git a/BUILD.gn b/BUILD.gn -index 17a0e5dd1a693b0f461396d9dcfb9ea812885472..5db09607a0f641f6e3906db1f596ce65cfbd28de 100644 ---- a/BUILD.gn -+++ b/BUILD.gn -@@ -756,7 +756,7 @@ config("internal_config") { - ":cppgc_header_features", - ] - -- if (is_component_build) { -+ if (is_component_build || is_electron_build) { - defines += [ "BUILDING_V8_SHARED" ] - } - -@@ -6901,7 +6901,7 @@ if (current_toolchain == v8_generator_toolchain) { - "src/interpreter/bytecodes.h", - ] - -- configs = [ ":internal_config" ] -+ configs = [ ":internal_config_base" ] - - deps = [ - ":v8_libbase", -@@ -6913,7 +6913,6 @@ if (current_toolchain == v8_generator_toolchain) { - - if (current_toolchain == v8_snapshot_toolchain) { - v8_executable("mksnapshot") { -- visibility = [ ":*" ] # Only targets in this file can depend on this. - - sources = [ - "src/snapshot/embedded/embedded-empty.cc", diff --git a/patches/v8/do_not_export_private_v8_symbols_on_windows.patch b/patches/v8/do_not_export_private_v8_symbols_on_windows.patch deleted file mode 100644 index e9fc0cd841f4..000000000000 --- a/patches/v8/do_not_export_private_v8_symbols_on_windows.patch +++ /dev/null @@ -1,52 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Tomas Rycl -Date: Mon, 13 May 2019 15:48:48 +0200 -Subject: Do not export private V8 symbols on Windows - -This change stops private V8 symbols and internal crt methods being exported. -It fixes an issue where native node modules can import -incorrect CRT methods and crash on Windows. -It also reduces size of node.lib by 75%. - -This patch can be safely removed if, when it is removed, `node.lib` does not -contain any standard C++ library exports (e.g. `std::ostringstream`). - -diff --git a/BUILD.gn b/BUILD.gn -index 5db09607a0f641f6e3906db1f596ce65cfbd28de..31b663ac42232ab2fdf7947a3b2ad0b7ab21016a 100644 ---- a/BUILD.gn -+++ b/BUILD.gn -@@ -756,6 +756,10 @@ config("internal_config") { - ":cppgc_header_features", - ] - -+ if (!is_component_build && is_electron_build) { -+ defines += [ "HIDE_PRIVATE_SYMBOLS" ] -+ } -+ - if (is_component_build || is_electron_build) { - defines += [ "BUILDING_V8_SHARED" ] - } -diff --git a/src/base/macros.h b/src/base/macros.h -index 56ff8736b3000644905f128fd365797424761798..52e0384705bd3bbd6c72f1f2cadd7922cc01ee0c 100644 ---- a/src/base/macros.h -+++ b/src/base/macros.h -@@ -388,13 +388,17 @@ bool is_inbounds(float_t v) { - - // Setup for Windows shared library export. - #define V8_EXPORT_ENUM -+#if defined(HIDE_PRIVATE_SYMBOLS) -+#define V8_EXPORT_PRIVATE -+#else //if !defined(HIDE_PRIVATE_SYMBOLS) - #ifdef BUILDING_V8_SHARED - #define V8_EXPORT_PRIVATE __declspec(dllexport) - #elif USING_V8_SHARED - #define V8_EXPORT_PRIVATE __declspec(dllimport) --#else -+#else //!(BUILDING_V8_SHARED || USING_V8_SHARED) - #define V8_EXPORT_PRIVATE --#endif // BUILDING_V8_SHARED -+#endif -+#endif - - #else // V8_OS_WIN -