Backport V8 5.1 to v6.x #8054
Merged
Backport V8 5.1 to v6.x #8054
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
nodejs-github-bot added the v8 engine 2 tasks
ofrobots added the semver-minor 
Member
| Rubber-stamp LGTM for the V8 update, thanks for putting this together! |
Contributor
| citgm-abi-smoker: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-abi-smoker/12/ fingers crossed 😄 |
Contributor
| And the results are in: https://ci.nodejs.org/job/citgm-abi-smoker/ |
Member
| I don’t know if |
3 tasks
Contributor Author
| Added some V8 5.1 backport commits that have landed on @thealphanerd could you take a quick look at the smoker results to see which failures are real / worth investigating? |
Contributor
| I am going to dig into all of these results today. Re running with the new commits, and also running on v6.x so we can compare results v6.x: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/347/ Both of these jobs are being run using the new junit reporter, which should make analyzing the output a nicer experience! |
2 tasks
Contributor
| @ofrobots it looks like this needs a rebase. |
ofrobots force-pushed the v6.x-switch-to-V8-5.1 branch from 67bed8e to 53da7cd Compare Contributor Author
| Rebased + added backport from #8099. |
2 tasks
Contributor
| Howdy all, I've got an update. The ABI smoker job is mostly working... I'm banging my head against a system failure on OSX, but this is CI issue, not related to ABI. --> https://ci.nodejs.org/view/Node.js-citgm/job/citgm-abi-smoker/ edit: I was running the smoker job against the wrong $HEAD originally. citgm ABI: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-abi-smoker/23/ edit 2: citgm abi is looking good, only expected failures. Running just the tests on that --> https://ci.nodejs.org/view/Node.js-citgm/job/citgm-abi-smoker/25/ edit 3: Look like everything passed on the run of only |
Contributor
| Smoke testing implies that there are no ABI breakages in this PR. LGTM |
Contributor Author
| @thealphanerd Thanks for all the work in getting this verified w/ citgm smoke testing! @nodejs/ctc Can I get some more LGTMs for landing this on |
Member
| Based on the Smoke testing LGTM. |
Contributor
| rubberstamp LGTM |
Member
| Also rubber-stamp LGTM.
For posterity, this looks to have been a clean cherry-pick. Only the commit hash is different. |
Contributor Author
| Thanks. I will land this today. |
ofrobots force-pushed the v6.x-switch-to-V8-5.1 branch from 53da7cd to c8981e8 Compare 
Pick up the latest branch-head for V8 5.1. This branch brings in improved language support and performance improvements. For full details: http://v8project.blogspot.com/2016/04/v8-release-51.html * Picks up the latest branch head for 5.1 [1] * Edit v8 gitignore to allow trace_event copy * Update V8 DEP trace_event as per deps/v8/DEPS [2] [1] https://chromium.googlesource.com/v8/v8.git/+/5.1.281.75 [2] https://chromium.googlesource.com/chromium/src/base/trace_event/common/+/c8c8665 Introduces a semver-minor overload of v8::Function::New() for use by v8_inspector. PR-URL: nodejs#8054 Refs: nodejs#7016 Refs: nodejs#7586 Refs: nodejs#7615 Reviewed-By: addaleax - Anna Henningsen <anna@addaleax.net> Reviewed-By: thealphanerd - Myles Borins <myles.borins@gmail.com> Reviewed-By: mhdawson - Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: evanlucas - Evan Lucas <evanlucas@me.com> Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
ofrobots force-pushed the v6.x-switch-to-V8-5.1 branch from c8981e8 to 723fa96 Compare V8 erroneously did null pointer checks on `this`. It can lead to a SIGSEGV crash if node is compiled with GCC 6. Backport relevant changes from [1] that fix this issue. [1]: https://codereview.chromium.org/1900423002 Fixes: nodejs#6272 PR-URL: nodejs#6544 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
V8 improved the error message for illegal token in v8/v8@879b617b. This breaks the recoverable error handling in repl. Ref: nodejs#6482 PR-URL: nodejs#7016 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
* Changes to messages. * V8 enabled proxy support by default. The --harmony_proxies flag is now gone. PR-URL: nodejs#6482 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
V8 5.1 changes the layout of stack frames. PR-URL: nodejs#6482 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
It is no longer necessary now that libplatform/libplatform.h uses proper includes. PR-URL: nodejs#7016 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The removal of the promise debug event is an API/ABI breaking change. Ref: https://codereview.chromium.org/1833563002 Ref: #23 PR-URL: nodejs#7016 Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Ref: #23 PR-URL: nodejs#7016 Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Remove the `_malloced_memory` field from the `HeapStatistics` class to achieve full ABI compatibility with V8 5.0. Ref: nodejs#7016 PR-URL: nodejs#7526 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Original commit message: Quit creating array literal boilerplates from Crankshaft. It's such a corner case. BUG= Review URL: https://codereview.chromium.org/1865013002 Cr-Commit-Position: refs/heads/master@{nodejs#35346} Fixes: nodejs#7454 PR-URL: nodejs#7632 Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Original commit message: Handle symbols in FrameMirror#invocationText(). Fix a TypeError when putting together the invocationText for a symbol method's stack frame. See nodejs#7536. Review-Url: https://codereview.chromium.org/2122793003 Cr-Commit-Position: refs/heads/master@{nodejs#37597} Fixes: nodejs#7536 PR-URL: nodejs#7612 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Original commit message: S390:Update inline asm constraint in test-platform The GetStackPointer() routine in test-platform uses an inline assembly code to store the current stack pointer value into a static variable sp_addr. The existing asm code for S390 uses an ST/STG instruction, with the memory operand associated with the general ('=g') constraint to sp_addr. On GCC 4.8.5, the GCC compiler got confused and treated sp_addr as an integer operand instead of memory operand, resulting in a store being emitted that writes to an invalid meory location. Given the specific store instructions being inlined here, we should restict the sp_addr operand to explicitly be a memory operand using '=m' instead of '=g'. R=bmeurer@chromium.org,jkummerow@chormium.org,rmcilroy@chromium.org,yangguo@chromium.org BUG= Review-Url: https://codereview.chromium.org/2158523002 Cr-Commit-Position: refs/heads/master@{nodejs#37809} Fixes: nodejs#7659 PR-URL: nodejs#7771 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com> Original commit message: If we can't rehash the backing store for weak sets & maps, do a last resort GC BUG=v8:4909 R=hpayer@chromium.org Committed: https://crrev.com/b93c80a6039c757723e70420ae73375b5d277814 Cr-Commit-Position: refs/heads/master@{#37591} Fixes: nodejs#6180 PR-URL: nodejs#7689 Reviewed-By: Matt Loring <mattloring@google.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Original commit message: Fix incorrect parameter to HasSufficientCapacity It takes the number of additional elements, not the total target capacity. Also, avoid right-shifting a negative integer as this is undefined in general BUG=v8:4909 R=verwaest@chromium.org Review-Url: https://codereview.chromium.org/2162333002 Cr-Commit-Position: refs/heads/master@{nodejs#37901} Fixes: nodejs#6180 PR-URL: nodejs#7689 Reviewed-By: Matt Loring <mattloring@google.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Original commit message: [regexp] Fix case-insensitive matching for one-byte subjects. The bug occurs because we do not canonicalize character class ranges before adding case equivalents. While adding case equivalents, we abort early for one-byte subject strings, assuming that the ranges are sorted. Which they are not. R=marja@chromium.org BUG=v8:5199 Review-Url: https://codereview.chromium.org/2159683002 Cr-Commit-Position: refs/heads/master@{nodejs#37833} Fixes: nodejs#7708 PR-URL: nodejs#7833 Reviewed-By: targos - Michaël Zasso <mic.besace@gmail.com> Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com>
Original commit message: [build] Add force_dynamic_crt option to build a static library with /… …MD on windows Adds option to build a V8 library statically, but with the options on windows that allows it to be subsequently included in another DLL. On Windows this is required for it to correclty link against the correct C++ runtime. Require for our Node.js shared library build. Reference: nodejs#7487 BUG= R=machenbach@chromium.org, michael_dawson@ca.ibm.com Committed: https://crrev.com/9cf88c1c364cf76c1e745aa63196768435e8ef5d Review-Url: https://codereview.chromium.org/2149963002 Cr-Original-Commit-Position: refs/heads/master@{nodejs#37814} Cr-Commit-Position: refs/heads/master@{nodejs#37856} PR-URL: nodejs#7802 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
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>
Original commit message: [Debugger] Fix StepNext over function with caught exception Without CL debugger on StepNext adds breakpoint to function where throw instruction is located. In case of StepNext we will skip pause in this function because StepNext shouldn't break in a deeper frame. BUG=chromium:604495 R=yangguo@chromium.org LOG=N Review URL: https://codereview.chromium.org/1894263002 Cr-Commit-Position: refs/heads/master@{nodejs#35627} Fixes: nodejs#7219 PR-URL: nodejs#8099 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Contributor Author
| Landed as 92ecbc4...723fa96. |
evanlucas added a commit that referenced this pull request Aug 26, 2016
Notable changes: * **buffer**: Fix regression introduced in v6.4.0 that prevented .write() at buffer end (Anna Henningsen) #8154 * **deps**: update V8 to 5.1.281.75 (Ali Ijaz Sheikh) #8054 * **inspector**: * fix inspector hang while disconnecting (Aleksei Koziatinskii) #8021 * add support for uncaught exception (Aleksei Koziatinskii) #8043 * **repl**: Fix saving editor mode text in `.save` (Prince J Wesley) #8145 * ***Revert*** "**repl,util**: insert carriage returns in output" (Evan Lucas) #8143 PR-URL: #8253
evanlucas added a commit that referenced this pull request Aug 26, 2016
Notable changes: * **buffer**: Fix regression introduced in v6.4.0 that prevented .write() at buffer end (Anna Henningsen) #8154 * **deps**: update V8 to 5.1.281.75 (Ali Ijaz Sheikh) #8054 * **inspector**: * fix inspector hang while disconnecting (Aleksei Koziatinskii) #8021 * add support for uncaught exception (Aleksei Koziatinskii) #8043 * **repl**: Fix saving editor mode text in `.save` (Prince J Wesley) #8145 * ***Revert*** "**repl,util**: insert carriage returns in output" (Evan Lucas) #8143 PR-URL: #8253
evanlucas added a commit that referenced this pull request Aug 26, 2016
Notable changes: * **buffer**: Fix regression introduced in v6.4.0 that prevented .write() at buffer end (Anna Henningsen) #8154 * **deps**: update V8 to 5.1.281.75 (Ali Ijaz Sheikh) #8054 * **inspector**: * fix inspector hang while disconnecting (Aleksei Koziatinskii) #8021 * add support for uncaught exception (Aleksei Koziatinskii) #8043 * **repl**: Fix saving editor mode text in `.save` (Prince J Wesley) #8145 * ***Revert*** "**repl,util**: insert carriage returns in output" (Evan Lucas) #8143 PR-URL: #8253
Contributor
| @nodejs/v8 Did this introduce any new compiler requirements? Our builds are failing on debian wheezy using |
Contributor
| Retroactively marking as |
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
14 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 -j4 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
deps, test, build, dtrace, repl
Description of change
This PR backports all the necessary commits for upgrading
v6.xto V8 5.1 as requested here: #7016 (comment). This PR is expected to be ABI compatible with V8 5.0.There were several commits on
masterthat incrementally upgraded to 5.1.281.75 that have been collapsed into a single commit updating directly to 5.1.281.75 (this commit needs a rubber stamp review)._NOTE:_ Omit
--whitespace=fixat landing time. There is significant whitespace in some of the V8 tests included in this PR./cc @thealphanerd @nodejs/v8