-
Notifications
You must be signed in to change notification settings - Fork 100
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
support thin strings #117
Comments
BTW I am working on this, still trying to figure out why |
@joyeecheung Does this (node) patch help? diff --git a/deps/v8/tools/gen-postmortem-metadata.py b/deps/v8/tools/gen-postmortem-metadata.py
index e793a91865..9e0b7c8a0c 100644
--- a/deps/v8/tools/gen-postmortem-metadata.py
+++ b/deps/v8/tools/gen-postmortem-metadata.py
@@ -72,6 +72,7 @@ consts_misc = [
{ 'name': 'ConsStringTag', 'value': 'kConsStringTag' },
{ 'name': 'ExternalStringTag', 'value': 'kExternalStringTag' },
{ 'name': 'SlicedStringTag', 'value': 'kSlicedStringTag' },
+ { 'name': 'ThinStringTag', 'value': 'kThinStringTag' },
{ 'name': 'HeapObjectTag', 'value': 'kHeapObjectTag' },
{ 'name': 'HeapObjectTagMask', 'value': 'kHeapObjectTagMask' }, |
@bnoordhuis Yes, that's what I need, thanks! I've got the thin string support working but then discovered the frame test was failing because Turbofan now puts stub frames on the stack in that scenario. I will open a PR fixing these two issues. |
@bnoordhuis BTW have you opened a CL to the V8 upstream yet? |
See: nodejs/llnode#117 Change-Id: Icc2830c8e9096610df33ffdc2f89e74cb1b35662 Reviewed-on: https://chromium-review.googlesource.com/618986 Reviewed-by: Michael Achenbach <machenbach@chromium.org> Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Cr-Commit-Position: refs/heads/master@{#47778}
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 <machenbach@chromium.org> Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> 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 <machenbach@chromium.org> Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Cr-Commit-Position: refs/heads/master@{#47778} PR-URL: #15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
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 <machenbach@chromium.org> Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Cr-Commit-Position: refs/heads/master@{#47778} PR-URL: #15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
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 <machenbach@chromium.org> Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Cr-Commit-Position: refs/heads/master@{#47778} PR-URL: #15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
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 <machenbach@chromium.org> Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Cr-Commit-Position: refs/heads/master@{#47778} PR-URL: #15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
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 <machenbach@chromium.org> Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Cr-Commit-Position: refs/heads/master@{nodejs#47778} PR-URL: nodejs#15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
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 <machenbach@chromium.org> Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Cr-Commit-Position: refs/heads/master@{#47778} PR-URL: #15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
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 <machenbach@chromium.org> Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Cr-Commit-Position: refs/heads/master@{nodejs#47778} PR-URL: nodejs#15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
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 <machenbach@chromium.org> Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Cr-Commit-Position: refs/heads/master@{nodejs#47778} PR-URL: nodejs#15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
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 <machenbach@chromium.org> Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Cr-Commit-Position: refs/heads/master@{nodejs#47778} PR-URL: nodejs#15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
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 <machenbach@chromium.org> Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> Cr-Commit-Position: refs/heads/master@{nodejs#47778} PR-URL: nodejs#15184 Ref: nodejs/llnode#117 Ref: nodejs/llnode#121 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
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 <machenbach@chromium.org> Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> 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 <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
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 <machenbach@chromium.org> Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> 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 <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
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 <machenbach@chromium.org> Commit-Queue: Ben Noordhuis <info@bnoordhuis.nl> 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 <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
Looks like in Node.js v8.3.0 (with v8 6.0) thin strings are enabled by default, since they are not recognized by llnode, when inspecting those strings the output is broken. e.g. when viewing the backtrace of
test/inspect-scenario.js
, script names that are thin strings will be displayed as "no script" (since llnode cannot doString::ToString()
on them) and frames with functions containing thin string arguments would be empty:Before (output in v8.2.1)
The text was updated successfully, but these errors were encountered: