deps: update V8 to 6.0 #14004

targos · 2017-06-30T12:01:35Z

As discussed in nodejs/CTC#146, I am opening this PR early so we can have test builds of Node with V8 6.0 as soon as possible.
The update is ready but blocked on five issues:

Build crashes on FreeBSD 10. See: update clang on FreeBSD 10 build#723 and Build failure on FreeBSD 10 node-v8#1.
Node crashes when running jest. See: V8_Fatal exception when running "jest" tests #13804, https://bugs.chromium.org/p/v8/issues/detail?id=6490 and https://github.com/targos/jest-crash.
SmartOS compilation fails: https://ci.nodejs.org/job/node-test-commit-smartos/9911/nodes=smartos15-64/console
async-hooks/test-zlib.zlib-binding.deflate can crash (seems to happen only with the 32-bit build): https://ci.nodejs.org/job/node-test-commit-linux/10874/
V8 cctest failure: https://ci.nodejs.org/job/node-test-commit-v8-linux/763/
--no-crankshaft option needs to be removed from tests #14531

/cc @nodejs/v8 @nodejs/ctc

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

Affected core subsystem(s)

V8

targos · 2017-06-30T12:07:05Z

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

targos · 2017-06-30T15:13:03Z

I forgot there are two other issues (initially reported here: nodejs/node-v8#7):

SmartOS compilation fails: https://ci.nodejs.org/job/node-test-commit-smartos/9911/nodes=smartos15-64/console (cc @nodejs/platform-smartos)
async-hooks/test-zlib.zlib-binding.deflate can crash (seems to happen only with the 32-bit build): https://ci.nodejs.org/job/node-test-commit-linux/10874/

And finally a V8 cctest fails: https://ci.nodejs.org/job/node-test-commit-v8-linux/763/

misterdjules · 2017-06-30T15:26:22Z

@targos When do you need those SmartOS build errors fixed? In other words, is it urgent?

joaocgreis · 2017-06-30T18:41:38Z

@targos https://chromium-review.googlesource.com/c/556599/ landed upstream, it would be better to replace my floating patch (deps,win: increase msvs_shard in gyp for V8) with a backport of that commit.

targos · 2017-07-03T09:11:43Z

@joaocgreis Thank you. I cherry-picked that commit.

targos · 2017-07-03T09:14:20Z

@misterdjules No, this is not urgent, and we have other issues to fix in the meantime

bmeurer · 2017-07-06T07:29:31Z

cc @natorion

natorion · 2017-07-11T11:03:06Z

Some comments/questions:

Node crashes when running jest. See: #13804, https://bugs.chromium.org/p/v8/issues/detail?id=6490 and https://github.com/targos/jest-crash.
==> scheduled to be merged upstream to 6.0

async-hooks/test-zlib.zlib-binding.deflate can crash (seems to happen only with the 32-bit build): https://ci.nodejs.org/job/node-test-commit-linux/10874/
==> Is there already a V8 issue? That sounds like something that might be fixed upstream. Sadly, I can't access ci.nodejs.org

V8 cctest failure: https://ci.nodejs.org/job/node-test-commit-v8-linux/763/
==> Same as above, it is hard to judge what needs to be done.

Compilation errors on SmartOS and FreeBSD =>
Are you going to float those patches? Are you expecting that they are going to be upstreamed? Please keep in mind that we will promote 6.0 to V8 stable next week. That means, that merges will have a much higher burden to overcome. Depending on the actual patch it might not be an issue though because of SmartOS and FreeBSD.

Trott · 2017-07-11T13:47:14Z

Sadly, I can't access ci.nodejs.org

@natorion It should be available again in a few hours, if all goes well.

Access is extremely restricted right now because it's being used for the scheduled security release of Node.js. Any changes to that schedule are likely to be posted to #14153. If necessary to know more, @MylesBorins can fill you in.

gibfahn · 2017-07-11T23:11:15Z

@natorion It should be available again in a few hours, if all goes well.

It's back btw.

targos · 2017-07-11T23:15:28Z

New CI run: https://ci.nodejs.org/job/node-test-pull-request/9075/

natorion · 2017-07-12T10:36:42Z

Thanks, seems like https://ci.nodejs.org/job/node-test-pull-request/9075/ was aborted and the other links produce 404s.

MylesBorins · 2017-07-12T10:50:29Z

@natorion it would appear that the test runner was hanging on centos 5

https://ci.nodejs.org/job/node-test-commit-linux/11105/nodes=centos5-64/console

targos · 2017-07-12T11:45:09Z

I updated the branch to V8 6.0.286.34 to include the fix for https://bugs.chromium.org/p/v8/issues/detail?id=6490.

New CI: https://ci.nodejs.org/job/node-test-commit/11071/
V8 tests: https://ci.nodejs.org/job/node-test-commit-v8-linux/776/

MylesBorins · 2017-07-12T12:32:09Z

@targos @addaleax any interest in floating the bsd / smart os packages upstream?

addaleax · 2017-07-12T12:41:00Z

Sorry, I didn’t quite follow here – which are the patches you are referring to?

MylesBorins · 2017-07-12T12:45:53Z

@addaleax patches that fix the compilation errors, Im not sure we have them yet...

was trying to summarize @natorion's questions in #14004 (comment)

targos · 2017-07-12T13:11:28Z

We don't have those patches yet and I don't know if anyone is working on it.

addaleax · 2017-07-12T13:14:22Z

async-hooks/test-zlib.zlib-binding.deflate can crash (seems to happen only with the 32-bit build): https://ci.nodejs.org/job/node-test-commit-linux/10874/

Do you have a CI log that contains that failure? Could it be the same as #14161?

targos · 2017-07-12T13:22:01Z

Do you have a CI log that contains that failure?

No, the logs have been deleted. It does not seem to be reproducible anymore. I'll mark this point as done for now.

PR-URL: #14004 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Major V8 updates are usually API/ABI incompatible with previous versions. This commit adapts NODE_MODULE_VERSION for V8 6.0. Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md PR-URL: #14004 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

regress/regress-crbug-514081 allocates a 2G block of memory and if there are multiple variants running at the same time this can lead to crashes, OOM kills or the OS failing to allocate memory. This patch limits us to running a single variant of the test Fixes: #6340 Refs: #6678 PR-URL: #14004 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

HGlobalValueNumberingPhase::CollectSideEffectsOnPathsToDominatedBlock() used to self-recurse before this commit, causing stack overflows on systems with small stack sizes. Make it non-recursive by storing intermediate results in a heap-allocated list. Fixes: #11991 Refs: #12460 PR-URL: #14004 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

VS2013 does not support defaulting move constructor and assignment operator. This adds explicit definitions of those methods for two classes. This fix is required because we still support building addons with VS2013 and the incompatibility is in v8.h. Fixes: nodejs/node-v8#4 Refs: #13263 PR-URL: #14004 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Original commit message: Fix GCC 7 build errors BUG=chromium:691681 R=franzih@chromium.org Change-Id: Id7e5698487f16dc217a804f6d3f24da7213c72b9 Reviewed-on: https://chromium-review.googlesource.com/530227 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Toon Verwaest <verwaest@chromium.org> Cr-Commit-Position: refs/heads/master@{#46045} Refs: #13517 Fixes: #10388 Refs: #12392 PR-URL: #14004 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

PR-URL: #14004 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Original commit messages: v8/v8@a2ab135 [snapshot] Rehash strings after deserialization. See https://goo.gl/6aN8xA Bug: v8:6593 Change-Id: Ic8b0b57195d01d41591397d5d45de3f0f3ebc3d9 Reviewed-on: https://chromium-review.googlesource.com/574527 Reviewed-by: Camillo Bruni <cbruni@chromium.org> Reviewed-by: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Commit-Queue: Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/heads/master@{#46732} v8/v8@182caaf Do not track transitions for built-in objects. Objects created during bootstrapping do not need a transition tree except for elements kind transitions. Bug: v8:6596 Change-Id: I237b8b2792f201336e1c9731c815095dd06bc182 Reviewed-on: https://chromium-review.googlesource.com/571750 Reviewed-by: Igor Sheludko <ishell@chromium.org> Commit-Queue: Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/heads/master@{#46693} Fixes: #14171 Refs: #14345 PR-URL: #14004 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

regress/regress-crbug-514081 allocates a 2G block of memory and if there are multiple variants running at the same time this can lead to crashes, OOM kills or the OS failing to allocate memory. This patch limits us to running a single variant of the test Fixes: #6340 Refs: #6678 Backport-PR-URL: #14574 Backport-Reviewed-By: Anna Henningsen <anna@addaleax.net> Backport-Reviewed-By: Refael Ackermann <refack@gmail.com> PR-URL: #14004 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

HGlobalValueNumberingPhase::CollectSideEffectsOnPathsToDominatedBlock() used to self-recurse before this commit, causing stack overflows on systems with small stack sizes. Make it non-recursive by storing intermediate results in a heap-allocated list. Fixes: #11991 Refs: #12460 Backport-PR-URL: #14574 Backport-Reviewed-By: Anna Henningsen <anna@addaleax.net> Backport-Reviewed-By: Refael Ackermann <refack@gmail.com> PR-URL: #14004 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

VS2013 does not support defaulting move constructor and assignment operator. This adds explicit definitions of those methods for two classes. This fix is required because we still support building addons with VS2013 and the incompatibility is in v8.h. Fixes: nodejs/node-v8#4 Refs: #13263 Backport-PR-URL: #14574 Backport-Reviewed-By: Anna Henningsen <anna@addaleax.net> Backport-Reviewed-By: Refael Ackermann <refack@gmail.com> PR-URL: #14004 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Original commit message: Fix GCC 7 build errors BUG=chromium:691681 R=franzih@chromium.org Change-Id: Id7e5698487f16dc217a804f6d3f24da7213c72b9 Reviewed-on: https://chromium-review.googlesource.com/530227 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Toon Verwaest <verwaest@chromium.org> Cr-Commit-Position: refs/heads/master@{#46045} Refs: #13517 Fixes: #10388 Refs: #12392 Backport-PR-URL: #14574 Backport-Reviewed-By: Anna Henningsen <anna@addaleax.net> Backport-Reviewed-By: Refael Ackermann <refack@gmail.com> PR-URL: #14004 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Backport-PR-URL: #14574 Backport-Reviewed-By: Anna Henningsen <anna@addaleax.net> Backport-Reviewed-By: Refael Ackermann <refack@gmail.com> PR-URL: #14004 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Original commit messages: v8/v8@a2ab135 [snapshot] Rehash strings after deserialization. See https://goo.gl/6aN8xA Bug: v8:6593 Change-Id: Ic8b0b57195d01d41591397d5d45de3f0f3ebc3d9 Reviewed-on: https://chromium-review.googlesource.com/574527 Reviewed-by: Camillo Bruni <cbruni@chromium.org> Reviewed-by: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Commit-Queue: Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/heads/master@{#46732} v8/v8@182caaf Do not track transitions for built-in objects. Objects created during bootstrapping do not need a transition tree except for elements kind transitions. Bug: v8:6596 Change-Id: I237b8b2792f201336e1c9731c815095dd06bc182 Reviewed-on: https://chromium-review.googlesource.com/571750 Reviewed-by: Igor Sheludko <ishell@chromium.org> Commit-Queue: Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/heads/master@{#46693} Fixes: #14171 Refs: #14345 Backport-PR-URL: #14574 Backport-Reviewed-By: Anna Henningsen <anna@addaleax.net> Backport-Reviewed-By: Refael Ackermann <refack@gmail.com> PR-URL: #14004 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

The v8 and test-hash-seed targets cannot be run in parallel because they need different copies of the deps/v8 directory. Ref: nodejs#14004 (comment) PR-URL: nodejs#14219 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

The v8 and test-hash-seed targets cannot be run in parallel because they need different copies of the deps/v8 directory. Ref: #14004 (comment) Backport-PR-URL: #15562 PR-URL: #14219 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

targos added blocked PRs that are blocked by other issues or PRs. 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. labels Jun 30, 2017

nodejs-github-bot added the v8 engine Issues and PRs related to the V8 dependency. label Jun 30, 2017

targos mentioned this pull request Jun 30, 2017

V8 5.9 & Node.js 8.0 nodejs/CTC#146

Closed

targos mentioned this pull request Jun 30, 2017

V8 6.0 failures nodejs/node-v8#7

Closed

targos force-pushed the v8-6.0 branch from ed2df05 to 2270ad6 Compare July 3, 2017 09:10

targos force-pushed the v8-6.0 branch from 2270ad6 to fec6890 Compare July 5, 2017 15:14

targos force-pushed the v8-6.0 branch from fec6890 to 42203ed Compare July 10, 2017 08:31

targos force-pushed the v8-6.0 branch from 42203ed to 4f744d8 Compare July 12, 2017 11:39

MylesBorins closed this Aug 1, 2017

MylesBorins added backported-to-v8.x labels Aug 1, 2017

This was referenced Aug 1, 2017

deps: update V8 to 6.0 8.x backport #14574

Closed

missing V8 6.0 patch #14582

Closed

addaleax mentioned this pull request Aug 2, 2017

v8.3.0 proposal #14594

Merged

targos deleted the v8-6.0 branch August 10, 2017 09:24

jasnell mentioned this pull request Oct 30, 2017

Proposal: 2017-10-31, Version 9.0.0 (Current) #15135

Merged

remojansen mentioned this pull request Dec 15, 2017

Node >= 8.0.0 inconsistently not throwing "Maximum call stack size exceeded" #17684

Closed

nfalco79 mentioned this pull request Feb 14, 2018

Add support for BSD platforms (such as FreeBSD) jenkinsci/nodejs-plugin#20

Closed

refack mentioned this pull request Nov 23, 2018

chore: list compiler versions used to build each node version on each platform nodejs/build#1522

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

deps: update V8 to 6.0 #14004

deps: update V8 to 6.0 #14004

targos commented Jun 30, 2017 •

edited by refack

Loading

targos commented Jun 30, 2017

targos commented Jun 30, 2017

misterdjules commented Jun 30, 2017

joaocgreis commented Jun 30, 2017

targos commented Jul 3, 2017

targos commented Jul 3, 2017

bmeurer commented Jul 6, 2017

natorion commented Jul 11, 2017

Trott commented Jul 11, 2017

gibfahn commented Jul 11, 2017

targos commented Jul 11, 2017

natorion commented Jul 12, 2017

MylesBorins commented Jul 12, 2017

targos commented Jul 12, 2017

MylesBorins commented Jul 12, 2017

addaleax commented Jul 12, 2017

MylesBorins commented Jul 12, 2017

targos commented Jul 12, 2017

addaleax commented Jul 12, 2017

targos commented Jul 12, 2017

deps: update V8 to 6.0 #14004

deps: update V8 to 6.0 #14004

Conversation

targos commented Jun 30, 2017 • edited by refack Loading

Checklist

Affected core subsystem(s)

targos commented Jun 30, 2017

targos commented Jun 30, 2017

misterdjules commented Jun 30, 2017

joaocgreis commented Jun 30, 2017

targos commented Jul 3, 2017

targos commented Jul 3, 2017

bmeurer commented Jul 6, 2017

natorion commented Jul 11, 2017

Trott commented Jul 11, 2017

gibfahn commented Jul 11, 2017

targos commented Jul 11, 2017

natorion commented Jul 12, 2017

MylesBorins commented Jul 12, 2017

targos commented Jul 12, 2017

MylesBorins commented Jul 12, 2017

addaleax commented Jul 12, 2017

MylesBorins commented Jul 12, 2017

targos commented Jul 12, 2017

addaleax commented Jul 12, 2017

targos commented Jul 12, 2017

targos commented Jun 30, 2017 •

edited by refack

Loading