yinghuitan created this revision.
yinghuitan added reviewers: clayborg, labath, jingham, jdoerfert, JDevlieghere, 
aadsm, kusmour.
Herald added a project: All.
yinghuitan requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch adds a new "target.auto-deduce-source-map" setting.

If enabled, this setting may auto deduce a source map entry based on requested
breakpoint path and the original path stored in debug info for resolved 
breakpoint.

As an example, if debug info contains "./a/b/c/main.cpp", user sets a source
breakpoint at "/root/repo/x/y/z/a/b/c/main.cpp". The breakpoint will resolve
correctly now with Greg's patch https://reviews.llvm.org/D130401. However, the
resolved breakpoint will use "./a/b/c/main.cpp" to locate source file during
stop event which would fail most of the time.

With the new "target.auto-deduce-source-map" setting enabled, a auto deduced 
source map entry "." => "/root/repo/x/y/z" will be added. This new mapping will
help lldb to map resolved breakpoint path "./a/b/c/main.cpp" back to 
"/root/repo/x/y/z/a/b/c/main.cpp" and locate it on disk.

If an existing source map entry is used the patch also concatenates the auto
deduced entry with any stripped reverse mapping prefix (see example below).

As a second example, debug info contains "./a/b/c/main.cpp" and user sets
breakpoint at "/root/repo/x/y/z/a/b/c/main.cpp". Let's say there is an existing
source map entry "." => "/root/repo"; this mapping would strip the prefix out 
of 
"/root/repo/x/y/z/a/b/c/main.cpp" and use "x/y/z/a/b/c/main.cpp" to resolve
breakpoint. "target.auto-deduce-source-map" setting would auto deduce a new 
potential mapping of "." => "x/y/z", then it detects that there is a stripped
prefix from reverse mapping and concatenates it as the new mapping:
 "." => "/root/repo/x/y/z" which would correct map "./a/b/c/main.cpp" path to
new path in disk.

This patches depends on https://reviews.llvm.org/D130401 to use new added
SBTarget::GetSourceMap() API for testing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133042

Files:
  lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
  lldb/include/lldb/Target/PathMappingList.h
  lldb/include/lldb/Target/Target.h
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/source/Target/PathMappingList.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td
  
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
  lldb/unittests/Target/PathMappingListTest.cpp

Index: lldb/unittests/Target/PathMappingListTest.cpp
===================================================================
--- lldb/unittests/Target/PathMappingListTest.cpp
+++ lldb/unittests/Target/PathMappingListTest.cpp
@@ -40,7 +40,7 @@
         map.RemapPath(ConstString(match.original.GetPath()), actual_remapped));
     EXPECT_EQ(FileSpec(actual_remapped.GetStringRef()), match.remapped);
     FileSpec unmapped_spec;
-    EXPECT_TRUE(map.ReverseRemapPath(match.remapped, unmapped_spec));
+    EXPECT_TRUE(map.ReverseRemapPath(match.remapped, unmapped_spec).hasValue());
     std::string unmapped_path = unmapped_spec.GetPath();
     EXPECT_EQ(unmapped_path, orig_normalized);
   }
Index: lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
===================================================================
--- lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++ lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -8,6 +8,7 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
+import json
 import os
 import side_effect
 
@@ -428,3 +429,84 @@
 
         bp_3 = target.FindBreakpointByID(bp_id_3)
         self.assertFalse(bp_3.IsValid(), "Didn't delete disabled breakpoint 3")
+
+
+    def get_source_map_json(self, target):
+        stream = lldb.SBStream()
+        target.GetSourceMap().GetAsJSON(stream)
+        return json.loads(stream.GetData())
+
+    def verify_source_map_entry_pair(self, entry, first, second):
+        self.assertEquals(entry["first"], first,
+            "source map entry 'first' does not match")
+        self.assertEquals(entry["second"], second,
+            "source map entry 'second' does not match")
+
+    @skipIf(oslist=["windows"])
+    @no_debug_info_test
+    def test_breakpoints_auto_deduce_source_map(self):
+        """
+            Test that with target.auto-deduce-source-map settings.
+
+            The "relative.yaml" contains a line table that is:
+
+            Line table for a/b/c/main.cpp in `a.out
+            0x0000000100003f94: a/b/c/main.cpp:1
+            0x0000000100003fb0: a/b/c/main.cpp:2:3
+            0x0000000100003fb8: a/b/c/main.cpp:2:3
+        """
+        src_dir = self.getSourceDir()
+        yaml_path = os.path.join(src_dir, "relative.yaml")
+        yaml_base, ext = os.path.splitext(yaml_path)
+        obj_path = self.getBuildArtifact("a.out")
+        self.yaml2obj(yaml_path, obj_path)
+
+        # Create a target with the object file we just created from YAML
+        target = self.dbg.CreateTarget(obj_path)
+        # We now have debug information with line table paths that start are
+        # "./a/b/c/main.cpp".
+
+        source_map_json = self.get_source_map_json(target)
+        self.assertEquals(len(source_map_json), 0, "source map should be empty initially")
+        self.runCmd("settings set target.auto-deduce-source-map true")
+
+        # Verify auto deduced source map when file path in debug info
+        # is a suffix of request breakpoint file path
+        path = "/x/y/a/b/c/main.cpp"
+        bp = target.BreakpointCreateByLocation(path, 2)
+        self.assertTrue(bp.GetNumLocations() > 0,
+                'Couldn\'t resolve breakpoint using full path "%s" in executate "%s" with '
+                'debug info that has relative path with matching suffix' % (path, self.getBuildArtifact("a.out")))
+
+        source_map_json = self.get_source_map_json(target)
+        self.assertEquals(len(source_map_json), 1, "source map should not be empty")
+        self.verify_source_map_entry_pair(source_map_json[0], ".", "/x/y")
+
+        # Reset source map.
+        self.runCmd("settings clear target.source-map")
+
+        # Verify auto deduced source map when file path of request breakpoint
+        # is a suffix of the file path in debug info.
+        path = "c/main.cpp"
+        bp = target.BreakpointCreateByLocation(path, 2)
+        self.assertTrue(bp.GetNumLocations() > 0,
+                'Couldn\'t resolve breakpoint using full path "%s" in executate "%s" with '
+                'debug info that has relative path with matching suffix' % (path, self.getBuildArtifact("a.out")))
+
+        source_map_json = self.get_source_map_json(target)
+        self.assertEquals(len(source_map_json), 1, "source map should not be empty")
+        self.verify_source_map_entry_pair(source_map_json[0], "a/b", ".")
+
+        # Reset source map.
+        self.runCmd("settings clear target.source-map")
+
+        # Verify source map will not auto deduced when file path of request breakpoint
+        # equals the file path in debug info.
+        path = "a/b/c/main.cpp"
+        bp = target.BreakpointCreateByLocation(path, 2)
+        self.assertTrue(bp.GetNumLocations() > 0,
+                'Couldn\'t resolve breakpoint using full path "%s" in executate "%s" with '
+                'debug info that has relative path with matching suffix' % (path, self.getBuildArtifact("a.out")))
+
+        source_map_json = self.get_source_map_json(target)
+        self.assertEquals(len(source_map_json), 0, "source map should not be deduced")
Index: lldb/source/Target/TargetProperties.td
===================================================================
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -37,6 +37,9 @@
   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 AutoDeduceSourceMap: Property<"auto-deduce-source-map", "Boolean">,
+    DefaultFalse,
+    Desc<"Automatically deduce source path mappings based on source file breakpoint resolution.">;
   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.">;
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -356,7 +356,9 @@
                                       bool hardware,
                                       LazyBool move_to_nearest_code) {
   FileSpec remapped_file;
-  if (!GetSourcePathMap().ReverseRemapPath(file, remapped_file))
+  llvm::Optional<llvm::StringRef> removed_prefix_opt =
+      GetSourcePathMap().ReverseRemapPath(file, remapped_file);
+  if (!removed_prefix_opt)
     remapped_file = file;
 
   if (check_inlines == eLazyBoolCalculate) {
@@ -400,7 +402,7 @@
     return nullptr;
 
   BreakpointResolverSP resolver_sp(new BreakpointResolverFileLine(
-      nullptr, offset, skip_prologue, location_spec));
+      nullptr, offset, skip_prologue, location_spec, removed_prefix_opt, shared_from_this()));
   return CreateBreakpoint(filter_sp, resolver_sp, internal, hardware, true);
 }
 
@@ -4269,6 +4271,12 @@
   return option_value->GetCurrentValue();
 }
 
+bool TargetProperties::GetAutoDeduceSourceMap() const {
+  const uint32_t idx = ePropertyAutoDeduceSourceMap;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+      nullptr, idx, g_target_properties[idx].default_uint_value != 0);
+}
+
 void TargetProperties::AppendExecutableSearchPaths(const FileSpec &dir) {
   const uint32_t idx = ePropertyExecutableSearchPaths;
   OptionValueFileSpecList *option_value =
Index: lldb/source/Target/PathMappingList.cpp
===================================================================
--- lldb/source/Target/PathMappingList.cpp
+++ lldb/source/Target/PathMappingList.cpp
@@ -76,6 +76,16 @@
   }
 }
 
+void PathMappingList::AppendUnique(llvm::StringRef path,
+                                   llvm::StringRef replacement, bool notify) {
+  for (const auto &pair : m_pairs) {
+    if (pair.first.GetStringRef().equals(NormalizePath(path)) &&
+        pair.second.GetStringRef().equals(NormalizePath(replacement)))
+      return;
+  }
+  Append(path, replacement, notify);
+}
+
 void PathMappingList::Insert(llvm::StringRef path, llvm::StringRef replacement,
                              uint32_t index, bool notify) {
   ++m_mod_id;
@@ -208,20 +218,22 @@
   return {};
 }
 
-bool PathMappingList::ReverseRemapPath(const FileSpec &file, FileSpec &fixed) const {
+llvm::Optional<llvm::StringRef>
+PathMappingList::ReverseRemapPath(const FileSpec &file, FileSpec &fixed) const {
   std::string path = file.GetPath();
   llvm::StringRef path_ref(path);
   for (const auto &it : m_pairs) {
+    llvm::StringRef removed_prefix = it.second.GetStringRef();
     if (!path_ref.consume_front(it.second.GetStringRef()))
       continue;
     auto orig_file = it.first.GetStringRef();
-    auto orig_style = FileSpec::GuessPathStyle(orig_file).value_or(
+    auto orig_style = FileSpec::GuessPathStyle(orig_file).getValueOr(
         llvm::sys::path::Style::native);
     fixed.SetFile(orig_file, orig_style);
     AppendPathComponents(fixed, path_ref, orig_style);
-    return true;
+    return removed_prefix;
   }
-  return false;
+  return llvm::None;
 }
 
 llvm::Optional<FileSpec> PathMappingList::FindFile(const FileSpec &orig_spec) const {
Index: lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
===================================================================
--- lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
+++ 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,13 @@
 // BreakpointResolverFileLine:
 BreakpointResolverFileLine::BreakpointResolverFileLine(
     const BreakpointSP &bkpt, lldb::addr_t offset, bool skip_prologue,
-    const SourceLocationSpec &location_spec)
+    const SourceLocationSpec &location_spec,
+    llvm::Optional<llvm::StringRef> removed_prefix_opt, 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),
+      m_removed_prefix_opt(removed_prefix_opt), m_target(target) {
+  m_auto_deduce_source_map = target && target->GetAutoDeduceSourceMap();
+}
 
 BreakpointResolver *BreakpointResolverFileLine::CreateFromStructuredData(
     const BreakpointSP &bkpt, const StructuredData::Dictionary &options_dict,
@@ -187,6 +192,77 @@
   }
 }
 
+void BreakpointResolverFileLine::DeduceSourceMapping(
+    SymbolContextList &sc_list) {
+  if (!m_auto_deduce_source_map || !m_target)
+    return;
+
+  Log *log = GetLog(LLDBLog::Breakpoints);
+  const llvm::StringRef path_separator = llvm::sys::path::get_separator(
+      m_location_spec.GetFileSpec().GetPathStyle());
+  // Check if "b" is a suffix of "a".
+  // And return llvm::None if not or the new path
+  // of "a" after consuming "b" from the back.
+  auto check_suffix =
+      [path_separator](llvm::StringRef a, llvm::StringRef b,
+                       bool case_sensitive) -> llvm::Optional<llvm::StringRef> {
+    if (case_sensitive ? a.consume_back(b) : a.consume_back_insensitive(b)) {
+      if (a.empty() || a.endswith(path_separator)) {
+        return a;
+      }
+    }
+    return llvm::None;
+  };
+
+  const bool case_sensitive = m_location_spec.GetFileSpec().IsCaseSensitive();
+  for (uint32_t i = 0; i < sc_list.GetSize(); ++i) {
+    SymbolContext sc;
+    sc_list.GetContextAtIndex(i, sc);
+
+    FileSpec sc_file = sc.line_entry.file;
+    FileSpec request_file = m_location_spec.GetFileSpec();
+
+    const bool full = !request_file.GetDirectory().IsEmpty();
+    if (FileSpec::Equal(sc_file, request_file, full))
+      continue;
+
+    llvm::StringRef sc_file_dir = sc_file.GetDirectory().GetStringRef();
+    llvm::StringRef request_file_dir =
+        request_file.GetDirectory().GetStringRef();
+
+    llvm::StringRef new_mapping_from;
+    llvm::SmallString<256> new_mapping_to;
+
+    // Adding back any potentially reverse mapping stripped prefix.
+    // for new_mapping_to.
+    if (m_removed_prefix_opt.hasValue())
+      new_mapping_to.append(m_removed_prefix_opt.getValue());
+
+    llvm::Optional<llvm::StringRef> new_mapping_from_opt =
+        check_suffix(sc_file_dir, request_file_dir, case_sensitive);
+    if (new_mapping_from_opt) {
+      new_mapping_from = new_mapping_from_opt.getValue();
+      if (new_mapping_to.empty())
+        new_mapping_to.append(".");
+    } else {
+      llvm::Optional<llvm::StringRef> new_mapping_to_opt =
+          check_suffix(request_file_dir, sc_file_dir, case_sensitive);
+      if (new_mapping_to_opt) {
+        new_mapping_from = ".";
+        new_mapping_to.append(new_mapping_to_opt.getValue());
+      }
+    }
+
+    if (!new_mapping_from.empty() && !new_mapping_to.empty()) {
+      LLDB_LOG(log, "generating auto source map from {0} to {1}",
+               new_mapping_from, new_mapping_to);
+      m_target->GetSourcePathMap().AppendUnique(new_mapping_from,
+                                                new_mapping_to,
+                                                /*notify*/ true);
+    }
+  }
+}
+
 Searcher::CallbackReturn BreakpointResolverFileLine::SearchCallback(
     SearchFilter &filter, SymbolContext &context, Address *addr) {
   SymbolContextList sc_list;
@@ -222,6 +298,9 @@
 
   FilterContexts(sc_list);
 
+  if (m_auto_deduce_source_map)
+    DeduceSourceMapping(sc_list);
+
   StreamString s;
   s.Printf("for %s:%d ",
            m_location_spec.GetFileSpec().GetFilename().AsCString("<Unknown>"),
Index: lldb/include/lldb/Target/Target.h
===================================================================
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -141,6 +141,8 @@
 
   PathMappingList &GetSourcePathMap() const;
 
+  bool GetAutoDeduceSourceMap() const;
+
   FileSpecList GetExecutableSearchPaths();
 
   void AppendExecutableSearchPaths(const FileSpec &);
Index: lldb/include/lldb/Target/PathMappingList.h
===================================================================
--- lldb/include/lldb/Target/PathMappingList.h
+++ lldb/include/lldb/Target/PathMappingList.h
@@ -37,6 +37,9 @@
 
   void Append(const PathMappingList &rhs, bool notify);
 
+  void AppendUnique(llvm::StringRef path, llvm::StringRef replacement,
+                    bool notify);
+
   void Clear(bool notify);
 
   // By default, dump all pairs.
@@ -88,7 +91,22 @@
                                      bool only_if_exists = false) const;
   bool RemapPath(const char *, std::string &) const = delete;
 
-  bool ReverseRemapPath(const FileSpec &file, FileSpec &fixed) const;
+  /// Perform reverse source path remap for input \a file.
+  /// Source maps contains a list of <from_original_path, to_new_path> mappings.
+  /// Reverse remap means locating a matching entry prefix using "to_new_path"
+  /// part and replacing it with "from_original_path" part if found.
+  ///
+  /// \param[in] file
+  ///     The source path to reverse remap.
+  /// \param[in] fixed
+  ///     The reversed mapped new path.
+  ///
+  /// \return
+  ///     llvm::None if no remapping happens, otherwise, the matching source map
+  ///     entry's ""to_new_pathto"" part (which is the prefix of \a file) is
+  ///     returned.
+  llvm::Optional<llvm::StringRef> ReverseRemapPath(const FileSpec &file,
+                                                   FileSpec &fixed) const;
 
   /// Finds a source file given a file spec using the path remappings.
   ///
Index: lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
===================================================================
--- lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
+++ lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
@@ -21,9 +21,11 @@
 
 class BreakpointResolverFileLine : public BreakpointResolver {
 public:
-  BreakpointResolverFileLine(const lldb::BreakpointSP &bkpt,
-                             lldb::addr_t offset, bool skip_prologue,
-                             const SourceLocationSpec &location_spec);
+  BreakpointResolverFileLine(
+      const lldb::BreakpointSP &bkpt, lldb::addr_t offset, bool skip_prologue,
+      const SourceLocationSpec &location_spec,
+      llvm::Optional<llvm::StringRef> removed_prefix_opt = llvm::None,
+      lldb::TargetSP target = nullptr);
 
   static BreakpointResolver *
   CreateFromStructuredData(const lldb::BreakpointSP &bkpt,
@@ -57,10 +59,16 @@
 
 protected:
   void FilterContexts(SymbolContextList &sc_list);
+  void DeduceSourceMapping(SymbolContextList &sc_list);
 
   friend class Breakpoint;
   SourceLocationSpec m_location_spec;
   bool m_skip_prologue;
+  // Any previously removed file path prefix by reverse source mapping.
+  // This is used to auto deduce source map if needed.
+  llvm::Optional<llvm::StringRef> m_removed_prefix_opt;
+  lldb::TargetSP m_target = nullptr;
+  bool m_auto_deduce_source_map = false;
 
 private:
   BreakpointResolverFileLine(const BreakpointResolverFileLine &) = delete;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to