deps: cherry-pick e020aae394 from V8 upstream #14913

MylesBorins · 2017-08-18T11:05:20Z

Original commit message:

Work around glibc thread-local storage bug

glibc before 2.17 has a bug that makes it impossible to execute
binaries that have single-byte thread-local variables:

    % node --version
    node: error while loading shared libraries: cannot allocate
    memory in static TLS block

Work around that by making the one instance in the V8 code base
an int.

See: https://sourceware.org/bugzilla/show_bug.cgi?id=14898
See: https://github.com/nodesource/distributions/issues/513
See: https://github.com/nodejs/build/pull/809
Change-Id: Iefd8009100cd93e26cf8dc5dc03f2d622b423385
Reviewed-on: https://chromium-review.googlesource.com/612351
Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-by: Eric Holk <eholk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47400}

Refs: nodejs/build#809

Original commit message: Work around glibc thread-local storage bug glibc before 2.17 has a bug that makes it impossible to execute binaries that have single-byte thread-local variables: % node --version node: error while loading shared libraries: cannot allocate memory in static TLS block Work around that by making the one instance in the V8 code base an int. See: https://sourceware.org/bugzilla/show_bug.cgi?id=14898 See: nodesource/distributions#513 See: nodejs/build#809 Change-Id: Iefd8009100cd93e26cf8dc5dc03f2d622b423385 Reviewed-on: https://chromium-review.googlesource.com/612351 Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-by: Eric Holk <eholk@chromium.org> Cr-Commit-Position: refs/heads/master@{nodejs#47400} Refs: nodejs/build#809

MylesBorins · 2017-08-18T11:06:18Z

CI: https://ci.nodejs.org/job/node-test-pull-request/9729/
v8-ci: https://ci.nodejs.org/job/node-test-commit-v8-linux/861/

bnoordhuis

LGTM, sweet code.

MylesBorins · 2017-08-18T11:14:42Z

FWIW the three-way merge algorithm in the latest git is good enough to apply the patch without specifying directory anymore

$ cd node
$ curl -L https://github.com/v8/v8/commit/e020aae394.patch | git am -3 # just works 🎉

instead of

$ cd node
$ curl -L https://github.com/v8/v8/commit/e020aae394.patch | git am -3 --directory=deps/v8

seishun · 2017-08-18T14:24:29Z

The CI failures seem unrelated. Can this PR go through?

mhdawson

LGTM

mhdawson · 2017-08-18T15:13:12Z

The v8 ci failing is concerning so I'd be thinking we should get that fixed up first ?

targos · 2017-08-18T15:15:28Z

I think the V8 CI is failing because of #14001

refack · 2017-08-18T15:46:02Z

So I'm trying to figure out how come the CI node jobs pass, but the V8 jobs fail, which made me think of d8, and it seems like maybe @matthewloring didn't patch d8 as part of #14001 ?

../src/d8.cc:229:12: error: ‘platform_’ was not declared in this scope
     return platform_->GetTracingController();
            ^

refack · 2017-08-18T15:52:28Z

Cross-ref: nodejs/build#387

refack · 2017-08-18T16:02:51Z

V8 ci on master for "negative control": https://ci.nodejs.org/job/node-test-commit-v8-linux/862/
Less interesting (https://ci.nodejs.org/job/node-test-commit-node-v8/115/)

matthewloring · 2017-08-18T21:41:54Z

It looks like #14001 had an implicit dependency on https://chromium-review.googlesource.com/c/509694. I think the V8 CI can be fixed by backporting this as well.

seishun · 2017-08-19T11:15:43Z

@matthewloring Are you planning to work on that?

matthewloring · 2017-08-19T16:40:24Z

@seishun Yup, I can put together the backport now.

seishun · 2017-08-22T18:19:23Z

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

seishun · 2017-08-22T19:03:32Z

CI is all green, I guess this PR is good to go?

MylesBorins · 2017-08-23T05:27:10Z

I'm afk for the next two weeks. If anyone can pick this up it would be excellent! On Aug 22, 2017 12:03 PM, "Nikolai Vavilov" <notifications@github.com> wrote: CI is all green, I guess this PR is good to go? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#14913 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAecV8lRxCulaS2w1Sx_vNa0mAuJd4Yuks5sayYMgaJpZM4O7aVm> .

jasnell · 2017-08-23T18:07:56Z

Is this ready to land?

targos · 2017-08-23T18:11:12Z

Yes.

Original commit message: Work around glibc thread-local storage bug glibc before 2.17 has a bug that makes it impossible to execute binaries that have single-byte thread-local variables: % node --version node: error while loading shared libraries: cannot allocate memory in static TLS block Work around that by making the one instance in the V8 code base an int. See: https://sourceware.org/bugzilla/show_bug.cgi?id=14898 See: nodesource/distributions#513 See: nodejs/build#809 Change-Id: Iefd8009100cd93e26cf8dc5dc03f2d622b423385 Reviewed-on: https://chromium-review.googlesource.com/612351 Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-by: Eric Holk <eholk@chromium.org> Cr-Commit-Position: refs/heads/master@{#47400} PR-URL: #14913 Ref: nodejs/build#809 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>

jasnell · 2017-08-24T22:52:12Z

landed in 64f59be

Original commit message: Work around glibc thread-local storage bug glibc before 2.17 has a bug that makes it impossible to execute binaries that have single-byte thread-local variables: % node --version node: error while loading shared libraries: cannot allocate memory in static TLS block Work around that by making the one instance in the V8 code base an int. See: https://sourceware.org/bugzilla/show_bug.cgi?id=14898 See: nodesource/distributions#513 See: nodejs/build#809 Change-Id: Iefd8009100cd93e26cf8dc5dc03f2d622b423385 Reviewed-on: https://chromium-review.googlesource.com/612351 Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-by: Eric Holk <eholk@chromium.org> Cr-Commit-Position: refs/heads/master@{#47400} PR-URL: nodejs/node#14913 Ref: nodejs/build#809 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Original commit message: Work around glibc thread-local storage bug glibc before 2.17 has a bug that makes it impossible to execute binaries that have single-byte thread-local variables: % node --version node: error while loading shared libraries: cannot allocate memory in static TLS block Work around that by making the one instance in the V8 code base an int. See: https://sourceware.org/bugzilla/show_bug.cgi?id=14898 See: nodesource/distributions#513 See: nodejs/build#809 Change-Id: Iefd8009100cd93e26cf8dc5dc03f2d622b423385 Reviewed-on: https://chromium-review.googlesource.com/612351 Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-by: Eric Holk <eholk@chromium.org> Cr-Commit-Position: refs/heads/master@{nodejs#47400} PR-URL: nodejs#14913 Ref: nodejs/build#809 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Original commit message: Work around glibc thread-local storage bug glibc before 2.17 has a bug that makes it impossible to execute binaries that have single-byte thread-local variables: % node --version node: error while loading shared libraries: cannot allocate memory in static TLS block Work around that by making the one instance in the V8 code base an int. See: https://sourceware.org/bugzilla/show_bug.cgi?id=14898 See: nodesource/distributions#513 See: nodejs/build#809 Change-Id: Iefd8009100cd93e26cf8dc5dc03f2d622b423385 Reviewed-on: https://chromium-review.googlesource.com/612351 Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-by: Eric Holk <eholk@chromium.org> Cr-Commit-Position: refs/heads/master@{#47400} PR-URL: nodejs/node#14913 Ref: nodejs/build#809 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Original commit message: Work around glibc thread-local storage bug glibc before 2.17 has a bug that makes it impossible to execute binaries that have single-byte thread-local variables: % node --version node: error while loading shared libraries: cannot allocate memory in static TLS block Work around that by making the one instance in the V8 code base an int. See: https://sourceware.org/bugzilla/show_bug.cgi?id=14898 See: nodesource/distributions#513 See: nodejs/build#809 Change-Id: Iefd8009100cd93e26cf8dc5dc03f2d622b423385 Reviewed-on: https://chromium-review.googlesource.com/612351 Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-by: Eric Holk <eholk@chromium.org> Cr-Commit-Position: refs/heads/master@{#47400} PR-URL: #14913 Ref: nodejs/build#809 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Original commit message: Work around glibc thread-local storage bug glibc before 2.17 has a bug that makes it impossible to execute binaries that have single-byte thread-local variables: % node --version node: error while loading shared libraries: cannot allocate memory in static TLS block Work around that by making the one instance in the V8 code base an int. See: https://sourceware.org/bugzilla/show_bug.cgi?id=14898 See: nodesource/distributions#513 See: nodejs/build#809 Change-Id: Iefd8009100cd93e26cf8dc5dc03f2d622b423385 Reviewed-on: https://chromium-review.googlesource.com/612351 Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-by: Eric Holk <eholk@chromium.org> Cr-Commit-Position: refs/heads/master@{nodejs#47400} PR-URL: nodejs#14913 Ref: nodejs/build#809 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Original commit message: Work around glibc thread-local storage bug glibc before 2.17 has a bug that makes it impossible to execute binaries that have single-byte thread-local variables: % node --version node: error while loading shared libraries: cannot allocate memory in static TLS block Work around that by making the one instance in the V8 code base an int. See: https://sourceware.org/bugzilla/show_bug.cgi?id=14898 See: nodesource/distributions#513 See: nodejs/build#809 Change-Id: Iefd8009100cd93e26cf8dc5dc03f2d622b423385 Reviewed-on: https://chromium-review.googlesource.com/612351 Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-by: Eric Holk <eholk@chromium.org> Cr-Commit-Position: refs/heads/master@{#47400} PR-URL: #14913 Ref: nodejs/build#809 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Original commit message: Work around glibc thread-local storage bug glibc before 2.17 has a bug that makes it impossible to execute binaries that have single-byte thread-local variables: % node --version node: error while loading shared libraries: cannot allocate memory in static TLS block Work around that by making the one instance in the V8 code base an int. See: https://sourceware.org/bugzilla/show_bug.cgi?id=14898 See: nodesource/distributions#513 See: nodejs/build#809 Change-Id: Iefd8009100cd93e26cf8dc5dc03f2d622b423385 Reviewed-on: https://chromium-review.googlesource.com/612351 Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-by: Eric Holk <eholk@chromium.org> Cr-Commit-Position: refs/heads/master@{nodejs#47400} PR-URL: nodejs#14913 Ref: nodejs/build#809 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Original commit message: Work around glibc thread-local storage bug glibc before 2.17 has a bug that makes it impossible to execute binaries that have single-byte thread-local variables: % node --version node: error while loading shared libraries: cannot allocate memory in static TLS block Work around that by making the one instance in the V8 code base an int. See: https://sourceware.org/bugzilla/show_bug.cgi?id=14898 See: nodesource/distributions#513 See: nodejs/build#809 Change-Id: Iefd8009100cd93e26cf8dc5dc03f2d622b423385 Reviewed-on: https://chromium-review.googlesource.com/612351 Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-by: Eric Holk <eholk@chromium.org> Cr-Commit-Position: refs/heads/master@{#47400} Backport-PR-URL: #15393 PR-URL: #14913 Ref: nodejs/build#809 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins requested review from bnoordhuis and targos August 18, 2017 11:05

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

bnoordhuis approved these changes Aug 18, 2017

View reviewed changes

targos approved these changes Aug 18, 2017

View reviewed changes

seishun approved these changes Aug 18, 2017

View reviewed changes

cjihrig approved these changes Aug 18, 2017

View reviewed changes

mhdawson approved these changes Aug 18, 2017

View reviewed changes

refack mentioned this pull request Aug 18, 2017

Expand coverage of v8 testing from Node.js tree nodejs/build#387

Closed

jasnell approved these changes Aug 18, 2017

View reviewed changes

bnoordhuis mentioned this pull request Aug 18, 2017

v8: fix compilation on PPC64 when libc musl is used instead of glibc #12081

Closed

jasnell closed this Aug 24, 2017

seishun mentioned this pull request Aug 25, 2017

ansible: use gcc 4.9 on CentOS 6 nodejs/build#809

Merged

MylesBorins mentioned this pull request Sep 10, 2017

v8.5.0 proposal #15308

Merged

MylesBorins added the dont-land-on-v6.x label Sep 20, 2017

MylesBorins mentioned this pull request Oct 3, 2017

v8.7.0 proposal #15762

Merged

MylesBorins deleted the fix-wasm branch November 14, 2017 17:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

deps: cherry-pick e020aae394 from V8 upstream #14913

deps: cherry-pick e020aae394 from V8 upstream #14913

MylesBorins commented Aug 18, 2017

MylesBorins commented Aug 18, 2017

bnoordhuis left a comment

MylesBorins commented Aug 18, 2017 •

edited

Loading

seishun commented Aug 18, 2017

mhdawson left a comment

mhdawson commented Aug 18, 2017

targos commented Aug 18, 2017

refack commented Aug 18, 2017

refack commented Aug 18, 2017

refack commented Aug 18, 2017 •

edited

Loading

matthewloring commented Aug 18, 2017 •

edited

Loading

seishun commented Aug 19, 2017

matthewloring commented Aug 19, 2017

seishun commented Aug 22, 2017 •

edited

Loading

seishun commented Aug 22, 2017

MylesBorins commented Aug 23, 2017 via email

jasnell commented Aug 23, 2017

targos commented Aug 23, 2017

jasnell commented Aug 24, 2017

deps: cherry-pick e020aae394 from V8 upstream #14913

deps: cherry-pick e020aae394 from V8 upstream #14913

Conversation

MylesBorins commented Aug 18, 2017

MylesBorins commented Aug 18, 2017

bnoordhuis left a comment

Choose a reason for hiding this comment

MylesBorins commented Aug 18, 2017 • edited Loading

seishun commented Aug 18, 2017

mhdawson left a comment

Choose a reason for hiding this comment

mhdawson commented Aug 18, 2017

targos commented Aug 18, 2017

refack commented Aug 18, 2017

refack commented Aug 18, 2017

refack commented Aug 18, 2017 • edited Loading

matthewloring commented Aug 18, 2017 • edited Loading

seishun commented Aug 19, 2017

matthewloring commented Aug 19, 2017

seishun commented Aug 22, 2017 • edited Loading

seishun commented Aug 22, 2017

MylesBorins commented Aug 23, 2017 via email

jasnell commented Aug 23, 2017

targos commented Aug 23, 2017

jasnell commented Aug 24, 2017

MylesBorins commented Aug 18, 2017 •

edited

Loading

refack commented Aug 18, 2017 •

edited

Loading

matthewloring commented Aug 18, 2017 •

edited

Loading

seishun commented Aug 22, 2017 •

edited

Loading