-
-
Notifications
You must be signed in to change notification settings - Fork 100
src,test: support V8 6.1 postmortem data #130
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
Conversation
|
Yikes, work in progress. Sorry for premature review! |
|
No worries, but yes, your two comments correspond to a couple of the remaining problems :-) |
|
@indutny thanks for the help on IRC. I was able to get all of the tests passing on V8 6.1. Apparently, the backing stores of typed arrays are not necessarily materialized like they were in older versions of V8. If you update the inspect-scenario to simply access Currently, I'm reporting unmaterialized backing stores as being unmaterialized. I think I can materialize the backing store in llnode, but it will take more work. |
125f7ec to
8a8e0f1
Compare
|
This is ready for review. I've done regression testing locally, and all tests are passing with Node 4.8.4 (latest Argon), 6.11.2 (latest Boron), 8.2.1 (latest Node 8 with Crankshaft). I also tested the V8 6.1 branch, with these changes to the postmortem metadata. |
|
|
|
@indutny I ran |
|
@cjihrig - Would you like me to merge this now or wait until 6.1 is landed in Node.js? (We've a few approved PR's on llnode atm. They do seem to merge cleanly when I test locally.) |
|
@hhellyer it should be fine to land this before 6.1 lands in core. |
Refs: nodejs/llnode#130 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/650746 PR-URL: #14730 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Refs: nodejs/llnode#130 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/650746 PR-URL: nodejs#14730 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Refs: nodejs/llnode#130 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/650746 PR-URL: nodejs#14730 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
See: nodejs/llnode#130 Change-Id: Ibce294f7620cd6ab0db4408a8c2b457c3a5aebcd Reviewed-on: https://chromium-review.googlesource.com/650746 Commit-Queue: Yang Guo <[email protected]> Reviewed-by: Yang Guo <[email protected]> Cr-Commit-Position: refs/heads/master@{#48021}
Refs: nodejs/llnode#130 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/650746 PR-URL: nodejs#14730 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Refs: nodejs/llnode#130 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/650746 Backport-PR-URL: #15393 PR-URL: #14730 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Refs: nodejs/llnode#130 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/650746 Backport-PR-URL: #15393 PR-URL: #14730 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Refs: nodejs/llnode#130 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/650746 Backport-PR-URL: #15393 PR-URL: #14730 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
This is still a work in progress, but adds postmortem debugging support for V8 6.1. The following issues still need to be resolved (all marked with
TODOin the code):Properly handle the end offset of function bodies. It previously needed to be shifted, but that is no longer the case. This just needs version detection logic.Properly load theThinStringTagconstant.V8DBG_THINSTRINGTAGappears to be defined, but the constant isn't loading. Right now, the value of0x5is hard coded.SupportUint8Arrays. Those tests are currently broken and commented out. The problem is that the backing store of theJSArrayBufferView'sJSArrayBufferis coming back as all zeros.UPDATE: The issue with the
Uint8Arrays is that the backing stores are not materialized. If the test is updated to reference the.bufferproperty, the backing store is materialized and shows up properly in llnode.