diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h b/lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h --- a/lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h +++ b/lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h @@ -23,7 +23,8 @@ public: BreakpointResolverFileLine(const lldb::BreakpointSP &bkpt, lldb::addr_t offset, bool skip_prologue, - const SourceLocationSpec &location_spec); + const SourceLocationSpec &location_spec, + lldb::TargetSP target = nullptr); static BreakpointResolver * CreateFromStructuredData(const lldb::BreakpointSP &bkpt, @@ -56,11 +57,27 @@ CopyForBreakpoint(lldb::BreakpointSP &breakpoint) override; protected: - void FilterContexts(SymbolContextList &sc_list, bool is_relative); + /// Filter \param sc_list for path matching. + /// If partial match is enabled symbol context is filtered with + /// with m_partial_match_dir_count setting. + /// If partial match is not enabled and request path is relative + /// symbol context is filtered if request path is not a valid suffix. + void FilterContextsForPaths(SymbolContextList &sc_list, bool is_relative); + + // 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 FilterContextsForLines(SymbolContextList &sc_list); friend class Breakpoint; SourceLocationSpec m_location_spec; bool m_skip_prologue; + bool m_partial_match = false; + uint64_t m_partial_match_dir_count = 0; private: BreakpointResolverFileLine(const BreakpointResolverFileLine &) = delete; diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -147,6 +147,10 @@ PathMappingList &GetSourcePathMap() const; + bool GetBreakpointPartialMatch() const; + + uint64_t GetBreakpointPartialMatchDirCount() const; + FileSpecList GetExecutableSearchPaths(); void AppendExecutableSearchPaths(const FileSpec &); diff --git a/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp b/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp --- a/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp +++ b/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp @@ -12,6 +12,7 @@ #include "lldb/Core/Module.h" #include "lldb/Symbol/CompileUnit.h" #include "lldb/Symbol/Function.h" +#include "lldb/Target/Target.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" #include "lldb/Utility/StreamString.h" @@ -22,9 +23,14 @@ // BreakpointResolverFileLine: BreakpointResolverFileLine::BreakpointResolverFileLine( const BreakpointSP &bkpt, lldb::addr_t offset, bool skip_prologue, - const SourceLocationSpec &location_spec) + const SourceLocationSpec &location_spec, lldb::TargetSP target) : BreakpointResolver(bkpt, BreakpointResolver::FileLineResolver, offset), - m_location_spec(location_spec), m_skip_prologue(skip_prologue) {} + m_location_spec(location_spec), m_skip_prologue(skip_prologue) { + if (target) { + m_partial_match = target->GetBreakpointPartialMatch(); + m_partial_match_dir_count = target->GetBreakpointPartialMatchDirCount(); + } +} BreakpointResolver *BreakpointResolverFileLine::CreateFromStructuredData( const BreakpointSP &bkpt, const StructuredData::Dictionary &options_dict, @@ -112,41 +118,134 @@ return WrapOptionsDict(options_dict_sp); } -// 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, - bool is_relative) { +void BreakpointResolverFileLine::FilterContextsForPaths( + SymbolContextList &sc_list, bool is_relative) { if (m_location_spec.GetExactMatch()) return; // Nothing to do. Contexts are precise. - llvm::StringRef relative_path; - if (is_relative) - relative_path = m_location_spec.GetFileSpec().GetDirectory().GetStringRef(); + llvm::StringRef request_path_dir = + m_location_spec.GetFileSpec().GetDirectory().GetStringRef(); + const char path_separator = llvm::sys::path::get_separator( + m_location_spec.GetFileSpec().GetPathStyle()) + .data()[0]; Log *log = GetLog(LLDBLog::Breakpoints); - for(uint32_t i = 0; i < sc_list.GetSize(); ++i) { + for (uint32_t i = 0; i < sc_list.GetSize(); ++i) { SymbolContext sc; sc_list.GetContextAtIndex(i, sc); - if (is_relative) { + llvm::StringRef sc_dir = sc.line_entry.file.GetDirectory().GetStringRef(); + + bool should_remove = false; + if (m_partial_match) { + uint64_t match_dir_count_goal = m_partial_match_dir_count; + + size_t max_match = std::min(request_path_dir.size(), sc_dir.size()); + + char last_match_char; + // The number of matched suffix characters. + size_t match_count = 0; + for (; match_count < max_match; ++match_count) { + if (match_dir_count_goal == 0) + break; // Meet goal. + + assert(request_path_dir.size() - 1 >= match_count && + "index will be out of range"); + assert(sc_dir.size() - 1 >= match_count && + "index will be out of range"); + // TODO: deal with case sensitivity. + if (request_path_dir[request_path_dir.size() - 1 - match_count] != + sc_dir[sc_dir.size() - 1 - match_count]) { + LLDB_LOG(log, + "removing not partial matching path {0} since it " + "doesn't meet {1} directories count with {2}", + sc_dir, m_partial_match_dir_count, request_path_dir); + // Failed to meet goal. + should_remove = true; + break; + } + last_match_char = sc_dir[sc_dir.size() - 1 - match_count]; + if (last_match_char == path_separator) + --match_dir_count_goal; + } + + // If conclusion hasn't reached after matching exaustion we should + // perform one last check: only if there is one extra directory left and + // remaining match_dir_count_goal is 1 can we meet the goal. + // + // Example: + // request_path_dir: /Users/unixname/foo/bar/file.cpp + // sc_dir: foo/bar/file.cpp + // m_partial_match_dir_count == 2 + // + // Otherwise, it is an invalid match. + + if (match_count == max_match && match_dir_count_goal > 0) { + if (last_match_char == path_separator) { + + // Last matched at directory boundary; no directory left to + // meet goal. + // + // Example: + // request_path_dir: /Users/unixname/foo/bar/file.cpp + // sc_dir: /foo/bar/file.cpp + // m_partial_match_dir_count == 3 + + should_remove = true; + } else if (match_dir_count_goal == 1) { + if (match_count == sc_dir.size() && + match_count == request_path_dir.size()) { + + // Both have exausted; no directory left to meet goal. + // + // Example: + // request_path_dir: foo/bar/file.cpp + // sc_dir: foo/bar/file.cpp + // m_partial_match_dir_count == 3 + + should_remove = true; + } else if (match_count == sc_dir.size()) { + char last_unmatched_char = + request_path_dir[request_path_dir.size() - 1 - match_count]; + should_remove = (last_unmatched_char != path_separator); + } else if (match_count == request_path_dir.size()) { + char last_unmatched_char = sc_dir[sc_dir.size() - 1 - match_count]; + should_remove = (last_unmatched_char != path_separator); + } + } else { + should_remove = true; + } + } + } else 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)) { + if (!sc_dir.endswith(request_path_dir)) { // 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; + LLDB_LOG(log, + "removing not matching relative path {0} since it " + "doesn't end with {1}", + sc_dir, request_path_dir); + should_remove = true; } } + if (should_remove) { + sc_list.RemoveContextAtIndex(i); + --i; + } + } +} + +void BreakpointResolverFileLine::FilterContextsForLines( + SymbolContextList &sc_list) { + if (m_location_spec.GetExactMatch()) + return; // Nothing to do. Contexts are precise. + + Log *log = GetLog(LLDBLog::Breakpoints); + for(uint32_t i = 0; i < sc_list.GetSize(); ++i) { + SymbolContext sc; + sc_list.GetContextAtIndex(i, sc); + if (!sc.block) continue; @@ -235,7 +334,7 @@ // of the CUs. FileSpec search_file_spec = m_location_spec.GetFileSpec(); const bool is_relative = search_file_spec.IsRelative(); - if (is_relative) + if (is_relative || m_partial_match) search_file_spec.GetDirectory().Clear(); SourceLocationSpec search_location_spec( search_file_spec, m_location_spec.GetLine().value_or(0), @@ -252,7 +351,8 @@ } } - FilterContexts(sc_list, is_relative); + FilterContextsForPaths(sc_list, is_relative); + FilterContextsForLines(sc_list); StreamString s; s.Printf("for %s:%d ", diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -4267,6 +4267,18 @@ return option_value->GetCurrentValue(); } +bool TargetProperties::GetBreakpointPartialMatch() const { + const uint32_t idx = ePropertyBreakpointPartialMatch; + return m_collection_sp->GetPropertyAtIndexAsBoolean( + nullptr, idx, g_target_properties[idx].default_uint_value != 0); +} + +uint64_t TargetProperties::GetBreakpointPartialMatchDirCount() const { + const uint32_t idx = ePropertyBreakpointPartialMatchDirCount; + return m_collection_sp->GetPropertyAtIndexAsUInt64( + nullptr, idx, g_target_properties[idx].default_uint_value != 0); +} + void TargetProperties::AppendExecutableSearchPaths(const FileSpec &dir) { const uint32_t idx = ePropertyExecutableSearchPaths; OptionValueFileSpecList *option_value = diff --git a/lldb/source/Target/TargetProperties.td b/lldb/source/Target/TargetProperties.td --- a/lldb/source/Target/TargetProperties.td +++ b/lldb/source/Target/TargetProperties.td @@ -37,6 +37,12 @@ def SourceMap: Property<"source-map", "PathMap">, DefaultStringValue<"">, Desc<"Source path remappings apply substitutions to the paths of source files, typically needed to debug from a different host than the one that built the target. The source-map property consists of an array of pairs, the first element is a path prefix, and the second is its replacement. The syntax is `prefix1 replacement1 prefix2 replacement2...`. The pairs are checked in order, the first prefix that matches is used, and that prefix is substituted with the replacement. A common pattern is to use source-map in conjunction with the clang -fdebug-prefix-map flag. In the build, use `-fdebug-prefix-map=/path/to/build_dir=.` to rewrite the host specific build directory to `.`. Then for debugging, use `settings set target.source-map . /path/to/local_dir` to convert `.` to a valid local path.">; + def BreakpointPartialMatch: Property<"breakpoint-partial-match", "Boolean">, + DefaultFalse, + Desc<"Enable partial matching breakpoint file path in debug info">; + def BreakpointPartialMatchDirCount: Property<"breakpoint-partial-match-dir-count", "UInt64">, + DefaultUnsignedValue<0>, + Desc<"Minimum number of directories need to match between requested breakpoint file path and the path in debug info. This option only takes effect when breakpoint-partial-match is set. Default value is zero which means matching file base name only.">; def ExecutableSearchPaths: Property<"exec-search-paths", "FileSpecList">, DefaultStringValue<"">, Desc<"Executable search paths to use when locating executable files whose paths don't match the local file system.">; diff --git a/lldb/test/API/breakpoint/Makefile b/lldb/test/API/breakpoint/Makefile new file mode 100644 --- /dev/null +++ b/lldb/test/API/breakpoint/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/breakpoint/TestBreakpointPartialMatch.py b/lldb/test/API/breakpoint/TestBreakpointPartialMatch.py new file mode 100644 --- /dev/null +++ b/lldb/test/API/breakpoint/TestBreakpointPartialMatch.py @@ -0,0 +1,193 @@ +""" +Test breakpoint partial match feature. +""" + + +import os + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +def split_all(path): + """Split path into parts""" + allparts = [] + while True: + parts = os.path.split(path) + if parts[0] == path: # sentinel for absolute paths + allparts.insert(0, parts[0]) + break + elif parts[1] == path: # sentinel for relative paths + allparts.insert(0, parts[1]) + break + else: + path = parts[0] + allparts.insert(0, parts[1]) + return allparts + + +class TestBreakpointPartialMatch(TestBase): + def setUp(self): + TestBase.setUp(self) + + self.source_file_name = "main.cpp" + self.line = line_number( + self.source_file_name, "// Set break point at this line." + ) + + dir_path = os.path.dirname(os.path.realpath(__file__)) + dirs = split_all(dir_path) + + self.partial_match_dir_count = min(2, len(dirs)) + + # Take last self.partial_match_dir_count directories out of dirs. + self.matched_dirs = dirs[-self.partial_match_dir_count :] + self.unmatched_dirs = ["non", "exist", "root"] + + # partial match debug info path with base name. + self.base_name_match_breakpoint_file = os.path.join( + *self.unmatched_dirs, self.source_file_name + ) + + # partial match debug info path with + # last self.partial_match_dir_count directories. + self.partial_match_breakpoint_file = os.path.join( + *self.unmatched_dirs, *self.matched_dirs, self.source_file_name + ) + + def test_default(self): + ''' + Test default settings of partial match. + ''' + self.build() + + # Disable partial match. + self.runCmd("settings set target.breakpoint-partial-match false") + + # Create a target by the debugger. + exe = self.getBuildArtifact("a.out") + error = lldb.SBError() + # Don't read in dependencies so we don't come across false matches that + # add unwanted breakpoint hits. + self.target = self.dbg.CreateTarget(exe, None, None, False, error) + self.assertTrue(self.target, VALID_TARGET) + + default_bp = self.target.BreakpointCreateByLocation( + self.partial_match_breakpoint_file, self.line + ) + # Breakpoint shouldn't bind due to mismatch path. + self.assertEqual(default_bp.GetNumLocations(), 0) + + # Enable partial match. + self.runCmd("settings set target.breakpoint-partial-match true") + + partial_match_bp = self.target.BreakpointCreateByLocation( + self.partial_match_breakpoint_file, self.line + ) + # Breakpoint should bind due to partial match. + self.assertNotEqual(partial_match_bp.GetNumLocations(), 0) + + basename_match_bp = self.target.BreakpointCreateByLocation( + self.base_name_match_breakpoint_file, self.line + ) + # Breakpoint should bind due to base name match + self.assertNotEqual(basename_match_bp.GetNumLocations(), 0) + + def test_dir_count(self): + ''' + Test partial match directory count. + ''' + self.build() + + # Enable partial match. + self.runCmd("settings set target.breakpoint-partial-match true") + + # Create a target by the debugger. + exe = self.getBuildArtifact("a.out") + error = lldb.SBError() + # Don't read in dependencies so we don't come across false matches that + # add unwanted breakpoint hits. + self.target = self.dbg.CreateTarget(exe, None, None, False, error) + self.assertTrue(self.target, VALID_TARGET) + + # Try a valid partial match dir count. + self.runCmd( + f"settings set target.breakpoint-partial-match-dir-count {self.partial_match_dir_count}" + ) + partial_match_bp = self.target.BreakpointCreateByLocation( + self.partial_match_breakpoint_file, self.line + ) + # Breakpoint should bind due to partial match. + self.assertNotEqual(partial_match_bp.GetNumLocations(), 0) + + # Then try a invalid partial match dir count. + self.runCmd( + f"settings set target.breakpoint-partial-match-dir-count {self.partial_match_dir_count + 1}" + ) + partial_match_fail_bp = self.target.BreakpointCreateByLocation( + self.partial_match_breakpoint_file, self.line + ) + # Breakpoint should not bind due to invalid match dir count. + self.assertEqual(partial_match_fail_bp.GetNumLocations(), 0) + + def test_exaustion_match(self): + ''' + Test partial match directory count during exaustion. + Exaustion -- one path matched all its length scenario. + ''' + self.build() + + # Enable partial match. + self.runCmd("settings set target.breakpoint-partial-match true") + + # Create a target by the debugger. + exe = self.getBuildArtifact("a.out") + error = lldb.SBError() + # Don't read in dependencies so we don't come across false matches that + # add unwanted breakpoint hits. + self.target = self.dbg.CreateTarget(exe, None, None, False, error) + self.assertTrue(self.target, VALID_TARGET) + + self.runCmd( + f"settings set target.breakpoint-partial-match-dir-count {self.partial_match_dir_count}" + ) + + ############################################################### + ### Verify that partial match works correct during exaustion + ### with or without leading separator + ############################################################### + exaustion_match_breakpoint_file_without_leading_separator = os.path.join( + *self.matched_dirs, self.source_file_name + ) + + partial_match_bp = self.target.BreakpointCreateByLocation( + exaustion_match_breakpoint_file_without_leading_separator, self.line + ) + # Breakpoint should bind due to partial match. + self.assertNotEqual(partial_match_bp.GetNumLocations(), 0) + + exaustion_match_breakpoint_file_with_leading_separator = ( + os.sep + exaustion_match_breakpoint_file_without_leading_separator + ) + + # Then try a invalid partial match dir count. + partial_match_bp = self.target.BreakpointCreateByLocation( + exaustion_match_breakpoint_file_with_leading_separator, self.line + ) + # Breakpoint should bind due to partial match. + self.assertNotEqual(partial_match_bp.GetNumLocations(), 0) + + ############################################################### + ### Verify that partial match wouldn't treat leading separator + ### as a directory match + ############################################################### + self.runCmd( + f"settings set target.breakpoint-partial-match-dir-count {self.partial_match_dir_count + 1}" + ) + partial_match_bp = self.target.BreakpointCreateByLocation( + exaustion_match_breakpoint_file_with_leading_separator, self.line + ) + # Breakpoint should not bind due to dir count match. + self.assertEqual(partial_match_bp.GetNumLocations(), 0) diff --git a/lldb/test/API/breakpoint/main.cpp b/lldb/test/API/breakpoint/main.cpp new file mode 100644 --- /dev/null +++ b/lldb/test/API/breakpoint/main.cpp @@ -0,0 +1,10 @@ +#include + +void foo() { + printf("hello world from foo"); // Set break point at this line. +} + +int main() { + foo(); + return 0; +}