Skip to content

deps: cherry-pick 46c4979e86 from upstream v8 #18920

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
Closed
bnoordhuis wants to merge 2 commits into from
Closed

deps: cherry-pick 46c4979e86 from upstream v8 #18920

bnoordhuis wants to merge 2 commits into from

Conversation

@bnoordhuis
Copy link
Member

@bnoordhuis bnoordhuis commented Feb 21, 2018

Original commit message:

Use wider types for max_old_space_size and co.

Make --max_old_space_size and friends work with values >= 2**31.
Such values did not work reliably (or sometimes not all) due to
signed integer overflow in size computations, which is UB.

Fixes #18786.

Bug: chromium:814138
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: Ibe23cef2417fd5b4a727022b8b0d4b50f1417182
Reviewed-on: https://chromium-review.googlesource.com/927063
Commit-Queue: Ben Noordhuis <[email protected]>
Reviewed-by: Michael Lippautz <[email protected]>
Cr-Commit-Position: refs/heads/master@{#51433}

Fixes: #18786

The second commit restores ABI for v8::ResourceConstraints. Not strictly needed on master but important for back-porting.

@bnoordhuis
deps: cherry-pick 46c4979e86 from upstream v8
0f20724 
Original commit message:

    Use wider types for max_old_space_size and co.

    Make --max_old_space_size and friends work with values >= 2**31.
    Such values did not work reliably (or sometimes not all) due to
    signed integer overflow in size computations, which is UB.

    Fixes nodejs#18786.

    Bug: chromium:814138
    Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
    Change-Id: Ibe23cef2417fd5b4a727022b8b0d4b50f1417182
    Reviewed-on: https://chromium-review.googlesource.com/927063
    Commit-Queue: Ben Noordhuis <[email protected]>
    Reviewed-by: Michael Lippautz <[email protected]>
    Cr-Commit-Position: refs/heads/master@{nodejs#51433}

Fixes: nodejs#18786
@bnoordhuis
squash! restore abi compat
a68a1db
@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 Feb 21, 2018
@BridgeAR BridgeAR added fast-track PRs that do not need to wait for 48 hours to land. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Feb 22, 2018
@BridgeAR
Copy link
Member

BridgeAR commented Feb 22, 2018

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

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Feb 22, 2018
@bnoordhuis @BridgeAR
deps: cherry-pick 46c4979e86 from upstream v8
13cb056 
Original commit message:

    Use wider types for max_old_space_size and co.

    Make --max_old_space_size and friends work with values >= 2**31.
    Such values did not work reliably (or sometimes not all) due to
    signed integer overflow in size computations, which is UB.

    Fixes nodejs#18786.

    Bug: chromium:814138
    Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
    Change-Id: Ibe23cef2417fd5b4a727022b8b0d4b50f1417182
    Reviewed-on: https://chromium-review.googlesource.com/927063
    Commit-Queue: Ben Noordhuis <[email protected]>
    Reviewed-by: Michael Lippautz <[email protected]>
    Cr-Commit-Position: refs/heads/master@{nodejs#51433}

PR-URL: nodejs#18920
Fixes: nodejs#18786
Reviewed-By: Yang Guo <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@BridgeAR
Copy link
Member

BridgeAR commented Feb 22, 2018

Landed in 13cb056 🎉

@BridgeAR BridgeAR closed this Feb 22, 2018
@targos
Copy link
Member

targos commented Feb 22, 2018

V8 CI to make sure it's not broken: https://ci.nodejs.org/job/node-test-commit-v8-linux/1231/

MylesBorins pushed a commit that referenced this pull request Mar 6, 2018
@bnoordhuis @MylesBorins
deps: cherry-pick 46c4979e86 from upstream v8
502bb86 
Original commit message:

    Use wider types for max_old_space_size and co.

    Make --max_old_space_size and friends work with values >= 2**31.
    Such values did not work reliably (or sometimes not all) due to
    signed integer overflow in size computations, which is UB.

    Fixes #18786.

    Bug: chromium:814138
    Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
    Change-Id: Ibe23cef2417fd5b4a727022b8b0d4b50f1417182
    Reviewed-on: https://chromium-review.googlesource.com/927063
    Commit-Queue: Ben Noordhuis <[email protected]>
    Reviewed-by: Michael Lippautz <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#51433}

PR-URL: #18920
Fixes: #18786
Reviewed-By: Yang Guo <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Mar 6, 2018
Merged
MylesBorins pushed a commit that referenced this pull request Mar 6, 2018
@bnoordhuis @MylesBorins
deps: cherry-pick 46c4979e86 from upstream v8
d3d1719 
Original commit message:

    Use wider types for max_old_space_size and co.

    Make --max_old_space_size and friends work with values >= 2**31.
    Such values did not work reliably (or sometimes not all) due to
    signed integer overflow in size computations, which is UB.

    Fixes #18786.

    Bug: chromium:814138
    Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
    Change-Id: Ibe23cef2417fd5b4a727022b8b0d4b50f1417182
    Reviewed-on: https://chromium-review.googlesource.com/927063
    Commit-Queue: Ben Noordhuis <[email protected]>
    Reviewed-by: Michael Lippautz <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#51433}

PR-URL: #18920
Fixes: #18786
Reviewed-By: Yang Guo <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 6, 2018
@bnoordhuis @MylesBorins
deps: cherry-pick 46c4979e86 from upstream v8
818c2fb 
Original commit message:

    Use wider types for max_old_space_size and co.

    Make --max_old_space_size and friends work with values >= 2**31.
    Such values did not work reliably (or sometimes not all) due to
    signed integer overflow in size computations, which is UB.

    Fixes #18786.

    Bug: chromium:814138
    Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
    Change-Id: Ibe23cef2417fd5b4a727022b8b0d4b50f1417182
    Reviewed-on: https://chromium-review.googlesource.com/927063
    Commit-Queue: Ben Noordhuis <[email protected]>
    Reviewed-by: Michael Lippautz <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#51433}

PR-URL: #18920
Fixes: #18786
Reviewed-By: Yang Guo <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 6, 2018
@bnoordhuis @MylesBorins
deps: cherry-pick 46c4979e86 from upstream v8
819ff1a 
Original commit message:

    Use wider types for max_old_space_size and co.

    Make --max_old_space_size and friends work with values >= 2**31.
    Such values did not work reliably (or sometimes not all) due to
    signed integer overflow in size computations, which is UB.

    Fixes #18786.

    Bug: chromium:814138
    Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
    Change-Id: Ibe23cef2417fd5b4a727022b8b0d4b50f1417182
    Reviewed-on: https://chromium-review.googlesource.com/927063
    Commit-Queue: Ben Noordhuis <[email protected]>
    Reviewed-by: Michael Lippautz <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#51433}

PR-URL: #18920
Fixes: #18786
Reviewed-By: Yang Guo <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@MylesBorins
Copy link
Contributor

MylesBorins commented Mar 7, 2018

We ended up having to back this out of 9.x as it was breaking the V8 CI

Looks like the error is:

20:38:11 ../src/api.cc:892:26: error: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'int' [-Werror,-Wshorten-64-to-32]
20:38:11   set_max_old_space_size(i::Heap::ComputeMaxOldGenerationSize(physical_memory));
20:38:11   ~~~~~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
failing on Ubuntu 16.04, so should be easy to reproduce.

Would someone be willing to backport?

MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
@bnoordhuis @MayaLekova
deps: cherry-pick 46c4979e86 from upstream v8
b3f4145 
Original commit message:

    Use wider types for max_old_space_size and co.

    Make --max_old_space_size and friends work with values >= 2**31.
    Such values did not work reliably (or sometimes not all) due to
    signed integer overflow in size computations, which is UB.

    Fixes nodejs#18786.

    Bug: chromium:814138
    Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
    Change-Id: Ibe23cef2417fd5b4a727022b8b0d4b50f1417182
    Reviewed-on: https://chromium-review.googlesource.com/927063
    Commit-Queue: Ben Noordhuis <[email protected]>
    Reviewed-by: Michael Lippautz <[email protected]>
    Cr-Commit-Position: refs/heads/master@{nodejs#51433}

PR-URL: nodejs#18920
Fixes: nodejs#18786
Reviewed-By: Yang Guo <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@MylesBorins
Copy link
Contributor

MylesBorins commented Aug 7, 2018

Should this be backported to v8.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BridgeAR BridgeAR BridgeAR approved these changes

+1 more reviewer

@hashseed hashseed hashseed approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. fast-track PRs that do not need to wait for 48 hours to land. 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.

6 participants

@bnoordhuis @BridgeAR @targos @MylesBorins @hashseed @nodejs-github-bot