[Lldb-commits] [PATCH] Implemented "thread jump" built-in command.

[jingham at apple.com]

This looks great.  One quibble and a suggestion.

The quibble: since the meaning of "force" is very specific, namely "don't leave the current function" it would be clearer to use that wording for the command line option.  I know I suggested the "force" but since you came up with a better name, we should use it for the command option.

The suggestion: the FindAddressesForLine function is more broadly useful than just for this jump feature.  For instance, another thing we haven't added to lldb yet is the "source info" command, which would be equivalent to gdb's "info line".  You should be able to say:

(lldb) info source -f foo.c -l 12 -s MyProject.dylib

or

(lldb) info source -f bar.h -l 13 --function MyFunction

and get the address ranges for that line in the given shared library or function or in the whole binary, something like:

Debug Info path:
   /Sources/MyProject/foo.c
Found path:
   /Storage/OldProjects/MyProject/foo.c

MyProject.dylib:
0x1234-0x1238
0x1244-0x1248

etc...

The last part of this output, and the general search mechanism, would use pretty much your function FindAddressesForLine.  So it shouldn't be a static helper function in this file.  The highest level entity that it depends on is the Target, but we tend to put lookup functions that only need the ModuleList directly in the module list (e.g. FindFunctions).  If that seems right to you, can you move that function there?  And for extra credit is should take an input ModuleList * that you would just stuff directly into the GetSearchFilterForModuleList call.  It doesn't need to be exactly as we would use it for "source info"; if it is living in the right place, then we can fix it up when we get around to implementing that command.  But if it's a static function off somewhere, then we'll write another copy which is not so good.

Jim

On Aug 28, 2013, at 8:51 PM, Richard Mitton <richard at codersnotes.com> wrote:

>  Update the patch to address the following issues:
> 
>  Fixed some of the command-line parser to check arguments more carefully.
> 
>  Added a 'force' command to prevent unintentionally jumping out of the current function.
> 
>  Moved the core jump logic into Thread::JumpToLine, and changed the command code to call that.
> 
>  Added a Python API version too.
> 
> http://llvm-reviews.chandlerc.com/D1452
> 
> CHANGE SINCE LAST DIFF
>  http://llvm-reviews.chandlerc.com/D1452?vs=3610&id=3883#toc
> 
> Files:
>  include/lldb/API/SBThread.h
>  include/lldb/Core/Error.h
>  include/lldb/Target/RegisterContext.h
>  include/lldb/Target/Thread.h
>  scripts/Python/interface/SBThread.i
>  source/API/SBThread.cpp
>  source/Commands/CommandObjectThread.cpp
>  source/Core/Error.cpp
>  source/Interpreter/CommandInterpreter.cpp
>  source/Target/RegisterContext.cpp
>  source/Target/Thread.cpp
>  test/functionalities/thread/jump/Makefile
>  test/functionalities/thread/jump/TestThreadJump.py
>  test/functionalities/thread/jump/main.cpp
>  test/functionalities/thread/jump/other.cpp
> <D1452.2.patch>_______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits