Cherry pick 588e15c and c0d4bb8 from upstream V8 #8038
Conversation
nodejs-github-bot added the v8 engine 
ofrobots changed the title 
| @cristiancavalli can you bump the patch level in https://github.com/nodejs/node/blob/master/deps/v8/include/v8-version.h |
| /cc @nodejs/v8 as the original didn't seem to get linkified. |
| @thealphanerd Bumped! |
| LGTM but seeing how this only affects the disassembler it's not a very serious bug, is it? Aside: the commit log for the second commit is a bit confusing in that it back-ports the test wholesale instead of just the fixes (which the commit log implies.) No action required, merely mentioning it for posterity. |
Actually this is a crash bug in the x32 assembler. See chromium issue: https://bugs.chromium.org/p/chromium/issues/detail?id=621926. This was identified as missing from node as we were going through bugs that were deemed important enough for Chromium to backport, but Node.js |
| This PR only contains changes to disasm-ia32.cc, it looks like it's missing the changes to assembler-ia32.cc from v8/v8@588e15c. |
Original commit message: Fixes a bug in cmpw. The opcodes for 'cmpw r/m16, r16' and 'cmpw r16, r/m16' were swapped, causing a few issues when less than/greater than comparison were performed. Adds a regression test. BUG=621926 Committed: https://crrev.com/efa7095e3e360fbadbe909d831ac11b268ca26b0 Review-Url: https://codereview.chromium.org/2103713003 Cr-Original-Commit-Position: refs/heads/master@{nodejs#37339} Cr-Commit-Position: refs/heads/master@{nodejs#37345}
Original commit message: Fixes a wrong use of Operand in a test. Operand(reg) -> reg Operand(reg, 0) -> [reg] BUG= Review-Url: https://codereview.chromium.org/2111503002 Cr-Commit-Position: refs/heads/master@{nodejs#37370}
cristiancavalli force-pushed the cherry-pick-14205ab branch from 767afc7 to 8c09bb9 Compare | @bnoordhuis Thanks for the spot! PR is updated accordingly. |
| LGTM. @fhinkel Can you also run the V8 test suite? |
| LGTM, to be squashed at landing time. V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/264/ |
| LGTM |
| Landed as 4dee6bd. |
Pick up an upstream bugfix for https://crbug.com/621926 and bump V8 version to 5.1.281.80. Original commit message for 588e15c: Fixes a bug in cmpw. The opcodes for 'cmpw r/m16, r16' and 'cmpw r16, r/m16' were swapped, causing a few issues when less than/greater than comparison were performed. Adds a regression test. BUG=621926 Committed: https://crrev.com/efa7095e3e360fbadbe909d831ac11b268ca26b0 Review-Url: https://codereview.chromium.org/2103713003 Cr-Original-Commit-Position: refs/heads/master@{#37339} Cr-Commit-Position: refs/heads/master@{#37345} Original commit message for c0d4bb8: Fixes a wrong use of Operand in a test. Operand(reg) -> reg Operand(reg, 0) -> [reg] BUG= Review-Url: https://codereview.chromium.org/2111503002 Cr-Commit-Position: refs/heads/master@{#37370} PR-URL: #8038 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com> Reviewed-By: mhdawson - Michael Dawson <michael_dawson@ca.ibm.com>
| Had to force-push to master, the commit SHA changed to 51d45db, sorry for the trouble. |
| This should not be landed on v6.x, correct? |
Pick up an upstream bugfix for https://crbug.com/621926 and bump V8 version to 5.1.281.80. Original commit message for 588e15c: Fixes a bug in cmpw. The opcodes for 'cmpw r/m16, r16' and 'cmpw r16, r/m16' were swapped, causing a few issues when less than/greater than comparison were performed. Adds a regression test. BUG=621926 Committed: https://crrev.com/efa7095e3e360fbadbe909d831ac11b268ca26b0 Review-Url: https://codereview.chromium.org/2103713003 Cr-Original-Commit-Position: refs/heads/master@{nodejs#37339} Cr-Commit-Position: refs/heads/master@{nodejs#37345} Original commit message for c0d4bb8: Fixes a wrong use of Operand in a test. Operand(reg) -> reg Operand(reg, 0) -> [reg] BUG= Review-Url: https://codereview.chromium.org/2111503002 Cr-Commit-Position: refs/heads/master@{nodejs#37370} PR-URL: nodejs#8038 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com> Reviewed-By: mhdawson - Michael Dawson <michael_dawson@ca.ibm.com>
9 participants
Checklist
make -j4 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
deps
Description of change
Backport of bugfix from upstream V8.
This is already fixed in versions 5.2 and 5.3 of V8 but is needed for 5.1.
cc @nodejs/v8