Skip to content

Conversation

targos
Copy link
Member

@targos targos commented Sep 8, 2025

This should replace #58491

Still blocked at least on nodejs/build#4083

V8 CI is also broken because --always-turbofan flag was removed. I tried to fix it in canary with ab7d369, but then test/v8-updates/test-linux-perf-logger.js fails because the functions don't get optimized: https://ci.nodejs.org/job/node-test-commit-v8-linux/6742/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/console

Notable changes since 13.7:

targos and others added 13 commits September 8, 2025 09:20
Major V8 updates are usually API/ABI incompatible with previous
versions. This commit adapts NODE_MODULE_VERSION for V8 14.1.

Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md
PR-URL: nodejs#54077
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
It's causing linker errors with node.lib in node-gyp and potentially
breaks other 3rd party tools

PR-URL: nodejs#56238
Refs: nodejs#55784
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
GCC emits warnings because of the trailing backslashes.

PR-URL: nodejs#58070
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
PR-URL: nodejs#58070
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
illumos pointers are VA48, can allocate from the top of the 64-bit range
as well.
In illumos, madvise(3C) now takes `void *` for its first argument
post-illumos#14418, but uses `caddr_t` pre-illumos#14418. This fix will
detect if the illumos mman.h file in use is pre-or-post-illumos#14418 so
builds can work either way.

PR-URL: nodejs#58237
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@targos targos added the semver-major PRs that contain breaking changes and should be released in the next major version. label Sep 8, 2025
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/security-wg
  • @nodejs/v8-update

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. labels Sep 8, 2025
@targos
Copy link
Member Author

targos commented Sep 8, 2025

@nodejs/cpp-reviewers Please take a look at 1bf9e85 and a540d99.

@targos
Copy link
Member Author

targos commented Sep 8, 2025

/cc @lukealbao In case you have ideas on how to adapt test-linux-perf-logger.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@targos
Copy link
Member Author

targos commented Sep 8, 2025

@nodejs/platform-ppc @nodejs/platform-s390 we quickly get a compiler error:

14:43:02 FAILED: obj/v8_bigint/mul-karatsuba.o 
14:43:02 ccache g++ -MD -MF obj/v8_bigint/mul-karatsuba.o.d -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_NONE -D_GLIBCXX_ASSERTIONS=1 -DUSE_UDEV -DUSE_AURA=1 -DUSE_GLIB=1 -DUSE_OZONE=1 -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DV8_TYPED_ARRAY_MAX_SIZE_IN_HEAP=64 -DENABLE_GDB_JIT_INTERFACE -DV8_INTL_SUPPORT -DV8_USE_EXTERNAL_STARTUP_DATA -DV8_ATOMIC_OBJECT_FIELD_WRITES -DV8_ENABLE_LAZY_SOURCE_POSITIONS -DV8_WIN64_UNWINDING_INFO -DV8_ENABLE_REGEXP_INTERPRETER_THREADED_DISPATCH -DV8_ENABLE_SPARKPLUG -DV8_ENABLE_TURBOFAN -DV8_ENABLE_WEBASSEMBLY -DV8_ENABLE_CONTINUATION_PRESERVED_EMBEDDER_DATA -DV8_ALLOCATION_FOLDING -DV8_ALLOCATION_SITE_TRACKING -DV8_ADVANCED_BIGINT_ALGORITHMS -DV8_USE_ZLIB -DV8_ENABLE_MAGLEV_GRAPH_PRINTER -DV8_ENABLE_EXTENSIBLE_RO_SNAPSHOT -DV8_ENABLE_BLACK_ALLOCATED_PAGES -DV8_ENABLE_LEAPTIERING -DV8_WASM_RANDOM_FUZZERS -DV8_ARRAY_BUFFER_INTERNAL_FIELD_COUNT=0 -DV8_ARRAY_BUFFER_VIEW_INTERNAL_FIELD_COUNT=0 -DV8_PROMISE_INTERNAL_FIELD_COUNT=0 -DV8_USE_DEFAULT_HASHER_SECRET=true -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS -DV8_HAVE_TARGET_OS -DV8_TARGET_OS_LINUX -DCPPGC_ENABLE_LARGER_CAGE -DCPPGC_SLIM_WRITE_BARRIER -DV8_TARGET_ARCH_PPC64 -I../.. -Igen -I../../include -Igen/include -Wall -Wno-unused-local-typedefs -Wno-maybe-uninitialized -Wno-deprecated-declarations -Wno-comments -Wno-packed-not-aligned -Wno-missing-field-initializers -Wno-unused-parameter -Wno-psabi -Werror -fno-strict-overflow -fno-ident -fno-math-errno -fno-strict-aliasing -fstack-protector -funwind-tables -fPIC -pipe -pthread -m64 -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -fno-omit-frame-pointer -g0 -ffp-contract=off -Wno-invalid-offsetof -Wno-strict-overflow -Wno-return-type -Wno-int-in-bool-context -Wno-deprecated -Wno-stringop-overflow -Wno-stringop-overread -Wno-restrict -Wno-array-bounds -Wno-nonnull -Wno-dangling-pointer -O3 -fdata-sections -ffunction-sections -fvisibility=default -Wno-narrowing -Wno-class-memaccess -Wno-invalid-offsetof -std=gnu++2a -fno-exceptions -fno-rtti  -c ../../src/bigint/mul-karatsuba.cc -o obj/v8_bigint/mul-karatsuba.o
14:43:02 ../../src/bigint/mul-karatsuba.cc: In function ‘uint32_t v8::bigint::{anonymous}::RoundUpLen(uint32_t)’:
14:43:02 ../../src/bigint/mul-karatsuba.cc:51:38: error: comparison of integer expressions of different signedness: ‘uint32_t’ {aka ‘unsigned int’} and ‘int’ [-Werror=sign-compare]
14:43:02    51 |   if (shift >= 2 && (len & additive) < (1 << (shift - 2))) {
14:43:02       |                     ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
14:43:02 cc1plus: all warnings being treated as errors

@richardlau
Copy link
Member

richardlau commented Sep 8, 2025

@nodejs/platform-ppc @nodejs/platform-s390 we quickly get a compiler error:

This might be a gcc issue. I'm working on a patch to make tools/make-v8.sh compatible with clang -- I'm testing with it on top of the branch for this PR (it didn't work with main because we need patches to V8's build dep which should be okay with V8 14.1): https://ci.nodejs.org/job/richardlau-node-test-commit-v8-linux/732/

@miladfarca
Copy link

We no longer maintain gcc builds for RHEL on ppc64/s390x and builds now are using Clang. This patch which enables it on said platforms should already be part of 14.1: https://crrev.com/c/6705431

@targos
Copy link
Member Author

targos commented Sep 8, 2025

@richardlau
Copy link
Member

@nodejs/platform-ppc @nodejs/platform-s390 we quickly get a compiler error:

This might be a gcc issue. I'm working on a patch to make tools/make-v8.sh compatible with clang -- I'm testing with it on top of the branch for this PR (it didn't work with main because we need patches to V8's build dep which should be okay with V8 14.1): https://ci.nodejs.org/job/richardlau-node-test-commit-v8-linux/732/

With clang 19: https://ci.nodejs.org/job/richardlau-node-test-commit-v8-linux/732/nodes=rhel8-s390x,v8test=v8test/console

14:19:17 In file included from ../../src/builtins/generate-bytecodes-builtins-list.cc:8:
14:19:17 In file included from ../../src/interpreter/bytecodes.h:12:
14:19:17 In file included from ../../src/common/globals.h:16:
14:19:17 In file included from ../../src/base/atomic-utils.h:13:
14:19:17 In file included from ../../src/base/atomicops.h:40:
14:19:17 In file included from ../../src/base/macros.h:13:
14:19:17 ../../src/base/logging.h:268:31: error: expected concept name with optional arguments
14:19:17   268 |              { t.begin() } -> std::forward_iterator;
14:19:17       |                               ^
14:19:17 In file included from ../../src/builtins/generate-bytecodes-builtins-list.cc:8:
14:19:17 In file included from ../../src/interpreter/bytecodes.h:12:
14:19:17 In file included from ../../src/common/globals.h:22:
14:19:17 ../../src/base/numbers/double.h:8:10: fatal error: 'bit' file not found
14:19:17     8 | #include <bit>
14:19:17       |          ^~~~~
14:19:17 2 warnings and 2 errors generated.

Above is with:

+ gn gen -v out.gn/s390x.release '--args=is_clang=true clang_base_path="/usr" clang_use_chrome_plugins=false treat_warnings_as_errors=false is_component_build=false is_debug=false use_custom_libcxx=false v8_target_cpu="s390x" target_cpu="s390x" v8_enable_backtrace=true cc_wrapper="ccache"'

FWIW https://ci.nodejs.org/job/node-test-commit-linuxone/51470/ (also with clang-19) passed.

@targos
Copy link
Member Author

targos commented Sep 9, 2025

Is it possible that it's using the wrong version of the C++ standard library? Both errors are related to C++20 features.

@richardlau
Copy link
Member

Is it possible that it's using the wrong version of the C++ standard library? Both errors are related to C++20 features.

Quite possibly. Node.js built successfully with clang 19 for the equivalent platform: https://ci.nodejs.org/job/node-test-commit-linuxone/51470/

The IBM team that maintains the upstream Linux ppc64le and s390x ports build V8 with use_custom_libcxx = true. We can't currently do that with the V8 CI in Node.js unless we pull in more DEPS for V8: https://ci.nodejs.org/job/richardlau-node-test-commit-v8-linux/734/nodes=rhel8-s390x,v8test=v8test/console

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. semver-major PRs that contain breaking changes and should be released in the next major version. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants