deps: update V8 to 7.2 #24875

targos · 2018-12-06T15:11:32Z

ETA: Jan 29th, 2019

nodejs-github-bot · 2018-12-06T15:11:42Z

@targos build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1853/pipeline

targos · 2018-12-06T15:23:53Z

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

targos · 2018-12-06T16:05:07Z

"unresolved external symbol" errors on Windows were already seen on canary. See nodejs/node-v8#89 (comment)
/cc @bzoz

targos · 2018-12-06T16:45:22Z

Error on Linux with GCC 6:

17:00:46 g++: internal compiler error: Killed (program cc1plus)
17:00:46 Please submit a full bug report,
17:00:46 with preprocessed source if appropriate.
17:00:46 See <http://bugzilla.redhat.com/bugzilla> for instructions.
17:00:46 deps/v8/gypfiles/v8_initializers.target.mk:199: recipe for target '/home/iojs/build/workspace/node-test-commit-linux/nodes/centos7-64-gcc6/out/Release/obj.target/v8_initializers/gen/torque-generated/builtins-array-from-dsl-gen.o' failed
17:00:46 make[2]: *** [/home/iojs/build/workspace/node-test-commit-linux/nodes/centos7-64-gcc6/out/Release/obj.target/v8_initializers/gen/torque-generated/builtins-array-from-dsl-gen.o] Error 4

CI: https://ci.nodejs.org/job/node-test-commit-linux/23806/nodes=centos7-64-gcc6/

bnoordhuis · 2018-12-06T19:12:43Z

Nine times out of ten that's the compiler getting OOM-killed. All the more likely because it was trying to compile a large generated file.

targos · 2019-01-01T15:58:08Z

Updated.

New CI: https://ci.nodejs.org/job/node-test-pull-request/19904/
V8: https://ci.nodejs.org/job/node-test-commit-v8-linux/1975/

Major V8 updates are usually API/ABI incompatible with previous versions. This commit adapts NODE_MODULE_VERSION for V8 7.2. Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md

Co-authored-by: Ujjwal Sharma <usharma1998@gmail.com> win: add v8_init to dependencies Fixes: nodejs/node-v8#89 Co-authored-by: Bartosz Sosnowski <bartosz@janeasystems.com>

Bump minimum version of ICU needed to build node to 63. Refs: v8/v8@30a350f Co-authored-by: Steven R Loomis <srloomis@us.ibm.com>

New heap space: code_large_object_space

targos · 2019-01-20T08:23:16Z

Updated and fixed v8.gyp (contained a file that's not in 7.2).

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

targos · 2019-01-20T08:47:29Z

/cc @nodejs/platform-ppc The executable produced for linuxone doesn't work (illegal instruction).
This is with V8 7.2.502.21

mhdawson · 2019-01-21T23:13:07Z

@john-yan can you take a look.

mhdawson · 2019-01-22T15:22:36Z

From@john-yan seems like V8 7.2 is green for us so new CI to see if it is repeatable: https://ci.nodejs.org/job/node-test-commit-v8-linux/2041/

mhdawson · 2019-01-22T22:35:58Z

CI failed again. The node executable built seems to crash.

mhdawson · 2019-01-22T22:42:53Z

I see that linuxone (s390x) also fails in the node CI in the same way it does for the v8 test. @sam-github can you help investigate what is going on? Looks like with the udpated version of V8 Node crashes on startup for some reason.

schuay · 2019-01-24T14:56:11Z

@schuay I'm wondering if there are any tests that could be added to the v8 testing that might have caught this problem? Maybe there are and we did not see them in the google CI because s390x testing is done through emulation?

Yes that is probably why we didn't see this before. IIRC runtime code is compiled with the host architecture in sim builds. On real s390 hosts we'd have seen this immediately.

john-yan · 2019-01-24T15:53:12Z

@schuay I'm wondering if there are any tests that could be added to the v8 testing that might have caught this problem? Maybe there are and we did not see them in the google CI because s390x testing is done through emulation?

Yes that is probably why we didn't see this before. IIRC runtime code is compiled with the host architecture in sim builds. On real s390 hosts we'd have seen this immediately.

Hello Jakob, we have v8 testing running locally on s390x host and we never see this either. Do you know which v8 test has coverage for this alignment issue?

schuay · 2019-01-24T16:05:06Z

Hello Jakob, we have v8 testing running locally on s390x host and we never see this either. Do you know which v8 test has coverage for this alignment issue?

From the backtrace I'd expect it to happen on every Isolate startup if the variable is unaligned. But I'd suggest moving this discussion to another bug to avoid cluttering here ;)

Original commit message: [snapshot] Always align embedded blob code pointer and size Other platforms besides ARM64 Windows may also have alignment requirements, e.g. PPC and s390. These requirements may affect both the code pointer field and the size field, and so they each need alignment directives because they are stored in different sections. Since aligning wastes a handful of bytes at most, not making alignment conditional on the platform type seems like a good idea. Refs: nodejs#24875 Change-Id: I1f58606af294be65e74a1f107cd05fc21e032704 Reviewed-on: https://chromium-review.googlesource.com/c/1433778 Commit-Queue: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Jakob Gruber <jgruber@chromium.org> Cr-Commit-Position: refs/heads/master@{#59058} Refs: v8/v8@fc0ddf5

Backport parts of v8/v8@9302db480e8cd7c88fd948baf0cf that fix compiler errors on Windows. Original commit message: [ubsan] Port HeapObject to the new design Merging the temporary HeapObjectPtr back into HeapObject. Bug: v8:3770 Change-Id: I5bcd23ca2f5ba862cf5b52955dca143e531c637b Reviewed-on: https://chromium-review.googlesource.com/c/1386492 Commit-Queue: Jakob Kummerow <jkummerow@chromium.org> Reviewed-by: Clemens Hammacher <clemensh@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Michael Stanton <mvstanton@chromium.org> Reviewed-by: Jakob Gruber <jgruber@chromium.org> Cr-Commit-Position: refs/heads/master@{#58410} Refs: v8/v8@9302db4

addaleax · 2019-01-24T16:46:24Z

CI for addaleax/node@85ffec2 which might (:crossed_fingers:) fix the Windows compiler errors: https://ci.nodejs.org/job/node-test-commit/25276/

addaleax · 2019-01-24T17:45:32Z

So, the compiler errors are fixed, but now there’s a linker error that’s probably going to need attention from somebody more knowledgeable with Windows/V8 snapshots than me:

v8_snapshot.lib(embedded.obj) : error LNK2026: module unsafe for SAFESEH image. [c:\workspace\node-compile-windows\node.vcxproj]

schuay · 2019-01-25T08:33:26Z

Haven't seen that one before. These Microsoft docs look relevant.

schuay · 2019-01-25T08:51:55Z

Is it an option to disable /SAFESEH? Here's another node issue where this has apparently been done before. Alternatively it may also be possible to add .SAFESEH MASM directives as explained here, but I have no experience with that.

paolosev · 2019-01-26T03:15:13Z

The problem seems to be in project v8_snapshot, where the file embedded.S is compiled with the MASM tool. We should pass /safeseh to ml.exe to make v8_snapshot.lib compatible with SAFESEH.

Other platforms besides ARM64 Windows may also have alignment requirements, e.g. PPC and s390. These requirements may affect both the code pointer field and the size field, and so they each need alignment directives because they are stored in different sections. Since aligning wastes a handful of bytes at most, not making alignment conditional on the platform type seems like a good idea. Refs: nodejs/node#24875 Change-Id: I1f58606af294be65e74a1f107cd05fc21e032704 Reviewed-on: https://chromium-review.googlesource.com/c/1433778 Commit-Queue: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Jakob Gruber <jgruber@chromium.org> Cr-Commit-Position: refs/heads/master@{#59058}

targos · 2019-01-31T14:03:58Z

Branch 7.3 was cut a week ago. I'll open a PR with it. We'll see if it's just easier to skip a version.

targos · 2019-01-31T16:22:34Z

Attempte to update directly to 7.3: #25852

Original commit message: [snapshot] Always align embedded blob code pointer and size Other platforms besides ARM64 Windows may also have alignment requirements, e.g. PPC and s390. These requirements may affect both the code pointer field and the size field, and so they each need alignment directives because they are stored in different sections. Since aligning wastes a handful of bytes at most, not making alignment conditional on the platform type seems like a good idea. Refs: nodejs#24875 Change-Id: I1f58606af294be65e74a1f107cd05fc21e032704 Reviewed-on: https://chromium-review.googlesource.com/c/1433778 Commit-Queue: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Jakob Gruber <jgruber@chromium.org> Cr-Commit-Position: refs/heads/master@{#59058} Refs: v8/v8@fc0ddf5

Original commit message: [snapshot] Always align embedded blob code pointer and size Other platforms besides ARM64 Windows may also have alignment requirements, e.g. PPC and s390. These requirements may affect both the code pointer field and the size field, and so they each need alignment directives because they are stored in different sections. Since aligning wastes a handful of bytes at most, not making alignment conditional on the platform type seems like a good idea. Refs: #24875 Change-Id: I1f58606af294be65e74a1f107cd05fc21e032704 Reviewed-on: https://chromium-review.googlesource.com/c/1433778 Commit-Queue: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Jakob Gruber <jgruber@chromium.org> Cr-Commit-Position: refs/heads/master@{#59058} Refs: v8/v8@fc0ddf5 PR-URL: #25852 Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>

targos added semver-major PRs that contain breaking changes and should be released in the next major version. blocked PRs that are blocked by other issues or PRs. labels Dec 6, 2018

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 Dec 6, 2018

targos removed the build Issues and PRs related to build files or the CI. label Dec 6, 2018

targos mentioned this pull request Dec 11, 2018

win: add v8_init to dependencies nodejs/node-v8#92

Closed

2 tasks

Flarna mentioned this pull request Dec 17, 2018

Ecosystem Modules that will break in Node 12 #25060

Closed

targos mentioned this pull request Dec 17, 2018

object spread operator mutates first argument #25089

Closed

targos force-pushed the v8-7.2 branch from 14b5f74 to 5107974 Compare January 1, 2019 15:53

targos and others added 7 commits January 20, 2019 08:45

deps: update V8 to 7.2.502.21

7d29926

build: reset embedder string to "-node.0"

bb0ffe7

src: update NODE_MODULE_VERSION to 69

a9cd973

Major V8 updates are usually API/ABI incompatible with previous versions. This commit adapts NODE_MODULE_VERSION for V8 7.2. Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md

deps: sync V8 gypfiles with 7.2

d1486e9

Co-authored-by: Ujjwal Sharma <usharma1998@gmail.com> win: add v8_init to dependencies Fixes: nodejs/node-v8#89 Co-authored-by: Bartosz Sosnowski <bartosz@janeasystems.com>

deps: bump minimum icu version to 63

20721dc

Bump minimum version of ICU needed to build node to 63. Refs: v8/v8@30a350f Co-authored-by: Steven R Loomis <srloomis@us.ibm.com>

test: remove apply calls over 65534 arg limit

8c3a8c8

test: update test-v8-stats

c04c8a2

New heap space: code_large_object_space

targos force-pushed the v8-7.2 branch from 5107974 to c04c8a2 Compare January 20, 2019 08:19

sam-github mentioned this pull request Jan 22, 2019

Request for temporary access to rhel72-s390x nodejs/build#1666

Closed

addaleax force-pushed the v8-7.2 branch from dd571ca to f9142b8 Compare January 24, 2019 16:28

targos mentioned this pull request Jan 31, 2019

deps: update V8 to 7.3 #25852

Closed

targos closed this Feb 1, 2019

jkunkee mentioned this pull request Feb 8, 2019

Support ARM64 Windows desktop #25998

Closed

targos deleted the v8-7.2 branch March 14, 2019 17:53

richardlau mentioned this pull request Mar 27, 2019

Node.js 12 SemVer Major Cut-off #26844

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

deps: update V8 to 7.2 #24875

deps: update V8 to 7.2 #24875

targos commented Dec 6, 2018

nodejs-github-bot commented Dec 6, 2018

targos commented Dec 6, 2018

targos commented Dec 6, 2018 •

edited

Loading

targos commented Dec 6, 2018

bnoordhuis commented Dec 6, 2018

targos commented Jan 1, 2019

targos commented Jan 20, 2019

targos commented Jan 20, 2019 •

edited

Loading

mhdawson commented Jan 21, 2019 •

edited

Loading

mhdawson commented Jan 22, 2019

mhdawson commented Jan 22, 2019

mhdawson commented Jan 22, 2019

schuay commented Jan 24, 2019

john-yan commented Jan 24, 2019

schuay commented Jan 24, 2019

addaleax commented Jan 24, 2019

addaleax commented Jan 24, 2019

schuay commented Jan 25, 2019

schuay commented Jan 25, 2019

paolosev commented Jan 26, 2019

targos commented Jan 31, 2019

targos commented Jan 31, 2019

deps: update V8 to 7.2 #24875

deps: update V8 to 7.2 #24875

Conversation

targos commented Dec 6, 2018

nodejs-github-bot commented Dec 6, 2018

targos commented Dec 6, 2018

targos commented Dec 6, 2018 • edited Loading

targos commented Dec 6, 2018

bnoordhuis commented Dec 6, 2018

targos commented Jan 1, 2019

targos commented Jan 20, 2019

targos commented Jan 20, 2019 • edited Loading

mhdawson commented Jan 21, 2019 • edited Loading

mhdawson commented Jan 22, 2019

mhdawson commented Jan 22, 2019

mhdawson commented Jan 22, 2019

schuay commented Jan 24, 2019

john-yan commented Jan 24, 2019

schuay commented Jan 24, 2019

addaleax commented Jan 24, 2019

addaleax commented Jan 24, 2019

schuay commented Jan 25, 2019

schuay commented Jan 25, 2019

paolosev commented Jan 26, 2019

targos commented Jan 31, 2019

targos commented Jan 31, 2019

targos commented Dec 6, 2018 •

edited

Loading

targos commented Jan 20, 2019 •

edited

Loading

mhdawson commented Jan 21, 2019 •

edited

Loading