Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 101
src: support thin strings and stub frames#121
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
joyeecheung commented Aug 15, 2017 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
joyeecheung commented Aug 16, 2017 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@indutny Doing I have a (very old) chromium checkout on my machine but setting |
joyeecheung commented Aug 16, 2017 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
OK, turns out silence was what it was supposed to be, the file would be formatted directly by the script. o.0 I've fixed the style, PTAL @indutny@bnoordhuis |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
bnoordhuis commented Aug 17, 2017
Still LGTM. I'm trying to sort out https://ci.nodejs.org/view/post-mortem/job/llnode-pipeline/ but you can land this if you want, @joyeecheung. |
bnoordhuis commented Aug 17, 2017
For good measure: https://ci.nodejs.org/view/post-mortem/job/llnode-pipeline/13/ |
Original commit message: Add postmortem metadata for thin strings. See: nodejs/llnode#117 Change-Id: Icc2830c8e9096610df33ffdc2f89e74cb1b35662 Reviewed-on: https://chromium-review.googlesource.com/618986 Reviewed-by: Michael Achenbach <[email protected]> Commit-Queue: Ben Noordhuis <[email protected]> Cr-Commit-Position: refs/heads/master@{nodejs#47778} Refs: nodejs/llnode#117 Refs: nodejs/llnode#121
Original commit message: Add postmortem metadata for thin strings. See: nodejs/llnode#117 Change-Id: Icc2830c8e9096610df33ffdc2f89e74cb1b35662 Reviewed-on: https://chromium-review.googlesource.com/618986 Reviewed-by: Michael Achenbach <[email protected]> Commit-Queue: Ben Noordhuis <[email protected]> Cr-Commit-Position: refs/heads/master@{#47778} PR-URL: #15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message: Add postmortem metadata for thin strings. See: nodejs/llnode#117 Change-Id: Icc2830c8e9096610df33ffdc2f89e74cb1b35662 Reviewed-on: https://chromium-review.googlesource.com/618986 Reviewed-by: Michael Achenbach <[email protected]> Commit-Queue: Ben Noordhuis <[email protected]> Cr-Commit-Position: refs/heads/master@{#47778} PR-URL: #15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message: Add postmortem metadata for thin strings. See: nodejs/llnode#117 Change-Id: Icc2830c8e9096610df33ffdc2f89e74cb1b35662 Reviewed-on: https://chromium-review.googlesource.com/618986 Reviewed-by: Michael Achenbach <[email protected]> Commit-Queue: Ben Noordhuis <[email protected]> Cr-Commit-Position: refs/heads/master@{#47778} PR-URL: #15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message: Add postmortem metadata for thin strings. See: nodejs/llnode#117 Change-Id: Icc2830c8e9096610df33ffdc2f89e74cb1b35662 Reviewed-on: https://chromium-review.googlesource.com/618986 Reviewed-by: Michael Achenbach <[email protected]> Commit-Queue: Ben Noordhuis <[email protected]> Cr-Commit-Position: refs/heads/master@{#47778} PR-URL: #15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message: Add postmortem metadata for thin strings. See: nodejs/llnode#117 Change-Id: Icc2830c8e9096610df33ffdc2f89e74cb1b35662 Reviewed-on: https://chromium-review.googlesource.com/618986 Reviewed-by: Michael Achenbach <[email protected]> Commit-Queue: Ben Noordhuis <[email protected]> Cr-Commit-Position: refs/heads/master@{nodejs#47778} PR-URL: nodejs#15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message: Add postmortem metadata for thin strings. See: nodejs/llnode#117 Change-Id: Icc2830c8e9096610df33ffdc2f89e74cb1b35662 Reviewed-on: https://chromium-review.googlesource.com/618986 Reviewed-by: Michael Achenbach <[email protected]> Commit-Queue: Ben Noordhuis <[email protected]> Cr-Commit-Position: refs/heads/master@{#47778} PR-URL: #15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message: Add postmortem metadata for thin strings. See: nodejs/llnode#117 Change-Id: Icc2830c8e9096610df33ffdc2f89e74cb1b35662 Reviewed-on: https://chromium-review.googlesource.com/618986 Reviewed-by: Michael Achenbach <[email protected]> Commit-Queue: Ben Noordhuis <[email protected]> Cr-Commit-Position: refs/heads/master@{nodejs#47778} PR-URL: nodejs#15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message: Add postmortem metadata for thin strings. See: nodejs/llnode#117 Change-Id: Icc2830c8e9096610df33ffdc2f89e74cb1b35662 Reviewed-on: https://chromium-review.googlesource.com/618986 Reviewed-by: Michael Achenbach <[email protected]> Commit-Queue: Ben Noordhuis <[email protected]> Cr-Commit-Position: refs/heads/master@{nodejs#47778} PR-URL: nodejs#15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message: Add postmortem metadata for thin strings. See: nodejs/llnode#117 Change-Id: Icc2830c8e9096610df33ffdc2f89e74cb1b35662 Reviewed-on: https://chromium-review.googlesource.com/618986 Reviewed-by: Michael Achenbach <[email protected]> Commit-Queue: Ben Noordhuis <[email protected]> Cr-Commit-Position: refs/heads/master@{nodejs#47778} PR-URL: nodejs#15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message: Add postmortem metadata for thin strings. See: nodejs/llnode#117 Change-Id: Icc2830c8e9096610df33ffdc2f89e74cb1b35662 Reviewed-on: https://chromium-review.googlesource.com/618986 Reviewed-by: Michael Achenbach <[email protected]> Commit-Queue: Ben Noordhuis <[email protected]> Cr-Commit-Position: refs/heads/master@{nodejs#47778} PR-URL: nodejs#15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message: Add postmortem metadata for thin strings. See: nodejs/llnode#117 Change-Id: Icc2830c8e9096610df33ffdc2f89e74cb1b35662 Reviewed-on: https://chromium-review.googlesource.com/618986 Reviewed-by: Michael Achenbach <[email protected]> Commit-Queue: Ben Noordhuis <[email protected]> Cr-Commit-Position: refs/heads/master@{#47778} Backport-PR-URL: #15393 PR-URL: #15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message: Add postmortem metadata for thin strings. See: nodejs/llnode#117 Change-Id: Icc2830c8e9096610df33ffdc2f89e74cb1b35662 Reviewed-on: https://chromium-review.googlesource.com/618986 Reviewed-by: Michael Achenbach <[email protected]> Commit-Queue: Ben Noordhuis <[email protected]> Cr-Commit-Position: refs/heads/master@{#47778} Backport-PR-URL: #15393 PR-URL: #15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message: Add postmortem metadata for thin strings. See: nodejs/llnode#117 Change-Id: Icc2830c8e9096610df33ffdc2f89e74cb1b35662 Reviewed-on: https://chromium-review.googlesource.com/618986 Reviewed-by: Michael Achenbach <[email protected]> Commit-Queue: Ben Noordhuis <[email protected]> Cr-Commit-Position: refs/heads/master@{#47778} Backport-PR-URL: #15393 PR-URL: #15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Fixes: #117
In Node.js v8.3.0 Turbofan has been enabled by default, so thin strings have also been enabled and now Turbofan would put stub frames on the stack (to store register states for bytecode handlers, I think).
Before this change, when viewing the backtrace of
test/fixtures/inspect-scenario.jswith Node.js v8.3.0, frames containing functions with thin string arguments or in thin string paths would be broken:See backtrace of inspect-scenario.js
After (with patch in #117 (comment)):
See backtrace of inspect-scenario.js
Also display stub frames as
<stub>to fixframe-test.js.This would need a patch in v8 to get the
kThinStringTagconstant: #117 (comment)Tested with v4.8.4, v6.11.1, v8.2.1 and v8.3.0(with patch from #117 (comment))