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.

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 6.8 #21079

Closed
wants to merge 18 commits into from
Closed

deps: update V8 to 6.8 #21079

wants to merge 18 commits into from

Conversation

targos
Copy link
Member

@targos targos commented Jun 1, 2018

ETA: July 24th

/cc @nodejs/v8-update @nodejs/tsc

@targos targos added semver-major PRs that contain breaking changes and should be released in the next major version. v8 engine Issues and PRs related to the V8 dependency. blocked PRs that are blocked by other issues or PRs. labels Jun 1, 2018
@targos targos requested a review from a team as a code owner June 1, 2018 09:05
@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 Jun 1, 2018
@targos
Copy link
Member Author

targos commented Jun 1, 2018

@targos targos removed the build Issues and PRs related to build files or the CI. label Jun 1, 2018
@mmarchini
Copy link
Contributor

Just an FYI (not a blocker): llnode is not ready for V8 6.8 yet, but I believe we'll finish the changes needed before June 24th.

PR if anyone is interested: nodejs/llnode#201

@psmarshall
Copy link
Contributor

We will need the V8 ABI compat patch to make the ABI look like 6.7 - I'll work on that in about 2 weeks and link it here.

@targos
Copy link
Member Author

targos commented Jun 1, 2018

macOS issue is a compiler issue. It is fixed in versions 10.11 and 10.12: https://ci.nodejs.org/job/node-test-commit-osx-targos/3/

@mhdawson

@ofrobots
Copy link
Contributor

ofrobots commented Jun 1, 2018

@psmarshall The V8 6.7 compat patch is not needed for this to land on master, but it would still be good to have it sooner rather than later.

@mhdawson
Copy link
Member

mhdawson commented Jun 5, 2018

We'll need to adjust the OSX machines for testing/release @gdams can you help to get an OSX 10.11 machine setup for release? @rvagg we should also schedule a talk with @gdams about OSX and mac stadium.

static void BackgroundRunner(void* data) {
namespace {

static void WorkerThreadMain(void* data) {
TRACE_EVENT_METADATA1("__metadata", "thread_name", "name",
"BackgroundTaskRunner");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to change this? @nodejs/trace-events

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense to change this to match the concept name actually used by V8, that is, 'WorkerThread'. However, things going to get really confusing since we also have worker_threads from #20876.

One of these concepts needs to be renamed for sanity's sake. /cc @addaleax

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One option would be to use something like 'PlatformWorkerThread' here.

@hashseed
Copy link
Member

@psmarshall The V8 6.7 compat patch is not needed for this to land on master, but it would still be good to have it sooner rather than later.

Peter will work on this next week.

@refack
Copy link
Contributor

refack commented Jun 10, 2018

macOS issue is a compiler issue. It is fixed in versions 10.11 and 10.12: ci.nodejs.org/job/node-test-commit-osx-targos/3

Googling and digging brought up this: https://stackoverflow.com/questions/23791060/c-thread-local-storage-clang-503-0-40-mac-osx

Might be helpful if someone from Apple would comment if simply updating xcode will solve this (cough @gibfahn cough)

@mhdawson
Copy link
Member

Issue for OSX in build repo: nodejs/build#1358

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rubber stamp LGTM

@targos
Copy link
Member Author

targos commented Jul 4, 2018

@targos
Copy link
Member Author

targos commented Jul 19, 2018

MylesBorins pushed a commit that referenced this pull request Aug 10, 2018
Backport-PR-URL: #21668
PR-URL: #21079
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
MylesBorins pushed a commit that referenced this pull request Aug 10, 2018
Backport-PR-URL: #21668
PR-URL: #21079
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
MylesBorins pushed a commit that referenced this pull request Aug 10, 2018
Synchronize source files list with upstream's BUILD.gn.

Teach v8.gyp to build and run torque, V8's DSL for generating builtins.

On Windows, the torque binary needs to be compiled and linked
with exception semantics and assume V8 is embedded.

Fixes: nodejs/node-v8#57

Co-Authored-By: Ben Noordhuis <info@bnoordhuis.nl>
Co-Authored-By: Refael Ackermann <refack@gmail.com>

Backport-PR-URL: #21668
PR-URL: #21079
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
MylesBorins pushed a commit that referenced this pull request Aug 10, 2018
Original commit message:

    [log][api] introduce public CodeEventListener API

    Introduce a new public API called CodeEventListener to allow embedders
    to better support external profilers and other diagnostic tools without
    relying on unsupported methods like --perf-basic-prof.

    Bug: v8:7694
    Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
    Change-Id: I063cc965394d59401358757634c9ea84c11517e9
    Co-authored-by: Daniel Beckert <daniel@sthima.com.br>
    Reviewed-on: https://chromium-review.googlesource.com/1028770
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Hannes Payer <hpayer@chromium.org>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Andreas Haas <ahaas@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#53382}

Refs: v8/v8@aa6ce3e

Backport-PR-URL: #21668
PR-URL: #21079
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
MylesBorins pushed a commit that referenced this pull request Aug 10, 2018
As per Node.js docs, vm.Script instance is not bound to any context.

However, this test was expecting otherwise and depended on
implementation details which are going to change.

Refs: https://chromium-review.googlesource.com/c/v8/v8/+/1013581

Backport-PR-URL: #21668
PR-URL: #21079
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
MylesBorins pushed a commit that referenced this pull request Aug 10, 2018
This commit updates the following postmortem metadata constant:

- v8dbg_context_idx_closure
  - Renamed: v8dbg_context_idx_scope_info
  - V8 commit: v8/v8@39496a9#diff-f3f182b0510ba2ee39ae87e421ff110b

Fixes: nodejs/node-v8#59

Backport-PR-URL: #21668
PR-URL: #21079
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
rvagg added a commit that referenced this pull request Aug 13, 2018
Notable changes:

* deps: Upgrade V8 from 6.7 to 6.8 (Michaël Zasso) #21079
  * Memory reduction and performance improvements, details at:
    https://v8project.blogspot.com/2018/06/v8-release-68.html
* fs: Implement a fs.mkdir() recursive option, similar to the mkdirp npm package
  or mkdir -p on the command line (Benjamin Coe) #21875
* http: http.get() and http.request() (and https variants) can now accept three
  arguments to allow for a URL and an options object (Sam Ruby) #21616
rvagg added a commit that referenced this pull request Aug 15, 2018
Notable changes:

* deps: Upgrade V8 from 6.7 to 6.8 (Michaël Zasso) #21079
  * Memory reduction and performance improvements, details at:
    https://v8project.blogspot.com/2018/06/v8-release-68.html
* fs: Implement a fs.mkdir() recursive option, similar to the mkdirp npm package
  or mkdir -p on the command line (Benjamin Coe) #21875
* http: http.get() and http.request() (and https variants) can now accept three
  arguments to allow for a URL and an options object (Sam Ruby) #21616
* Added new collaborators
  * Sam Ruby (https://github.com/rubys)
  * George Adams (https://github.com/gdams)
rvagg added a commit that referenced this pull request Aug 16, 2018
Notable changes:

* buffer:
  * Fix out-of-bounds (OOB) write in `Buffer.write()` for UCS-2 encoding
    (CVE-2018-12115)
  * Fix unintentional exposure of uninitialized memory in `Buffer.alloc()`
    (CVE-2018-7166)
* deps:
  * Upgrade to OpenSSL 1.1.0i, fixing:
    - Client DoS due to large DH parameter (CVE-2018-0732)
    - ECDSA key extraction via local side-channel (CVE not assigned)
  * Upgrade V8 from 6.7 to 6.8 (Michaël Zasso) #21079
    - Memory reduction and performance improvements, details at:
      https://v8project.blogspot.com/2018/06/v8-release-68.html
* http: `http.get()` and `http.request()` (and `https` variants) can now accept
  three arguments to allow for a `URL` _and_ an `options` object
  (Sam Ruby) #21616
* Added new collaborators
  * Sam Ruby (https://github.com/rubys)
  * George Adams (https://github.com/gdams)
rvagg added a commit that referenced this pull request Aug 16, 2018
Notable changes:

* buffer:
  * Fix out-of-bounds (OOB) write in `Buffer.write()` for UCS-2 encoding
    (CVE-2018-12115)
  * Fix unintentional exposure of uninitialized memory in `Buffer.alloc()`
    (CVE-2018-7166)
* deps:
  * Upgrade to OpenSSL 1.1.0i, fixing:
    - Client DoS due to large DH parameter (CVE-2018-0732)
    - ECDSA key extraction via local side-channel (CVE not assigned)
  * Upgrade V8 from 6.7 to 6.8 (Michaël Zasso) #21079
    - Memory reduction and performance improvements, details at:
      https://v8project.blogspot.com/2018/06/v8-release-68.html
* http: `http.get()` and `http.request()` (and `https` variants) can now accept
  three arguments to allow for a `URL` _and_ an `options` object
  (Sam Ruby) #21616
* Added new collaborators
  * Sam Ruby (https://github.com/rubys)
  * George Adams (https://github.com/gdams)
nornagon pushed a commit to electron/node that referenced this pull request Sep 18, 2018
Precursor to removing deprecated APIs on the v8 side @
https://chromium-review.googlesource.com/c/v8/v8/+/1045310

PR-URL: nodejs/node#21079
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
(cherry-picked from 0f3c2c6)
codebytere pushed a commit to electron/node that referenced this pull request Oct 2, 2018
Precursor to removing deprecated APIs on the v8 side @
https://chromium-review.googlesource.com/c/v8/v8/+/1045310

PR-URL: nodejs/node#21079
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
(cherry-picked from 0f3c2c6)
blattersturm pushed a commit to citizenfx/node that referenced this pull request Nov 3, 2018
Precursor to removing deprecated APIs on the v8 side @
https://chromium-review.googlesource.com/c/v8/v8/+/1045310

PR-URL: nodejs#21079
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
blattersturm pushed a commit to citizenfx/node that referenced this pull request Nov 3, 2018
Refs: nodejs#21079 (comment)

PR-URL: nodejs#21982
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-major PRs that contain breaking changes and should be released in the next major version. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.