[lldb] Add support for synthetic LineEntry objects without valid address ranges #158811
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
medismailben
changed the title
Member
|
@llvm/pr-subscribers-lldb Author: Med Ismail Bennani (medismailben) ChangesPatch is 37.34 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/158811.diff 26 Files Affected:
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index ab2e5e170559d..6a8d9ccfc8bc7 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -279,7 +279,8 @@ class BreakpointLocation
/// The line entry must have the same start address as the address for this
/// location.
bool SetPreferredLineEntry(const LineEntry &line_entry) {
- if (m_address == line_entry.range.GetBaseAddress()) {
+ if (line_entry.HasValidRange() &&
+ m_address == line_entry.GetRange().GetBaseAddress()) {
m_preferred_line_entry = line_entry;
return true;
}
diff --git a/lldb/include/lldb/Core/Address.h b/lldb/include/lldb/Core/Address.h
index 85b2ab7bb3cfe..a16cb55680a68 100644
--- a/lldb/include/lldb/Core/Address.h
+++ b/lldb/include/lldb/Core/Address.h
@@ -30,7 +30,7 @@ class Stream;
class Symbol;
class SymbolContext;
class Target;
-struct LineEntry;
+class LineEntry;
/// \class Address Address.h "lldb/Core/Address.h"
/// A section + offset based address class.
diff --git a/lldb/include/lldb/Symbol/LineEntry.h b/lldb/include/lldb/Symbol/LineEntry.h
index 8da59cf0bd24a..755729a2a1442 100644
--- a/lldb/include/lldb/Symbol/LineEntry.h
+++ b/lldb/include/lldb/Symbol/LineEntry.h
@@ -13,12 +13,14 @@
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/SupportFile.h"
#include "lldb/lldb-private.h"
+#include <optional>
namespace lldb_private {
/// \class LineEntry LineEntry.h "lldb/Symbol/LineEntry.h"
/// A line table entry class.
-struct LineEntry {
+class LineEntry {
+public:
/// Default constructor.
///
/// Initialize all member variables to invalid values.
@@ -133,9 +135,21 @@ struct LineEntry {
/// Helper to access the file.
const FileSpec &GetFile() const { return file_sp->GetSpecOnly(); }
- /// The section offset address range for this line entry.
- AddressRange range;
+ /// Get the address range for this line entry.
+ /// \return The address range if valid, otherwise an invalid AddressRange.
+ const AddressRange &GetRange() const;
+ /// Check if this line entry has a valid address range.
+ bool HasValidRange() const;
+
+ /// Set the address range for this line entry.
+ void SetRange(const AddressRange &range);
+
+private:
+ /// The section offset address range for this line entry (optional).
+ std::optional<AddressRange> m_range;
+
+public:
/// The source file, possibly mapped by the target.source-map setting.
lldb::SupportFileSP file_sp;
diff --git a/lldb/include/lldb/lldb-forward.h b/lldb/include/lldb/lldb-forward.h
index af5656b3dcad1..1dedfd0d644f7 100644
--- a/lldb/include/lldb/lldb-forward.h
+++ b/lldb/include/lldb/lldb-forward.h
@@ -306,7 +306,7 @@ class WatchpointResource;
class WatchpointResourceCollection;
class WatchpointSetOptions;
struct CompilerContext;
-struct LineEntry;
+class LineEntry;
struct PropertyDefinition;
struct ScriptSummaryFormat;
struct StatisticsOptions;
diff --git a/lldb/source/API/SBLineEntry.cpp b/lldb/source/API/SBLineEntry.cpp
index 0f4936f32a074..78ff488e8927f 100644
--- a/lldb/source/API/SBLineEntry.cpp
+++ b/lldb/source/API/SBLineEntry.cpp
@@ -50,8 +50,8 @@ SBAddress SBLineEntry::GetStartAddress() const {
LLDB_INSTRUMENT_VA(this);
SBAddress sb_address;
- if (m_opaque_up)
- sb_address.SetAddress(m_opaque_up->range.GetBaseAddress());
+ if (m_opaque_up && m_opaque_up->HasValidRange())
+ sb_address.SetAddress(m_opaque_up->GetRange().GetBaseAddress());
return sb_address;
}
@@ -60,9 +60,9 @@ SBAddress SBLineEntry::GetEndAddress() const {
LLDB_INSTRUMENT_VA(this);
SBAddress sb_address;
- if (m_opaque_up) {
- sb_address.SetAddress(m_opaque_up->range.GetBaseAddress());
- sb_address.OffsetAddress(m_opaque_up->range.GetByteSize());
+ if (m_opaque_up && m_opaque_up->HasValidRange()) {
+ sb_address.SetAddress(m_opaque_up->GetRange().GetBaseAddress());
+ sb_address.OffsetAddress(m_opaque_up->GetRange().GetByteSize());
}
return sb_address;
}
diff --git a/lldb/source/API/SBThread.cpp b/lldb/source/API/SBThread.cpp
index 4e4aa48bc9a2e..da240a84db48c 100644
--- a/lldb/source/API/SBThread.cpp
+++ b/lldb/source/API/SBThread.cpp
@@ -515,9 +515,14 @@ void SBThread::StepInto(const char *target_name, uint32_t end_line,
if (frame_sp && frame_sp->HasDebugInformation()) {
SymbolContext sc(frame_sp->GetSymbolContext(eSymbolContextEverything));
AddressRange range;
- if (end_line == LLDB_INVALID_LINE_NUMBER)
- range = sc.line_entry.range;
- else {
+ if (end_line == LLDB_INVALID_LINE_NUMBER) {
+ if (sc.line_entry.HasValidRange())
+ range = sc.line_entry.GetRange();
+ else {
+ error = Status::FromErrorString("No valid range for line entry");
+ return;
+ }
+ } else {
llvm::Error err = sc.GetAddressRangeFromHereToEndLine(end_line, range);
if (err) {
error = Status::FromErrorString(llvm::toString(std::move(err)).c_str());
@@ -787,7 +792,7 @@ SBError SBThread::StepOverUntil(lldb::SBFrame &sb_frame,
eSymbolContextLineEntry, sc_list);
for (const SymbolContext &sc : sc_list) {
addr_t step_addr =
- sc.line_entry.range.GetBaseAddress().GetLoadAddress(target);
+ sc.line_entry.GetRange().GetBaseAddress().GetLoadAddress(target);
if (step_addr != LLDB_INVALID_ADDRESS) {
AddressRange unused_range;
if (frame_sc.function->GetRangeContainingLoadAddress(step_addr, *target,
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp b/lldb/source/Breakpoint/BreakpointResolver.cpp
index 4ac40501a5df5..a91805156a63c 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -273,11 +273,19 @@ void BreakpointResolver::SetSCMatchesByLine(
}
// Sort by file address.
- llvm::sort(worklist_begin, worklist_end,
- [](const SymbolContext &a, const SymbolContext &b) {
- return a.line_entry.range.GetBaseAddress().GetFileAddress() <
- b.line_entry.range.GetBaseAddress().GetFileAddress();
- });
+ llvm::sort(
+ worklist_begin, worklist_end,
+ [](const SymbolContext &a, const SymbolContext &b) {
+ auto a_addr =
+ a.line_entry.HasValidRange()
+ ? a.line_entry.GetRange().GetBaseAddress().GetFileAddress()
+ : LLDB_INVALID_ADDRESS;
+ auto b_addr =
+ b.line_entry.HasValidRange()
+ ? b.line_entry.GetRange().GetBaseAddress().GetFileAddress()
+ : LLDB_INVALID_ADDRESS;
+ return a_addr < b_addr;
+ });
// Go through and see if there are line table entries that are
// contiguous, and if so keep only the first of the contiguous range.
@@ -307,7 +315,9 @@ void BreakpointResolver::AddLocation(SearchFilter &filter,
bool skip_prologue,
llvm::StringRef log_ident) {
Log *log = GetLog(LLDBLog::Breakpoints);
- Address line_start = sc.line_entry.range.GetBaseAddress();
+ Address line_start = sc.line_entry.HasValidRange()
+ ? sc.line_entry.GetRange().GetBaseAddress()
+ : Address();
if (!line_start.IsValid()) {
LLDB_LOGF(log,
"error: Unable to set breakpoint %s at file address "
diff --git a/lldb/source/Commands/CommandObjectDisassemble.cpp b/lldb/source/Commands/CommandObjectDisassemble.cpp
index c0553d2c6c8b2..5c32e985c78b8 100644
--- a/lldb/source/Commands/CommandObjectDisassemble.cpp
+++ b/lldb/source/Commands/CommandObjectDisassemble.cpp
@@ -356,8 +356,8 @@ CommandObjectDisassemble::GetCurrentLineRanges() {
LineEntry pc_line_entry(
frame->GetSymbolContext(eSymbolContextLineEntry).line_entry);
- if (pc_line_entry.IsValid())
- return std::vector<AddressRange>{pc_line_entry.range};
+ if (pc_line_entry.IsValid() && pc_line_entry.HasValidRange())
+ return std::vector<AddressRange>{pc_line_entry.GetRange()};
// No line entry, so just disassemble around the current pc
m_options.show_mixed = false;
diff --git a/lldb/source/Commands/CommandObjectSource.cpp b/lldb/source/Commands/CommandObjectSource.cpp
index 0b4599b16ef0d..fe38aa82ae5c0 100644
--- a/lldb/source/Commands/CommandObjectSource.cpp
+++ b/lldb/source/Commands/CommandObjectSource.cpp
@@ -1032,9 +1032,11 @@ class CommandObjectSourceList : public CommandObjectParsed {
bool show_inlined_frames = true;
const bool show_function_arguments = true;
const bool show_function_name = true;
+ Address addr = sc.line_entry.HasValidRange()
+ ? sc.line_entry.GetRange().GetBaseAddress()
+ : Address();
sc.DumpStopContext(&result.GetOutputStream(),
- m_exe_ctx.GetBestExecutionContextScope(),
- sc.line_entry.range.GetBaseAddress(),
+ m_exe_ctx.GetBestExecutionContextScope(), addr,
show_fullpaths, show_module, show_inlined_frames,
show_function_arguments, show_function_name);
result.GetOutputStream().EOL();
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index 0f96fa92a731d..43a6211a18a33 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -1628,8 +1628,8 @@ static void DumpSymbolContextList(
strm.EOL();
Address addr;
- if (sc.line_entry.IsValid())
- addr = sc.line_entry.range.GetBaseAddress();
+ if (sc.line_entry.IsValid() && sc.line_entry.HasValidRange())
+ addr = sc.line_entry.GetRange().GetBaseAddress();
else if (sc.block && sc.block->GetContainingInlinedBlock())
sc.block->GetContainingInlinedBlock()->GetStartAddress(addr);
else
diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp
index bbec714642ec9..9c9573cba9529 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -519,7 +519,7 @@ class CommandObjectThreadStepWithTypeAndScope : public CommandObjectParsed {
block_range.GetByteSize() - pc_offset_in_block;
range = AddressRange(pc_address, range_length);
} else {
- range = sc.line_entry.range;
+ range = sc.line_entry.GetRange();
}
new_plan_sp = thread->QueueThreadPlanForStepInRange(
@@ -999,7 +999,7 @@ class CommandObjectThreadUntil : public CommandObjectParsed {
while (idx < end_func_idx) {
if (line_idx_ranges.FindEntryIndexThatContains(idx) != UINT32_MAX) {
addr_t address =
- line_entry.range.GetBaseAddress().GetLoadAddress(target);
+ line_entry.GetRange().GetBaseAddress().GetLoadAddress(target);
if (address != LLDB_INVALID_ADDRESS)
address_list.push_back(address);
}
diff --git a/lldb/source/Core/AddressResolverFileLine.cpp b/lldb/source/Core/AddressResolverFileLine.cpp
index 6ab3b8fcee154..be2f3aa4bebbe 100644
--- a/lldb/source/Core/AddressResolverFileLine.cpp
+++ b/lldb/source/Core/AddressResolverFileLine.cpp
@@ -46,8 +46,8 @@ AddressResolverFileLine::SearchCallback(SearchFilter &filter,
cu->ResolveSymbolContext(m_src_location_spec, eSymbolContextEverything,
sc_list);
for (const SymbolContext &sc : sc_list) {
- Address line_start = sc.line_entry.range.GetBaseAddress();
- addr_t byte_size = sc.line_entry.range.GetByteSize();
+ Address line_start = sc.line_entry.GetRange().GetBaseAddress();
+ addr_t byte_size = sc.line_entry.GetRange().GetByteSize();
if (line_start.IsValid()) {
AddressRange new_range(line_start, byte_size);
m_address_ranges.push_back(new_range);
diff --git a/lldb/source/Core/FormatEntity.cpp b/lldb/source/Core/FormatEntity.cpp
index 491f5c6320d97..2548dfd1620fd 100644
--- a/lldb/source/Core/FormatEntity.cpp
+++ b/lldb/source/Core/FormatEntity.cpp
@@ -1918,10 +1918,11 @@ bool FormatEntity::Format(const Entry &entry, Stream &s,
return false;
case Entry::Type::FunctionLineOffset:
- if (sc)
- return (DumpAddressOffsetFromFunction(
- s, sc, exe_ctx, sc->line_entry.range.GetBaseAddress(), false, false,
- false));
+ if (sc) {
+ Address line_addr = sc->line_entry.GetRange().GetBaseAddress();
+ return (DumpAddressOffsetFromFunction(s, sc, exe_ctx, line_addr, false,
+ false, false));
+ }
return false;
case Entry::Type::FunctionPCOffset:
@@ -1986,11 +1987,11 @@ bool FormatEntity::Format(const Entry &entry, Stream &s,
case Entry::Type::LineEntryStartAddress:
case Entry::Type::LineEntryEndAddress:
- if (sc && sc->line_entry.range.GetBaseAddress().IsValid()) {
- Address addr = sc->line_entry.range.GetBaseAddress();
+ if (sc && sc->line_entry.HasValidRange()) {
+ Address addr = sc->line_entry.GetRange().GetBaseAddress();
if (entry.type == Entry::Type::LineEntryEndAddress)
- addr.Slide(sc->line_entry.range.GetByteSize());
+ addr.Slide(sc->line_entry.GetRange().GetByteSize());
if (DumpAddressAndContent(s, sc, exe_ctx, addr, false))
return true;
}
diff --git a/lldb/source/DataFormatters/TypeSummary.cpp b/lldb/source/DataFormatters/TypeSummary.cpp
index 6aa290698cd12..489d6c43f5092 100644
--- a/lldb/source/DataFormatters/TypeSummary.cpp
+++ b/lldb/source/DataFormatters/TypeSummary.cpp
@@ -105,8 +105,8 @@ bool StringSummaryFormat::FormatObject(ValueObject *valobj, std::string &retval,
retval = std::string(s.GetString());
return true;
} else {
- if (FormatEntity::Format(m_format, s, &sc, &exe_ctx,
- &sc.line_entry.range.GetBaseAddress(), valobj,
+ Address temp_addr = sc.line_entry.GetRange().GetBaseAddress();
+ if (FormatEntity::Format(m_format, s, &sc, &exe_ctx, &temp_addr, valobj,
false, false)) {
retval.assign(std::string(s.GetString()));
return true;
diff --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
index 0e2ca1784e7e9..58d0c7f309784 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -947,7 +947,7 @@ uint32_t SymbolFilePDB::ResolveSymbolContext(
continue;
auto file_vm_addr =
- sc.line_entry.range.GetBaseAddress().GetFileAddress();
+ sc.line_entry.GetRange().GetBaseAddress().GetFileAddress();
if (file_vm_addr == LLDB_INVALID_ADDRESS || file_vm_addr == 0)
continue;
diff --git a/lldb/source/Symbol/CompileUnit.cpp b/lldb/source/Symbol/CompileUnit.cpp
index 166f111ef6220..64e4e29d99c6b 100644
--- a/lldb/source/Symbol/CompileUnit.cpp
+++ b/lldb/source/Symbol/CompileUnit.cpp
@@ -335,7 +335,7 @@ void CompileUnit::ResolveSymbolContext(
// line entry will be the in function that contained the line that might
// be a CallSite, and we can just iterate over that function to find any
// inline records, and dig up their call sites.
- Address start_addr = line_entry.range.GetBaseAddress();
+ Address start_addr = line_entry.GetRange().GetBaseAddress();
Function *function = start_addr.CalculateSymbolContextFunction();
// Record the size of the list to see if we added to it:
size_t old_sc_list_size = sc_list.GetSize();
@@ -390,8 +390,9 @@ void CompileUnit::ResolveSymbolContext(
*src_location_spec.GetColumn() == call_site_line.column))
matches_spec = true;
}
- if (matches_spec &&
- sibling_block->GetRangeAtIndex(0, call_site_line.range)) {
+ AddressRange range;
+ if (matches_spec && sibling_block->GetRangeAtIndex(0, range)) {
+ call_site_line.SetRange(range);
SymbolContext call_site_sc(sc.target_sp, sc.module_sp,
sc.comp_unit, sc.function, sc.block,
&call_site_line, sc.symbol);
@@ -446,8 +447,9 @@ void CompileUnit::ResolveSymbolContext(
if (resolve_scope == eSymbolContextLineEntry) {
sc_list.Append(sc);
} else {
- line_entry.range.GetBaseAddress().CalculateSymbolContext(&resolved_sc,
- resolve_scope);
+ if (line_entry.HasValidRange())
+ line_entry.GetRange().GetBaseAddress().CalculateSymbolContext(
+ &resolved_sc, resolve_scope);
// Sometimes debug info is bad and isn't able to resolve the line entry's
// address back to the same compile unit and/or line entry. If the compile
// unit changed, then revert back to just the compile unit and line entry.
@@ -474,7 +476,9 @@ void CompileUnit::ResolveSymbolContext(
"unable to resolve a line table file address {0:x16} back "
"to a compile unit, please file a bug and attach the address "
"and file.",
- line_entry.range.GetBaseAddress().GetFileAddress());
+ line_entry.HasValidRange()
+ ? line_entry.GetRange().GetBaseAddress().GetFileAddress()
+ : 0);
}
sc_list.Append(sc);
}
diff --git a/lldb/source/Symbol/Function.cpp b/lldb/source/Symbol/Function.cpp
index 6114eccd935ee..39e85a10665d0 100644
--- a/lldb/source/Symbol/Function.cpp
+++ b/lldb/source/Symbol/Function.cpp
@@ -593,7 +593,7 @@ uint32_t Function::GetPrologueByteSize() {
if (first_line_entry.is_prologue_end) {
prologue_end_file_addr =
- first_line_entry.range.GetBaseAddress().GetFileAddress();
+ first_line_entry.GetRange().GetBaseAddress().GetFileAddress();
prologue_end_line_idx = first_line_entry_idx;
} else {
// Check the first few instructions and look for one that has
@@ -605,7 +605,7 @@ uint32_t Function::GetPrologueByteSize() {
if (line_table->GetLineEntryAtIndex(idx, line_entry)) {
if (line_entry.is_prologue_end) {
prologue_end_file_addr =
- line_entry.range.GetBaseAddress().GetFileAddress();
+ line_entry.GetRange().GetBaseAddress().GetFileAddress();
prologue_end_line_idx = idx;
break;
}
@@ -625,7 +625,7 @@ uint32_t Function::GetPrologueByteSize() {
if (line_table->GetLineEntryAtIndex(idx, line_entry)) {
if (line_entry.line != first_line_entry.line) {
prologue_end_file_addr =
- line_entry.range.GetBaseAddress().GetFileAddress();
+ line_entry.GetRange().GetBaseAddress().GetFileAddress();
prologue_end_line_idx = idx;
break;
}
@@ -634,8 +634,8 @@ uint32_t Function::GetPrologueByteSize() {
if (prologue_end_file_addr == LLDB_INVALID_ADDRESS) {
prologue_end_file_addr =
- first_line_entry.range.GetBaseAddress().GetFileAddress() +
- first_line_entry.range.GetByteSize();
+ first_line_entry.GetRange().GetBaseAddress().GetFileAddress() +
+ first_line_entry.GetRange().GetByteSize();
prologue_end_line_idx = first_line_entry_idx;
}
}
@@ -659,7 +659,7 @@ uint32_t Function::GetPrologueByteSize() {
if (line_entry.line != 0)
break;
}
- if (line_entry.range.GetBaseAddress().GetFileAddress() >=
+ if (line_entry.GetRange().GetBaseAddress().GetFileAddress() >=
range_end_file_addr)
break;
@@ -670,8 +670,9 @@ uint32_t Function::GetPrologueByteSize() {
LineEntry first_non_zero_entry;
if (line_table->GetLineEntryAtIndex(first_non_zero_line,
...
[truncated]
|
medismailben
requested review from
DavidSpickett,
bulbazord,
jimingham and
labath
Member
|
Can we break this up into an NFC patch that makes |
Member
+1. Why do you want to turn it into a class? In C++, the only difference between a class and a struct is the default access level. Structs are public by default, Classes are private by default. |
medismailben
force-pushed
the
line-entry-optional-range
branch
3 times, most recently
from
86e61fb to
0562715
Compare
medismailben
changed the title
medismailben
force-pushed
the
line-entry-optional-range
branch
from
0562715 to
5d34b21
Compare
Member
Author
To Alex's point, this didn't require to turn the |
JDevlieghere
requested changes
Dec 2, 2025
Member
JDevlieghere
left a comment
JDevlieghere
left a comment
There was a problem hiding this comment.
I strongly dislike this API because it easy to misuse and hard to use correctly. That's based on my very recent experience with the SupportFileSP. When it's used incorrectly (which it will, just like SupportFileSP was) it will lead to subtle and hard to diagnose bugs. My main qualm is the GetRange accessor which gives a false sense of security. If we need to make the AddressRange optional, then we should just keep it a (public) member and force every call site to consider that it may not be present.
bulbazord
requested changes
Dec 2, 2025
Member
bulbazord
left a comment
bulbazord
left a comment
There was a problem hiding this comment.
Instead of modifying the AddressRange member to be an optional, why not only relax the LineEntry validity check? There's a lot of code that needs to be changed here to make the range member optional and you need to remember to call HasValidRange. Not every place that accesses the AddressRange checks for a valid range either.
Functionally, the only benefit to having AddressRange be an optional is you can tell the difference between "The AddressRange was set to an invalid value" versus "The AddressRange member was untouched". Is there value in knowing the difference?
Requesting changes because my current impression is that this adds unnecessary complexity to LineEntry.
medismailben
force-pushed
the
line-entry-optional-range
branch
from
5d34b21 to
d588fce
Compare
medismailben
changed the title
JDevlieghere
requested changes
Dec 3, 2025
Member
There was a problem hiding this comment.
Can you please update the PR description with more details about why this is necessary? The current description doesn't explain the problem this is solving nor how this is the (best) solution.
In the context of the line table it makes sense to require a line entry to have an address (range). This PR also needs to motivate why dropping this requirement makes sense.
Finally, we have a bunch of places across LLDB that call IsValid. I see it referenced in SymbolContext, StackFrame, Disassembler, FormatEntity, CommandObjectSource to name a few. Did you look through all its uses and confirmed that this change in behavior remains correct? The fact that the test suite passes is promising but far from a guarantee as I doubt someone went out of their way to cover this previously unexpected situation.
I'm not saying this isn't the right approach, but right now there's not enough information in this PR to come to the conclusion that it is.
clayborg
reviewed
Dec 3, 2025
Collaborator
clayborg
left a comment
clayborg
left a comment
There was a problem hiding this comment.
How does anyone find a LineEntry with no address ranges. Is there anything we can put into the address fields of a LineEntry that has no PC value that helps to identify where it comes from? Code or byte offset? Is this for a specific target like wasm?
Collaborator
|
Lots of things might break in LLDB if the |
clayborg
reviewed
Dec 3, 2025
Collaborator
|
It might be worth having an extra bool in |
medismailben
force-pushed
the
line-entry-optional-range
branch
from
d588fce to
1fcece5
Compare
medismailben
changed the title
JDevlieghere
approved these changes
Dec 4, 2025
bulbazord
approved these changes
Dec 4, 2025
Member
bulbazord
left a comment
bulbazord
left a comment
There was a problem hiding this comment.
I do like the is_synthetic member better than an optional AddressRange because it conveys the intent better. In the future, it might also be able to explain why another field might appear invalid or otherwise strange.
medismailben
force-pushed
the
line-entry-optional-range
branch
3 times, most recently
from
7e22806 to
1ab9264
Compare
Member
Author
|
@clayborg Are you fine with this approach ? |
clayborg
approved these changes
Dec 4, 2025
Collaborator
clayborg
left a comment
clayborg
left a comment
There was a problem hiding this comment.
Looks good! Just a few inline comments that can be implemented or ignored.
lldb/include/lldb/Symbol/LineEntry.h
Outdated
|
|
||
| /// This gets set for LineEntries created without a valid address range. | ||
| /// When set, `LineEntry::IsValid` doesn't check the `range` validity. | ||
| bool synthetic; |
Collaborator
There was a problem hiding this comment.
initialize this with false here just in case new constructors are added later?
bool synthetic = false;
Comment on lines
+135
to
+136
| if (!ref().range.IsValid()) | ||
| ref().synthetic = true; |
Collaborator
There was a problem hiding this comment.
We don't have the ability to set the address range through the public API right? If we ever do add this, then we will need to modify this value as needed to be true or false.
Member
Author
There was a problem hiding this comment.
Right, we don't that's why I didn't bother making a setter for it for now.
…ess ranges Scripted Frames that materialize Python functions are PC-less by design, meaning they don't have valid address ranges. Previously, LineEntry::IsValid() required both a valid address range and a line number, preventing scripted frames from creating valid line entries for these synthetic stack frames. Relaxing this requirement is necessary since `SBSymbolContext::SetLineEntry` will first check if the LineEntry is valid and discard it otherwise. This change introduces an `synthetic` flag that gets set when LineEntry objects are created or modified through the SBAPI (specifically via SetLine). When this flag is set, IsValid() no longer requires a valid address range, only a valid line number. This is risk-free because the flag is only set for LineEntry objects created through the SBAPI, which are primarily used by scripted processes and frames. Regular debug information-derived line entries continue to require valid address ranges. Signed-off-by: Med Ismail Bennani <[email protected]>
medismailben
force-pushed
the
line-entry-optional-range
branch
from
1ab9264 to
177ced5
Compare
medismailben
added a commit
to medismailben/llvm-project
that referenced
this pull request
Dec 6, 2025
…ess ranges (llvm#158811) Scripted frames that materialize Python functions are PC-less by design, meaning they don't have valid address ranges. Previously, LineEntry::IsValid() required both a valid address range and a line number, preventing scripted frames from creating valid line entries for these synthetic stack frames. Relaxing this requirement is necessary since `SBSymbolContext::SetLineEntry` will first check if the LineEntry is valid and discard it otherwise. This change introduces an `synthetic` flag that gets set when LineEntry objects are created or modified through the SBAPI (specifically via SetLine). When this flag is set, IsValid() no longer requires a valid address range, only a valid line number. This is risk-free because the flag is only set for LineEntry objects created through the SBAPI, which are primarily used by scripted processes and frames. Regular debug information-derived line entries continue to require valid address ranges. Signed-off-by: Med Ismail Bennani <[email protected]> (cherry picked from commit 41f643a)
medismailben
added a commit
to medismailben/llvm-project
that referenced
this pull request
Dec 6, 2025
…ess ranges (llvm#158811) Scripted frames that materialize Python functions are PC-less by design, meaning they don't have valid address ranges. Previously, LineEntry::IsValid() required both a valid address range and a line number, preventing scripted frames from creating valid line entries for these synthetic stack frames. Relaxing this requirement is necessary since `SBSymbolContext::SetLineEntry` will first check if the LineEntry is valid and discard it otherwise. This change introduces an `synthetic` flag that gets set when LineEntry objects are created or modified through the SBAPI (specifically via SetLine). When this flag is set, IsValid() no longer requires a valid address range, only a valid line number. This is risk-free because the flag is only set for LineEntry objects created through the SBAPI, which are primarily used by scripted processes and frames. Regular debug information-derived line entries continue to require valid address ranges. Signed-off-by: Med Ismail Bennani <[email protected]> (cherry picked from commit 41f643a)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Scripted frames that materialize Python functions are PC-less by design,
meaning they don't have valid address ranges. Previously, LineEntry::IsValid()
required both a valid address range and a line number, preventing scripted
frames from creating valid line entries for these synthetic stack frames.
Relaxing this requirement is necessary since
SBSymbolContext::SetLineEntrywill first check if the LineEntry is valid and discard it otherwise.
This change introduces an
syntheticflag that gets set when LineEntryobjects are created or modified through the SBAPI (specifically via SetLine).
When this flag is set, IsValid() no longer requires a valid address range,
only a valid line number.
This is risk-free because the flag is only set for LineEntry objects created
through the SBAPI, which are primarily used by scripted processes and frames.
Regular debug information-derived line entries continue to require valid
address ranges.
Signed-off-by: Med Ismail Bennani [email protected]