Skip to content

[lldb] Store SupportFiles in SourceManager::File (NFC) #106639

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions lldb/include/lldb/Core/SourceManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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; }

Expand All @@ -70,15 +73,13 @@ 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 support file. If the target has source mappings, this might be
/// different from the original support file passed to the constructor.
lldb::SupportFileSP m_support_file_sp;

// Keep the modification time that this file data is valid for
llvm::sys::TimePoint<> m_mod_time;
Expand All @@ -93,7 +94,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;
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Commands/CommandObjectSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
14 changes: 8 additions & 6 deletions lldb/source/Core/IOHandlerCursesGUI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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());
Expand All @@ -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.
Expand Down
139 changes: 79 additions & 60 deletions lldb/source/Core/SourceManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
}
Expand All @@ -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.
Expand Down Expand Up @@ -444,25 +449,25 @@ 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_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_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)
Expand All @@ -472,65 +477,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) {
Expand Down Expand Up @@ -603,7 +619,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;
}
Expand Down Expand Up @@ -644,7 +661,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);
Expand Down Expand Up @@ -698,7 +716,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;
}
Expand Down Expand Up @@ -778,9 +797,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) {
Expand Down
12 changes: 7 additions & 5 deletions lldb/unittests/Core/SourceManagerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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);
Expand Down
Loading