yinghuitan updated this revision to Diff 460270.
yinghuitan added a comment.
Address review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133042/new/
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,69 @@
bp_3 = target.FindBreakpointByID(bp_id_3)
self.assertFalse(bp_3.IsValid(), "Didn't delete disabled breakpoint 3")
+
+
+ def get_source_map_json(self):
+ stream = lldb.SBStream()
+ self.dbg.GetSetting("target.source-map").GetAsJSON(stream)
+ return json.loads(stream.GetData())
+
+ def verify_source_map_entry_pair(self, entry, original, replacement):
+ self.assertEquals(entry[0], original,
+ "source map entry 'original' does not match")
+ self.assertEquals(entry[1], replacement,
+ "source map entry 'replacement' 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()
+ 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()
+ 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 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()
+ 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. It only deduces source mapping if source file breakpoint request is using full path.">;
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));
return CreateBreakpoint(filter_sp, resolver_sp, internal, hardware, true);
}
@@ -4270,6 +4272,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,18 @@
}
}
+void PathMappingList::AppendUnique(llvm::StringRef path,
+ llvm::StringRef replacement, bool notify) {
+ auto normalized_path = NormalizePath(path);
+ auto normalized_replacement = NormalizePath(replacement);
+ for (const auto &pair : m_pairs) {
+ if (pair.first.GetStringRef().equals(normalized_path) &&
+ pair.second.GetStringRef().equals(normalized_replacement))
+ return;
+ }
+ Append(path, replacement, notify);
+}
+
void PathMappingList::Insert(llvm::StringRef path, llvm::StringRef replacement,
uint32_t index, bool notify) {
++m_mod_id;
@@ -207,20 +219,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,11 @@
// 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)
: 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) {}
BreakpointResolver *BreakpointResolverFileLine::CreateFromStructuredData(
const BreakpointSP &bkpt, const StructuredData::Dictionary &options_dict,
@@ -187,6 +190,85 @@
}
}
+void BreakpointResolverFileLine::DeduceSourceMapping(
+ SymbolContextList &sc_list) {
+ Target &target = GetBreakpoint()->GetTarget();
+ if (!target.GetAutoDeduceSourceMap())
+ 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;
+ };
+
+ FileSpec request_file = m_location_spec.GetFileSpec();
+
+ // Only auto deduce source map if breakpoint is full path.
+ // Note: an existing source map reverse mapping (m_removed_prefix_opt has
+ // value) may make request_file relative.
+ if (!m_removed_prefix_opt.has_value() && request_file.IsRelative())
+ return;
+
+ const bool case_sensitive = request_file.IsCaseSensitive();
+ const bool full = !request_file.GetDirectory().IsEmpty();
+ for (uint32_t i = 0; i < sc_list.GetSize(); ++i) {
+ SymbolContext sc;
+ sc_list.GetContextAtIndex(i, sc);
+
+ FileSpec sc_file = sc.line_entry.file;
+
+ 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())
+ llvm::sys::path::append(new_mapping_to, 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 = ".";
+ } 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 = ".";
+ llvm::sys::path::append(new_mapping_to, ".",
+ 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);
+ 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 +304,9 @@
FilterContexts(sc_list);
+ if (GetBreakpoint()->GetTarget().GetAutoDeduceSourceMap())
+ 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,10 @@
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);
static BreakpointResolver *
CreateFromStructuredData(const lldb::BreakpointSP &bkpt,
@@ -57,10 +58,14 @@
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;
private:
BreakpointResolverFileLine(const BreakpointResolverFileLine &) = delete;
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
