Skip to content

[lldb] Fix source display for artificial locations #115876

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.

Sign up for GitHub

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 1 commit into from
Nov 13, 2024
Merged

[lldb] Fix source display for artificial locations #115876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion lldb/include/lldb/Symbol/Function.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,8 @@ class Function : public UserID, public SymbolContextScope {
///
/// \param[out] line_no
/// The line number.
void GetStartLineSourceInfo(FileSpec &source_file, uint32_t &line_no);
void GetStartLineSourceInfo(lldb::SupportFileSP &source_file_sp,
uint32_t &line_no);

/// Find the file and line number of the source location of the end of the
/// function.
Expand Down
13 changes: 8 additions & 5 deletions lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,21 +139,23 @@ void BreakpointResolverFileLine::FilterContexts(SymbolContextList &sc_list) {
if (!sc.block)
continue;

FileSpec file;
SupportFileSP file_sp;
uint32_t line;
const Block *inline_block = sc.block->GetContainingInlinedBlock();
if (inline_block) {
const Declaration &inline_declaration = inline_block->GetInlinedFunctionInfo()->GetDeclaration();
if (!inline_declaration.IsValid())
continue;
file = inline_declaration.GetFile();
file_sp = std::make_shared<SupportFile>(inline_declaration.GetFile());
line = inline_declaration.GetLine();
} else if (sc.function)
sc.function->GetStartLineSourceInfo(file, line);
sc.function->GetStartLineSourceInfo(file_sp, line);
else
continue;

if (file != sc.line_entry.GetFile()) {
if (!file_sp ||
!file_sp->Equal(*sc.line_entry.file_sp,
SupportFile::eEqualFileSpecAndChecksumIfSet)) {
LLDB_LOG(log, "unexpected symbol context file {0}",
sc.line_entry.GetFile());
continue;
Expand Down Expand Up @@ -190,7 +192,8 @@ void BreakpointResolverFileLine::FilterContexts(SymbolContextList &sc_list) {
const int decl_line_is_too_late_fudge = 1;
if (line &&
m_location_spec.GetLine() < line - decl_line_is_too_late_fudge) {
LLDB_LOG(log, "removing symbol context at {0}:{1}", file, line);
LLDB_LOG(log, "removing symbol context at {0}:{1}",
file_sp->GetSpecOnly(), line);
sc_list.RemoveContextAtIndex(i);
--i;
}
Expand Down
4 changes: 1 addition & 3 deletions lldb/source/Commands/CommandObjectSource.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -784,9 +784,7 @@ class CommandObjectSourceList : public CommandObjectParsed {

if (sc.block == nullptr) {
// Not an inlined function
FileSpec function_file_spec;
sc.function->GetStartLineSourceInfo(function_file_spec, start_line);
start_file = std::make_shared<SupportFile>(function_file_spec);
sc.function->GetStartLineSourceInfo(start_file, start_line);
if (start_line == 0) {
result.AppendErrorWithFormat("Could not find line information for "
"start of function: \"%s\".\n",
Expand Down
30 changes: 19 additions & 11 deletions lldb/source/Core/Disassembler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,16 +205,20 @@ Disassembler::GetFunctionDeclLineEntry(const SymbolContext &sc) {
return {};

LineEntry prologue_end_line = sc.line_entry;
FileSpec func_decl_file;
SupportFileSP func_decl_file_sp;
Copy link
Member

Choose a reason for hiding this comment

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

As you've noticed, SupportFileSP is always valid in a few places (like in LineEntry). I don't think that's something that should generally hold (mostly because it's hard to enforce) so I would have done the same thing here. The alternative would be to initialize this to an empty SupportFile and have GetStartLineSourceInfo do the same instead of calling ::reset, though I think that's pretty inefficient. Anyway, just an observation, no need to change anything.

uint32_t func_decl_line;
sc.function->GetStartLineSourceInfo(func_decl_file, func_decl_line);
sc.function->GetStartLineSourceInfo(func_decl_file_sp, func_decl_line);

if (func_decl_file != prologue_end_line.GetFile() &&
func_decl_file != prologue_end_line.original_file_sp->GetSpecOnly())
if (!func_decl_file_sp)
return {};
if (!func_decl_file_sp->Equal(*prologue_end_line.file_sp,
SupportFile::eEqualFileSpecAndChecksumIfSet) &&
!func_decl_file_sp->Equal(*prologue_end_line.original_file_sp,
SupportFile::eEqualFileSpecAndChecksumIfSet))
return {};

SourceLine decl_line;
decl_line.file = func_decl_file;
decl_line.file = func_decl_file_sp->GetSpecOnly();
decl_line.line = func_decl_line;
// TODO: Do we care about column on these entries? If so, we need to plumb
// that through GetStartLineSourceInfo.
Expand Down Expand Up @@ -410,20 +414,24 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch,
LineEntry prologue_end_line = sc.line_entry;
if (!ElideMixedSourceAndDisassemblyLine(exe_ctx, sc,
prologue_end_line)) {
FileSpec func_decl_file;
SupportFileSP func_decl_file_sp;
uint32_t func_decl_line;
sc.function->GetStartLineSourceInfo(func_decl_file,
sc.function->GetStartLineSourceInfo(func_decl_file_sp,
func_decl_line);
if (func_decl_file == prologue_end_line.GetFile() ||
func_decl_file ==
prologue_end_line.original_file_sp->GetSpecOnly()) {
if (func_decl_file_sp &&
(func_decl_file_sp->Equal(
*prologue_end_line.file_sp,
SupportFile::eEqualFileSpecAndChecksumIfSet) ||
func_decl_file_sp->Equal(
*prologue_end_line.original_file_sp,
SupportFile::eEqualFileSpecAndChecksumIfSet))) {
// Add all the lines between the function declaration and
// the first non-prologue source line to the list of lines
// to print.
for (uint32_t lineno = func_decl_line;
lineno <= prologue_end_line.line; lineno++) {
SourceLine this_line;
this_line.file = func_decl_file;
this_line.file = func_decl_file_sp->GetSpecOnly();
this_line.line = lineno;
source_lines_to_display.lines.push_back(this_line);
}
Expand Down
9 changes: 5 additions & 4 deletions lldb/source/Symbol/Function.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,10 +287,10 @@ Function::Function(CompileUnit *comp_unit, lldb::user_id_t func_uid,

Function::~Function() = default;

void Function::GetStartLineSourceInfo(FileSpec &source_file,
void Function::GetStartLineSourceInfo(SupportFileSP &source_file_sp,
uint32_t &line_no) {
line_no = 0;
source_file.Clear();
source_file_sp.reset();

if (m_comp_unit == nullptr)
return;
Expand All @@ -299,7 +299,8 @@ void Function::GetStartLineSourceInfo(FileSpec &source_file,
GetType();

if (m_type != nullptr && m_type->GetDeclaration().GetLine() != 0) {
source_file = m_type->GetDeclaration().GetFile();
source_file_sp =
std::make_shared<SupportFile>(m_type->GetDeclaration().GetFile());
line_no = m_type->GetDeclaration().GetLine();
} else {
LineTable *line_table = m_comp_unit->GetLineTable();
Expand All @@ -310,7 +311,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.GetFile();
source_file_sp = line_entry.file_sp;
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions lldb/source/Target/StackFrame.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1918,15 +1918,15 @@ bool StackFrame::GetStatus(Stream &strm, bool show_frame_info, bool show_source,
if (m_sc.comp_unit && m_sc.line_entry.IsValid()) {
have_debuginfo = true;
if (source_lines_before > 0 || source_lines_after > 0) {
SupportFileSP source_file_sp = m_sc.line_entry.file_sp;
uint32_t start_line = m_sc.line_entry.line;
if (!start_line && m_sc.function) {
FileSpec source_file;
m_sc.function->GetStartLineSourceInfo(source_file, start_line);
m_sc.function->GetStartLineSourceInfo(source_file_sp, start_line);
}

size_t num_lines =
target->GetSourceManager().DisplaySourceLinesWithLineNumbers(
m_sc.line_entry.file_sp, start_line, m_sc.line_entry.column,
source_file_sp, start_line, m_sc.line_entry.column,
source_lines_before, source_lines_after, "->", &strm);
if (num_lines != 0)
have_source = true;
Expand Down
19 changes: 14 additions & 5 deletions lldb/test/API/source-manager/TestSourceManager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,19 +314,28 @@ def test_set_breakpoint_with_absolute_path(self):
)

def test_artificial_source_location(self):
src_file = "artificial_location.c"
d = {"C_SOURCES": src_file}
src_file = "artificial_location.cpp"
d = {"C_SOURCES": "", "CXX_SOURCES": src_file}
self.build(dictionary=d)

lldbutil.run_to_source_breakpoint(
self, "main", lldb.SBFileSpec(src_file, False)
)
target = lldbutil.run_to_breakpoint_make_target(self)

# Find the instruction with line=0 and put a breakpoint there.
sc_list = target.FindFunctions("A::foo")
self.assertEqual(len(sc_list), 1)
insns = sc_list[0].function.GetInstructions(target)
insn0 = next(filter(lambda insn: insn.addr.line_entry.line == 0, insns))
bkpt = target.BreakpointCreateBySBAddress(insn0.addr)
self.assertGreater(bkpt.GetNumLocations(), 0)

lldbutil.run_to_breakpoint_do_run(self, target, bkpt)

self.expect(
"process status",
substrs=[
"stop reason = breakpoint",
f"{src_file}:0",
"static int foo();",
"Note: this address is compiler-generated code in function",
"that has no source code associated with it.",
],
Expand Down
6 changes: 0 additions & 6 deletions lldb/test/API/source-manager/artificial_location.c
View file
Open in desktop

This file was deleted.

8 changes: 8 additions & 0 deletions lldb/test/API/source-manager/artificial_location.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#include "artificial_location.h"

int A::foo() {
#line 0
Copy link
Member

Choose a reason for hiding this comment

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

FYI there's some discussion about this test going on in #107849, in case you had some thoughts on Jeremy's points

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the pointer. Quite a coincidence that I end up touching the same test just as Jaremy ends up breaking it :P

return 42;
}

int main() { return A::foo(); }
3 changes: 3 additions & 0 deletions lldb/test/API/source-manager/artificial_location.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
struct A {
static int foo();
};
Loading