[Lldb-commits] [PATCH] D74136: [LLDB] WIP: Optionally follow DW_AT_decl_file when setting breakpoint

Thu Feb 6 08:50:34 PST 2020

kwk created this revision.
Herald added subscribers: lldb-commits, dexonsmith, aprantl, mehdi_amini, mgorny.
Herald added a reviewer: jdoerfert.
Herald added a project: LLDB.
kwk planned changes to this revision.
kwk added a comment.

I plan to refactor some of this patch.

To set a breakpoint on a function called `YOURFUNCTION` you do this:

  (lldb) breakpoint set -n YOURFUNCTION

The output then could look like this:

  Breakpoint 1: where = YOURTARGET`YOURFUNCTION() + 4 at YOURFILE:1:20, address = 0x01234

LLDB does a good job in finding `YOURFUNCTION` because, indeed,
`YOURFILE` contains the definition of `YOURFUNCTION`.

Let's say, you not only want to find `YOURFUNCTION` anywhere in
`YOURTARGET`. Say, you want to find `YOURFUNCTION` in the file that LLDB
found it in.

This is how you'd do that:

  (lldb) breakpoint set -n YOURFUNCTION -f YOURFILE

This is where things get a bit tricky. If `YOURFILE` points to a header
file in which `YOURFUNCTION` is not only declared but also defined, then
chances are that it might be inlined into `YOURTARGET` and LLDB cannot
find it. *Why?* You might ask and the answer is that LLDB uses only the
`DW_TAG_compile_unit`'s name that it finds under the `DW_AT_name`
attribute.

Suppose this is a snippet of `llvm-darfdump YOURTARGET`

  0x0000000b: DW_TAG_compile_unit
                DW_AT_name ("main.c")

  0x0000002a: DW_TAG_subprogram
                DW_AT_name ("YOURFUNCTION")
                DW_AT_decl_file ("YOURFILE")

Then LLDB only looks at the file called `main.c` and ignores the
`DW_AT_decl_file`.

With this change I introduce a switch that teaches LLDB to optionally
also respect the `DW_AT_decl_file` all you have to do is pass an option
to the previously mentioned breakpoint command.

  (lldb) breakpoint set -n YOURFUNCTION -f YOURFILE
  --search-source-files

I'm happy to change the name of the flag from `--search-source-files` or
`-Q` to something else. Maybe `--respect-decl-file` or
`--follow-decl-file` are better names, but for now this is about the
working of this patch and not about names.

If you're not convinced that this change has any purpose, think about
LTO and how it might inline your functions with logic that is hard to
reason about when you just have the source code and a binary.

For reference, here's the definition for `DW_AT_decl_file` from the DWARF
spec in chapter 2.14:

> It is sometimes useful in a debugger to be able to associate a
>  declaration with its occurrence in the program source.
>  [...]
>  The value of the DW_AT_decl_file attribute corresponds to a file
>  number from the line number information table for the compilation unit
>  containing the debugging information entry and represents the source
>  file in which the declaration appeared (see Section 6.2 on page 148).
>  The value 0 indicates that no source file has been specified.

Reference to lldb-dev mailing list
==================================

I once brought this topic up on the lldb-dev mailing list
(http://lists.llvm.org/pipermail/lldb-dev/2019-November/015707.html) and
since then worked on it.

Performance considerations
--------------------------

That is where Jim Ingham wrote:

> You probably also want to take some care not to regress the
>  performance of the case where the function is defined in a CU, since
>  that's the more common usage.

Since I didn't change the default behavior in LLDB, I think the
performance is not in danger.

Documentation issue
-------------------

Jim also brought up an idea of documenting and querying options of
commands:

> Another thing I've thought about doing is adding the ability to have
>  help include one of the non-optional, non-overlapping options to the
> command, so you could say:
> 
>   (lldb) help break set -n
> 
> and that would tell you that this is a "by function name" breakpoint,
>  and in that case -n means...  That might help reduce the information
>  overload, and give a better sense of what these complex commands do.

I haven't implemented this yet but I wanted to bubble this idea up here
so it receives more attention.

Thoughts on LTO
---------------

Jim's thoughts on LTO:

> Back to your original query...  If the function is defined in a .h
>  file, or gets inlined by LTO, this filtering is trickier, and I
>  didn't implement that behavior when I implemented this breakpoint
>  type.  So in that case, and in the case where LTO inlines a function,
>  the feature isn't implemented correctly.  The -n searche always looks
>  for out of line and inline instances when doing the search.  So we
>  already get the searcher to all the instances.
>  You would just have to widen the search beyond "Does the CU match" to
>  try to figure out where the inlined instance was defined.

That is essentially what I do in this change.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74136

Files:
  lldb/include/lldb/Core/SearchFilter.h
  lldb/include/lldb/Target/Target.h
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Breakpoint/CMakeLists.txt
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/SearchFilter.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Target/Target.cpp
  lldb/test/Shell/Breakpoint/Inputs/search-source-files.cpp
  lldb/test/Shell/Breakpoint/Inputs/search-source-files.h
  lldb/test/Shell/Breakpoint/Inputs/search-source-files2.h
  lldb/test/Shell/Breakpoint/search-source-files.test

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D74136.242919.patch
Type: text/x-patch
Size: 26523 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200206/708918c8/attachment-0001.bin>