[Lldb-commits] [lldb] Realpath symlinks for breakpoints (PR #102223)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Aug 6 13:58:42 PDT 2024
https://github.com/royitaqi created https://github.com/llvm/llvm-project/pull/102223
# TL;DR
Improve the chance of resolving file/line breakpoints by, as a second matching attempt, realpath'ing the support files who has a matching basename and whose path matches one of the prefixes in a setting.
Telemetries and tests are included.
# Problem
In some development environment, some source files are symlinked for some reasons, then the symlink files are used in the build. This causes the debug info to contain the path to the symlink files, not the original source files. However, during debugging, the IDE opens and uses the original source file to set breakpoints. The result is a mismatch between the source file's real path v.s. the symlink's path, causing file/line breakpoints to be unresolved for these files.
# Feature
After the normal file/path matching attempt fails, if the basename of the files match yet the directories don't AND the support file's path matches of the prefixes in the setting (see below), realpath the support file, then reattempt the match.
# Setting
A list of prefixes, which the support file's path has to match in order to be realpath'ed. The default is an empty list, which will cost nothing.
# Telemetries
are added. They can be obtained as a part of the Target's Statistics (e.g. `statistics dump`).
1. The number of realpath attempts made.
2. The number of times such realpath'ing help found a match.
# Tests
are added:
```
ninja lldb-unit-test-deps
tools/lldb/unittests/Utility/UtilityTests --gtest_filter="SupportFileList*"
tools/lldb/unittests/Utility/UtilityTests --gtest_filter="RealpathPrefixes*"
```
>From c629defe38a510ffdd23122b2e9cfac68933cf34 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Fri, 2 Aug 2024 06:35:45 -0700
Subject: [PATCH 1/4] [lldb] Allow realpathing support files when resolving
file/line breakpoints
Differential Revision: https://phabricator.intern.facebook.com/D60317677
---
lldb/include/lldb/Symbol/CompileUnit.h | 10 +-
lldb/include/lldb/Target/Target.h | 4 +
lldb/include/lldb/Utility/FileSpecList.h | 9 +-
lldb/include/lldb/Utility/RealpathPrefixes.h | 55 +++++++++++
.../Breakpoint/BreakpointResolverFileLine.cpp | 6 +-
lldb/source/Symbol/CompileUnit.cpp | 14 +--
lldb/source/Target/Target.cpp | 8 ++
lldb/source/Target/TargetProperties.td | 3 +
lldb/source/Utility/CMakeLists.txt | 1 +
lldb/source/Utility/FileSpecList.cpp | 95 ++++++++++++-------
lldb/source/Utility/RealpathPrefixes.cpp | 57 +++++++++++
11 files changed, 216 insertions(+), 46 deletions(-)
create mode 100644 lldb/include/lldb/Utility/RealpathPrefixes.h
create mode 100644 lldb/source/Utility/RealpathPrefixes.cpp
diff --git a/lldb/include/lldb/Symbol/CompileUnit.h b/lldb/include/lldb/Symbol/CompileUnit.h
index c20a37e3283075..6a9ef3e6c75bcb 100644
--- a/lldb/include/lldb/Symbol/CompileUnit.h
+++ b/lldb/include/lldb/Symbol/CompileUnit.h
@@ -24,6 +24,8 @@
#include "llvm/ADT/DenseSet.h"
namespace lldb_private {
+class RealpathPrefixes;
+
/// \class CompileUnit CompileUnit.h "lldb/Symbol/CompileUnit.h"
/// A class that describes a compilation unit.
///
@@ -390,9 +392,11 @@ class CompileUnit : public std::enable_shared_from_this<CompileUnit>,
/// entries appended to.
///
/// \see enum SymbolContext::Scope
- void ResolveSymbolContext(const SourceLocationSpec &src_location_spec,
- lldb::SymbolContextItem resolve_scope,
- SymbolContextList &sc_list);
+ void
+ ResolveSymbolContext(const SourceLocationSpec &src_location_spec,
+ lldb::SymbolContextItem resolve_scope,
+ SymbolContextList &sc_list,
+ const RealpathPrefixes *realpath_prefixes = nullptr);
/// Get whether compiler optimizations were enabled for this compile unit
///
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 119dff4d498199..d6d5f05e9258f2 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -39,6 +39,8 @@
namespace lldb_private {
+class RealpathPrefixes;
+
OptionEnumValues GetDynamicValueTypes();
enum InlineStrategy {
@@ -117,6 +119,8 @@ class TargetProperties : public Properties {
InlineStrategy GetInlineStrategy() const;
+ RealpathPrefixes GetSourceRealpathPrefixes() const;
+
llvm::StringRef GetArg0() const;
void SetArg0(llvm::StringRef arg);
diff --git a/lldb/include/lldb/Utility/FileSpecList.h b/lldb/include/lldb/Utility/FileSpecList.h
index 6eb3bb9971f13a..99a99a45a1dad2 100644
--- a/lldb/include/lldb/Utility/FileSpecList.h
+++ b/lldb/include/lldb/Utility/FileSpecList.h
@@ -17,6 +17,7 @@
#include <vector>
namespace lldb_private {
+class RealpathPrefixes;
class Stream;
/// A list of support files for a CompileUnit.
@@ -64,10 +65,16 @@ class SupportFileList {
/// \param[in] file
/// The file specification to search for.
///
+ /// \param[in] realpath_prefixes
+ /// Paths that start with one of the prefixes in this list will be
+ /// realpath'ed to resolve any symlinks.
+ ///
/// \return
/// The index of the file that matches \a file if it is found,
/// else UINT32_MAX is returned.
- size_t FindCompatibleIndex(size_t idx, const FileSpec &file) const;
+ size_t FindCompatibleIndex(
+ size_t idx, const FileSpec &file,
+ const RealpathPrefixes *realpath_prefixes = nullptr) const;
template <class... Args> void EmplaceBack(Args &&...args) {
m_files.push_back(
diff --git a/lldb/include/lldb/Utility/RealpathPrefixes.h b/lldb/include/lldb/Utility/RealpathPrefixes.h
new file mode 100644
index 00000000000000..ba39259980c067
--- /dev/null
+++ b/lldb/include/lldb/Utility/RealpathPrefixes.h
@@ -0,0 +1,55 @@
+//===-- RealpathPrefixes.h --------------------------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_CORE_REALPATHPREFIXES_H
+#define LLDB_CORE_REALPATHPREFIXES_H
+
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/Support/VirtualFileSystem.h"
+
+#include <optional>
+#include <string>
+#include <vector>
+
+namespace lldb_private {
+class FileSpec;
+class FileSpecList;
+} // namespace lldb_private
+
+namespace lldb_private {
+
+class RealpathPrefixes {
+public:
+ // Prefixes are obtained from FileSpecList, through FileSpec::GetPath(), which
+ // ensures that the paths are normalized. For example:
+ // "./foo/.." -> ""
+ // "./foo/../bar" -> "bar"
+ explicit RealpathPrefixes(const FileSpecList &file_spec_list);
+
+ // Sets an optional filesystem to use for realpath'ing. If not set, the real
+ // filesystem will be used.
+ void SetFileSystem(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs);
+
+ std::optional<FileSpec> ResolveSymlinks(const FileSpec &file_spec) const;
+
+private:
+ // Paths that start with one of the prefixes in this list will be realpath'ed
+ // to resolve any symlinks.
+ //
+ // Wildcard prefixes:
+ // - "" (empty string) will match all paths.
+ // - "/" will match all absolute paths.
+ std::vector<std::string> m_prefixes;
+
+ // The filesystem to use for realpath'ing.
+ llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> m_fs;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_CORE_REALPATHPREFIXES_H
diff --git a/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp b/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
index 16c4ee1b88d162..315e017350d80b 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,13 +291,16 @@ 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();
+ RealpathPrefixes realpath_prefixes =
+ GetBreakpoint()->GetTarget().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);
}
}
diff --git a/lldb/source/Symbol/CompileUnit.cpp b/lldb/source/Symbol/CompileUnit.cpp
index ddeacf18e855ee..f60745cb2e38e4 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,
+ const 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,
+ const 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/Target.cpp b/lldb/source/Target/Target.cpp
index 129683c43f0c1a..5a5d689e03fbc0 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 ef538678670fea..d037293c5a09f4 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 that don't match the original source file locations that the user will use to set breakpoints, then this setting can help resolve breakpoints correctly.">;
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 e9954d66cd1a52..397db0e8976023 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 7647e04a820451..c43659c95a2d09 100644
--- a/lldb/source/Utility/FileSpecList.cpp
+++ b/lldb/source/Utility/FileSpecList.cpp
@@ -8,6 +8,7 @@
#include "lldb/Utility/FileSpecList.h"
#include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/RealpathPrefixes.h"
#include "lldb/Utility/Stream.h"
#include <cstdint>
@@ -108,52 +109,76 @@ 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,
+ kCompatible = 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 the 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::kCompatible;
+ } else if (curr_file.IsRelative() || file_spec_relative) {
+ llvm::StringRef curr_file_dir = curr_file.GetDirectory().GetStringRef();
+ if (curr_file_dir.empty())
+ return IsCompatibleResult::kCompatible; // 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 IsCompatibleResult::kCompatible;
+ }
+ return IsCompatibleResult::kOnlyFileMatch;
+}
+
+size_t SupportFileList::FindCompatibleIndex(
+ size_t start_idx, const FileSpec &file_spec,
+ const 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::kCompatible)
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))
+ return idx;
+ }
}
}
diff --git a/lldb/source/Utility/RealpathPrefixes.cpp b/lldb/source/Utility/RealpathPrefixes.cpp
new file mode 100644
index 00000000000000..a54145981061ed
--- /dev/null
+++ b/lldb/source/Utility/RealpathPrefixes.cpp
@@ -0,0 +1,57 @@
+//===-- 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/Host/FileSystem.h"
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/FileSpecList.h"
+
+using namespace lldb_private;
+
+RealpathPrefixes::RealpathPrefixes(const FileSpecList &file_spec_list)
+ : m_fs(llvm::vfs::getRealFileSystem()) {
+ m_prefixes.reserve(file_spec_list.GetSize());
+ for (const FileSpec &file_spec : file_spec_list) {
+ m_prefixes.emplace_back(file_spec.GetPath());
+ }
+}
+
+void RealpathPrefixes::SetFileSystem(
+ llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs) {
+ m_fs = fs;
+}
+
+std::optional<FileSpec>
+RealpathPrefixes::ResolveSymlinks(const FileSpec &file_spec) const {
+ if (m_prefixes.empty())
+ return std::nullopt;
+
+ auto is_prefix = [](llvm::StringRef a, llvm::StringRef b,
+ bool case_sensitive) -> bool {
+ return case_sensitive ? a.consume_front(b) : a.consume_front_insensitive(b);
+ };
+ std::string file_spec_path = file_spec.GetPath();
+ for (const std::string &prefix : m_prefixes) {
+ if (is_prefix(file_spec_path, prefix, file_spec.IsCaseSensitive())) {
+ // 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);
+
+ // 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;
+}
>From 8de590f656fc2b8ed3cb4348fde2a9b243acbad4 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Fri, 2 Aug 2024 06:37:28 -0700
Subject: [PATCH 2/4] Add unit tests
Summary:
Test Plan:
Reviewers:
Subscribers:
Tasks:
Tags:
Differential Revision: https://phabricator.intern.facebook.com/D60668386
---
lldb/unittests/Core/CMakeLists.txt | 1 -
lldb/unittests/Utility/CMakeLists.txt | 2 +
.../{Core => Utility}/FileSpecListTest.cpp | 130 ++++++++++++++++
.../unittests/Utility/MockSymlinkFileSystem.h | 63 ++++++++
.../Utility/RealpathPrefixesTest.cpp | 146 ++++++++++++++++++
5 files changed, 341 insertions(+), 1 deletion(-)
rename lldb/unittests/{Core => Utility}/FileSpecListTest.cpp (57%)
create mode 100644 lldb/unittests/Utility/MockSymlinkFileSystem.h
create mode 100644 lldb/unittests/Utility/RealpathPrefixesTest.cpp
diff --git a/lldb/unittests/Core/CMakeLists.txt b/lldb/unittests/Core/CMakeLists.txt
index d40c357e3f463b..949963fd403463 100644
--- a/lldb/unittests/Core/CMakeLists.txt
+++ b/lldb/unittests/Core/CMakeLists.txt
@@ -3,7 +3,6 @@ add_lldb_unittest(LLDBCoreTests
DiagnosticEventTest.cpp
DumpDataExtractorTest.cpp
DumpRegisterInfoTest.cpp
- FileSpecListTest.cpp
FormatEntityTest.cpp
MangledTest.cpp
ModuleSpecTest.cpp
diff --git a/lldb/unittests/Utility/CMakeLists.txt b/lldb/unittests/Utility/CMakeLists.txt
index 8e12815d51541c..1aaf87af699ceb 100644
--- a/lldb/unittests/Utility/CMakeLists.txt
+++ b/lldb/unittests/Utility/CMakeLists.txt
@@ -12,6 +12,7 @@ add_lldb_unittest(UtilityTests
DataExtractorTest.cpp
EnvironmentTest.cpp
EventTest.cpp
+ FileSpecListTest.cpp
FileSpecTest.cpp
FlagsTest.cpp
ListenerTest.cpp
@@ -22,6 +23,7 @@ add_lldb_unittest(UtilityTests
ProcessInstanceInfoTest.cpp
RangeMapTest.cpp
RangeTest.cpp
+ RealpathPrefixesTest.cpp
RegisterValueTest.cpp
RegularExpressionTest.cpp
ScalarTest.cpp
diff --git a/lldb/unittests/Core/FileSpecListTest.cpp b/lldb/unittests/Utility/FileSpecListTest.cpp
similarity index 57%
rename from lldb/unittests/Core/FileSpecListTest.cpp
rename to lldb/unittests/Utility/FileSpecListTest.cpp
index e63f4a00bc3a94..06efdd4a70458c 100644
--- a/lldb/unittests/Core/FileSpecListTest.cpp
+++ b/lldb/unittests/Utility/FileSpecListTest.cpp
@@ -8,7 +8,9 @@
#include "gtest/gtest.h"
+#include "MockSymlinkFileSystem.h"
#include "lldb/Utility/FileSpecList.h"
+#include "lldb/Utility/RealpathPrefixes.h"
using namespace lldb_private;
@@ -123,3 +125,131 @@ TEST(SupportFileListTest, RelativePathMatchesWindows) {
EXPECT_EQ((size_t)2, files.FindCompatibleIndex(0, rel3_wrong));
EXPECT_EQ((size_t)6, files.FindCompatibleIndex(3, rel3_wrong));
}
+
+// Support file is a symlink to the breakpoint file.
+// A matching prefix is set.
+// Should find the two compatible.
+// Absolute paths are used.
+TEST(SupportFileListTest, SymlinkedAbsolutePaths) {
+ // Prepare FS
+ llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem(
+ FileSpec("/symlink_dir/foo.h"), FileSpec("/real_dir/foo.h")));
+
+ // Prepare RealpathPrefixes
+ FileSpecList temp;
+ temp.EmplaceBack("/symlink_dir");
+ RealpathPrefixes prefixes(std::move(temp));
+ prefixes.SetFileSystem(fs);
+
+ // Prepare FileSpecList
+ SupportFileList support_file_list;
+ support_file_list.EmplaceBack(FileSpec("/symlink_dir/foo.h"));
+
+ // Test
+ size_t ret = support_file_list.FindCompatibleIndex(
+ 0, FileSpec("/real_dir/foo.h"), &prefixes);
+ EXPECT_EQ(ret, (size_t)0);
+}
+
+// Support file is a symlink to the breakpoint file.
+// A matching prefix is set.
+// Should find the two compatible.
+// Relative paths are used.
+TEST(SupportFileListTest, SymlinkedRelativePaths) {
+ // Prepare FS
+ llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem(
+ FileSpec("symlink_dir/foo.h"), FileSpec("real_dir/foo.h")));
+
+ // Prepare RealpathPrefixes
+ FileSpecList temp;
+ temp.EmplaceBack("symlink_dir");
+ RealpathPrefixes prefixes(std::move(temp));
+ prefixes.SetFileSystem(fs);
+
+ // Prepare FileSpecList
+ SupportFileList support_file_list;
+ support_file_list.EmplaceBack(FileSpec("symlink_dir/foo.h"));
+
+ // Test
+ size_t ret = support_file_list.FindCompatibleIndex(
+ 0, FileSpec("real_dir/foo.h"), &prefixes);
+ EXPECT_EQ(ret, (size_t)0);
+}
+
+// Support file is a symlink to the breakpoint file.
+// A matching prefix is set.
+// However, the breakpoint is set with a partial path.
+// Should find the two compatible.
+TEST(SupportFileListTest, PartialBreakpointPath) {
+ // Prepare FS
+ llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem(
+ FileSpec("symlink_dir/foo.h"), FileSpec("/real_dir/foo.h")));
+
+ // Prepare RealpathPrefixes
+ FileSpecList temp;
+ temp.EmplaceBack("symlink_dir");
+ RealpathPrefixes prefixes(std::move(temp));
+ prefixes.SetFileSystem(fs);
+
+ // Prepare FileSpecList
+ SupportFileList support_file_list;
+ support_file_list.EmplaceBack(FileSpec("symlink_dir/foo.h"));
+
+ // Test
+ size_t ret = support_file_list.FindCompatibleIndex(
+ 0, FileSpec("real_dir/foo.h"), &prefixes);
+ EXPECT_EQ(ret, (size_t)0);
+}
+
+// Support file is a symlink to the breakpoint file.
+// A matching prefix is set.
+// However, the basename is different between the symlink and its target.
+// Should find the two incompatible.
+TEST(SupportFileListTest, DifferentBasename) {
+ // Prepare FS
+ llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem(
+ FileSpec("/symlink_dir/foo.h"), FileSpec("/real_dir/bar.h")));
+
+ // Prepare RealpathPrefixes
+ FileSpecList temp;
+ temp.EmplaceBack("/symlink_dir");
+ RealpathPrefixes prefixes(std::move(temp));
+ prefixes.SetFileSystem(fs);
+
+ // Prepare FileSpecList
+ SupportFileList support_file_list;
+ support_file_list.EmplaceBack(FileSpec("/symlink_dir/foo.h"));
+
+ // Test
+ size_t ret = support_file_list.FindCompatibleIndex(
+ 0, FileSpec("real_dir/bar.h"), &prefixes);
+ EXPECT_EQ(ret, UINT32_MAX);
+}
+
+// No prefixes are configured.
+// The support file and the breakpoint file are different.
+// Should find the two incompatible.
+TEST(SupportFileListTest, NoPrefixes) {
+ // Prepare FileSpecList
+ SupportFileList support_file_list;
+ support_file_list.EmplaceBack(FileSpec("/real_dir/bar.h"));
+
+ // Test
+ size_t ret = support_file_list.FindCompatibleIndex(
+ 0, FileSpec("/real_dir/foo.h"), nullptr);
+ EXPECT_EQ(ret, UINT32_MAX);
+}
+
+// No prefixes are configured.
+// The support file and the breakpoint file are the same.
+// Should find the two compatible.
+TEST(SupportFileListTest, SameFile) {
+ // Prepare FileSpecList
+ SupportFileList support_file_list;
+ support_file_list.EmplaceBack(FileSpec("/real_dir/foo.h"));
+
+ // Test
+ size_t ret = support_file_list.FindCompatibleIndex(
+ 0, FileSpec("/real_dir/foo.h"), nullptr);
+ EXPECT_EQ(ret, (size_t)0);
+}
diff --git a/lldb/unittests/Utility/MockSymlinkFileSystem.h b/lldb/unittests/Utility/MockSymlinkFileSystem.h
new file mode 100644
index 00000000000000..3326fc8046819d
--- /dev/null
+++ b/lldb/unittests/Utility/MockSymlinkFileSystem.h
@@ -0,0 +1,63 @@
+//===-- MockSymlinkFileSystem.h
+//--------------------------------------------------===//
+//
+// 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/FileSpec.h"
+#include "llvm/Support/VirtualFileSystem.h"
+
+namespace lldb_private {
+
+// A mock file system that realpath's a given symlink to a given realpath.
+class MockSymlinkFileSystem : public llvm::vfs::FileSystem {
+public:
+ // Treat all files as non-symlinks.
+ MockSymlinkFileSystem() = default;
+
+ /// Treat \a symlink as a symlink to \a realpath. Treat all other files as
+ /// non-symlinks.
+ MockSymlinkFileSystem(FileSpec &&symlink, FileSpec &&realpath)
+ : m_symlink(std::move(symlink)), m_realpath(std::move(realpath)) {}
+
+ /// If \a Path matches the symlink given in the ctor, put the realpath given
+ /// in the ctor into \a Output.
+ std::error_code getRealPath(const llvm::Twine &Path,
+ llvm::SmallVectorImpl<char> &Output) override {
+ if (FileSpec(Path.str()) == m_symlink) {
+ std::string path = m_realpath.GetPath();
+ Output.assign(path.begin(), path.end());
+ } else {
+ Path.toVector(Output);
+ }
+ return {};
+ }
+
+ // Implement the rest of the interface
+ llvm::ErrorOr<llvm::vfs::Status> status(const llvm::Twine &Path) override {
+ return llvm::errc::operation_not_permitted;
+ }
+ llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
+ openFileForRead(const llvm::Twine &Path) override {
+ return llvm::errc::operation_not_permitted;
+ }
+ llvm::vfs::directory_iterator dir_begin(const llvm::Twine &Dir,
+ std::error_code &EC) override {
+ return llvm::vfs::directory_iterator();
+ }
+ std::error_code setCurrentWorkingDirectory(const llvm::Twine &Path) override {
+ return llvm::errc::operation_not_permitted;
+ }
+ llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override {
+ return llvm::errc::operation_not_permitted;
+ }
+
+private:
+ FileSpec m_symlink;
+ FileSpec m_realpath;
+};
+
+} // namespace lldb_private
diff --git a/lldb/unittests/Utility/RealpathPrefixesTest.cpp b/lldb/unittests/Utility/RealpathPrefixesTest.cpp
new file mode 100644
index 00000000000000..15b06f9854b8fa
--- /dev/null
+++ b/lldb/unittests/Utility/RealpathPrefixesTest.cpp
@@ -0,0 +1,146 @@
+//===-- RealpathPrefixesTest.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 "gtest/gtest.h"
+
+#include "MockSymlinkFileSystem.h"
+#include "lldb/Utility/FileSpecList.h"
+#include "lldb/Utility/RealpathPrefixes.h"
+
+using namespace lldb_private;
+
+// Should resolve a symlink which match an absolute prefix
+TEST(RealpathPrefixesTest, MatchingAbsolutePrefix) {
+ // Prepare FS
+ llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem(
+ FileSpec("/dir1/link.h"), FileSpec("/dir2/real.h")));
+
+ // Prepare RealpathPrefixes
+ FileSpecList file_spec_list;
+ file_spec_list.EmplaceBack("/dir1");
+ RealpathPrefixes prefixes(std::move(file_spec_list));
+ prefixes.SetFileSystem(fs);
+
+ // Test
+ std::optional<FileSpec> ret =
+ prefixes.ResolveSymlinks(FileSpec("/dir1/link.h"));
+ EXPECT_EQ(ret, FileSpec("/dir2/real.h"));
+}
+
+// Should resolve a symlink which match a relative prefix
+TEST(RealpathPrefixesTest, MatchingRelativePrefix) {
+ // Prepare FS
+ llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem(
+ FileSpec("dir1/link.h"), FileSpec("dir2/real.h")));
+
+ // Prepare RealpathPrefixes
+ FileSpecList file_spec_list;
+ file_spec_list.EmplaceBack("dir1");
+ RealpathPrefixes prefixes(std::move(file_spec_list));
+ prefixes.SetFileSystem(fs);
+
+ // Test
+ std::optional<FileSpec> ret =
+ prefixes.ResolveSymlinks(FileSpec("dir1/link.h"));
+ EXPECT_EQ(ret, FileSpec("dir2/real.h"));
+}
+
+// Should resolve a symlink which match a prefix with a case-insensitive support
+// file
+TEST(RealpathPrefixesTest, CaseInsensitive) {
+ // Prepare FS
+ llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem(
+ FileSpec("/dir1/link.h"), FileSpec("/dir2/real.h")));
+
+ // Prepare RealpathPrefixes
+ FileSpecList file_spec_list;
+ file_spec_list.EmplaceBack("/dir1");
+ RealpathPrefixes prefixes(std::move(file_spec_list));
+ prefixes.SetFileSystem(fs);
+
+ // Test
+ std::optional<FileSpec> ret = prefixes.ResolveSymlinks(
+ FileSpec("/DIR1/LINK.H", FileSpec::Style::posix));
+ EXPECT_EQ(ret, FileSpec("/dir2/real.h"));
+}
+
+// Should resolve a symlink when there is mixture of matching and mismatching
+// prefixex
+TEST(RealpathPrefixesTest, MatchingAndMismatchingPrefix) {
+ // Prepare FS
+ llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem(
+ FileSpec("/dir1/link.h"), FileSpec("/dir2/real.h")));
+
+ // Prepare RealpathPrefixes
+ FileSpecList file_spec_list;
+ file_spec_list.EmplaceBack("/fake/path1");
+ file_spec_list.EmplaceBack("/dir1"); // Matching prefix
+ file_spec_list.EmplaceBack("/fake/path2");
+ RealpathPrefixes prefixes(std::move(file_spec_list));
+ prefixes.SetFileSystem(fs);
+
+ // Test
+ std::optional<FileSpec> ret =
+ prefixes.ResolveSymlinks(FileSpec("/dir1/link.h"));
+ EXPECT_EQ(ret, FileSpec("/dir2/real.h"));
+}
+
+// Should resolve a symlink when the prefixes matches after normalization
+TEST(RealpathPrefixesTest, ComplexPrefixes) {
+ // Prepare FS
+ llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem(
+ FileSpec("dir1/link.h"), FileSpec("dir2/real.h")));
+
+ // Prepare RealpathPrefixes
+ FileSpecList file_spec_list;
+ file_spec_list.EmplaceBack("./dir1/foo/../bar/.."); // Equivalent to "/dir1"
+ RealpathPrefixes prefixes(std::move(file_spec_list));
+ prefixes.SetFileSystem(fs);
+
+ // Test
+ std::optional<FileSpec> ret =
+ prefixes.ResolveSymlinks(FileSpec("dir1/link.h"));
+ EXPECT_EQ(ret, FileSpec("dir2/real.h"));
+}
+
+// Should not resolve a symlink which doesn't match any prefixes
+TEST(RealpathPrefixesTest, MismatchingPrefixes) {
+ // Prepare FS
+ llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem(
+ FileSpec("/dir1/link.h"), FileSpec("/dir2/real.h")));
+
+ // Prepare RealpathPrefixes
+ FileSpecList file_spec_list;
+ file_spec_list.EmplaceBack("/dir3");
+ RealpathPrefixes prefixes(std::move(file_spec_list));
+ prefixes.SetFileSystem(fs);
+
+ // Test
+ std::optional<FileSpec> ret =
+ prefixes.ResolveSymlinks(FileSpec("/dir1/link.h"));
+ EXPECT_EQ(ret, std::nullopt);
+}
+
+// Should not resolve a realpath
+TEST(RealpathPrefixesTest, Realpath) {
+ // Prepare FS
+ llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(
+ new MockSymlinkFileSystem());
+
+ // Prepare RealpathPrefixes
+ FileSpecList file_spec_list;
+ file_spec_list.EmplaceBack("/symlink_dir");
+ RealpathPrefixes prefixes(std::move(file_spec_list));
+ prefixes.SetFileSystem(fs);
+
+ // Test
+ std::optional<FileSpec> ret =
+ prefixes.ResolveSymlinks(FileSpec("/dir/real.h"));
+ EXPECT_EQ(ret, std::nullopt);
+}
>From 02c015d3e16fa5dd9495fc4f663cd65afbb4aba4 Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Fri, 2 Aug 2024 08:30:53 -0700
Subject: [PATCH 3/4] Add logging and telemetry
Summary:
Test Plan:
Reviewers:
Subscribers:
Tasks:
Tags:
Differential Revision: https://phabricator.intern.facebook.com/D60673160
---
lldb/include/lldb/Target/Statistics.h | 4 ++++
lldb/include/lldb/Utility/RealpathPrefixes.h | 8 ++++++++
lldb/source/Target/Statistics.cpp | 12 ++++++++++++
lldb/source/Target/Target.cpp | 4 +++-
lldb/source/Utility/FileSpecList.cpp | 15 ++++++++++++++-
lldb/source/Utility/RealpathPrefixes.cpp | 15 ++++++++++++---
lldb/unittests/Utility/CMakeLists.txt | 1 +
lldb/unittests/Utility/MockSymlinkFileSystem.h | 9 ++++++---
lldb/unittests/Utility/RealpathPrefixesTest.cpp | 15 ++++++++-------
9 files changed, 68 insertions(+), 15 deletions(-)
diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index 35bd7f8a66e055..2ce56309aefc39 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -184,6 +184,8 @@ class TargetStats {
void SetFirstPrivateStopTime();
void SetFirstPublicStopTime();
void IncreaseSourceMapDeduceCount();
+ void IncreaseSourceRealpathAttemptCount();
+ void IncreaseSourceRealpathCompatibleCount();
StatsDuration &GetCreateTime() { return m_create_time; }
StatsSuccessFail &GetExpressionStats() { return m_expr_eval; }
@@ -198,6 +200,8 @@ class TargetStats {
StatsSuccessFail m_frame_var{"frameVariable"};
std::vector<intptr_t> m_module_identifiers;
uint32_t m_source_map_deduce_count = 0;
+ uint32_t m_source_realpath_attempt_count = 0;
+ uint32_t m_source_realpath_compatible_count = 0;
void CollectStats(Target &target);
};
diff --git a/lldb/include/lldb/Utility/RealpathPrefixes.h b/lldb/include/lldb/Utility/RealpathPrefixes.h
index ba39259980c067..b9387ec0dee731 100644
--- a/lldb/include/lldb/Utility/RealpathPrefixes.h
+++ b/lldb/include/lldb/Utility/RealpathPrefixes.h
@@ -19,6 +19,7 @@
namespace lldb_private {
class FileSpec;
class FileSpecList;
+class Target;
} // namespace lldb_private
namespace lldb_private {
@@ -35,6 +36,10 @@ class RealpathPrefixes {
// filesystem will be used.
void SetFileSystem(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs);
+ // Sets an optional Target instance to gather statistics.
+ void SetTarget(Target *target) { m_target = target; }
+ Target *GetTarget() const { return m_target; }
+
std::optional<FileSpec> ResolveSymlinks(const FileSpec &file_spec) const;
private:
@@ -48,6 +53,9 @@ class RealpathPrefixes {
// The filesystem to use for realpath'ing.
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> m_fs;
+
+ // The optional Target instance to gather statistics.
+ Target *m_target;
};
} // namespace lldb_private
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index 583d1524881fc3..894c247571ff90 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() {
+ ++m_source_realpath_attempt_count;
+}
+
+void TargetStats::IncreaseSourceRealpathCompatibleCount() {
+ ++m_source_realpath_compatible_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 5a5d689e03fbc0..48feb97db6d083 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -4359,7 +4359,9 @@ InlineStrategy TargetProperties::GetInlineStrategy() const {
// 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, {}));
+ auto prefixes = RealpathPrefixes(GetPropertyAtIndexAs<FileSpecList>(idx, {}));
+ prefixes.SetTarget(this->m_target);
+ return prefixes;
}
llvm::StringRef TargetProperties::GetArg0() const {
diff --git a/lldb/source/Utility/FileSpecList.cpp b/lldb/source/Utility/FileSpecList.cpp
index c43659c95a2d09..d16789d65e8017 100644
--- a/lldb/source/Utility/FileSpecList.cpp
+++ b/lldb/source/Utility/FileSpecList.cpp
@@ -7,7 +7,11 @@
//===----------------------------------------------------------------------===//
#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"
@@ -176,8 +180,17 @@ size_t SupportFileList::FindCompatibleIndex(
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))
+ if (IsCompatible(*resolved_curr_file, file_spec)) {
+ // Stats and logging.
+ if (Target *target = realpath_prefixes->GetTarget())
+ target->GetStatistics().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
index a54145981061ed..646f08ef409c44 100644
--- a/lldb/source/Utility/RealpathPrefixes.cpp
+++ b/lldb/source/Utility/RealpathPrefixes.cpp
@@ -8,14 +8,17 @@
#include "lldb/Utility/RealpathPrefixes.h"
-#include "lldb/Host/FileSystem.h"
+#include "lldb/Target/Statistics.h"
+#include "lldb/Target/Target.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)
- : m_fs(llvm::vfs::getRealFileSystem()) {
+ : m_fs(llvm::vfs::getRealFileSystem()), m_target(nullptr) {
m_prefixes.reserve(file_spec_list.GetSize());
for (const FileSpec &file_spec : file_spec_list) {
m_prefixes.emplace_back(file_spec.GetPath());
@@ -39,12 +42,18 @@ RealpathPrefixes::ResolveSymlinks(const FileSpec &file_spec) const {
std::string file_spec_path = file_spec.GetPath();
for (const std::string &prefix : m_prefixes) {
if (is_prefix(file_spec_path, prefix, file_spec.IsCaseSensitive())) {
+ // Stats and logging.
+ if (m_target)
+ m_target->GetStatistics().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);
+ FileSpec realpath(buff, file_spec.GetPathStyle());
// Only return realpath if it is different from the original file_spec.
if (realpath != file_spec)
diff --git a/lldb/unittests/Utility/CMakeLists.txt b/lldb/unittests/Utility/CMakeLists.txt
index 1aaf87af699ceb..40e0959fc01d14 100644
--- a/lldb/unittests/Utility/CMakeLists.txt
+++ b/lldb/unittests/Utility/CMakeLists.txt
@@ -51,6 +51,7 @@ add_lldb_unittest(UtilityTests
XcodeSDKTest.cpp
LINK_LIBS
+ lldbTarget
lldbUtility
lldbUtilityHelpers
LLVMTestingSupport
diff --git a/lldb/unittests/Utility/MockSymlinkFileSystem.h b/lldb/unittests/Utility/MockSymlinkFileSystem.h
index 3326fc8046819d..7fa1f93bfa38a9 100644
--- a/lldb/unittests/Utility/MockSymlinkFileSystem.h
+++ b/lldb/unittests/Utility/MockSymlinkFileSystem.h
@@ -20,14 +20,16 @@ class MockSymlinkFileSystem : public llvm::vfs::FileSystem {
/// Treat \a symlink as a symlink to \a realpath. Treat all other files as
/// non-symlinks.
- MockSymlinkFileSystem(FileSpec &&symlink, FileSpec &&realpath)
- : m_symlink(std::move(symlink)), m_realpath(std::move(realpath)) {}
+ MockSymlinkFileSystem(FileSpec &&symlink, FileSpec &&realpath,
+ FileSpec::Style style = FileSpec::Style::native)
+ : m_symlink(std::move(symlink)), m_realpath(std::move(realpath)),
+ m_style(style) {}
/// If \a Path matches the symlink given in the ctor, put the realpath given
/// in the ctor into \a Output.
std::error_code getRealPath(const llvm::Twine &Path,
llvm::SmallVectorImpl<char> &Output) override {
- if (FileSpec(Path.str()) == m_symlink) {
+ if (FileSpec(Path.str(), m_style) == m_symlink) {
std::string path = m_realpath.GetPath();
Output.assign(path.begin(), path.end());
} else {
@@ -58,6 +60,7 @@ class MockSymlinkFileSystem : public llvm::vfs::FileSystem {
private:
FileSpec m_symlink;
FileSpec m_realpath;
+ FileSpec::Style m_style;
};
} // namespace lldb_private
diff --git a/lldb/unittests/Utility/RealpathPrefixesTest.cpp b/lldb/unittests/Utility/RealpathPrefixesTest.cpp
index 15b06f9854b8fa..c6896c23b437da 100644
--- a/lldb/unittests/Utility/RealpathPrefixesTest.cpp
+++ b/lldb/unittests/Utility/RealpathPrefixesTest.cpp
@@ -51,23 +51,24 @@ TEST(RealpathPrefixesTest, MatchingRelativePrefix) {
EXPECT_EQ(ret, FileSpec("dir2/real.h"));
}
-// Should resolve a symlink which match a prefix with a case-insensitive support
-// file
-TEST(RealpathPrefixesTest, CaseInsensitive) {
+// Should resolve in Windows and/or with a case-insensitive support file
+TEST(RealpathPrefixesTest, WindowsAndCaseInsensitive) {
// Prepare FS
llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem(
- FileSpec("/dir1/link.h"), FileSpec("/dir2/real.h")));
+ FileSpec("f:\\dir1\\link.h", FileSpec::Style::windows),
+ FileSpec("f:\\dir2\\real.h", FileSpec::Style::windows),
+ FileSpec::Style::windows));
// Prepare RealpathPrefixes
FileSpecList file_spec_list;
- file_spec_list.EmplaceBack("/dir1");
+ file_spec_list.EmplaceBack(FileSpec("f:\\dir1", FileSpec::Style::windows));
RealpathPrefixes prefixes(std::move(file_spec_list));
prefixes.SetFileSystem(fs);
// Test
std::optional<FileSpec> ret = prefixes.ResolveSymlinks(
- FileSpec("/DIR1/LINK.H", FileSpec::Style::posix));
- EXPECT_EQ(ret, FileSpec("/dir2/real.h"));
+ FileSpec("F:\\DIR1\\LINK.H", FileSpec::Style::windows));
+ EXPECT_EQ(ret, FileSpec("f:\\dir2\\real.h", FileSpec::Style::windows));
}
// Should resolve a symlink when there is mixture of matching and mismatching
>From b99ced042c907b968dc97a1e3181f84657c4b00d Mon Sep 17 00:00:00 2001
From: Roy Shi <royshi at meta.com>
Date: Tue, 6 Aug 2024 13:28:23 -0700
Subject: [PATCH 4/4] Address additional comments in diffs
---
lldb/include/lldb/Symbol/CompileUnit.h | 4 ++++
lldb/include/lldb/Target/Target.h | 3 +--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/lldb/include/lldb/Symbol/CompileUnit.h b/lldb/include/lldb/Symbol/CompileUnit.h
index 6a9ef3e6c75bcb..c8ce99b01b62f3 100644
--- a/lldb/include/lldb/Symbol/CompileUnit.h
+++ b/lldb/include/lldb/Symbol/CompileUnit.h
@@ -391,6 +391,10 @@ class CompileUnit : public std::enable_shared_from_this<CompileUnit>,
/// A SymbolContext list class that will get any matching
/// entries appended to.
///
+ /// \param[in] realpath_prefixes
+ /// Paths that start with one of the prefixes in this list will be
+ /// realpath'ed to resolve any symlinks.
+ ///
/// \see enum SymbolContext::Scope
void
ResolveSymbolContext(const SourceLocationSpec &src_location_spec,
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index d6d5f05e9258f2..7f4d607f5427df 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -34,13 +34,12 @@
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/Broadcaster.h"
#include "lldb/Utility/LLDBAssert.h"
+#include "lldb/Utility/RealpathPrefixes.h"
#include "lldb/Utility/Timeout.h"
#include "lldb/lldb-public.h"
namespace lldb_private {
-class RealpathPrefixes;
-
OptionEnumValues GetDynamicValueTypes();
enum InlineStrategy {
More information about the lldb-commits
mailing list