Skip to content

Commit

Permalink
[lldb] Store SupportFile in FileEntry (NFC) (#85468)
Browse files Browse the repository at this point in the history 
This is another step towards supporting DWARF5 checksums and inline
source code in LLDB.
  • Loading branch information
@JDevlieghere
JDevlieghere committed Mar 15, 2024
1 parent 8ebf408 commit d5a277d
Show file tree
Hide file tree
Showing 26 changed files with 89 additions and 71 deletions.
2 changes: 1 addition & 1 deletion lldb/include/lldb/Core/Disassembler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
34 changes: 22 additions & 12 deletions lldb/include/lldb/Symbol/LineEntry.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,18 +130,28 @@ struct LineEntry {
/// Shared pointer to the target this LineEntry belongs to.
void ApplyFileMappings(lldb::TargetSP target_sp);

// Member variables.
AddressRange range; ///< The section offset address range for this line entry.
FileSpec file; ///< The source file, possibly mapped by the target.source-map
///setting
lldb::SupportFileSP
original_file_sp; ///< The original source file, from debug info.
uint32_t line = LLDB_INVALID_LINE_NUMBER; ///< The source line number, or zero
///< if there is no line number
/// information.
uint16_t column =
0; ///< The column number of the source line, or zero if there
/// is no column information.
const FileSpec &GetFile() const {
assert(file_sp);
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.
lldb::SupportFileSP file_sp;

/// The original source file, from debug info.
lldb::SupportFileSP original_file_sp;

/// The source line number, or LLDB_INVALID_LINE_NUMBER if there is no line
/// number information.
uint32_t line = LLDB_INVALID_LINE_NUMBER;

/// The column number of the source line, or zero if there is no column
/// information.
uint16_t column = 0;

uint16_t is_start_of_statement : 1, ///< Indicates this entry is the beginning
///of a statement.
is_start_of_basic_block : 1, ///< Indicates this entry is the beginning of
Expand Down
3 changes: 3 additions & 0 deletions lldb/include/lldb/Utility/SupportFile.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 5 additions & 5 deletions lldb/source/API/SBLineEntry.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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->Update(filespec.ref());
else
ref().file.Clear();
ref().file_sp->Update(FileSpec());
}
void SBLineEntry::SetLine(uint32_t line) {
LLDB_INSTRUMENT_VA(this, line);
Expand Down Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/API/SBThread.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Breakpoint/BreakpointResolver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 4 additions & 3 deletions lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Commands/CommandObjectBreakpoint.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand Down
14 changes: 7 additions & 7 deletions lldb/source/Commands/CommandObjectSource.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Commands/CommandObjectThread.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.file_sp->GetSpecOnly();
if (m_options.m_filenames.GetSize() == 1)
file = m_options.m_filenames.GetFileSpecAtIndex(0);

Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Core/Address.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
8 changes: 4 additions & 4 deletions lldb/source/Core/Disassembler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.file_sp->GetSpecOnly() &&
func_decl_file != prologue_end_line.original_file_sp->GetSpecOnly())
return {};

Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Core/FormatEntity.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
11 changes: 6 additions & 5 deletions lldb/source/Core/IOHandlerCursesGUI.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Core/SourceManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Symbol/CompileUnit.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Symbol/Function.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
}
Expand All @@ -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();
}
}

Expand Down

1 comment on commit d5a277d

@slackito
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has broken stepping in one of our tests. I'm still figuring out how this works but I wanted to give you an early heads up. Hopefully this info is enough for you to figure out the problem, as I don't have a lot of experience with the lldb code base.

The divergence happens in Block::GetContainingInlinedBlockWithCallSite, with a stacktrace like this:

#0  lldb_private::Block::GetContainingInlinedBlockWithCallSite (this=0x73d9ffeee0e0, 
    find_call_site=...)
    at /proc/self/cwd/third_party/llvm/llvm-project/lldb/source/Symbol/Block.cpp:234
#1  0x000055556c0723f5 in lldb_private::LineEntry::GetSameLineContiguousAddressRange (
    this=0x73d9fe25c0c0, include_inlined_functions=true)
    at /proc/self/cwd/third_party/llvm/llvm-project/lldb/source/Symbol/LineEntry.cpp:222
#2  0x000055556c2b94e7 in lldb_private::Thread::QueueThreadPlanForStepOverRange (
    this=0x73d9fbf64a18, abort_other_plans=false, line_entry=..., addr_context=..., 
    stop_other_threads=lldb::eOnlyDuringStepping, status=..., 
    step_out_avoids_code_withoug_debug_info=lldb_private::eLazyBoolCalculate)
    at third_party/llvm/llvm-project/lldb/source/Target/Thread.cpp:1274
#3  0x000055556b74d1d9 in CommandObjectThreadStepWithTypeAndScope::DoExecute (
    this=0x73d9fefa2800, command=..., result=...)
    at third_party/llvm/llvm-project/lldb/source/Commands/CommandObjectThread.cpp:538
#4  0x000055556b7dd877 in lldb_private::CommandObjectParsed::Execute (this=0x73d9fefa2800, 
    args_string=0x7fffffffc228 "", result=...)
    at third_party/llvm/llvm-project/lldb/source/Interpreter/CommandObject.cpp:829
#5  0x000055556b4dafc8 in lldb_private::CommandInterpreter::HandleCommand (this=0x73d9ff71c280, 
    command_line=0x73d9ffee35c0 "n", lazy_add_to_history=lldb_private::eLazyBoolCalculate, 
    result=..., force_repeat_command=false)
    at third_party/llvm/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:2038
#6  0x000055556b4e0ac8 in lldb_private::CommandInterpreter::IOHandlerInputComplete (
    this=0x73d9ff71c280, io_handler=..., line=...)
    at third_party/llvm/llvm-project/lldb/source/Interpreter/CommandInterpreter.cpp:3118

After this commit, when we get to the marked line in the code below, find_call_site contains a FileSpec where m_directory has been mapped by target.source-map, whereas function_info->GetCallSite() has NOT been mapped. This causes the comparison to fail and the n command to compute the stopping address incorrectly.

Block *Block::GetContainingInlinedBlockWithCallSite(
    const Declaration &find_call_site) {
  Block *inlined_block = GetContainingInlinedBlock();

  while (inlined_block) {
    const auto *function_info = inlined_block->GetInlinedFunctionInfo();

    if (function_info &&
        function_info->GetCallSite().FileAndLineEqual(find_call_site))  <-------------- here
      return inlined_block;
    inlined_block = inlined_block->GetInlinedParent();
  }
  return nullptr;
}

Could you please take a look? If you need any additional information please let me know

Please sign in to comment.