[Lldb-commits] [PATCH] D129307: Add a new breakpoint partial match settings

Thu Jul 7 11:45:58 PDT 2022

clayborg added a comment.

In D129307#3636573 <https://reviews.llvm.org/D129307#3636573>, @jingham wrote:

> I'm not entirely clear what problem this is solving.  Any actor setting breakpoints can already choose the match depth by simply providing that many directory components.  I.e. if I know I have subdir1/foo.cpp and subdir2/foo.cpp I can set a breakpoint on only one of them by doing:
>
>   (lldb) break set -f subdir1/foo.cpp -l 10
>
> So an IDE could implement this by simply passing either the base name or the base name plus one sub directory, etc.  As you say, that's what command line users do anyway, they seldom type out full paths.  So this is really about IDE's running lldb, and this seems more like a feature the IDE should offer, not lldb.

Xcode has this same issue if you download a dSYM file from the Apple build infrastructure if that dSYM doesn't contain path re-mappings in the UUID plists. Xcode, the IDE, will send down full paths to the source files that it knows about if you set breakpoints. Granted Xcode could work around this, but we added the ability to remap things at the module level in LLDB to work around this issue. So yes, it would be great if all IDEs would do this, but currently none do.

> It also seems awkward to do all this work as part of the breakpoint filtering - which as you have seen is somewhat delicate code.  After all, you can implement "only match the last N components" by only submitting the last N components when looking up the files.  So it would be much more straightforward to strip the leading path components from the file name when you create the breakpoint and you don't have to muck with the filtering at all.  That would also help remove some confusion when you see that the path submitted was /foo/bar/baz.cpp but I matched /some/other/bar/baz.cpp (but not /some/other/different/baz.cpp) because I had set the match suffix count to 1.  It would be nice to have break list show me exactly what it was going to match on.

We might need to always store the path that was given in source file and line breakpoints and then have the breakpoint update its locations when/if either of these two settings are modified. This would also help with your example below where you comment on breakpoint settings depend on the history of the session (they shouldn't).

> I am also not sure doing this as a general setting for values other than 0 is really helpful.  So far as I can tell, you would use a value of 1 because you know that though you might have files with duplicate names they are always disambiguated by their parent directory name.

Exactly the case I was worried about. Usually zero would be the default IMHO, but if you have two different binaries that both have a "main.cpp" it could help. We can remove the depth and have this setting only set breakpoints by basename if needed if no one wants the extra complexity.

> Using a value of 2 says the names are disambiguated by the two parent directory names.  But there's no guarantee that one value is going to work for all the files in your project and having to change a global setting from breakpoint to breakpoint is awkward.
>
> I also don't like that this makes breakpoint settings depend on the history of the session.  For instance:
>
> a) Set the partial match to on and the depth to 2
> b) Set a file & line breakpoint
> c) Change the partial match depth to 0
> d) a library gets loaded with a file that wouldn't have matched with depth of 2 but does with 0

We would need to store the full path in the source file and line breakpoints all the time and then set all of the source breakpoints again if this setting changes (enabled/disabled or dir depth changes).

> Also, you are supposed to be able to save breakpoints & restore them and provided your binaries haven't changed you will get the same breakpoints.  Now, for that to work, you also have to restore some target setting that wasn't saved with the breakpoints.

We wouldn't actually need to if we always store the path we are given when the breakpoint is originally set, and then update the breakpoint locations when/if the settings are changed (which might cause more locations to show up, or less).

> I wouldn't mind so much if this were passed in to the breakpoint setting directly, though again, I don't really see the point since this is mostly for IDE's and they can strip however much they want off a path before submitting it w/o involving lldb.

So neither Xcode nor Visual Studio Code make any attempt to remap sources for anyone. The IDEs always pass down the full path currently.

Just to let everyone know where we are going with this: we want to implement an auto source map feature after this patch where if the user sets a breakpoint with "/some/build/path/<src-root>/bar/baz/foo.cpp", and the debug info contains "<any-path>/<src-root>/bar/baz.foo.cpp", we can have LLDB automatically create a source mapping for "/some/build/path" -> "<any-path>" where "<any-path>" can be a relative path like "." or an absolute path like "/local/file/path". The BUCK build system uses relative paths in the DWARF for all source files so that cached compiled versions of .o and .a files can be downloaded from the build infrastructure if no one has changed sources in a particular directory. This lets build servers build and cache intermediate files on another machine and send them over to user machines to speed up local builds. Apple's B&I builds on some remote build server using different paths and then uploads results to other machines in different directories and also archives the sources in other locations. So having this feature will help users to be able to set breakpoints more reliably without having to set the source mapping settings manually.

We can implement this feature in lldb-vscode, but I would much rather have a solution in LLDB itself if this is useful to anyone other than just lldb-vscode. I am not aware of any IDEs that do source remapping for debugger in any way, so I think this feature it would be valuable to just about any IDE, but we can just put the feature in lldb-vscode and let everyone else fend for themselves if no one wants this feature in LLDB.

We would welcome feedback from anyone on the review or subscriber list.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129307/new/

https://reviews.llvm.org/D129307