https://github.com/JDevlieghere created
https://github.com/llvm/llvm-project/pull/106639
To support detecting MD5 checksum mismatches, store a SupportFile rather than a
plain FileSpec in SourceManager::File.
>From 64b6c6419722ecd5865cdf3e734b63ed51763174 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jo...@devlieghere.com>
Date: Thu, 29 Aug 2024 15:03:23 -0700
Subject: [PATCH] [lldb] Store SupportFiles in SourceManager::File (NFC)
To support detecting MD5 checksum mismatches, store a SupportFile rather
than a plain FileSpec in SourceManager::File.
---
lldb/include/lldb/Core/SourceManager.h | 26 ++--
lldb/source/Commands/CommandObjectSource.cpp | 4 +-
lldb/source/Core/IOHandlerCursesGUI.cpp | 14 +-
lldb/source/Core/SourceManager.cpp | 141 +++++++++++--------
lldb/unittests/Core/SourceManagerTest.cpp | 12 +-
5 files changed, 114 insertions(+), 83 deletions(-)
diff --git a/lldb/include/lldb/Core/SourceManager.h
b/lldb/include/lldb/Core/SourceManager.h
index 5239ac6f4055f5..3459a1031a10d7 100644
--- a/lldb/include/lldb/Core/SourceManager.h
+++ b/lldb/include/lldb/Core/SourceManager.h
@@ -37,8 +37,8 @@ class SourceManager {
const SourceManager::File &rhs);
public:
- File(const FileSpec &file_spec, lldb::TargetSP target_sp);
- File(const FileSpec &file_spec, lldb::DebuggerSP debugger_sp);
+ File(lldb::SupportFileSP support_file_sp, lldb::TargetSP target_sp);
+ File(lldb::SupportFileSP support_file_sp, lldb::DebuggerSP debugger_sp);
bool ModificationTimeIsStale() const;
bool PathRemappingIsStale() const;
@@ -56,7 +56,10 @@ class SourceManager {
bool LineIsValid(uint32_t line);
- const FileSpec &GetFileSpec() { return m_file_spec; }
+ lldb::SupportFileSP GetSupportFile() const {
+ assert(m_support_file_sp && "SupportFileSP must always be valid");
+ return m_support_file_sp;
+ }
uint32_t GetSourceMapModificationID() const { return m_source_map_mod_id; }
@@ -70,15 +73,17 @@ class SourceManager {
protected:
/// Set file and update modification time.
- void SetFileSpec(FileSpec file_spec);
+ void SetSupportFile(lldb::SupportFileSP support_file_sp);
bool CalculateLineOffsets(uint32_t line = UINT32_MAX);
- FileSpec m_file_spec_orig; // The original file spec that was used (can be
- // different from m_file_spec)
- FileSpec m_file_spec; // The actually file spec being used (if the target
- // has source mappings, this might be different from
- // m_file_spec_orig)
+ /// The original support file that was used.
+ lldb::SupportFileSP m_original_support_file_sp;
+
+ /// The actually support file being used. If the target
+ /// has source mappings, this might be different from
+ /// the original support file.
+ lldb::SupportFileSP m_support_file_sp;
// Keep the modification time that this file data is valid for
llvm::sys::TimePoint<> m_mod_time;
@@ -93,7 +98,8 @@ class SourceManager {
lldb::TargetWP m_target_wp;
private:
- void CommonInitializer(const FileSpec &file_spec, lldb::TargetSP
target_sp);
+ void CommonInitializer(lldb::SupportFileSP support_file_sp,
+ lldb::TargetSP target_sp);
};
typedef std::shared_ptr<File> FileSP;
diff --git a/lldb/source/Commands/CommandObjectSource.cpp
b/lldb/source/Commands/CommandObjectSource.cpp
index 5ddd46ac5fdc07..1a0629c6765d41 100644
--- a/lldb/source/Commands/CommandObjectSource.cpp
+++ b/lldb/source/Commands/CommandObjectSource.cpp
@@ -1076,8 +1076,8 @@ class CommandObjectSourceList : public
CommandObjectParsed {
target.GetSourceManager().GetLastFile());
if (last_file_sp) {
const bool show_inlines = true;
- m_breakpoint_locations.Reset(last_file_sp->GetFileSpec(), 0,
- show_inlines);
+ m_breakpoint_locations.Reset(
+ last_file_sp->GetSupportFile()->GetSpecOnly(), 0,
show_inlines);
SearchFilterForUnconstrainedSearches target_search_filter(
target.shared_from_this());
target_search_filter.Search(m_breakpoint_locations);
diff --git a/lldb/source/Core/IOHandlerCursesGUI.cpp
b/lldb/source/Core/IOHandlerCursesGUI.cpp
index d922d32f910583..8f44e3d0cd016b 100644
--- a/lldb/source/Core/IOHandlerCursesGUI.cpp
+++ b/lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -6894,8 +6894,8 @@ class SourceFileWindowDelegate : public WindowDelegate {
if (context_changed)
m_selected_line = m_pc_line;
- if (m_file_sp &&
- m_file_sp->GetFileSpec() == m_sc.line_entry.GetFile()) {
+ if (m_file_sp && m_file_sp->GetSupportFile()->GetSpecOnly() ==
+ m_sc.line_entry.GetFile()) {
// Same file, nothing to do, we should either have the lines or
// not (source file missing)
if (m_selected_line >= static_cast<size_t>(m_first_visible_line)) {
@@ -7001,7 +7001,8 @@ class SourceFileWindowDelegate : public WindowDelegate {
LineEntry bp_loc_line_entry;
if (bp_loc_sp->GetAddress().CalculateSymbolContextLineEntry(
bp_loc_line_entry)) {
- if (m_file_sp->GetFileSpec() == bp_loc_line_entry.GetFile()) {
+ if (m_file_sp->GetSupportFile()->GetSpecOnly() ==
+ bp_loc_line_entry.GetFile()) {
bp_lines.insert(bp_loc_line_entry.line);
}
}
@@ -7332,7 +7333,7 @@ class SourceFileWindowDelegate : public WindowDelegate {
if (exe_ctx.HasProcessScope() && exe_ctx.GetProcessRef().IsAlive()) {
BreakpointSP bp_sp = exe_ctx.GetTargetRef().CreateBreakpoint(
nullptr, // Don't limit the breakpoint to certain modules
- m_file_sp->GetFileSpec(), // Source file
+ m_file_sp->GetSupportFile()->GetSpecOnly(), // Source file
m_selected_line +
1, // Source line number (m_selected_line is zero based)
0, // Unspecified column.
@@ -7478,7 +7479,8 @@ class SourceFileWindowDelegate : public WindowDelegate {
LineEntry bp_loc_line_entry;
if (bp_loc_sp->GetAddress().CalculateSymbolContextLineEntry(
bp_loc_line_entry)) {
- if (m_file_sp->GetFileSpec() == bp_loc_line_entry.GetFile() &&
+ if (m_file_sp->GetSupportFile()->GetSpecOnly() ==
+ bp_loc_line_entry.GetFile() &&
m_selected_line + 1 == bp_loc_line_entry.line) {
bool removed =
exe_ctx.GetTargetRef().RemoveBreakpointByID(bp_sp->GetID());
@@ -7492,7 +7494,7 @@ class SourceFileWindowDelegate : public WindowDelegate {
// No breakpoint found on the location, add it.
BreakpointSP bp_sp = exe_ctx.GetTargetRef().CreateBreakpoint(
nullptr, // Don't limit the breakpoint to certain modules
- m_file_sp->GetFileSpec(), // Source file
+ m_file_sp->GetSupportFile()->GetSpecOnly(), // Source file
m_selected_line +
1, // Source line number (m_selected_line is zero based)
0, // No column specified.
diff --git a/lldb/source/Core/SourceManager.cpp
b/lldb/source/Core/SourceManager.cpp
index 0d70c554e5342b..75da550314c322 100644
--- a/lldb/source/Core/SourceManager.cpp
+++ b/lldb/source/Core/SourceManager.cpp
@@ -87,8 +87,10 @@ SourceManager::FileSP SourceManager::GetFile(const FileSpec
&file_spec) {
LLDB_LOG(log, "Source file caching disabled: creating new source file:
{0}",
file_spec);
if (target_sp)
- return std::make_shared<File>(file_spec, target_sp);
- return std::make_shared<File>(file_spec, debugger_sp);
+ return std::make_shared<File>(std::make_shared<SupportFile>(file_spec),
+ target_sp);
+ return std::make_shared<File>(std::make_shared<SupportFile>(file_spec),
+ debugger_sp);
}
ProcessSP process_sp = target_sp ? target_sp->GetProcessSP() : ProcessSP();
@@ -136,7 +138,8 @@ SourceManager::FileSP SourceManager::GetFile(const FileSpec
&file_spec) {
}
// Check if the file exists on disk.
- if (file_sp && !FileSystem::Instance().Exists(file_sp->GetFileSpec())) {
+ if (file_sp && !FileSystem::Instance().Exists(
+ file_sp->GetSupportFile()->GetSpecOnly())) {
LLDB_LOG(log, "File doesn't exist on disk: {0}", file_spec);
file_sp.reset();
}
@@ -148,9 +151,11 @@ SourceManager::FileSP SourceManager::GetFile(const
FileSpec &file_spec) {
// (Re)create the file.
if (target_sp)
- file_sp = std::make_shared<File>(file_spec, target_sp);
+ file_sp =
std::make_shared<File>(std::make_shared<SupportFile>(file_spec),
+ target_sp);
else
- file_sp = std::make_shared<File>(file_spec, debugger_sp);
+ file_sp =
std::make_shared<File>(std::make_shared<SupportFile>(file_spec),
+ debugger_sp);
// Add the file to the debugger and process cache. If the file was
// invalidated, this will overwrite it.
@@ -444,25 +449,27 @@ void SourceManager::FindLinesMatchingRegex(FileSpec
&file_spec,
match_lines);
}
-SourceManager::File::File(const FileSpec &file_spec,
+SourceManager::File::File(SupportFileSP support_file_sp,
lldb::DebuggerSP debugger_sp)
- : m_file_spec_orig(file_spec), m_file_spec(), m_mod_time(),
+ : m_original_support_file_sp(support_file_sp),
+ m_support_file_sp(std::make_shared<SupportFile>()), m_mod_time(),
m_debugger_wp(debugger_sp), m_target_wp(TargetSP()) {
- CommonInitializer(file_spec, {});
+ CommonInitializer(support_file_sp, {});
}
-SourceManager::File::File(const FileSpec &file_spec, TargetSP target_sp)
- : m_file_spec_orig(file_spec), m_file_spec(), m_mod_time(),
+SourceManager::File::File(SupportFileSP support_file_sp, TargetSP target_sp)
+ : m_original_support_file_sp(support_file_sp),
+ m_support_file_sp(std::make_shared<SupportFile>()), m_mod_time(),
m_debugger_wp(target_sp ? target_sp->GetDebugger().shared_from_this()
: DebuggerSP()),
m_target_wp(target_sp) {
- CommonInitializer(file_spec, target_sp);
+ CommonInitializer(support_file_sp, target_sp);
}
-void SourceManager::File::CommonInitializer(const FileSpec &file_spec,
+void SourceManager::File::CommonInitializer(SupportFileSP support_file_sp,
TargetSP target_sp) {
// Set the file and update the modification time.
- SetFileSpec(file_spec);
+ SetSupportFile(support_file_sp);
// Always update the source map modification ID if we have a target.
if (target_sp)
@@ -472,65 +479,76 @@ void SourceManager::File::CommonInitializer(const
FileSpec &file_spec,
if (m_mod_time == llvm::sys::TimePoint<>()) {
if (target_sp) {
// If this is just a file name, try finding it in the target.
- if (!file_spec.GetDirectory() && file_spec.GetFilename()) {
- bool check_inlines = false;
- SymbolContextList sc_list;
- size_t num_matches =
- target_sp->GetImages().ResolveSymbolContextForFilePath(
- file_spec.GetFilename().AsCString(), 0, check_inlines,
- SymbolContextItem(eSymbolContextModule |
- eSymbolContextCompUnit),
- sc_list);
- bool got_multiple = false;
- if (num_matches != 0) {
- if (num_matches > 1) {
- CompileUnit *test_cu = nullptr;
- for (const SymbolContext &sc : sc_list) {
- if (sc.comp_unit) {
- if (test_cu) {
- if (test_cu != sc.comp_unit)
- got_multiple = true;
- break;
- } else
- test_cu = sc.comp_unit;
+ {
+ FileSpec file_spec = support_file_sp->GetSpecOnly();
+ if (!file_spec.GetDirectory() && file_spec.GetFilename()) {
+ bool check_inlines = false;
+ SymbolContextList sc_list;
+ size_t num_matches =
+ target_sp->GetImages().ResolveSymbolContextForFilePath(
+ file_spec.GetFilename().AsCString(), 0, check_inlines,
+ SymbolContextItem(eSymbolContextModule |
+ eSymbolContextCompUnit),
+ sc_list);
+ bool got_multiple = false;
+ if (num_matches != 0) {
+ if (num_matches > 1) {
+ CompileUnit *test_cu = nullptr;
+ for (const SymbolContext &sc : sc_list) {
+ if (sc.comp_unit) {
+ if (test_cu) {
+ if (test_cu != sc.comp_unit)
+ got_multiple = true;
+ break;
+ } else
+ test_cu = sc.comp_unit;
+ }
}
}
- }
- if (!got_multiple) {
- SymbolContext sc;
- sc_list.GetContextAtIndex(0, sc);
- if (sc.comp_unit)
- SetFileSpec(sc.comp_unit->GetPrimaryFile());
+ if (!got_multiple) {
+ SymbolContext sc;
+ sc_list.GetContextAtIndex(0, sc);
+ if (sc.comp_unit)
+ SetSupportFile(std::make_shared<SupportFile>(
+ sc.comp_unit->GetPrimaryFile()));
+ }
}
}
}
// Try remapping the file if it doesn't exist.
- if (!FileSystem::Instance().Exists(m_file_spec)) {
- // Check target specific source remappings (i.e., the
- // target.source-map setting), then fall back to the module
- // specific remapping (i.e., the .dSYM remapping dictionary).
- auto remapped = target_sp->GetSourcePathMap().FindFile(m_file_spec);
- if (!remapped) {
- FileSpec new_spec;
- if (target_sp->GetImages().FindSourceFile(m_file_spec, new_spec))
- remapped = new_spec;
+ {
+ FileSpec file_spec = support_file_sp->GetSpecOnly();
+ if (!FileSystem::Instance().Exists(file_spec)) {
+ // Check target specific source remappings (i.e., the
+ // target.source-map setting), then fall back to the module
+ // specific remapping (i.e., the .dSYM remapping dictionary).
+ auto remapped = target_sp->GetSourcePathMap().FindFile(file_spec);
+ if (!remapped) {
+ FileSpec new_spec;
+ if (target_sp->GetImages().FindSourceFile(file_spec, new_spec))
+ remapped = new_spec;
+ }
+ if (remapped)
+ SetSupportFile(std::make_shared<SupportFile>(
+ *remapped, support_file_sp->GetChecksum()));
}
- if (remapped)
- SetFileSpec(*remapped);
}
}
}
// If the file exists, read in the data.
if (m_mod_time != llvm::sys::TimePoint<>())
- m_data_sp = FileSystem::Instance().CreateDataBuffer(m_file_spec);
+ m_data_sp = FileSystem::Instance().CreateDataBuffer(
+ m_support_file_sp->GetSpecOnly());
}
-void SourceManager::File::SetFileSpec(FileSpec file_spec) {
+void SourceManager::File::SetSupportFile(lldb::SupportFileSP support_file_sp) {
+ FileSpec file_spec = support_file_sp->GetSpecOnly();
resolve_tilde(file_spec);
- m_file_spec = std::move(file_spec);
- m_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec);
+ m_support_file_sp =
+ std::make_shared<SupportFile>(file_spec, support_file_sp->GetChecksum());
+ m_mod_time = FileSystem::Instance().GetModificationTime(file_spec);
}
uint32_t SourceManager::File::GetLineOffset(uint32_t line) {
@@ -603,7 +621,8 @@ bool SourceManager::File::ModificationTimeIsStale() const {
// TODO: use host API to sign up for file modifications to anything in our
// source cache and only update when we determine a file has been updated.
// For now we check each time we want to display info for the file.
- auto curr_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec);
+ auto curr_mod_time = FileSystem::Instance().GetModificationTime(
+ m_support_file_sp->GetSpecOnly());
return curr_mod_time != llvm::sys::TimePoint<>() &&
m_mod_time != curr_mod_time;
}
@@ -644,7 +663,8 @@ size_t SourceManager::File::DisplaySourceLines(uint32_t
line,
debugger_sp->GetStopShowColumnAnsiSuffix());
HighlighterManager mgr;
- std::string path = GetFileSpec().GetPath(/*denormalize*/ false);
+ std::string path =
+ GetSupportFile()->GetSpecOnly().GetPath(/*denormalize*/ false);
// FIXME: Find a way to get the definitive language this file was written in
// and pass it to the highlighter.
const auto &h = mgr.getHighlighterFor(lldb::eLanguageTypeUnknown, path);
@@ -698,7 +718,8 @@ void SourceManager::File::FindLinesMatchingRegex(
bool lldb_private::operator==(const SourceManager::File &lhs,
const SourceManager::File &rhs) {
- if (lhs.m_file_spec != rhs.m_file_spec)
+ if (!lhs.GetSupportFile()->Equal(*rhs.GetSupportFile(),
+ SupportFile::eEqualChecksumIfSet))
return false;
return lhs.m_mod_time == rhs.m_mod_time;
}
@@ -778,9 +799,9 @@ void SourceManager::SourceFileCache::AddSourceFile(const
FileSpec &file_spec,
assert(file_sp && "invalid FileSP");
AddSourceFileImpl(file_spec, file_sp);
- const FileSpec &resolved_file_spec = file_sp->GetFileSpec();
+ const FileSpec &resolved_file_spec =
file_sp->GetSupportFile()->GetSpecOnly();
if (file_spec != resolved_file_spec)
- AddSourceFileImpl(file_sp->GetFileSpec(), file_sp);
+ AddSourceFileImpl(file_sp->GetSupportFile()->GetSpecOnly(), file_sp);
}
void SourceManager::SourceFileCache::RemoveSourceFile(const FileSP &file_sp) {
diff --git a/lldb/unittests/Core/SourceManagerTest.cpp
b/lldb/unittests/Core/SourceManagerTest.cpp
index 58d6f6cb3f8503..26ab0edffb398d 100644
--- a/lldb/unittests/Core/SourceManagerTest.cpp
+++ b/lldb/unittests/Core/SourceManagerTest.cpp
@@ -8,6 +8,7 @@
#include "lldb/Core/SourceManager.h"
#include "lldb/Host/FileSystem.h"
+#include "lldb/Utility/SupportFile.h"
#include "gtest/gtest.h"
#include "TestingSupport/MockTildeExpressionResolver.h"
@@ -29,8 +30,8 @@ TEST_F(SourceFileCache, FindSourceFileFound) {
// Insert: foo
FileSpec foo_file_spec("foo");
- auto foo_file_sp =
- std::make_shared<SourceManager::File>(foo_file_spec, lldb::DebuggerSP());
+ auto foo_file_sp = std::make_shared<SourceManager::File>(
+ std::make_shared<SupportFile>(foo_file_spec), lldb::DebuggerSP());
cache.AddSourceFile(foo_file_spec, foo_file_sp);
// Query: foo, expect found.
@@ -43,8 +44,8 @@ TEST_F(SourceFileCache, FindSourceFileNotFound) {
// Insert: foo
FileSpec foo_file_spec("foo");
- auto foo_file_sp =
- std::make_shared<SourceManager::File>(foo_file_spec, lldb::DebuggerSP());
+ auto foo_file_sp = std::make_shared<SourceManager::File>(
+ std::make_shared<SupportFile>(foo_file_spec), lldb::DebuggerSP());
cache.AddSourceFile(foo_file_spec, foo_file_sp);
// Query: bar, expect not found.
@@ -63,7 +64,8 @@ TEST_F(SourceFileCache, FindSourceFileByUnresolvedPath) {
// Create the file with the resolved file spec.
auto foo_file_sp = std::make_shared<SourceManager::File>(
- resolved_foo_file_spec, lldb::DebuggerSP());
+ std::make_shared<SupportFile>(resolved_foo_file_spec),
+ lldb::DebuggerSP());
// Cache the result with the unresolved file spec.
cache.AddSourceFile(foo_file_spec, foo_file_sp);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
