-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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
[lldb] Reland: Store SupportFile in FileEntry (NFC) #85892
Conversation
✅ With the latest revision this PR passed the C/C++ code formatter. |
I tested the two patches separately. With the first one (Make LineEntry::file private) my problematic test case still passes. When I apply the second one (Swap FileSpec for SupportFile) on top, the test fails as observed with the original change. |
@slackito Thanks, that's what I expected, but it still helps narrow things down. Could you do me another favor and try swapping out line 292 in LineTable.cpp for:
I'm wondering if we somehow end up comparing it against the file set in
|
With that modification the test goes back to passing :) |
a9a62f2
to
86819e8
Compare
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) ChangesThis is another step towards supporting DWARF5 checksums and inline Patch is 27.54 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/85892.diff 27 Files Affected:
diff --git a/lldb/include/lldb/Core/Disassembler.h b/lldb/include/lldb/Core/Disassembler.h
index 885ac1bb4a7ef8..e037a49f152c74 100644
--- a/lldb/include/lldb/Core/Disassembler.h
+++ b/lldb/include/lldb/Core/Disassembler.h
@@ -538,7 +538,7 @@ class Disassembler : public std::enable_shared_from_this<Disassembler>,
ElideMixedSourceAndDisassemblyLine(const ExecutionContext &exe_ctx,
const SymbolContext &sc, LineEntry &line) {
SourceLine sl;
- sl.file = line.file;
+ sl.file = line.GetFile();
sl.line = line.line;
sl.column = line.column;
return ElideMixedSourceAndDisassemblyLine(exe_ctx, sc, sl);
diff --git a/lldb/include/lldb/Symbol/LineEntry.h b/lldb/include/lldb/Symbol/LineEntry.h
index 31e1cd0b36f96e..8da59cf0bd24aa 100644
--- a/lldb/include/lldb/Symbol/LineEntry.h
+++ b/lldb/include/lldb/Symbol/LineEntry.h
@@ -130,11 +130,14 @@ struct LineEntry {
/// Shared pointer to the target this LineEntry belongs to.
void ApplyFileMappings(lldb::TargetSP target_sp);
+ /// Helper to access the file.
+ const FileSpec &GetFile() const { return file_sp->GetSpecOnly(); }
+
/// The section offset address range for this line entry.
AddressRange range;
/// The source file, possibly mapped by the target.source-map setting.
- FileSpec file;
+ lldb::SupportFileSP file_sp;
/// The original source file, from debug info.
lldb::SupportFileSP original_file_sp;
diff --git a/lldb/include/lldb/Utility/SupportFile.h b/lldb/include/lldb/Utility/SupportFile.h
index 0ea0ca4e7c97a1..7505d7f345c5dd 100644
--- a/lldb/include/lldb/Utility/SupportFile.h
+++ b/lldb/include/lldb/Utility/SupportFile.h
@@ -45,6 +45,9 @@ class SupportFile {
/// Materialize the file to disk and return the path to that temporary file.
virtual const FileSpec &Materialize() { return m_file_spec; }
+ /// Change the file name.
+ void Update(const FileSpec &file_spec) { m_file_spec = file_spec; }
+
protected:
FileSpec m_file_spec;
Checksum m_checksum;
diff --git a/lldb/source/API/SBLineEntry.cpp b/lldb/source/API/SBLineEntry.cpp
index 28d12e65fdaf8a..99a7b8fe644cb5 100644
--- a/lldb/source/API/SBLineEntry.cpp
+++ b/lldb/source/API/SBLineEntry.cpp
@@ -81,8 +81,8 @@ SBFileSpec SBLineEntry::GetFileSpec() const {
LLDB_INSTRUMENT_VA(this);
SBFileSpec sb_file_spec;
- if (m_opaque_up.get() && m_opaque_up->file)
- sb_file_spec.SetFileSpec(m_opaque_up->file);
+ if (m_opaque_up.get() && m_opaque_up->GetFile())
+ sb_file_spec.SetFileSpec(m_opaque_up->GetFile());
return sb_file_spec;
}
@@ -109,9 +109,9 @@ void SBLineEntry::SetFileSpec(lldb::SBFileSpec filespec) {
LLDB_INSTRUMENT_VA(this, filespec);
if (filespec.IsValid())
- ref().file = filespec.ref();
+ ref().file_sp = std::make_shared<SupportFile>(filespec.ref());
else
- ref().file.Clear();
+ ref().file_sp = std::make_shared<SupportFile>();
}
void SBLineEntry::SetLine(uint32_t line) {
LLDB_INSTRUMENT_VA(this, line);
@@ -168,7 +168,7 @@ bool SBLineEntry::GetDescription(SBStream &description) {
if (m_opaque_up) {
char file_path[PATH_MAX * 2];
- m_opaque_up->file.GetPath(file_path, sizeof(file_path));
+ m_opaque_up->GetFile().GetPath(file_path, sizeof(file_path));
strm.Printf("%s:%u", file_path, GetLine());
if (GetColumn() > 0)
strm.Printf(":%u", GetColumn());
diff --git a/lldb/source/API/SBThread.cpp b/lldb/source/API/SBThread.cpp
index fa4c80e59d973f..eb9cf063802cd4 100644
--- a/lldb/source/API/SBThread.cpp
+++ b/lldb/source/API/SBThread.cpp
@@ -819,7 +819,7 @@ SBError SBThread::StepOverUntil(lldb::SBFrame &sb_frame,
step_file_spec = sb_file_spec.ref();
} else {
if (frame_sc.line_entry.IsValid())
- step_file_spec = frame_sc.line_entry.file;
+ step_file_spec = frame_sc.line_entry.GetFile();
else {
sb_error.SetErrorString("invalid file argument or no file for frame");
return sb_error;
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp b/lldb/source/Breakpoint/BreakpointResolver.cpp
index 1861a0fe7c4fe4..ff4e2a9985197b 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -221,7 +221,7 @@ void BreakpointResolver::SetSCMatchesByLine(
auto &match = all_scs[0];
auto worklist_begin = std::partition(
all_scs.begin(), all_scs.end(), [&](const SymbolContext &sc) {
- if (sc.line_entry.file == match.line_entry.file ||
+ if (sc.line_entry.GetFile() == match.line_entry.GetFile() ||
*sc.line_entry.original_file_sp ==
*match.line_entry.original_file_sp) {
// When a match is found, keep track of the smallest line number.
diff --git a/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp b/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
index cc4e1d26724f04..d7d8c714867e3e 100644
--- a/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
@@ -147,8 +147,9 @@ void BreakpointResolverFileLine::FilterContexts(SymbolContextList &sc_list) {
else
continue;
- if (file != sc.line_entry.file) {
- LLDB_LOG(log, "unexpected symbol context file {0}", sc.line_entry.file);
+ if (file != sc.line_entry.GetFile()) {
+ LLDB_LOG(log, "unexpected symbol context file {0}",
+ sc.line_entry.GetFile());
continue;
}
@@ -223,7 +224,7 @@ void BreakpointResolverFileLine::DeduceSourceMapping(
const bool case_sensitive = request_file.IsCaseSensitive();
for (const SymbolContext &sc : sc_list) {
- FileSpec sc_file = sc.line_entry.file;
+ FileSpec sc_file = sc.line_entry.GetFile();
if (FileSpec::Equal(sc_file, request_file, /*full*/ true))
continue;
diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index fbece865f11314..cd4c7790f447e1 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -780,8 +780,8 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
} else {
const SymbolContext &sc =
cur_frame->GetSymbolContext(eSymbolContextLineEntry);
- if (sc.line_entry.file) {
- file = sc.line_entry.file;
+ if (sc.line_entry.GetFile()) {
+ file = sc.line_entry.GetFile();
} else {
result.AppendError("Can't find the file for the selected frame to "
"use as the default file.");
diff --git a/lldb/source/Commands/CommandObjectSource.cpp b/lldb/source/Commands/CommandObjectSource.cpp
index fde74f02aea644..0c1267456a1845 100644
--- a/lldb/source/Commands/CommandObjectSource.cpp
+++ b/lldb/source/Commands/CommandObjectSource.cpp
@@ -158,7 +158,7 @@ class CommandObjectSourceInfo : public CommandObjectParsed {
if (module_list.GetSize() &&
module_list.GetIndexForModule(module) == LLDB_INVALID_INDEX32)
continue;
- if (!FileSpec::Match(file_spec, line_entry.file))
+ if (!FileSpec::Match(file_spec, line_entry.GetFile()))
continue;
if (start_line > 0 && line_entry.line < start_line)
continue;
@@ -239,7 +239,7 @@ class CommandObjectSourceInfo : public CommandObjectParsed {
num_matches++;
if (num_lines > 0 && num_matches > num_lines)
break;
- assert(cu_file_spec == line_entry.file);
+ assert(cu_file_spec == line_entry.GetFile());
if (!cu_header_printed) {
if (num_matches > 0)
strm << "\n\n";
@@ -760,11 +760,11 @@ class CommandObjectSourceList : public CommandObjectParsed {
bool operator<(const SourceInfo &rhs) const {
if (function.GetCString() < rhs.function.GetCString())
return true;
- if (line_entry.file.GetDirectory().GetCString() <
- rhs.line_entry.file.GetDirectory().GetCString())
+ if (line_entry.GetFile().GetDirectory().GetCString() <
+ rhs.line_entry.GetFile().GetDirectory().GetCString())
return true;
- if (line_entry.file.GetFilename().GetCString() <
- rhs.line_entry.file.GetFilename().GetCString())
+ if (line_entry.GetFile().GetFilename().GetCString() <
+ rhs.line_entry.GetFile().GetFilename().GetCString())
return true;
if (line_entry.line < rhs.line_entry.line)
return true;
@@ -799,7 +799,7 @@ class CommandObjectSourceList : public CommandObjectParsed {
sc.function->GetEndLineSourceInfo(end_file, end_line);
} else {
// We have an inlined function
- start_file = source_info.line_entry.file;
+ start_file = source_info.line_entry.GetFile();
start_line = source_info.line_entry.line;
end_line = start_line + m_options.num_lines;
}
diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp
index cf4f8ccaa0c4aa..3dbbfd4f9d344d 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -1705,7 +1705,7 @@ class CommandObjectThreadJump : public CommandObjectParsed {
line = sym_ctx.line_entry.line + m_options.m_line_offset;
// Try the current file, but override if asked.
- FileSpec file = sym_ctx.line_entry.file;
+ FileSpec file = sym_ctx.line_entry.GetFile();
if (m_options.m_filenames.GetSize() == 1)
file = m_options.m_filenames.GetFileSpecAtIndex(0);
diff --git a/lldb/source/Core/Address.cpp b/lldb/source/Core/Address.cpp
index 6f5c366ab38a30..b23398883fa553 100644
--- a/lldb/source/Core/Address.cpp
+++ b/lldb/source/Core/Address.cpp
@@ -398,7 +398,7 @@ bool Address::GetDescription(Stream &s, Target &target,
"Non-brief descriptions not implemented");
LineEntry line_entry;
if (CalculateSymbolContextLineEntry(line_entry)) {
- s.Printf(" (%s:%u:%u)", line_entry.file.GetFilename().GetCString(),
+ s.Printf(" (%s:%u:%u)", line_entry.GetFile().GetFilename().GetCString(),
line_entry.line, line_entry.column);
return true;
}
diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp
index 7b07fcb2681307..e31746fa0b8b9d 100644
--- a/lldb/source/Core/Disassembler.cpp
+++ b/lldb/source/Core/Disassembler.cpp
@@ -201,7 +201,7 @@ Disassembler::GetFunctionDeclLineEntry(const SymbolContext &sc) {
uint32_t func_decl_line;
sc.function->GetStartLineSourceInfo(func_decl_file, func_decl_line);
- if (func_decl_file != prologue_end_line.file &&
+ if (func_decl_file != prologue_end_line.GetFile() &&
func_decl_file != prologue_end_line.original_file_sp->GetSpecOnly())
return {};
@@ -354,7 +354,7 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch,
}
if (sc.line_entry.IsValid()) {
SourceLine this_line;
- this_line.file = sc.line_entry.file;
+ this_line.file = sc.line_entry.GetFile();
this_line.line = sc.line_entry.line;
this_line.column = sc.line_entry.column;
if (!ElideMixedSourceAndDisassemblyLine(exe_ctx, sc, this_line))
@@ -406,7 +406,7 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch,
uint32_t func_decl_line;
sc.function->GetStartLineSourceInfo(func_decl_file,
func_decl_line);
- if (func_decl_file == prologue_end_line.file ||
+ if (func_decl_file == prologue_end_line.GetFile() ||
func_decl_file ==
prologue_end_line.original_file_sp->GetSpecOnly()) {
// Add all the lines between the function declaration and
@@ -439,7 +439,7 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch,
if (sc != prev_sc && sc.comp_unit && sc.line_entry.IsValid()) {
SourceLine this_line;
- this_line.file = sc.line_entry.file;
+ this_line.file = sc.line_entry.GetFile();
this_line.line = sc.line_entry.line;
if (!ElideMixedSourceAndDisassemblyLine(exe_ctx, sc,
diff --git a/lldb/source/Core/FormatEntity.cpp b/lldb/source/Core/FormatEntity.cpp
index cf82676beddabe..ba62e26252591f 100644
--- a/lldb/source/Core/FormatEntity.cpp
+++ b/lldb/source/Core/FormatEntity.cpp
@@ -1792,7 +1792,7 @@ bool FormatEntity::Format(const Entry &entry, Stream &s,
if (sc && sc->line_entry.IsValid()) {
Module *module = sc->module_sp.get();
if (module) {
- if (DumpFile(s, sc->line_entry.file, (FileKind)entry.number))
+ if (DumpFile(s, sc->line_entry.GetFile(), (FileKind)entry.number))
return true;
}
}
diff --git a/lldb/source/Core/IOHandlerCursesGUI.cpp b/lldb/source/Core/IOHandlerCursesGUI.cpp
index f86dce247135f8..d922d32f910583 100644
--- a/lldb/source/Core/IOHandlerCursesGUI.cpp
+++ b/lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -6894,7 +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.file) {
+ if (m_file_sp &&
+ m_file_sp->GetFileSpec() == 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)) {
@@ -6909,8 +6910,8 @@ class SourceFileWindowDelegate : public WindowDelegate {
} else {
// File changed, set selected line to the line with the PC
m_selected_line = m_pc_line;
- m_file_sp =
- m_debugger.GetSourceManager().GetFile(m_sc.line_entry.file);
+ m_file_sp = m_debugger.GetSourceManager().GetFile(
+ m_sc.line_entry.GetFile());
if (m_file_sp) {
const size_t num_lines = m_file_sp->GetNumLines();
m_line_width = 1;
@@ -7000,7 +7001,7 @@ 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.file) {
+ if (m_file_sp->GetFileSpec() == bp_loc_line_entry.GetFile()) {
bp_lines.insert(bp_loc_line_entry.line);
}
}
@@ -7477,7 +7478,7 @@ 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.file &&
+ if (m_file_sp->GetFileSpec() == bp_loc_line_entry.GetFile() &&
m_selected_line + 1 == bp_loc_line_entry.line) {
bool removed =
exe_ctx.GetTargetRef().RemoveBreakpointByID(bp_sp->GetID());
diff --git a/lldb/source/Core/SourceManager.cpp b/lldb/source/Core/SourceManager.cpp
index 517a4b0268d2a0..0d70c554e5342b 100644
--- a/lldb/source/Core/SourceManager.cpp
+++ b/lldb/source/Core/SourceManager.cpp
@@ -418,7 +418,7 @@ bool SourceManager::GetDefaultFileAndLine(FileSpec &file_spec, uint32_t &line) {
if (sc.function->GetAddressRange()
.GetBaseAddress()
.CalculateSymbolContextLineEntry(line_entry)) {
- SetDefaultFileAndLine(line_entry.file, line_entry.line);
+ SetDefaultFileAndLine(line_entry.GetFile(), line_entry.line);
file_spec = m_last_file_spec;
line = m_last_line;
return true;
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
index 3d43ed3f99ffba..3b601726388d6b 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
@@ -417,7 +417,7 @@ bool ClangExpressionSourceCode::GetText(
if (sc.comp_unit && sc.line_entry.IsValid()) {
DebugMacros *dm = sc.comp_unit->GetDebugMacros();
if (dm) {
- AddMacroState state(sc.line_entry.file, sc.line_entry.line);
+ AddMacroState state(sc.line_entry.GetFile(), sc.line_entry.line);
AddMacros(dm, sc.comp_unit, state, debug_macros_stream);
}
}
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index 10a1fe03918984..bcb04fae15bd46 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -106,13 +106,13 @@ bool lldb_private::formatters::LibcxxFunctionSummaryProvider(
case CPPLanguageRuntime::LibCppStdFunctionCallableCase::Lambda:
stream.Printf(
" Lambda in File %s at Line %u",
- callable_info.callable_line_entry.file.GetFilename().GetCString(),
+ callable_info.callable_line_entry.GetFile().GetFilename().GetCString(),
callable_info.callable_line_entry.line);
break;
case CPPLanguageRuntime::LibCppStdFunctionCallableCase::CallableObject:
stream.Printf(
" Function in File %s at Line %u",
- callable_info.callable_line_entry.file.GetFilename().GetCString(),
+ callable_info.callable_line_entry.GetFile().GetFilename().GetCString(),
callable_info.callable_line_entry.line);
break;
case CPPLanguageRuntime::LibCppStdFunctionCallableCase::FreeOrMemberFunction:
diff --git a/lldb/source/Symbol/CompileUnit.cpp b/lldb/source/Symbol/CompileUnit.cpp
index 1b3cd23d94006a..ddeacf18e855ee 100644
--- a/lldb/source/Symbol/CompileUnit.cpp
+++ b/lldb/source/Symbol/CompileUnit.cpp
@@ -320,7 +320,7 @@ void CompileUnit::ResolveSymbolContext(
src_location_spec.GetColumn() ? std::optional<uint16_t>(line_entry.column)
: std::nullopt;
- SourceLocationSpec found_entry(line_entry.file, line_entry.line, column,
+ SourceLocationSpec found_entry(line_entry.GetFile(), line_entry.line, column,
inlines, exact);
while (line_idx != UINT32_MAX) {
diff --git a/lldb/source/Symbol/Function.cpp b/lldb/source/Symbol/Function.cpp
index fdc090355771c6..194f89bc51d807 100644
--- a/lldb/source/Symbol/Function.cpp
+++ b/lldb/source/Symbol/Function.cpp
@@ -289,7 +289,7 @@ void Function::GetStartLineSourceInfo(FileSpec &source_file,
if (line_table->FindLineEntryByAddress(GetAddressRange().GetBaseAddress(),
line_entry, nullptr)) {
line_no = line_entry.line;
- source_file = line_entry.file;
+ source_file = line_entry.GetFile();
}
}
}
@@ -311,7 +311,7 @@ void Function::GetEndLineSourceInfo(FileSpec &source_file, uint32_t &line_no) {
LineEntry line_entry;
if (line_table->FindLineEntryByAddress(scratch_addr, line_entry, nullptr)) {
line_no = line_entry.line;
- source_file = line_entry.file;
+ source_file = line_entry.GetFile();
}
}
diff --git a/lldb/source/Symbol/LineEntry.cpp b/lldb/source/Symbol/LineEntry.cpp
index 389f8dcb65d8d8..9e0c06b6ff73ec 100644
--- a/lldb/source/Symbol/LineEntry.cpp
+++ b/lldb/source/Symbol/LineEntry.cpp
@@ -14,12 +14,12 @@
using namespace lldb_private;
LineEntry::LineEntry()
- : range(), file(), is_start_of_statement(0), is_start_of_basic_block(0),
+ : range(), is_start_of_statement(0), is_start_of_basic_block(0),
is_prologue_end(0), is_epilogue_begin(0), is_terminal_entry(0) {}
void LineEntry::Clear() {
range.Clear();
- file.Clear();
+ file_sp = std::make_shared<SupportFile>();
original_file_sp = std::make_shared<SupportFile>();
line = LLDB_INVALID_LINE_NUMBER;
column = 0;
@@ -35,6 +35,7 @@ bool LineEntry::IsValid() const {
}
bool LineEntry::DumpStopContext(Stream *s, bool show_fullpaths) const {
+ const FileSpec &file = file_sp->GetSpecOnly();
if (file) {
if (show_fullpaths)
...
[truncated]
|
This is another step towards supporting DWARF5 checksums and inline source code in LLDB. This is a reland of llvm#85468 but without the functional change of storing the support file from the line table (yet).
This is another step towards supporting DWARF5 checksums and inline
source code in LLDB. This is a reland of #85468 but without the functional
change of storing the support file from the line table (yet).