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.

Sign up for GitHub

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

Jump to bottom

deps: update V8 to 11.5 #48456

Closed
wants to merge 14 commits into from
Closed

deps: update V8 to 11.5 #48456

wants to merge 14 commits into from

Conversation

targos
Copy link
Member

@targos targos commented Jun 14, 2023

Based on #48029 with two additional commits.

  • deps: update V8 to 11.5.150.8
  • build: reset embedder string to "-node.0"
  • src: update NODE_MODULE_VERSION to 118
  • deps: always define V8_EXPORT_PRIVATE as no-op
  • deps: silence irrelevant V8 warning
  • deps: patch V8 to support compilation on win-arm64
  • deps: disable V8 concurrent sparkplug compilation
  • tools: update V8 gypfiles for 11.4
  • tools: add new V8 headers to distribution
  • lib: update usage of always on Atomics API
  • test: adapt debugger tests to V8 11.4
  • test: update flag to disable SharedArrayBuffer
  • tools: update V8 gypfiles for 11.5
  • test: adapt test-fs-write to V8 internal changes

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp
  • @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 Jun 14, 2023
@targos targos mentioned this pull request Jun 14, 2023
Closed
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 14, 2023 •
edited by targos
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/52250/
V8: https://ci.nodejs.org/job/node-test-commit-v8-linux/5395/

@targos
Copy link
Member Author

targos commented Jun 14, 2023

@StefanStojanovic I've kept 63589e5. Can you point me to the V8 CL that would make it obsolete?

@nodejs nodejs deleted a comment from nodejs-github-bot Jun 14, 2023
@nodejs nodejs deleted a comment from nodejs-github-bot Jun 14, 2023
@nodejs nodejs deleted a comment from nodejs-github-bot Jun 14, 2023
@StefanStojanovic
Copy link
Contributor

@StefanStojanovic I've kept 63589e5. Can you point me to the V8 CL that would make it obsolete?

My third CL landed 5 days ago. That should be the last one needed. In case you need the other CLs, this is the second one that landed a month ago, and the first one that landed 2 months ago

@targos
Copy link
Member Author

targos commented Jun 14, 2023

Thanks, I updated the PR to cherry-pick that last commit instead of the floating patch.

@targos
Copy link
Member Author

targos commented Jun 14, 2023 •
edited
Loading

Failure in shared lib build:

12:29:30 /home/iojs/build/workspace/node-test-commit-linux-containered/out/Release/obj.target/v8_zlib/deps/v8/third_party/zlib/cpu_features.o:(.bss.x86_cpu_enable_avx512+0x0): multiple definition of `x86_cpu_enable_avx512'
12:29:30 /home/iojs/build/workspace/node-test-commit-linux-containered/out/Release/obj.target/zlib/deps/zlib/cpu_features.o:(.bss+0x0): first defined here
12:29:30 collect2: error: ld returned 1 exit status
12:29:30 libnode.target.mk:546: recipe for target '/home/iojs/build/workspace/node-test-commit-linux-containered/out/Release/obj.target/libnode.so.118' failed
12:29:30 make[2]: *** [/home/iojs/build/workspace/node-test-commit-linux-containered/out/Release/obj.target/libnode.so.118] Error 1

Same on SmartOS:

12:27:09 ld: fatal: symbol 'x86_cpu_enable_avx512' is multiply-defined:
12:27:09 	(file /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos20-64/out/Release/obj.target/deps/zlib/libzlib.a(cpu_features.o) type=OBJT; file /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos20-64/out/Release/obj.target/tools/v8_gypfiles/libv8_zlib.a(cpu_features.o) type=OBJT);
12:27:09 ld: fatal: file processing errors. No output written to /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos20-64/out/Release/cctest

@targos targos mentioned this pull request Jun 14, 2023
Closed
This was referenced Jun 15, 2023
Open
Open
targos and others added 13 commits June 21, 2023 12:14
@targos
deps: update V8 to 11.5.150.10
0ddf2b3
@targos
build: reset embedder string to "-node.0"
e8726f8
@targos
src: update NODE_MODULE_VERSION to 118
5801de2 
Major V8 updates are usually API/ABI incompatible with previous
versions. This commit adapts NODE_MODULE_VERSION for V8 11.5.

Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md
@targos
deps: always define V8_EXPORT_PRIVATE as no-op
b3fd0cd 
dllexport introduces issues when compiling with MSVC.

PR-URL: nodejs#47251
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
@targos
deps: silence irrelevant V8 warning
dc3c787 
PR-URL: nodejs#45579
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#47251
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
@targos
deps: disable V8 concurrent sparkplug compilation
5258be6 
It introduces process hangs on some platforms because Node.js doesn't
tear down V8 correctly.
Disable it while we work on a solution.

Refs: nodejs#47297
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=13902
PR-URL: nodejs#47450
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos
deps: V8: cherry-pick 15e31d629ac0
f6c4436 
Original commit message:

    [win][msvc] fix cross-compilation for arm64

    Cross-compiling x64 to ARM64 on Windows fails with MSVC. The reason is
    the ProbeMemory function which uses asm() code prohibited in this case.
    This change adds conditionalizing on V8_TRAP_HANDLER_SUPPORTED thus
    removing the problematic parts of the code in MSVC cross-compilation.

    This extends https://chromium-review.googlesource.com/c/v8/v8/+/3964232
    in a way, as it wraps ProbeMemory usage inside of the "#ifdef
    V8_TRAP_HANDLER_VIA_SIMULATOR" blocks.

    This follows https://chromium-review.googlesource.com/c/v8/v8/+/4403215
    and https://chromium-review.googlesource.com/c/v8/v8/+/4489305 as a part
    of an effort to completely fix cross-compilation with MSVC.

    Change-Id: I59d9a995fbc8ee1cee2807429fd44d8043c178fb
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4574697
    Reviewed-by: Andreas Haas <ahaas@chromium.org>
    Reviewed-by: Mark Seaborn <mseaborn@chromium.org>
    Commit-Queue: Mark Seaborn <mseaborn@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#88147}

Refs: v8/v8@15e31d6
@targos
tools: update V8 gypfiles for 11.4
273686f
@targos
tools: add new V8 headers to distribution
fd29941
@targos
lib: update usage of always on Atomics API
baa57f1
@pfaffe @targos
test: adapt debugger tests to V8 11.4
cc27e4e 
Accept a new `step` break message.
@targos
test: update flag to disable SharedArrayBuffer
ada4cc2 
`--no-harmony-sharedarraybuffer` was removed from V8 but it's still
possible to disable the feature with `--enable-sharedarraybuffer-per-context`.
@targos
tools: update V8 gypfiles for 11.5
83435a2
@targos
Copy link
Member Author

targos commented Jun 21, 2023

I don't see what's special about x86_cpu_enable_avx512. It's defined the same way as, for example, x86_cpu_enable_simd, which isn't causing any issues.

/cc @nodejs/cpp-reviewers

@wa-Nadoo
Copy link

5258be6 can be dropped due to upstream bug has been fixed.

@targos
Copy link
Member Author

targos commented Jun 26, 2023

5258be6 can be dropped due to upstream bug has been fixed.

Upstream bug was closed, but a fix needs to happen on the Node.js side.

See #47297 and #47452

@targos
Copy link
Member Author

targos commented Jul 9, 2023

#48710

@targos targos closed this Jul 9, 2023
@targos targos deleted the v8-115 branch September 13, 2023 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@targos @nodejs-github-bot @StefanStojanovic @wa-Nadoo @pfaffe