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

[v15.x] deps: update V8 to 8.7 #36182

Closed
wants to merge 24 commits into from
Closed

[v15.x] deps: update V8 to 8.7 #36182

wants to merge 24 commits into from

Conversation

targos
Copy link
Member

@targos targos commented Nov 19, 2020

Backport of #35700

I probably missed some compatibility changes in other headers, but I think v8.h is safe.

/cc @addaleax

BTW, it would be nice to document somewhere which V8 header files are officially supported and should not have backwards-incompatible changes within a release line (I don't even know them myself).

targos and others added 24 commits November 19, 2020 21:14
@targos
deps: update V8 to 8.7.220
6327cfe
@targos
build: reset embedder string to "-node.0"
500ead2
@refack @targos
deps: V8: un-cherry-pick bd019bd
ad8eed3 
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
6da0120 
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
3fac798 
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
a79d681 
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
60fb216 
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
dc721ee 
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>
@mmarchini @targos
deps: patch V8 to run on Xcode 8
b4e1287 
Patch V8 (wasm/wasm-module.cc) to remove const qualifier from type
passed to template call of `OwnedVector::Of`. Xcode 8 can't convert
'OwnedVector<unsigned char>' to 'OwnedVector<const unsigned char>' when
returning from a function (which is likely a bug on Xcode, considering
this worked on the prior version of Xcode as well as newer versions).
This workaround shouldn't affect the application, since the const
qualifier is preserved in the AsmJsOffsetInformation::encoded_offset_.

There's also a V8 test passing a const-qualified type to ::Of, but since
we don't test V8 on Xcode 8, it should be fine to leave it as is.

Signed-off-by: Matheus Marchini <mmarchini@netflix.com>

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>
@cjihrig @targos
deps: update V8 postmortem metadata script
cfd934f 
This commit updates V8's gen-postmortem-metadata.py script
to fix SmartOS compilation for V8 8.4.

PR-URL: nodejs#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>
@cjihrig @targos
deps: update V8 postmortem metadata script
7f96548 
This commit updates V8's gen-postmortem-metadata.py script to
fix SmartOS compilation with V8 8.5.

PR-URL: nodejs#35415
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@gengjiawen @targos
deps: fix V8 build issue with inline methods
5e10ebb 
PR-URL: nodejs#35415
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@targos
deps: V8: cherry-pick d76abfed3512
ed898f7 
Original commit message:

    [wasm-simd][arm64] fix cross compile arm64 on windows

    See: nodejs#35415
    Change-Id: I5d77ae202d4bbfb0971246fe5de9c0ad17c485ff
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2459491
    Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
    Auto-Submit: Jiawen Geng <technicalcute@gmail.com>
    Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70444}

Refs: v8/v8@d76abfe

PR-URL: nodejs#35415
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@addaleax @targos
deps: V8: cherry-pick 3176bfd447a9
e262dab 
Original commit message:

    [heap-profiler] Fix crash when a snapshot deleted while taking one

    Fix a crash/hang that occurred when deleting a snapshot during the
    GC that is part of taking another one.

    Specifically, when deleting the only other snapshot in such
    a situation, the `v8::HeapSnapshot::Delete()` method sees that there
    is only one (complete) snapshot at that point, and decides that it is
    okay to perform “delete all snapshots” instead of just deleting
    the requested one. That resets the internal string lookup table
    of the heap profiler, but the new snapshot that is currently in
    progress still holds references to the old string lookup table,
    leading to a use-after-free segfault or infinite loop.

    Fix this by guarding against resetting the string table while
    another heap snapshot is being taken, and add a test that would
    crash before this fix.

    This can be triggered in Node.js by repeatedly calling
    `v8.getHeapSnapshot()`, which provides heap snapshots as weakly
    held host objects.

    Change-Id: If9ac3728bf79114000982f1e7bb05e8034299e3c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2464823
    Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
    Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70445}

Refs: v8/v8@3176bfd

PR-URL: nodejs#35612
Refs: nodejs#35559
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
@targos
deps: V8: cherry-pick 7b3a27b7ae65
6218c1c 
Original commit message:

    fix: correct calling convention for Windows on Arm

    Corrects a "Check failed: kFPParamRegisterCount == kParamRegisterCount"
    message when compiling v8_snapshot for Windows on Arm.

    Unlike x64, Windows on Arm's calling convention does not alternate
    between integer and float registers.

    Bug: chromium:1052746
    Change-Id: I4c9cdafcd6e43742b94613f85b2983761cc0891a
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2440717
    Commit-Queue: Maya Lekova <mslekova@chromium.org>
    Reviewed-by: Jakob Gruber <jgruber@chromium.org>
    Reviewed-by: Maya Lekova <mslekova@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70257}

Refs: v8/v8@7b3a27b

PR-URL: nodejs#35700
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
@targos
deps: V8: cherry-pick 45e49775f5a3
685fbe2 
Original commit message:

    [arm64][msvc] fix arm64 build on msvc

    See: nodejs#35415 (comment)

    Co-authored-by: Richard Townsend <richard.townsend@arm.com>
    Change-Id: I440644f55dc8c8ec3108e5015ebbce2829dd8207
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2479602
    Reviewed-by: Jakob Gruber <jgruber@chromium.org>
    Commit-Queue: Jakob Gruber <jgruber@chromium.org>
    Auto-Submit: Jiawen Geng <technicalcute@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#70648}

Refs: v8/v8@45e4977

PR-URL: nodejs#35700
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
@danbev @targos
deps: cherry-pick 9a49b22 from V8 upstream
1772fe1 
Original commit message:
  Fix alloc/dealloc size mismatch for v8::BackingStore

  On newer compilers the {operator delete} with explicit {size_t}
  argument would be instantiated for {v8::BackingStore} and used
  in the destructor of {std::unique_ptr<v8::BackingStore>}. The {size_t}
  argument is wrong though, since the pointer actually points
  to a {v8::internal::BackingStore} object.
  The solution is to explicitly provide a {operator delete}, preventing
  an implicitly generated {size_t} operator.

  Bug:v8:11081

  Change-Id: Iee0aa47a67f0e41000bea628942f7e3d70198b83
  Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2506712
  Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
  Reviewed-by: Camillo Bruni <cbruni@chromium.org>
  Cr-Commit-Position: refs/heads/master@{#70916}

PR-URL: nodejs#35939
Fixes: nodejs#35669
Refs: v8/v8@9a49b22
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
@targos
deps: V8: cherry-pick 821fb3883a8e
f4305dd 
Original commit message:

    [serializer] avoid deferring objects with embedder fields

    JS objects with embedder fields cannot be deferred because the
    serialize/deserialize callbacks need the back reference immediately
    to identify the object.

    Refs: nodejs/node-v8#175
    Bug: v8:11146
    Change-Id: I4292f2ab0041f7b0779620437ed26905c194cd9b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2531195
    Reviewed-by: Jakob Gruber <jgruber@chromium.org>
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Cr-Commit-Position: refs/heads/master@{#71134}

Refs: v8/v8@821fb38

PR-URL: nodejs#35700
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
@oleavr @targos
deps: V8: cherry-pick 086eecbd96b6
d074cfd 
Original commit message:

    [platform] Add Permission::kNoAccessWillJitLater enum value

    This value is unused for now. This CL is part 1 of a 3-step dance.
    Part 2 will be teaching Chrome's Platform implementation to accept
    the new value. Part 3 will then actually use it in V8.

    Bug: chromium:1117591
    Change-Id: Ie3aed20d4cc58f3def3be2a3a03bba4c3a37bf44
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2450056
    Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70335}

PR-URL: nodejs#35986
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
@oleavr @targos
deps: V8: cherry-pick 4e077ff0444a
3e15799 
Original commit message:

    [mac] Set MAP_JIT only when necessary

    This is a "minimal" change to achieve the required goal: seeing that
    there is only one place where we need to indicate that memory should
    be reserved with MAP_JIT, we can add a value to the Permissions enum
    instead of adding a second, orthogonal parameter.
    That way we avoid changing public API functions, which makes this CL
    easier to undo once we have platform-independent w^x in Wasm.

    Bug: chromium:1117591
    Change-Id: I6333d69ab29d5900c689f08dcc892a5f1c1159b8
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2435365
    Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    Reviewed-by: Clemens Backes <clemensb@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70379}

PR-URL: nodejs#35986
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
@oleavr @targos
deps: V8: cherry-pick 1d0f426311d4
de67931 
Original commit message:

    [mac-arm64] Fix missing #include

    For an "#if defined(MAP_JIT)" test to work as expected, <sys/mman.h>
    must be included in the compilation unit.

    Bug: chromium:1144200
    Change-Id: Ia0bf35ec1872c02457f1fbc0ee6689c7f7d27d4a
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2517689
    Reviewed-by: Clemens Backes <clemensb@chromium.org>
    Reviewed-by: Igor Sheludko <ishell@chromium.org>
    Reviewed-by: Nico Weber <thakis@chromium.org>
    Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70986}

PR-URL: nodejs#35986
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
@gengjiawen @targos
src: fix v8 api deprecation
685cb6e 
PR-URL: nodejs#35700
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
@targos
tools: update V8 gypfiles for 8.7
b7311fb 
PR-URL: nodejs#35700
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
@targos
deps: V8: restore compatibility with 8.6
ad2fd53 
Revert "[unwinder] Delete the old unwinder API"
Refs: v8/v8@9c5eed7
@targos targos added the semver-minor PRs that contain new features and should be released in the next minor version. label Nov 19, 2020
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency. labels Nov 19, 2020
@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 19, 2020
@targos
Copy link
Member Author

targos commented Nov 19, 2020

self-reminder: increment embedder string in the last commit

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 19, 2020
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@gengjiawen
Copy link
Member

@targos This is needs rebase.

@bricss
Copy link

bricss commented Nov 30, 2020

Ping 📡

@bricss
Copy link

bricss commented Dec 6, 2020 •
edited
Loading

Maybe the conflict can be resolved, tho it will get into the next 15th release? 🙄

@Tarnadas Tarnadas mentioned this pull request Dec 12, 2020
Closed
@gengjiawen
Copy link
Member

@targos ping.

@targos
Copy link
Member Author

targos commented Jan 3, 2021

I don't want to spend more time on this before someone helps me with the ABI compat patch

@gengjiawen
Copy link
Member

I don't want to spend more time on this before someone helps me with the ABI compat patch

You mean the current merge conflicts or other issue ?

@targos
Copy link
Member Author

targos commented Jan 3, 2021

I mean extend ad2fd53 to revert all breaking changes in the API/ABI (or confirm that there are no remaining changes to revert)

@targos
Copy link
Member Author

targos commented Jan 13, 2021

I'm going to close this. Anyone feel free to pick it up if you feel strongly about it. I'll keep the branch on my fork until v15.x is EoL.

@targos targos closed this Jan 13, 2021
@targos targos deleted the v8-87-v15.x branch April 12, 2021 15:06
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

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. semver-minor PRs that contain new features and should be released in the next minor version. 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.