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

[v14.x] deps: update V8 to 8.4 #34356

Closed
wants to merge 21 commits into from
Closed

[v14.x] deps: update V8 to 8.4 #34356

wants to merge 21 commits into from

Conversation

targos
Copy link
Member

@targos targos commented Jul 14, 2020

Backport of #33579

Headers diff: https://gist.github.com/targos/b6f16016f80fa01c328156642c6c0eb4 (git diff abb5e1a476...HEAD -- deps/v8/include ':(exclude)deps/v8/include/cppgc')

This probably needs some changes for ABI compatibility. /cc @nodejs/v8-update @addaleax

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. v14.x v8 engine Issues and PRs related to the V8 dependency. labels Jul 14, 2020
@MylesBorins
Copy link
Contributor

I think that this needs to land with this patch

#34272

@MylesBorins MylesBorins mentioned this pull request Jul 14, 2020
Closed
4 tasks
@targos
Copy link
Member Author

targos commented Jul 14, 2020

@MylesBorins #34272 is already part of this PR: 8865013

@addaleax
Copy link
Member

@targos I’ve pushed a few changes that should bring this to ABI compatibility :)

@MylesBorins
Copy link
Contributor

I'm prepping a 14.x release today. Should I punt the release by a day or two so we can get this landed?

@addaleax
Copy link
Member

@MylesBorins I would be in favor of waiting for this to land, yes

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32340/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 14, 2020 •
edited by MylesBorins
Loading

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

@github-actions github-actions bot mentioned this pull request Jul 15, 2020
Open
13 tasks
@targos
Copy link
Member Author

targos commented Jul 15, 2020

@addaleax thanks a lot! It looks like we need to disable some tests that use the PostJob API.

@MylesBorins did you mean to push this merge commit?

@addaleax
Copy link
Member

@targos Ah, yeah, I only ran make test, not make test-v8 – lmk if you’d like me to fix that up

@targos
Copy link
Member Author

targos commented Jul 15, 2020

I'll try to do it 👍

@MylesBorins
Copy link
Contributor

@targos I did mean to push that, apologies if it complicated things. There was a conflict due to a V8 change that landed on staging

@addaleax
Copy link
Member

@MylesBorins I don’t think there are circumstances in Node.js’s development model where merge commits are useful, because they will need to be removed again when rebasing or landing anyway

@MylesBorins
Copy link
Contributor

@addaleax I'll avoid it in the future

@MylesBorins
Copy link
Contributor

Going to delay the release one more day so we can try and get this in, but If we cant get this landed by tomorrow I think we should wait until 14.7.0 which is scheduled to land in 2 weeks

@MylesBorins MylesBorins mentioned this pull request Jul 15, 2020
Merged
@github-actions github-actions bot mentioned this pull request Jul 16, 2020
Open
13 tasks
@targos targos mentioned this pull request Jul 16, 2020
Closed
targos and others added 8 commits July 16, 2020 14:16
@targos
deps: update V8 to 8.4.371.19
101a4e5
@targos
build: reset embedder string to "-node.0"
d0a3a41
@refack @targos
deps: V8: un-cherry-pick bd019bd
8be298d 
Original commit message:

    [testrunner] delete ancient junit compatible format support

    Testrunner has ancient support for JUnit compatible XML output.

    This CL removes this old feature.

    R=mstarzinger@chromium.org,jgruber@chromium.org,jkummerow@chromium.org
    CC=​machenbach@chromium.org

    Bug: v8:8728
    Change-Id: I7e1beb011dbaec3aa1a27398a5c52abdd778eaf0
    Reviewed-on: https://chromium-review.googlesource.com/c/1430065
    Reviewed-by: Jakob Gruber <jgruber@chromium.org>
    Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
    Commit-Queue: Tamer Tas <tmrts@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#59045}

Refs: v8/v8@bd019bd

PR-URL: nodejs#32116
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@ryzokuken @targos
deps: patch V8 to run on older XCode versions
332374e 
Patch V8 (compiler/js-heap-broker.cc) to remove the use of an optional
property, which is a fairly new C++ feature, since that requires a newer
XCode version than the minimum requirement in BUILDING.md and thus
breaks CI.

PR-URL: nodejs#32116
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@refack @targos
deps: V8: patch register-arm64.h
dc9a411 
Fixes a compilation issue on some platforms

PR-URL: nodejs#32116
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@refack @targos
deps: V8: forward declaration of Rtl*FunctionTable
b27fe7e 
This should be semver-patch since actual invocation is version
conditional.

PR-URL: nodejs#32116
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@joaocgreis @targos
deps: make v8.h compatible with VS2015
f997e43 
There is a bug in the most recent version of VS2015 that affects v8.h
and therefore prevents compilation of addons.

Refs: https://stackoverflow.com/q/38378693

PR-URL: nodejs#32116
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@targos
deps: V8: silence irrelevant warnings
183e0ae 
PR-URL: nodejs#26685
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>

PR-URL: nodejs#32116
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Jul 16, 2020
@joyeecheung @MylesBorins
deps: V8: backport 22014de00115
fb180ac 
Original commit message:

    Reland "[snapshot] rehash JSMap and JSSet during deserialization"

    This is a reland of 8374feed55a5b3010f2e9593560a2d84f9f6725f.

    Fixed rehashing of global proxy keys by creating its identity hash
    early, before the deserialization of the context snapshot.

    Original change's description:
    > [snapshot] rehash JSMap and JSSet during deserialization
    >
    > To rehash JSMap and JSSet, we simply replace the backing store
    > with a new one created with the new hash.
    >
    > Bug: v8:9187
    > Change-Id: I90c25b18b33b7bc2b6ffe1b89fe17aa5f978b517
    > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2143983
    > Commit-Queue: Joyee Cheung <joyee@igalia.com>
    > Reviewed-by: Jakob Gruber <jgruber@chromium.org>
    > Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    > Cr-Commit-Position: refs/heads/master@{#67663}

    Bug: v8:9187, v8:10523
    Change-Id: I7a0319b1d10ff07644de902fec43e7c2b1dd8da9
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2212085
    Reviewed-by: Leszek Swirski <leszeks@chromium.org>
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Jakob Gruber <jgruber@chromium.org>
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Cr-Commit-Position: refs/heads/master@{#67999}

Refs: v8/v8@22014de

Backport-PR-URL: #34356
PR-URL: #33300
Refs: v8/v8@ea0719b
Refs: v8/v8@bb9f0c2
Refs: #17058
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 16, 2020
@Qard @MylesBorins
deps: V8: cherry-pick eec10a2fd8fa
4c37837 
Original commit message:

    [promisehook] Add before/after hooks to thenable tasks

    This will allow Node.js to properly track async context in thenables.

    Change-Id: If441423789a78307a57ad7e645daabf551cddb57
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2215624
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Sathya Gunasekaran  <gsathya@chromium.org>
    Commit-Queue: Gus Caplan <me@gus.host>
    Cr-Commit-Position: refs/heads/master@{#68207}

Refs: v8/v8@eec10a2

Backport-PR-URL: #34356
PR-URL: #33778
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request Jul 16, 2020
@cjihrig @MylesBorins
deps: update V8 postmortem metadata script
3fa7ad3 
This commit updates V8's gen-postmortem-metadata.py script
to fix SmartOS compilation for V8 8.4.

Backport-PR-URL: #34356
PR-URL: #33579
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 16, 2020
@ryzokuken @MylesBorins
tools: update V8 gypfiles for 8.4
4e2fa43 
Backport-PR-URL: #34356
PR-URL: #33579
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 16, 2020
@targos @MylesBorins
deps: bump minimum icu version to 67
9f886c9 
Bump minimum version of ICU needed to build node to 67.

Refs: v8/v8@611e412

Backport-PR-URL: #34356
PR-URL: #33579
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 16, 2020
@cjihrig @MylesBorins
test: remove test/v8-updates/test-postmortem-metadata.js
1af8943 
The postmortem metadata test is no longer used to maintain
postmortem debugging tools. Since it frequently breaks on
V8 updates, it makes more sense to just remove it.

Backport-PR-URL: #34356
PR-URL: #33579
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 16, 2020
@addaleax @MylesBorins
deps: V8: undo header change of 9dbab9bbdb979
2079fef 
Refs: v8/v8@9dbab9b

PR-URL: #34356
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 16, 2020
@addaleax @MylesBorins
deps: V8: re-add dummy Isolate::CheckMemoryPressure
447b1e8 
Undoes the ABI-breaking part of v8/v8@92a44876bd66aa.

Refs: v8/v8@92a4487

PR-URL: #34356
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 16, 2020
@addaleax @MylesBorins
deps: V8: revert de4c0042cbe6 from upstream V8
2c9fd6e 
Original commit message:

    [weakrefs] Remove deprecated FinalizationGroup V8 API

    Bug: v8:8179
    Change-Id: I16170a197028beb35309b15613004b29a956896c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2171696
    Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
    Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
    Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
    Auto-Submit: Shu-yu Guo <syg@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#67492}

Refs: v8/v8@de4c004

PR-URL: #34356
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 16, 2020
@addaleax @MylesBorins
deps: V8: update headers for ABI compatibility
95afc2e 
Remove new functionality that interferes with ABI compatibility.

PR-URL: #34356
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@MylesBorins
Copy link
Contributor

This change broke the shot test suite on v14.x due to detecting leaks.

Specifically

The following leaks were detected:FinalizationRegistry, WeakRef

This was referenced Jul 17, 2020
Open
Open
Open
Open
Open
This was referenced Jun 17, 2021
Closed
Open
Closed
Closed
richardlau added a commit to richardlau/node-1 that referenced this pull request Jul 12, 2021
@richardlau
deps: restore minimum ICU version to 65
fba278c 
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
nodejs#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede
richardlau added a commit that referenced this pull request Jul 15, 2021
@richardlau
deps: restore minimum ICU version to 65
3543ed2 
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: #39068
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
jameshilliard pushed a commit to jameshilliard/node that referenced this pull request Jul 19, 2021
@richardlau @jameshilliard
deps: restore minimum ICU version to 65
a095b32 
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
nodejs#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: nodejs#39068
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
richardlau added a commit that referenced this pull request Jul 20, 2021
@richardlau
deps: restore minimum ICU version to 65
ccecea5 
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: #39068
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
richardlau added a commit to jameshilliard/node that referenced this pull request Jul 23, 2021
@richardlau
deps: restore minimum ICU version to 65
7d8c7a4 
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
nodejs#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: nodejs#39068
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
richardlau added a commit that referenced this pull request Jul 23, 2021
@richardlau
deps: restore minimum ICU version to 65
632a3ae 
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: #39068
Backport-PR-URL: #39451
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
guybedford pushed a commit that referenced this pull request Jul 26, 2021
@richardlau @guybedford
deps: restore minimum ICU version to 65
1b7af5b 
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: #39068
Backport-PR-URL: #39451
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
richardlau added a commit that referenced this pull request Jul 26, 2021
@richardlau
deps: restore minimum ICU version to 65
39e9cd5 
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: #39068
Backport-PR-URL: #39451
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
@richardlau @foxxyz
deps: restore minimum ICU version to 65
2d9a0db 
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
nodejs#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: nodejs#39068
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gengjiawen gengjiawen gengjiawen approved these changes

@addaleax addaleax addaleax approved these changes

@MylesBorins MylesBorins MylesBorins approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. 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.