Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2758373002:
[tools] add lldbinit (Closed)
Description[tools] add lldbinit
The goal of this commit is to add the equivalent to gdbinit but
for lldb. I've tried to replicate the commands as close as possible
but I'm unsure about the jss command and hoping to get some feedback
on it in addition to the bta command which I'm not sure how/when this
could be used. This is probably just inexperience on my part.
The lldbinit file can be placed into a directory prefixed with dot
(.lldbinit) and the python script is currently expected to be in the
same directory. The path to the script can be changed manually if needed
as well.
NOTRY=true
Review-Url: https://codereview.chromium.org/2758373002
Cr-Commit-Position: refs/heads/master@{#44136}
Committed: https://chromium.googlesource.com/v8/v8/+/ec1ffe39ed9313d309e9c1a3235ff6813ae3ccbb
Patch Set 1 #
Total comments: 2
Patch Set 2 : rename TypeFeedbackMetadata -> FeedbackMetadata #
Total comments: 2
Patch Set 3 : *args and line limit #Messages
Total messages: 24 (10 generated)
daniel.bevenius@gmail.com changed reviewers: + machenbach@chromium.org
Would you mind giving this a review and see what you think? I've added a comment in the commit message about the bta and jss commands which I'm not really sure what the expected behaviour is for them. If you know of any place that I can set a break point to see how they work in gdb I'd be able to compare them with the lldb versions. Thanks
machenbach@chromium.org changed reviewers: + ishell@chromium.org, yangguo@chromium.org
+ more owners of gdb
https://codereview.chromium.org/2758373002/diff/1/tools/lldbinit File tools/lldbinit (right): https://codereview.chromium.org/2758373002/diff/1/tools/lldbinit#newcode17 tools/lldbinit:17: # Print TypeFeedbackMetadata s/TypeFeedbackMetadata/FeedbackMetadata/ https://codereview.chromium.org/2758373002/diff/1/tools/lldbinit#newcode18 tools/lldbinit:18: command regex -h 'Print a v8 TypeFeedbackMetadata object' jfm 's/(.+)/expr -- '_v8_internal_Print_TypeFeedbackMetadata((void*)(%1))/' Same here.
Thanks for the review! This patch set contains the changes requested by Igor Sheludko,
lgtm
lgtm rubber-stamp https://codereview.chromium.org/2758373002/diff/20001/tools/lldb_commands.py File tools/lldb_commands.py (right): https://codereview.chromium.org/2758373002/diff/20001/tools/lldb_commands.py#... tools/lldb_commands.py:8: def jst(debugger, command, result, dict): nit: since command, result, dict are always unused, maybe just use *args and maybe **kwargs, does that work? Also the var name dict is masking the built-in dict. https://codereview.chromium.org/2758373002/diff/20001/tools/lldb_commands.py#... tools/lldb_commands.py:23: js_entry_sp = frame.EvaluateExpression("v8::internal::Isolate::Current()->thread_local_top()->js_entry_sp_;").GetValue() nit: 80 char line limit
Updated with a patch set including the *args and line limit changes.
still lgtm
On 2017/03/23 13:25:13, Michael Achenbach wrote: > still lgtm Thanks! Should I allow more time for others to review or is the LGTM from you and Igor Sheludko enough?
Description was changed from ========== [tools] add lldbinit The goal of this commit is to add the equivalent to gdbinit but for lldb. I've tried to replicate the commands as close as possible but I'm unsure about the jss command and hoping to get some feedback on it in addition to the bta command which I'm not sure how/when this could be used. This is probably just inexperience on my part. The lldbinit file can be placed into a directory prefixed with dot (.lldbinit) and the python script is currently expected to be in the same directory. The path to the script can be changed manually if needed as well. BUG= ========== to ========== [tools] add lldbinit The goal of this commit is to add the equivalent to gdbinit but for lldb. I've tried to replicate the commands as close as possible but I'm unsure about the jss command and hoping to get some feedback on it in addition to the bta command which I'm not sure how/when this could be used. This is probably just inexperience on my part. The lldbinit file can be placed into a directory prefixed with dot (.lldbinit) and the python script is currently expected to be in the same directory. The path to the script can be changed manually if needed as well. NOTRY=true ==========
This can land IMO. I added NOTRY=true as there's no point to have CQ run trybots...
The CQ bit was checked by machenbach@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2017/03/24 12:09:50, Michael Achenbach wrote: > This can land IMO. I added NOTRY=true as there's no point to have CQ run > trybots... Igor Sheludko are you ok with me checking the commit checkbox as well?
On 2017/03/27 06:14:46, danbev wrote: > On 2017/03/24 12:09:50, Michael Achenbach wrote: > > This can land IMO. I added NOTRY=true as there's no point to have CQ run > > trybots... > > Igor Sheludko are you ok with me checking the commit checkbox as well? Yes. Lgtm
The CQ bit was checked by daniel.bevenius@gmail.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490596974083650,
"parent_rev": "b117c9a7891442e8eb60c8948863db612e66ea49", "commit_rev":
"ec1ffe39ed9313d309e9c1a3235ff6813ae3ccbb"}
Message was sent while issue was closed.
Description was changed from ========== [tools] add lldbinit The goal of this commit is to add the equivalent to gdbinit but for lldb. I've tried to replicate the commands as close as possible but I'm unsure about the jss command and hoping to get some feedback on it in addition to the bta command which I'm not sure how/when this could be used. This is probably just inexperience on my part. The lldbinit file can be placed into a directory prefixed with dot (.lldbinit) and the python script is currently expected to be in the same directory. The path to the script can be changed manually if needed as well. NOTRY=true ========== to ========== [tools] add lldbinit The goal of this commit is to add the equivalent to gdbinit but for lldb. I've tried to replicate the commands as close as possible but I'm unsure about the jss command and hoping to get some feedback on it in addition to the bta command which I'm not sure how/when this could be used. This is probably just inexperience on my part. The lldbinit file can be placed into a directory prefixed with dot (.lldbinit) and the python script is currently expected to be in the same directory. The path to the script can be changed manually if needed as well. NOTRY=true Review-Url: https://codereview.chromium.org/2758373002 Cr-Commit-Position: refs/heads/master@{#44136} Committed: https://chromium.googlesource.com/v8/v8/+/ec1ffe39ed9313d309e9c1a3235ff6813ae... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/v8/v8/+/ec1ffe39ed9313d309e9c1a3235ff6813ae... |
