Allow relative file paths when settings source breakpoints

[clayborg]

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.

[jingham]

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

packages/Python/lldbsuite/test/lldbutil.py

579 ↗

(On Diff #142258)

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

[clayborg]

packages/Python/lldbsuite/test/lldbutil.py

579 ↗

(On Diff #142258)

sure thing

[clayborg]

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

[jingham]

Looks good.

[davide]

Thanks for this! Just one minor inline.

source/Breakpoint/BreakpointResolverFileLine.cpp

132 ↗

(On Diff #142289)

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

[davide]

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?

[clayborg]

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

[davide]

thanks!

lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileLine.h

public:
   }
   static inline bool classof(const BreakpointResolver *V) {
     return V->getResolverID() == BreakpointResolver::FileLineResolver;
   }
 
   lldb::BreakpointResolverSP CopyForBreakpoint(Breakpoint &breakpoint) override;
 
 protected:
-  void FilterContexts(SymbolContextList &sc_list);
+  void FilterContexts(SymbolContextList &sc_list, bool is_relative);
 
   friend class Breakpoint;
   FileSpec m_file_spec;   // This is the file spec we are looking for.
   uint32_t m_line_number; // This is the line number that we are looking for.
   bool m_inlines; // This determines whether the resolver looks for inlined
                   // functions or not.
   bool m_skip_prologue;
   bool m_exact_match;
 
 private:
   DISALLOW_COPY_AND_ASSIGN(BreakpointResolverFileLine);
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_BreakpointResolverFileLine_h_

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py

def breakpoint_command_sequence(self):
         lldbutil.run_break_set_by_file_and_line(
             self, "main.c", self.line, num_expected_locations=1, loc_exact=True)
         lldbutil.run_break_set_by_file_and_line(
             self, "main.c", self.line, num_expected_locations=1, loc_exact=True)
         # Breakpoint 4 - set at the same location as breakpoint 1 to test
         # setting breakpoint commands on two breakpoints at a time
         lldbutil.run_break_set_by_file_and_line(
             self, None, self.line, num_expected_locations=1, loc_exact=True)
-
+        # Make sure relative path source breakpoints work as expected. We test
+        # with partial paths with and without "./" prefixes.
+        lldbutil.run_break_set_by_file_and_line(
+            self, "./main.c", self.line,
+            num_expected_locations=1, loc_exact=True)
+        lldbutil.run_break_set_by_file_and_line(
+            self, "breakpoint_command/main.c", self.line,
+            num_expected_locations=1, loc_exact=True)
+        lldbutil.run_break_set_by_file_and_line(
+            self, "./breakpoint_command/main.c", self.line,
+            num_expected_locations=1, loc_exact=True)
+        lldbutil.run_break_set_by_file_and_line(
+            self, "breakpoint/breakpoint_command/main.c", self.line,
+            num_expected_locations=1, loc_exact=True)
+        lldbutil.run_break_set_by_file_and_line(
+            self, "./breakpoint/breakpoint_command/main.c", self.line,
+            num_expected_locations=1, loc_exact=True)
+        # Test relative breakpoints with incorrect paths and make sure we get
+        # no breakpoint locations
+        lldbutil.run_break_set_by_file_and_line(
+            self, "invalid/main.c", self.line,
+            num_expected_locations=0, loc_exact=True)
+        lldbutil.run_break_set_by_file_and_line(
+            self, "./invalid/main.c", self.line,
+            num_expected_locations=0, loc_exact=True)
         # Now add callbacks for the breakpoints just created.
         self.runCmd(
             "breakpoint command add -s command -o 'frame variable --show-types --scope' 1 4")
         self.runCmd(
             "breakpoint command add -s python -o 'import side_effect; side_effect.one_liner = \"one liner was here\"' 2")
         self.runCmd(
             "breakpoint command add --python-function bktptcmd.function 3")
 

lldb/trunk/packages/Python/lldbsuite/test/lldbutil.py

elif num_locations != -2:
             (num_locations,
              out_num_locations))
 
     if file_name:
         out_file_name = ""
         if 'file' in break_results:
             out_file_name = break_results['file']
         test.assertTrue(
-            file_name == out_file_name,
+            file_name.endswith(out_file_name),
             "Breakpoint file name '%s' doesn't match resultant name '%s'." %
             (file_name,
              out_file_name))
 
     if line_number != -1:
         out_line_number = -1
         if 'line_no' in break_results:
             out_line_number = break_results['line_no']

lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp

 
 // Filter the symbol context list to remove contexts where the line number was
 // moved into a new function. We do this conservatively, so if e.g. we cannot
 // resolve the function in the context (which can happen in case of
 // line-table-only debug info), we leave the context as is. The trickiest part
 // here is handling inlined functions -- in this case we need to make sure we
 // look at the declaration line of the inlined function, NOT the function it was
 // inlined into.
-void BreakpointResolverFileLine::FilterContexts(SymbolContextList &sc_list) {
+void BreakpointResolverFileLine::FilterContexts(SymbolContextList &sc_list,
+                                                bool is_relative) {
   if (m_exact_match)
     return; // Nothing to do. Contexts are precise.
 
+  llvm::StringRef relative_path;
+  if (is_relative)
+    relative_path = m_file_spec.GetNormalizedPath().GetDirectory().GetStringRef();
+
   Log * log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS);
   for(uint32_t i = 0; i < sc_list.GetSize(); ++i) {
     SymbolContext sc;
     sc_list.GetContextAtIndex(i, sc);
-    if (! sc.block)
+    if (is_relative) {
+      // If the path was relative, make sure any matches match as long as the
+      // relative parts of the path match the path from support files
+      auto sc_dir = sc.line_entry.file.GetDirectory().GetStringRef();
+      if (!sc_dir.endswith(relative_path)) {
+        // We had a relative path specified and the relative directory
+        // doesn't match so remove this one
+        LLDB_LOG(log, "removing not matching relative path {0} since it "
+                "doesn't end with {1}", sc_dir, relative_path);
+        sc_list.RemoveContextAtIndex(i);
+        --i;
+        continue;
+      }
+    }
+
+    if (!sc.block)
       continue;
 
     FileSpec file;
     uint32_t line;
     const Block *inline_block = sc.block->GetContainingInlinedBlock();
     if (inline_block) {
       const Declaration &inline_declaration = inline_block->GetInlinedFunctionInfo()->GetDeclaration();
       if (!inline_declaration.IsValid())
BreakpointResolverFileLine::SearchCallback(SearchFilter &filter,
   // only set breakpoints on that match.
 
   // Note also that if file_spec only had a file name and not a directory, there
   // may be many different file spec's in the resultant list.  The closest line
   // match for one will not be right for some totally different file.  So we go
   // through the match list and pull out the sets that have the same file spec
   // in their line_entry and treat each set separately.
 
+  FileSpec search_file_spec = m_file_spec;
+  const bool is_relative = m_file_spec.IsRelative();
+  if (is_relative)
+    search_file_spec.GetDirectory().Clear();
+
   const size_t num_comp_units = context.module_sp->GetNumCompileUnits();
   for (size_t i = 0; i < num_comp_units; i++) {
     CompUnitSP cu_sp(context.module_sp->GetCompileUnitAtIndex(i));
     if (cu_sp) {
       if (filter.CompUnitPasses(*cu_sp))
-        cu_sp->ResolveSymbolContext(m_file_spec, m_line_number, m_inlines,
+        cu_sp->ResolveSymbolContext(search_file_spec, m_line_number, m_inlines,
                                     m_exact_match, eSymbolContextEverything,
                                     sc_list);
     }
   }
 
-  FilterContexts(sc_list);
+  FilterContexts(sc_list, is_relative);
 
   StreamString s;
   s.Printf("for %s:%d ", m_file_spec.GetFilename().AsCString("<Unknown>"),
            m_line_number);
 
   SetSCMatchesByLine(filter, sc_list, m_skip_prologue, s.GetString());
 
   return Searcher::eCallbackReturnContinue;