Page MenuHomePhabricator

Allow relative file paths when settings source breakpoints
ClosedPublic

Authored by clayborg on Apr 12 2018, 2:12 PM.

Details

Summary

Many IDEs set breakpoints using absolute paths and this causes problems when the full path of the source file path doesn't match what is in the debug info. This can be due to different build systems and do or do not resolve symlinks. This patch allows relative breakpoint to be set correctly without needing to do any target.source-map tricks. If IDEs want to, they can send down relative paths like:

./main.c
./src/main.c
src/main.c
foo/bar/src/main.c

I used the breakpoint resolver to match on the file basename and then we weed out anything whose relative paths don't match. This will be a huge improvement for IDEs as they can specify as much of a relative path as desired to uniquely identify a source file in the current project.

Event Timeline

clayborg created this revision.Apr 12 2018, 2:12 PM

Except for a minor comment, this seems fine to me, and is a useful addition!

packages/Python/lldbsuite/test/lldbutil.py
579 ↗(On Diff #142289)

The output file name should always END with the original file name, right? Can we make this check reflect that more closely?

clayborg added inline comments.Apr 12 2018, 3:50 PM
packages/Python/lldbsuite/test/lldbutil.py
579 ↗(On Diff #142289)

sure thing

clayborg updated this revision to Diff 142289.Apr 12 2018, 3:56 PM

Fixed the breakpoint verification in lldbutil to make sure the file name ends with the right thing

clayborg marked an inline comment as done.Apr 12 2018, 3:56 PM
jingham accepted this revision.Apr 12 2018, 4:11 PM

Looks good.

This revision is now accepted and ready to land.Apr 12 2018, 4:11 PM
davide added a subscriber: davide.Apr 12 2018, 5:45 PM

Thanks for this! Just one minor inline.

source/Breakpoint/BreakpointResolverFileLine.cpp
132 ↗(On Diff #142289)

Just one typo, "reltive" -> "relative".

davide added inline comments.Apr 12 2018, 5:46 PM
source/Breakpoint/BreakpointResolverFileLine.cpp
219–221 ↗(On Diff #142289)

For consistency with the rest of the codestyle (and what you use above), can you remove the braces before committing?

This revision was automatically updated to reflect the committed changes.
clayborg marked 2 inline comments as done.Apr 13 2018, 8:09 AM

Didn't update the diffs, but I did fix the things Davide requested before submission