deps: update V8 to 12.5 #52651
Closed
deps: update V8 to 12.5 #52651
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Review requested:
|
nodejs-github-bot
added
build
targos
added
the
semver-major
|
@nodejs/platform-windows there's one little error for now: |
|
@targos, with these changes the compilation passes (should probably become part of 0b738c9) From f52e086c0142311f1c4b8b2ccf160050d8f2bdd5 Mon Sep 17 00:00:00 2001
From: StefanStojanovic <[email protected]>
Date: Wed, 24 Apr 2024 16:30:39 +0200
Subject: [PATCH 1/1] V8 v12.5 Win fix
---
deps/v8/src/api/api.cc | 6 ++----
deps/v8/src/flags/flag-definitions.h | 3 ++-
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/deps/v8/src/api/api.cc b/deps/v8/src/api/api.cc
index e99e7d9d135..bd2254d82a7 100644
--- a/deps/v8/src/api/api.cc
+++ b/deps/v8/src/api/api.cc
@@ -6323,8 +6323,7 @@ void v8::Object::SetAlignedPointerInInternalFields(int argc, int indices[],
// static
void* v8::Object::Unwrap(v8::Isolate* isolate, i::Address wrapper_obj,
CppHeapPointerTag tag) {
- return i::JSApiWrapper(i::JSObject::cast(i::Tagged<i::Object>(
- reinterpret_cast<i::Address>(wrapper_obj))))
+ return i::JSApiWrapper(i::JSObject::cast(i::Tagged<i::Object>((wrapper_obj))))
.GetCppHeapWrappable(reinterpret_cast<i::Isolate*>(isolate),
static_cast<i::ExternalPointerTag>(tag));
}
@@ -6332,8 +6331,7 @@ void* v8::Object::Unwrap(v8::Isolate* isolate, i::Address wrapper_obj,
// static
void v8::Object::Wrap(v8::Isolate* isolate, i::Address wrapper_obj,
CppHeapPointerTag tag, void* wrappable) {
- return i::JSApiWrapper(i::JSObject::cast(i::Tagged<i::Object>(
- reinterpret_cast<i::Address>(wrapper_obj))))
+ return i::JSApiWrapper(i::JSObject::cast(i::Tagged<i::Object>((wrapper_obj))))
.SetCppHeapWrappable(reinterpret_cast<i::Isolate*>(isolate), wrappable,
static_cast<i::ExternalPointerTag>(tag));
}
diff --git a/deps/v8/src/flags/flag-definitions.h b/deps/v8/src/flags/flag-definitions.h
index 68d98ae63e2..cd469b7537e 100644
--- a/deps/v8/src/flags/flag-definitions.h
+++ b/deps/v8/src/flags/flag-definitions.h
@@ -848,10 +848,11 @@ DEFINE_INT(invocation_count_for_feedback_allocation, 8,
// Tiering: Maglev.
#if defined(ANDROID)
DEFINE_INT(invocation_count_for_maglev, 1000,
+ "invocation count required for optimizing with Maglev")
#else
DEFINE_INT(invocation_count_for_maglev, 400,
-#endif // ANDROID
"invocation count required for optimizing with Maglev")
+#endif // ANDROID
DEFINE_INT(invocation_count_for_maglev_osr, 100,
"invocation count required for maglev OSR")
DEFINE_BOOL(osr_from_maglev, false,
--
2.44.0.windows.1However, there is another issue I got when running basic debug build sanity check (fails with Release too) Here is the logBased on the errors, looks like not using |
github-actions
bot
added
request-ci-failed
This comment was marked as outdated.
This comment was marked as outdated.
targos
removed
the
request-ci-failed
|
@StefanStojanovic Not sure what's wrong with your local tests but Windows CI is green! |
This makes me both happy and sad... 😂 Hopefully it was something on my machine, I'll see if I can reproduce it again. |
github-actions
bot
removed
the
request-ci
@nodejs/platform-ppc |
|
@targos that's likely a bug in gcc10, this should fix it: |
|
@miladfarca Oh yeah I forgot to look at the floating patches we already have in |
Major V8 updates are usually API/ABI incompatible with previous versions. This commit adapts NODE_MODULE_VERSION for V8 12.5. Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md
dllexport introduces issues when compiling with MSVC. PR-URL: nodejs#47251 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Richard Lau <[email protected]>
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 <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=14221 PR-URL: nodejs#49639 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#49639 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#52293 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Richard Lau <[email protected]>
After enabling -std:c++20 on Windows, patch is now much smaller. PR-URL: nodejs#52465 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
On Windows debug builds, it is not allowed to dereference empty iterators. Refs: https://chromium-review.googlesource.com/c/v8/v8/+/5331688 PR-URL: nodejs#52465 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Original commit message:
[turboshaft] initialize constant_value_ to an empty value
gcc-10 seems to have a bug were not initializing this value
throws this compilation error:
```
src/compiler/turboshaft/assembler.h:680:16: error: ‘<anonymous>’ is used uninitialized in this function [-Werror=uninitialized]
680 | return Get();
```
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86465
Bug: v8:12783
Change-Id: I7a5fee5009b866a801326fba734c156c3cfdb1b0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5503350
Reviewed-by: Nico Hartmann <[email protected]>
Commit-Queue: Milad Farazmand <[email protected]>
Cr-Commit-Position: refs/heads/main@{#93675}
Refs: v8/v8@f6bef09
PR-URL: nodejs#52802
Fixes: nodejs#52661
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Namely: - `v8::ObjectTemplate::SetAccessor(v8::Local<v8::String>, ...);` - `v8::ObjectTemplate::SetNativeDataProperty` with `AccessControl` Refs: v8/v8@46c241e Refs: v8/v8@6ec8839 Co-authored-by: Michaël Zasso <[email protected]>
github-actions
bot
removed
the
request-ci
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I know, it never ends 😅