[PATCH] D36347: Add new script to launch lldb and set breakpoints for diagnostics all diagnostics seen.
jingham added a comment. I can't see anything wrong with the SB API use here. I don't feel qualified to comment on the most effective workflow for an analysis I've never had to do, however. https://reviews.llvm.org/D36347 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API
jingham added a comment. IIUC, when Expected returns fails, it returns an Error object that might have information about what went wrong. Would it be possible to include the contents of that error n the log message? We often get "I can't run an expression in a really complex proprietary code base" and need to try to sort out what went wrong from these logs. So every crumb of information we can preserve is useful... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61438/new/ https://reviews.llvm.org/D61438 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D91103: [tooling] Add support for fixits that indicate code will need reformatting
jingham added a comment. IIUC, the expression parser part of this change suppresses any Fixits that are clang-tidy type rewrites, is that right? If so that is indeed the correct behavior. But that fact that this change implements that behavior is entirely non-obvious at the call site. Could you either add a comment here explaining to point of the test or maybe wrap the test in a method on the FixItHint class so that it's self documenting? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91103/new/ https://reviews.llvm.org/D91103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle
jingham added inline comments.
Comment at:
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py:15
+
+class TestCoroutineHandle(TestBase):
+def do_test(self, stdlib_type):
aprantl wrote:
> Nice test!
Would you mind changing moving this into lldbutil.py? This seems generally
useful. If you return the thread & breakpoint it would be even more generally
useful?
Comment at:
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py:44
+
+lldbutil.run_to_source_breakpoint(self, '// Break after co_yield',
+lldb.SBFileSpec("main.cpp", False))
avogelsgesang wrote:
> aprantl wrote:
> > Does this launch a new process? It might be faster to just set an
> > additional breakpoint and run `process.Continue()`.
> somehow, I assumed `run_to_source_breakpoint` would actually continue the
> running process instead of starting a new process. But looking into
> `run_to_source_breakpoint`, you are right, this launches a new process...
>
> I changed this to instead continue execution of the already launched process
There is also lldbutil.continue_to_breakpoint,
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132415/new/
https://reviews.llvm.org/D132415
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle
jingham added inline comments.
Comment at:
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py:44
+
+lldbutil.run_to_source_breakpoint(self, '// Break after co_yield',
+lldb.SBFileSpec("main.cpp", False))
jingham wrote:
> avogelsgesang wrote:
> > aprantl wrote:
> > > Does this launch a new process? It might be faster to just set an
> > > additional breakpoint and run `process.Continue()`.
> > somehow, I assumed `run_to_source_breakpoint` would actually continue the
> > running process instead of starting a new process. But looking into
> > `run_to_source_breakpoint`, you are right, this launches a new process...
> >
> > I changed this to instead continue execution of the already launched process
> There is also lldbutil.continue_to_breakpoint,
Forgot to delete that inline comment before saving...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132415/new/
https://reviews.llvm.org/D132415
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables
jingham added a comment. I don't know off the top of my head. Did anybody working on this patch try printing variables in lldb with this name change, particularly global variables? Global lookups are particularly tricky, but it shouldn't be hard to check. You'd want to check that you can print them with frame var and expr. Repository: rC Clang https://reviews.llvm.org/D44842 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables
jingham added a comment. @ormris Thanks! I you come up with some examples that test this, I can whip up an lldb test case (or you can if you are feeling adventurous.) If you want to try I'd be happy to give you a hand. Repository: rC Clang https://reviews.llvm.org/D44842 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables
jingham added a comment. Apparently this makes a global variable, so "frame var" wouldn't show it by default. What does "frame var -g" show? We would also need to get: (lldb) frame var -g crazy or something like it to work. "frame var" has its own parser to support "->" and ".". That doesn't currently work with: (lldb) frame var -g crazy error: unexpected char '<' encountered after "crazy" in "" so for that part to work, either the name lookup must work w/o the or frame var's parser must be updated to cope with this (and of course with any arbitrarily complex type that could get substituted in there). That's likely a non-trivial bit of work. I wonder if expr is failing because this is a C++14 extension, lldb sets CPlusPlus11 to true in the LangOpts for the compiler it makes, but not CPlusPlus14. Repository: rC Clang https://reviews.llvm.org/D44842 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36347: New lldb python module for adding diagnostic breakpoints
jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. Use the form of the command that gets an SBExecutionContext, then you can avoid having to cache the target at all. Comment at: utils/clangdiag.py:98-100 +def the_diag_command(debugger, command, result, dict): +# Use the Shell Lexer to properly parse up command options just like a +# shell would If you use the form of the command function that takes an execution context: def command_function(debugger, command, exe_ctx, result, internal_dict): then you can grab the target from there when the command gets invoked and pass it to your enable & disable funcs. That way you won't have to rely on GetSelectedTarget. That's important, for instance, if you were running a debug session with two targets and you wanted to invoke your command in a breakpoint command of a breakpoint in target A. There's no guarantee when target A hits the breakpoint that A is the currently selected target (it won't get selected till it actually decides to stop.) But when the breakpoint runs its command, it sets the right target, & thread in the execution context that gets passed in. https://reviews.llvm.org/D36347 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36347: New lldb python module for adding diagnostic breakpoints
jingham added a comment.
Yes definitely use names for your breakpoints. It makes them easier to handle.
Note starting a month or two ago you can set names to not get deleted except
by an explicit gesture. That hasn't shown up in releases yet, but once you can
rely on its being there, you can set the names to not get auto-deleted or
disabled, and then if somebody does:
(lldb) break disable
not thinking they are affecting your utility, you won't get messed up by this.
Comment at: utils/clangdiag.py:87
+def disable(debugger):
+global target
+global diagtool
clayborg wrote:
> remove and use:
> ```
> target = debugger.GetSelectedTarget()
> ```
See my comment. Don't use selected targets, use the command form that takes an
SBExecutionContext. That's been around for more than a couple of years now, so
it's pretty safe to use.
Comment at: utils/clangdiag.py:91-92
+# Remove all diag breakpoints.
+for bp in Breakpoints:
+target.BreakpointDelete(bp.GetID())
+target = None
clayborg wrote:
> Another way to do this would be to give the breakpoints you create using
> "target.BreakpointCreateXXX" to have a name when you create them:
>
> ```
> bp = target.BreakpointCreateByName('DiagnosticsEngine::Report')
> bp.AddName("llvm::Diagnostic")
> ```
>
> Then here you can iterate over all breakpoints in the target:
>
> ```
> for bp in target.breakpoint_iter():
> if bp.MatchesName("llvm::Diagnostic"):
> target.BreakpointDelete(bp.GetID())
> ```
>
> Then you don't need a global list?
>
If you name them you can just do:
target.BreakpointDeleteByName("llvm::Diagnostic")
That's even simpler.
https://reviews.llvm.org/D36347
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36347: New lldb python module for adding diagnostic breakpoints
jingham added a comment. Note, BTW, Enrico also added a form of the command add that allows you to use a Python class to wrap the callable. That was in early 2015 so it's probably safe to use as well by now. That's even more convenient, and obviates the need for globals at all. One instance of the class is made per debugger as the command gets loaded, so it's ivars live for the life of the command - spanning execution. There's an example of it here: http://llvm.org/svn/llvm-project/lldb/trunk/examples/python/disassembly_mode.py and it is mentioned in the Python Ref. You don't by any means have to use this form, but if you are having fun playing around with this stuff, it makes it much more pleasant to write commands. Comment at: utils/clangdiag.py:100 +# Use the Shell Lexer to properly parse up command options just like a +# shell would +command_args = shlex.split(command) clayborg wrote: > Great idea. Forgot about the execution context variant. The "exe_ctx" is a > lldb.SBExecutionContext object. You get your target using: > > ``` > target = exe_ctx.GetTarget() > ``` > Yeah, if I could think of a friendly way to do it, I would outlaw the older form. That was an oversight, and makes commands that aren't guaranteed to behave correctly in breakpoint callbacks. I'll go make the docs a little stronger now that the exe_ctx form has been there long enough that it's generally safe to use. https://reviews.llvm.org/D36347 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36347: New lldb python module for adding diagnostic breakpoints
jingham added a comment. Ack, my bad. I should remember not to rely on my memory... I thought at one point I was going to do it that way, then got annoyed I'd have to have "BreakpointDisableByName" etc... So apparently I made: SBTarget.FindBreakpointByName that takes a name & an SBBreakpointList, and the list gets filled with the breakpoints that match that name. https://reviews.llvm.org/D36347 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
