diff options
author | royitaqi <royitaqi@users.noreply.github.com> | 2024-08-15 11:26:24 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-08-15 11:26:24 -0700 |
commit | 47721d46187f89c12a13d07b5857496301cf5d6e (patch) | |
tree | 948ec5d1e574b63e8b71226807874ffefe4a8bd6 /lldb/source | |
parent | 52337d5f9d108f04b2ed06069b21a255c232dc1f (diff) | |
download | llvm-47721d46187f89c12a13d07b5857496301cf5d6e.zip llvm-47721d46187f89c12a13d07b5857496301cf5d6e.tar.gz llvm-47721d46187f89c12a13d07b5857496301cf5d6e.tar.bz2 |
[lldb] Realpath symlinks for breakpoints (#102223)
Improve the chance of resolving file/line breakpoints by realpath'ing the support files before doing a second match attempt, with some conditions applied.
A working [hello-world example](https://github.com/royitaqi/lldb_demos/blob/main/realpath/README.md).
See [patch](https://github.com/llvm/llvm-project/pull/102223) for more info about problem/motivation, details of the feature, new settings, telemetries and tests.
Diffstat (limited to 'lldb/source')
-rw-r--r-- | lldb/source/Breakpoint/BreakpointResolverFileLine.cpp | 12 | ||||
-rw-r--r-- | lldb/source/Symbol/CompileUnit.cpp | 14 | ||||
-rw-r--r-- | lldb/source/Target/Statistics.cpp | 12 | ||||
-rw-r--r-- | lldb/source/Target/Target.cpp | 8 | ||||
-rw-r--r-- | lldb/source/Target/TargetProperties.td | 3 | ||||
-rw-r--r-- | lldb/source/Utility/CMakeLists.txt | 1 | ||||
-rw-r--r-- | lldb/source/Utility/FileSpecList.cpp | 108 | ||||
-rw-r--r-- | lldb/source/Utility/RealpathPrefixes.cpp | 70 |
8 files changed, 186 insertions, 42 deletions
diff --git a/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp b/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp index 16c4ee1..5087540 100644 --- a/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp +++ b/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp @@ -15,6 +15,7 @@ #include "lldb/Target/Target.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" +#include "lldb/Utility/RealpathPrefixes.h" #include "lldb/Utility/StreamString.h" #include <optional> @@ -290,16 +291,25 @@ Searcher::CallbackReturn BreakpointResolverFileLine::SearchCallback( const uint32_t line = m_location_spec.GetLine().value_or(0); const std::optional<uint16_t> column = m_location_spec.GetColumn(); + Target &target = GetBreakpoint()->GetTarget(); + RealpathPrefixes realpath_prefixes = target.GetSourceRealpathPrefixes(); + 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_location_spec, eSymbolContextEverything, - sc_list); + sc_list, &realpath_prefixes); } } + // Gather stats into the Target + target.GetStatistics().IncreaseSourceRealpathAttemptCount( + realpath_prefixes.GetSourceRealpathAttemptCount()); + target.GetStatistics().IncreaseSourceRealpathCompatibleCount( + realpath_prefixes.GetSourceRealpathCompatibleCount()); + FilterContexts(sc_list); DeduceSourceMapping(sc_list); diff --git a/lldb/source/Symbol/CompileUnit.cpp b/lldb/source/Symbol/CompileUnit.cpp index ddeacf18..db8f8ce 100644 --- a/lldb/source/Symbol/CompileUnit.cpp +++ b/lldb/source/Symbol/CompileUnit.cpp @@ -213,11 +213,12 @@ VariableListSP CompileUnit::GetVariableList(bool can_create) { return m_variables; } -std::vector<uint32_t> FindFileIndexes(const SupportFileList &files, - const FileSpec &file) { +std::vector<uint32_t> +FindFileIndexes(const SupportFileList &files, const FileSpec &file, + RealpathPrefixes *realpath_prefixes = nullptr) { std::vector<uint32_t> result; uint32_t idx = -1; - while ((idx = files.FindCompatibleIndex(idx + 1, file)) != + while ((idx = files.FindCompatibleIndex(idx + 1, file, realpath_prefixes)) != UINT32_MAX) result.push_back(idx); return result; @@ -247,7 +248,8 @@ uint32_t CompileUnit::FindLineEntry(uint32_t start_idx, uint32_t line, void CompileUnit::ResolveSymbolContext( const SourceLocationSpec &src_location_spec, - SymbolContextItem resolve_scope, SymbolContextList &sc_list) { + SymbolContextItem resolve_scope, SymbolContextList &sc_list, + RealpathPrefixes *realpath_prefixes) { const FileSpec file_spec = src_location_spec.GetFileSpec(); const uint32_t line = src_location_spec.GetLine().value_or(0); const bool check_inlines = src_location_spec.GetCheckInlines(); @@ -275,8 +277,8 @@ void CompileUnit::ResolveSymbolContext( return; } - std::vector<uint32_t> file_indexes = FindFileIndexes(GetSupportFiles(), - file_spec); + std::vector<uint32_t> file_indexes = + FindFileIndexes(GetSupportFiles(), file_spec, realpath_prefixes); const size_t num_file_indexes = file_indexes.size(); if (num_file_indexes == 0) return; diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp index 583d152..390e04c 100644 --- a/lldb/source/Target/Statistics.cpp +++ b/lldb/source/Target/Statistics.cpp @@ -192,6 +192,10 @@ TargetStats::ToJSON(Target &target, } target_metrics_json.try_emplace("sourceMapDeduceCount", m_source_map_deduce_count); + target_metrics_json.try_emplace("sourceRealpathAttemptCount", + m_source_realpath_attempt_count); + target_metrics_json.try_emplace("sourceRealpathCompatibleCount", + m_source_realpath_compatible_count); return target_metrics_json; } @@ -220,6 +224,14 @@ void TargetStats::IncreaseSourceMapDeduceCount() { ++m_source_map_deduce_count; } +void TargetStats::IncreaseSourceRealpathAttemptCount(uint32_t count) { + m_source_realpath_attempt_count += count; +} + +void TargetStats::IncreaseSourceRealpathCompatibleCount(uint32_t count) { + m_source_realpath_compatible_count += count; +} + bool DebuggerStats::g_collecting_stats = false; llvm::json::Value DebuggerStats::ReportStatistics( diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 129683c..5a5d689 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -60,6 +60,7 @@ #include "lldb/Utility/LLDBAssert.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" +#include "lldb/Utility/RealpathPrefixes.h" #include "lldb/Utility/State.h" #include "lldb/Utility/StreamString.h" #include "lldb/Utility/Timer.h" @@ -4354,6 +4355,13 @@ InlineStrategy TargetProperties::GetInlineStrategy() const { static_cast<InlineStrategy>(g_target_properties[idx].default_uint_value)); } +// Returning RealpathPrefixes, but the setting's type is FileSpecList. We do +// this because we want the FileSpecList to normalize the file paths for us. +RealpathPrefixes TargetProperties::GetSourceRealpathPrefixes() const { + const uint32_t idx = ePropertySourceRealpathPrefixes; + return RealpathPrefixes(GetPropertyAtIndexAs<FileSpecList>(idx, {})); +} + llvm::StringRef TargetProperties::GetArg0() const { const uint32_t idx = ePropertyArg0; return GetPropertyAtIndexAs<llvm::StringRef>( diff --git a/lldb/source/Target/TargetProperties.td b/lldb/source/Target/TargetProperties.td index ef53867..421252a 100644 --- a/lldb/source/Target/TargetProperties.td +++ b/lldb/source/Target/TargetProperties.td @@ -150,6 +150,9 @@ let Definition = "target" in { DefaultEnumValue<"eInlineBreakpointsAlways">, EnumValues<"OptionEnumValues(g_inline_breakpoint_enums)">, Desc<"The strategy to use when settings breakpoints by file and line. Breakpoint locations can end up being inlined by the compiler, so that a compile unit 'a.c' might contain an inlined function from another source file. Usually this is limited to breakpoint locations from inlined functions from header or other include files, or more accurately non-implementation source files. Sometimes code might #include implementation files and cause inlined breakpoint locations in inlined implementation files. Always checking for inlined breakpoint locations can be expensive (memory and time), so if you have a project with many headers and find that setting breakpoints is slow, then you can change this setting to headers. This setting allows you to control exactly which strategy is used when setting file and line breakpoints.">; + def SourceRealpathPrefixes: Property<"source-realpath-prefixes", "FileSpecList">, + DefaultStringValue<"">, + Desc<"Realpath any source paths that start with one of these prefixes. If the debug info contains symlinks which match the original source file's basename but don't match its location that the user will use to set breakpoints, then this setting can help resolve breakpoints correctly. This handles both symlinked files and directories. Wild card prefixes: An empty string matches all paths. A forward slash matches absolute paths.">; def DisassemblyFlavor: Property<"x86-disassembly-flavor", "Enum">, DefaultEnumValue<"eX86DisFlavorDefault">, EnumValues<"OptionEnumValues(g_x86_dis_flavor_value_types)">, diff --git a/lldb/source/Utility/CMakeLists.txt b/lldb/source/Utility/CMakeLists.txt index e9954d6..397db0e 100644 --- a/lldb/source/Utility/CMakeLists.txt +++ b/lldb/source/Utility/CMakeLists.txt @@ -51,6 +51,7 @@ add_lldb_library(lldbUtility NO_INTERNAL_DEPENDENCIES Log.cpp NameMatches.cpp ProcessInfo.cpp + RealpathPrefixes.cpp RegisterValue.cpp RegularExpression.cpp Instrumentation.cpp diff --git a/lldb/source/Utility/FileSpecList.cpp b/lldb/source/Utility/FileSpecList.cpp index 7647e04..5852367 100644 --- a/lldb/source/Utility/FileSpecList.cpp +++ b/lldb/source/Utility/FileSpecList.cpp @@ -7,7 +7,12 @@ //===----------------------------------------------------------------------===// #include "lldb/Utility/FileSpecList.h" +#include "lldb/Target/Statistics.h" +#include "lldb/Target/Target.h" #include "lldb/Utility/ConstString.h" +#include "lldb/Utility/LLDBLog.h" +#include "lldb/Utility/Log.h" +#include "lldb/Utility/RealpathPrefixes.h" #include "lldb/Utility/Stream.h" #include <cstdint> @@ -108,52 +113,85 @@ size_t SupportFileList::FindFileIndex(size_t start_idx, }); } -size_t SupportFileList::FindCompatibleIndex(size_t start_idx, - const FileSpec &file_spec) const { - const size_t num_files = m_files.size(); - if (start_idx >= num_files) - return UINT32_MAX; +enum IsCompatibleResult { + kNoMatch = 0, + kOnlyFileMatch = 1, + kBothDirectoryAndFileMatch = 2, +}; +IsCompatibleResult IsCompatible(const FileSpec &curr_file, + const FileSpec &file_spec) { const bool file_spec_relative = file_spec.IsRelative(); const bool file_spec_case_sensitive = file_spec.IsCaseSensitive(); // When looking for files, we will compare only the filename if the directory // argument is empty in file_spec const bool full = !file_spec.GetDirectory().IsEmpty(); + // Always start by matching the filename first + if (!curr_file.FileEquals(file_spec)) + return IsCompatibleResult::kNoMatch; + + // Only compare the full name if the we were asked to and if the current + // file entry has a directory. If it doesn't have a directory then we only + // compare the filename. + if (FileSpec::Equal(curr_file, file_spec, full)) { + return IsCompatibleResult::kBothDirectoryAndFileMatch; + } else if (curr_file.IsRelative() || file_spec_relative) { + llvm::StringRef curr_file_dir = curr_file.GetDirectory().GetStringRef(); + if (curr_file_dir.empty()) + // Basename match only for this file in the list + return IsCompatibleResult::kBothDirectoryAndFileMatch; + + // Check if we have a relative path in our file list, or if "file_spec" is + // relative, if so, check if either ends with the other. + llvm::StringRef file_spec_dir = file_spec.GetDirectory().GetStringRef(); + // We have a relative path in our file list, it matches if the + // specified path ends with this path, but we must ensure the full + // component matches (we don't want "foo/bar.cpp" to match "oo/bar.cpp"). + auto is_suffix = [](llvm::StringRef a, llvm::StringRef b, + bool case_sensitive) -> bool { + if (case_sensitive ? a.consume_back(b) : a.consume_back_insensitive(b)) + return a.empty() || a.ends_with("/"); + return false; + }; + const bool case_sensitive = + file_spec_case_sensitive || curr_file.IsCaseSensitive(); + if (is_suffix(curr_file_dir, file_spec_dir, case_sensitive) || + is_suffix(file_spec_dir, curr_file_dir, case_sensitive)) + return IsCompatibleResult::kBothDirectoryAndFileMatch; + } + return IsCompatibleResult::kOnlyFileMatch; +} + +size_t SupportFileList::FindCompatibleIndex( + size_t start_idx, const FileSpec &file_spec, + RealpathPrefixes *realpath_prefixes) const { + const size_t num_files = m_files.size(); + if (start_idx >= num_files) + return UINT32_MAX; + for (size_t idx = start_idx; idx < num_files; ++idx) { const FileSpec &curr_file = m_files[idx]->GetSpecOnly(); - // Always start by matching the filename first - if (!curr_file.FileEquals(file_spec)) - continue; - - // Only compare the full name if the we were asked to and if the current - // file entry has the a directory. If it doesn't have a directory then we - // only compare the filename. - if (FileSpec::Equal(curr_file, file_spec, full)) { + IsCompatibleResult result = IsCompatible(curr_file, file_spec); + if (result == IsCompatibleResult::kBothDirectoryAndFileMatch) return idx; - } else if (curr_file.IsRelative() || file_spec_relative) { - llvm::StringRef curr_file_dir = curr_file.GetDirectory().GetStringRef(); - if (curr_file_dir.empty()) - return idx; // Basename match only for this file in the list - - // Check if we have a relative path in our file list, or if "file_spec" is - // relative, if so, check if either ends with the other. - llvm::StringRef file_spec_dir = file_spec.GetDirectory().GetStringRef(); - // We have a relative path in our file list, it matches if the - // specified path ends with this path, but we must ensure the full - // component matches (we don't want "foo/bar.cpp" to match "oo/bar.cpp"). - auto is_suffix = [](llvm::StringRef a, llvm::StringRef b, - bool case_sensitive) -> bool { - if (case_sensitive ? a.consume_back(b) : a.consume_back_insensitive(b)) - return a.empty() || a.ends_with("/"); - return false; - }; - const bool case_sensitive = - file_spec_case_sensitive || curr_file.IsCaseSensitive(); - if (is_suffix(curr_file_dir, file_spec_dir, case_sensitive) || - is_suffix(file_spec_dir, curr_file_dir, case_sensitive)) - return idx; + + if (realpath_prefixes && result == IsCompatibleResult::kOnlyFileMatch) { + if (std::optional<FileSpec> resolved_curr_file = + realpath_prefixes->ResolveSymlinks(curr_file)) { + if (IsCompatible(*resolved_curr_file, file_spec) == + IsCompatibleResult::kBothDirectoryAndFileMatch) { + // Stats and logging. + realpath_prefixes->IncreaseSourceRealpathCompatibleCount(); + Log *log = GetLog(LLDBLog::Source); + LLDB_LOGF(log, + "Realpath'ed support file %s is compatible to input file", + resolved_curr_file->GetPath().c_str()); + // We found a match + return idx; + } + } } } diff --git a/lldb/source/Utility/RealpathPrefixes.cpp b/lldb/source/Utility/RealpathPrefixes.cpp new file mode 100644 index 0000000..14c81ee --- /dev/null +++ b/lldb/source/Utility/RealpathPrefixes.cpp @@ -0,0 +1,70 @@ +//===-- RealpathPrefixes.cpp ----------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Utility/RealpathPrefixes.h" + +#include "lldb/Utility/FileSpec.h" +#include "lldb/Utility/FileSpecList.h" +#include "lldb/Utility/LLDBLog.h" +#include "lldb/Utility/Log.h" + +using namespace lldb_private; + +RealpathPrefixes::RealpathPrefixes( + const FileSpecList &file_spec_list, + llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs) + : m_fs(fs) { + m_prefixes.reserve(file_spec_list.GetSize()); + for (const FileSpec &file_spec : file_spec_list) { + m_prefixes.emplace_back(file_spec.GetPath()); + } +} + +std::optional<FileSpec> +RealpathPrefixes::ResolveSymlinks(const FileSpec &file_spec) { + if (m_prefixes.empty()) + return std::nullopt; + + // Test if `b` is a *path* prefix of `a` (not just *string* prefix). + // E.g. "/foo/bar" is a path prefix of "/foo/bar/baz" but not "/foo/barbaz". + auto is_path_prefix = [](llvm::StringRef a, llvm::StringRef b, + bool case_sensitive, + llvm::sys::path::Style style) -> bool { + if (case_sensitive ? a.consume_front(b) : a.consume_front_insensitive(b)) + // If `b` isn't "/", then it won't end with "/" because it comes from + // `FileSpec`. After `a` consumes `b`, `a` should either be empty (i.e. + // `a` == `b`) or end with "/" (the remainder of `a` is a subdirectory). + return b == "/" || a.empty() || + llvm::sys::path::is_separator(a[0], style); + return false; + }; + std::string file_spec_path = file_spec.GetPath(); + for (const std::string &prefix : m_prefixes) { + if (is_path_prefix(file_spec_path, prefix, file_spec.IsCaseSensitive(), + file_spec.GetPathStyle())) { + // Stats and logging. + IncreaseSourceRealpathAttemptCount(); + Log *log = GetLog(LLDBLog::Source); + LLDB_LOGF(log, "Realpath'ing support file %s", file_spec_path.c_str()); + + // One prefix matched. Try to realpath. + llvm::SmallString<PATH_MAX> buff; + std::error_code ec = m_fs->getRealPath(file_spec_path, buff); + if (ec) + return std::nullopt; + FileSpec realpath(buff, file_spec.GetPathStyle()); + + // Only return realpath if it is different from the original file_spec. + if (realpath != file_spec) + return realpath; + return std::nullopt; + } + } + // No prefix matched + return std::nullopt; +} |