★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/27533Nouvelle recherche | Portail wanayoo
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

Jump to bottom

deps: V8: cherry-pick e0a109c #27533

Closed
wants to merge 2 commits into from
Closed

deps: V8: cherry-pick e0a109c #27533

wants to merge 2 commits into from

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented May 2, 2019

Original commit message:

[api] Implement StartupData::CanBeRehashed() for the snapshot blob

This enables the embedder to check if the snapshot generated
from SnapshotCreator::CreateBlob() can be rehashed and the seed
can be recomputed during deserialization.

The lack of this functionality resulted in a temporary vunerability
in Node.js: https://github.com/nodejs/node/pull/27365

Change-Id: I88d52337217c40f79c26438be3c87d2db874d980
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1578661
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61175}

Refs: v8/v8@e0a109c

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

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented May 2, 2019

Lite-CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3425

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented May 2, 2019

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

@joyeecheung
Copy link
Member Author

@joyeecheung joyeecheung commented May 2, 2019

V8-CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/2266/

@joyeecheung joyeecheung requested a review from targos May 2, 2019
targos
targos approved these changes May 2, 2019
View changes
@joyeecheung
Copy link
Member Author

@joyeecheung joyeecheung commented May 2, 2019

hmm, somehow on our branch the snapshots generated in the tests are not rehashable https://ci.nodejs.org/job/node-test-commit-v8-linux/2266/

There may be some other commits that we need to backport together? @hashseed

@joyeecheung
Copy link
Member Author

@joyeecheung joyeecheung commented May 2, 2019 •
edited

Ah, the failures are all Check failed: !blob.CanBeRehashed(), which means the ExtractRehashability() on our branch may be bogus..(because having maps in the snapshot here does fail the pummel/test-hash-seed test)

joyeecheung added 2 commits Jun 5, 2019
@joyeecheung
deps: V8: cherry-pick e0a109c
2702abb 
Original commit message:

    [api] Implement StartupData::CanBeRehashed() for the snapshot blob

    This enables the embedder to check if the snapshot generated
    from SnapshotCreator::CreateBlob() can be rehashed and the seed
    can be recomputed during deserialization.

    The lack of this functionality resulted in a temporary vunerability
    in Node.js: nodejs#27365

    Change-Id: I88d52337217c40f79c26438be3c87d2db874d980
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1578661
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#61175}

Refs: v8/v8@e0a109c
@joyeecheung
fixup! deps: V8: cherry-pick e0a109c
0b06ddf
@joyeecheung
Copy link
Member Author

@joyeecheung joyeecheung commented Jun 6, 2019

OK, looks like somehow the patch was applied incorrectly and the assertions went to the wrong places. I've moved them to be in the correct location.

@targos maybe you want to take another look? (though it's just correcting the patch to be the same as the one in the upstream)

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jun 6, 2019

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

@joyeecheung
Copy link
Member Author

@joyeecheung joyeecheung commented Jun 6, 2019

V8-CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/2361/

refack
refack approved these changes Jun 6, 2019
View changes
targos
targos approved these changes Jun 6, 2019
View changes
Trott
Trott approved these changes Jun 6, 2019
View changes
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Jun 9, 2019

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

@joyeecheung
Copy link
Member Author

@joyeecheung joyeecheung commented Jun 9, 2019

V8-CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/2366

@joyeecheung
Copy link
Member Author

@joyeecheung joyeecheung commented Jun 11, 2019

Landed in d2634be

joyeecheung added a commit that referenced this issue Jun 11, 2019
@joyeecheung
deps: V8: cherry-pick e0a109c
d2634be 
Original commit message:

    [api] Implement StartupData::CanBeRehashed() for the snapshot blob

    This enables the embedder to check if the snapshot generated
    from SnapshotCreator::CreateBlob() can be rehashed and the seed
    can be recomputed during deserialization.

    The lack of this functionality resulted in a temporary vunerability
    in Node.js: #27365

    Change-Id: I88d52337217c40f79c26438be3c87d2db874d980
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1578661
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#61175}

Refs: v8/v8@e0a109c

PR-URL: #27533
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BridgeAR added a commit that referenced this issue Jun 17, 2019
@joyeecheung @BridgeAR
deps: V8: cherry-pick e0a109c
9382b3b 
Original commit message:

    [api] Implement StartupData::CanBeRehashed() for the snapshot blob

    This enables the embedder to check if the snapshot generated
    from SnapshotCreator::CreateBlob() can be rehashed and the seed
    can be recomputed during deserialization.

    The lack of this functionality resulted in a temporary vunerability
    in Node.js: #27365

    Change-Id: I88d52337217c40f79c26438be3c87d2db874d980
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1578661
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#61175}

Refs: v8/v8@e0a109c

PR-URL: #27533
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Jun 17, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@refack refack

@Trott Trott

@targos targos

Assignees
No one assigned
Labels
author ready build v8 engine
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet
5 participants
@joyeecheung @nodejs-github-bot @refack @Trott @targos