-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
deps: update V8 to 13.7 #58064
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
deps: update V8 to 13.7 #58064
Conversation
Review requested:
|
Debug build fails on my mac:
|
These two JSPI CLs might need to be included if not already part of this change, for ppc64 and s390x: |
@anonrig any idea why macOS fails to build on GitHub actions (simdutf-related error)? I didn't get this error locally. |
@miladfarca thanks for the heads up. I believe both commits are included already. |
I'm not sure. @lemire any suggestions to why simdutf is failing on macOS? https://github.com/nodejs/node/actions/runs/14713137908/job/41290246774?pr=58064 |
I believe that macos builds are going to be dependent on Xcode >=16.3, as V8 now depends on simdutf features that are contingent on This might necessitate changing the build image to macos-15. |
I see, thanks @Renegade334. I think the easiest for now is to revert https://chromium-review.googlesource.com/c/v8/v8/+/6449193, since it touches a feature that's in development and behind a flag. |
It looks like something is broken on the release builds too.
|
Some tests with a log in
|
c8a886e
to
c93adf2
Compare
I started a discussion on the Chromium Slack about the build issues. The DCHECK in the snapshot serializer happens with a value of type
I don't know which interceptor this is about. |
diff --git a/deps/v8/src/snapshot/serializer.cc b/deps/v8/src/snapshot/serializer.cc
index f4cc04b6f4..ae2bd8f256 100644
--- a/deps/v8/src/snapshot/serializer.cc
+++ b/deps/v8/src/snapshot/serializer.cc
@@ -1150,10 +1150,13 @@ void Serializer::ObjectSerializer::VisitExternalPointer(
Tagged<HeapObject> host, ExternalPointerSlot slot) {
PtrComprCageBase cage_base(isolate());
InstanceType instance_type = object_->map(cage_base)->instance_type();
+ InstanceType host_instance_type = host->map(cage_base)->instance_type();
+
if (InstanceTypeChecker::IsForeign(instance_type) ||
InstanceTypeChecker::IsJSExternalObject(instance_type) ||
InstanceTypeChecker::IsAccessorInfo(instance_type) ||
- InstanceTypeChecker::IsFunctionTemplateInfo(instance_type)) {
+ InstanceTypeChecker::IsFunctionTemplateInfo(instance_type) ||
+ InstanceTypeChecker::IsInterceptorInfo(host_instance_type)) {
// Output raw data payload, if any.
OutputRawData(slot.address());
Address value = slot.load(isolate()); This seems enough to make the crash go away. I will trying upstreaming it. |
Nice! I rebased and added your patch so we can try in CI. |
CI looks good, modulo the usual build memory issues. |
Previously the ObjectSerializer didn't serialize the ExternalPointers in the InterceptorInfo properly, but this case can be shadowed by the fact that they get promoted to RO space by default and don't get serialized by ObjectSerializer. This patch fixes up the missing handling in ObjectSerializer and adds a test case for this path. Refs: nodejs/node#58064 Change-Id: Icc62a01b006eaf68d1d2be1e3bc98b448f0c66dc Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6516091 Reviewed-by: Leszek Swirski <[email protected]> Commit-Queue: Joyee Cheung <[email protected]> Reviewed-by: Igor Sheludko <[email protected]> Cr-Commit-Position: refs/heads/main@{#100315}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Landed in ccf227e...a8217a9 |
PR-URL: #58064 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
PR-URL: #58064 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
Major V8 updates are usually API/ABI incompatible with previous versions. This commit adapts NODE_MODULE_VERSION for V8 13.7. Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md PR-URL: #58064 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
PR-URL: #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]> PR-URL: #58064 Reviewed-By: Paolo Insogna <[email protected]>
PR-URL: #53134 Refs: #52809 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[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: Joyee Cheung <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> PR-URL: #58064 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Marco Ippolito <[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: #56238 Refs: #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]> PR-URL: #58064 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: #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]> PR-URL: #58064 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. PR-URL: #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]> PR-URL: #58064 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: #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]> PR-URL: #58064 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
simdutf's `atomic_binary_to_base64` requires `std::atomic_ref`. `std::atomic_ref` is only available in LLVM 19, which is only available in Xcode 16.3, which is only available on macOS 15. Refs: llvm/llvm-project#76647 Refs: v8/v8@e3cddbe PR-URL: #58064 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
It depends on `std::atomic_ref`, which is not available in Xcode 16.1. Refs: v8/v8@6d6c1e6 PR-URL: #58064 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
Original commit message: [serializer] serialize ExternalPointers in InterceptorInfo properly Previously the ObjectSerializer didn't serialize the ExternalPointers in the InterceptorInfo properly, but this case can be shadowed by the fact that they get promoted to RO space by default and don't get serialized by ObjectSerializer. This patch fixes up the missing handling in ObjectSerializer and adds a test case for this path. Refs: #58064 Change-Id: Icc62a01b006eaf68d1d2be1e3bc98b448f0c66dc Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6516091 Reviewed-by: Leszek Swirski <[email protected]> Commit-Queue: Joyee Cheung <[email protected]> Reviewed-by: Igor Sheludko <[email protected]> Cr-Commit-Position: refs/heads/main@{#100315} Refs: v8/v8@d2ad518 PR-URL: #58064 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
Original commit message: [explicit-resource-management] disallow using in switch cases This CL disallows `using` and `await using` in switch cases. This was a normative change that got consensus in TC39 meetings: rbuckton/ecma262#14 Bug: 409478039 Change-Id: I077e75d7d0d632c8b34150cfc76e4903984d6091 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6500234 Reviewed-by: Shu-yu Guo <[email protected]> Commit-Queue: Rezvan Mahdavi Hezaveh <[email protected]> Cr-Commit-Position: refs/heads/main@{#100037} Refs: v8/v8@044b9b6 PR-URL: #58230 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> PR-URL: #58064 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
Original commit message: [explicit-resource-management] Turn flag on by default This feature has shipped since M134 on the blink side, and is highly unlikely to be unshipped now, so flip the flag on. Bug: 42203506 Change-Id: I9554cea721983464b150c0de70b58a4b50fd477b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6513391 Reviewed-by: Rezvan Mahdavi Hezaveh <[email protected]> Commit-Queue: Shu-yu Guo <[email protected]> Cr-Commit-Position: refs/heads/main@{#100069} Refs: v8/v8@4f38995 PR-URL: #58230 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> PR-URL: #58064 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
Original commit message: [float16array] Turn flag on by default Float16Array has shipped in blink since M135. It is unlikely it'll unship by now, so turn the flag on by default. Bug: 42203953 Change-Id: Ibd9de407b8810dd7bcdb46194fe04fc290ff8fb8 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6513988 Reviewed-by: Rezvan Mahdavi Hezaveh <[email protected]> Commit-Queue: Shu-yu Guo <[email protected]> Cr-Commit-Position: refs/heads/main@{#100104} Refs: v8/v8@1d3362c Co-authored-by: Michaël Zasso <[email protected]> PR-URL: #58230 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> PR-URL: #58064 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
Refs: v8/v8@1eb7d3c Refs: v8/v8@f942a49 PR-URL: #58064 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
Notable changes: