deps: backport 2bd7464 from upstream V8#10169
Closed
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original commit message: For global object property cells, we did not check that the map on the previous object is still the same for which we actually optimized. So the optimized code was not in sync with the actual state of the property cell. When loading from such a global object property cell, Crankshaft optimizes away any map checks (based on the stable map assumption), leading to arbitrary memory access in the worst case. TurboFan has the same bug for stores, but is safe on loads because we do appropriate map checks there. However mixing TurboFan and Crankshaft still exposes the bug. [email protected] BUG=chromium:659475 Review-Url: https://codereview.chromium.org/2444233004 Cr-Commit-Position: refs/heads/master@{nodejs#40592}
nodejs-github-bot added v6.x v8 engine 
Author
cristiancavalli commented Dec 7, 2016
cc @ofrobots |
Contributor
ofrobots commented Dec 7, 2016
/cc @nodejs/v8 |
Member
V8 test CI run: https://ci.nodejs.org/job/node-test-commit-v8-linux/457/ |
bnoordhuis approved these changes Dec 7, 2016
Contributor
ofrobots commented Dec 8, 2016
Member
mhdawson commented Dec 8, 2016
New CI run as seems to have failed a good part of the arm run : https://ci.nodejs.org/job/node-test-pull-request/5312/ |
Contributor
ofrobots commented Dec 13, 2016
The arm-fanned issues in the latest CI seem unrelated infrastructure issues. Rest is green. |
ofrobots approved these changes Dec 13, 2016
ofrobots pushed a commit that referenced this pull request Dec 13, 2016
Original commit message: For global object property cells, we did not check that the map on the previous object is still the same for which we actually optimized. So the optimized code was not in sync with the actual state of the property cell. When loading from such a global object property cell, Crankshaft optimizes away any map checks (based on the stable map assumption), leading to arbitrary memory access in the worst case. TurboFan has the same bug for stores, but is safe on loads because we do appropriate map checks there. However mixing TurboFan and Crankshaft still exposes the bug. [email protected] BUG=chromium:659475 Review-Url: https://codereview.chromium.org/2444233004 Cr-Commit-Position: refs/heads/master@{#40592} PR-URL: #10169 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]> Reviewed-By: ofrobots - Ali Ijaz Sheikh <[email protected]>
Contributor
ofrobots commented Dec 13, 2016
Landed on @cristiancavalli This doesn't cleanly apply to |
Contributor
edit: I misread the above comment. While the patch lands cleanly there is a missing symbol this is no longer on |
Author
cristiancavalli commented Dec 13, 2016
@ofrobots will do |
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
Original commit message: For global object property cells, we did not check that the map on the previous object is still the same for which we actually optimized. So the optimized code was not in sync with the actual state of the property cell. When loading from such a global object property cell, Crankshaft optimizes away any map checks (based on the stable map assumption), leading to arbitrary memory access in the worst case. TurboFan has the same bug for stores, but is safe on loads because we do appropriate map checks there. However mixing TurboFan and Crankshaft still exposes the bug. [email protected] BUG=chromium:659475 Review-Url: https://codereview.chromium.org/2444233004 Cr-Commit-Position: refs/heads/master@{#40592} PR-URL: #10169 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]> Reviewed-By: ofrobots - Ali Ijaz Sheikh <[email protected]>
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
make -j8 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
deps
Description of change
Backport of bugfix from upstream V8
Original commit message:
For global object property cells, we did not check that the map on the
previous object is still the same for which we actually optimized. So
the optimized code was not in sync with the actual state of the property
cell. When loading from such a global object property cell, Crankshaft
optimizes away any map checks (based on the stable map assumption),
leading to arbitrary memory access in the worst case.
TurboFan has the same bug for stores, but is safe on loads because we
do appropriate map checks there. However mixing TurboFan and Crankshaft
still exposes the bug.
R=[email protected]
BUG=chromium:659475
Review-Url: https://codereview.chromium.org/2444233004
Cr-Commit-Position: refs/heads/master@{#40592}
V8 commit: v8/v8@2bd7464