Skip to content
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
8ed8c54
[lldb] Add DWARFExpressionEntry and GetExpressionEntryAtAddress() to …
UltimateForce21 Jun 11, 2025
1db5002
Update lldb/include/lldb/Expression/DWARFExpressionList.h
UltimateForce21 Jun 19, 2025
a26010b
Update lldb/include/lldb/Expression/DWARFExpressionList.h
UltimateForce21 Jun 19, 2025
72237b7
Update lldb/source/Expression/DWARFExpressionList.cpp
UltimateForce21 Jun 19, 2025
94e4951
Update DWARFExpressionList.h
UltimateForce21 Jun 24, 2025
e8142da
Update DWARFExpressionList.cpp
UltimateForce21 Jun 24, 2025
7e8741e
Update DWARFExpressionList.h
UltimateForce21 Jun 28, 2025
c4cd77f
Update DWARFExpressionList.cpp
UltimateForce21 Jun 28, 2025
62c02a9
Change GetExpressionEntryAtAddress to return std::optional instead of…
UltimateForce21 Jun 29, 2025
d015971
Update DWARFExpressionList.cpp
UltimateForce21 Jul 2, 2025
60898ea
Add underflow/overflow checks to GetExpressionEntryAtAddressi
UltimateForce21 Jul 3, 2025
3462165
Make file_range optional in DWARFExpressionEntry for always-valid expr
UltimateForce21 Jul 8, 2025
2ed8443
Annotate Instruction::Dump() with DWARF variable locations
UltimateForce21 Jul 3, 2025
8c6b22d
Added Initial Basic API test for rich variable annotation in disassem…
UltimateForce21 Jul 5, 2025
842a9e5
Improved DWARF variable annotation printing and alignment
UltimateForce21 Jul 6, 2025
2fa6d24
Filter out partial DWARF decoding errors from disassembly annotations
UltimateForce21 Jul 6, 2025
6bbc8aa
Ignore annotations with only decoding errors
UltimateForce21 Jul 6, 2025
cbbc924
Add tests for disassembly variable annotations and decoding edge cases
UltimateForce21 Jul 6, 2025
b887db2
Rebase disassembler annotations branch onto updated DWARFExpressionEn…
UltimateForce21 Jul 8, 2025
912ba6d
Add `PrintRegisterOnly` flag in `struct DIDumpOptions` and created ne…
UltimateForce21 Jul 9, 2025
09c4d04
Add high-level comment explaining rich disassembly annotation logic i…
UltimateForce21 Jul 20, 2025
6e17f77
Add comment clarifying annotation column length check in Instruction:…
UltimateForce21 Jul 20, 2025
31431c0
Refactor variable annotation logic in `Instruction::Dump` using `anno…
UltimateForce21 Jul 20, 2025
9c5cb8f
Use range-based for loop for variable list iteration in Instruction::…
UltimateForce21 Jul 20, 2025
ca8510c
Consolidated DumpLocation and DumpLocationWithOptions using default D…
UltimateForce21 Jul 20, 2025
ffefe5f
Use `llvm::join` to simplify annotation output formatting
UltimateForce21 Jul 20, 2025
fae745a
Merge branch 'main' into add-disassembler-annotations
UltimateForce21 Aug 4, 2025
dcddf16
Fix formatting to match LLVM style
UltimateForce21 Aug 4, 2025
7bac074
More formatting fixes
UltimateForce21 Aug 4, 2025
79c0a9e
Fix formatting for code and tests
UltimateForce21 Aug 5, 2025
3d19b02
Added `--rich` option for disassembler annotations and updated SBFram…
UltimateForce21 Aug 8, 2025
6ca4bb6
Formatting changes.
UltimateForce21 Aug 8, 2025
4bf584e
Merge branch 'main' into add-disassembler-annotations
UltimateForce21 Aug 8, 2025
b1f13e7
Redo Workflow tests
UltimateForce21 Aug 9, 2025
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
1 change: 1 addition & 0 deletions lldb/include/lldb/Expression/DWARFExpression.h
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ class DWARFExpression {
}

void DumpLocation(Stream *s, lldb::DescriptionLevel level, ABI *abi) const;
void DumpLocationWithOptions(Stream *s, lldb::DescriptionLevel level, ABI *abi, llvm::DIDumpOptions options) const;
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not have one version with a default argument?

  void DumpLocationWithOptions(Stream *s, lldb::DescriptionLevel level, ABI *abi, llvm::DIDumpOptions options = {}) const;

Copy link
Contributor Author

@UltimateForce21 UltimateForce21 Jul 20, 2025

Choose a reason for hiding this comment

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

I've updated the code now to use a single DumpLocation method with a default DIDumpOptions argument, as recommended. The DumpLocationWithOptions variant has been removed in commit ca8510c.


bool MatchesOperand(StackFrame &frame, const Instruction::Operand &op) const;

Expand Down
15 changes: 15 additions & 0 deletions lldb/include/lldb/Expression/DWARFExpressionList.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#ifndef LLDB_EXPRESSION_DWARFEXPRESSIONLIST_H
#define LLDB_EXPRESSION_DWARFEXPRESSIONLIST_H

#include "lldb/Core/AddressRange.h"
#include "lldb/Core/Value.h"
#include "lldb/Expression/DWARFExpression.h"
#include "lldb/Utility/RangeMap.h"
Expand Down Expand Up @@ -58,6 +59,20 @@ class DWARFExpressionList {
}

lldb::addr_t GetFuncFileAddress() { return m_func_file_addr; }

/// Represents an entry in the DWARFExpressionList with all needed metadata.
struct DWARFExpressionEntry {
/// Represents a DWARF location range in the DWARF unit’s file‐address space
std::optional<AddressRange> file_range; ///< None = always-valid single expr
const DWARFExpression *expr;
};

/// Returns a DWARFExpressionEntry whose file_range contains the given
/// load‐address. `func_load_addr` is the load‐address of the function
/// start; `load_addr` is the full runtime PC. On success, `expr` is non-null.
std::optional<DWARFExpressionEntry>
GetExpressionEntryAtAddress(lldb::addr_t func_load_addr,
lldb::addr_t load_addr) const;

const DWARFExpression *GetExpressionAtAddress(lldb::addr_t func_load_addr,
lldb::addr_t load_addr) const;
Expand Down
75 changes: 75 additions & 0 deletions lldb/source/Core/Disassembler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@
#include "lldb/Symbol/Function.h"
#include "lldb/Symbol/Symbol.h"
#include "lldb/Symbol/SymbolContext.h"
#include "lldb/Symbol/Variable.h"
#include "lldb/Symbol/VariableList.h"
#include "lldb/Target/ExecutionContext.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/SectionLoadList.h"
#include "lldb/Target/StackFrame.h"
#include "lldb/Target/Target.h"
Expand Down Expand Up @@ -702,6 +705,78 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size,
ss.FillLastLineToColumn(opcode_pos + opcode_column_width, ' ');
ss.PutCString(mnemonics);

const size_t annotation_column = 150;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@JDevlieghere Question for a future PR: Should we consider threading through and checking the actual width of the Terminal to see if the annotations would fit?


if (exe_ctx && exe_ctx->GetFramePtr()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a high-level comment here to explain what this block of code is doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

StackFrame *frame = exe_ctx->GetFramePtr();
TargetSP target_sp = exe_ctx->GetTargetSP();
if (frame && target_sp) {
addr_t current_pc = m_address.GetLoadAddress(target_sp.get());
addr_t original_pc = frame->GetFrameCodeAddress().GetLoadAddress(target_sp.get());
if (frame->ChangePC(current_pc)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is m_address here? Is it really necessary to modify the PC of frame?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

m_address here refers to the address of the current instruction being dumped in the disassembler output. Since Instruction::Dump is called per-instruction, m_address gives us the load address of the instruction currently being printed.

I used frame->ChangePC(current_pc) to temporarily update the frame's PC to this instruction address. This is necessary because GetInScopeVariableList(true) relies on the frame's current PC to determine which variables are in scope and what their locations are at that exact address. Without setting the PC to the instruction being dumped, the variable list might reflect a different point in the function, which could lead to incorrect or missing annotations.

That said, I make sure to save the original PC with frame->GetFrameCodeAddress() and restore it afterward with frame->ChangePC(original_pc) to avoid any persistent side effects on the frame state.

If there’s a better way to get the in-scope variables for a given address without mutating the frame, I’m happy to revise the approach.

VariableListSP var_list_sp = frame->GetInScopeVariableList(true);
SymbolContext sc = frame->GetSymbolContext(eSymbolContextFunction);
addr_t func_load_addr = LLDB_INVALID_ADDRESS;
if (sc.function)
func_load_addr = sc.function->GetAddress().GetLoadAddress(target_sp.get());


if(ss.GetSizeOfLastLine() < annotation_column) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this check if there is enough space for an annotation? Maybe comment that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, exactly. I have now added the comment in commit: 6e17f77c4300654c7664b52efc045983bb59721d


std::vector<std::string> annotations;

if (var_list_sp) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

LLVM coding style prefers early exits over checks. Maybe you can wrap this whole block in a lambda or static function, then this can become

if (!var_list_sp)
   return; 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion! I've pushed the change in 31431c0, which moves the annotation logic into a local annotate_variables lambda with early exits.

for (size_t i = 0; i < var_list_sp->GetSize(); ++i) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can this be a a range-based for? (i.e, for (VariableSP var_sp : *var_list_sp))

Copy link
Contributor Author

@UltimateForce21 UltimateForce21 Jul 20, 2025

Choose a reason for hiding this comment

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

I've updated the loop to use a range-based for as recommended. The change is now committed in 9c5cb8f.

VariableSP var_sp = var_list_sp->GetVariableAtIndex(i);
if (!var_sp)
continue;

const char *name = var_sp->GetName().AsCString();
auto &expr_list = var_sp->LocationExpressionList();
if (!expr_list.IsValid())
continue;
// Handle std::optional<DWARFExpressionEntry>.
if (auto entryOrErr = expr_list.GetExpressionEntryAtAddress(func_load_addr, current_pc)) {
auto entry = *entryOrErr;

// Check if entry has a file_range, and filter on address if so.
if (!entry.file_range || entry.file_range->ContainsFileAddress(
(current_pc - func_load_addr) + expr_list.GetFuncFileAddress())) {

StreamString loc_str;
ABI *abi = exe_ctx->GetProcessPtr()->GetABI().get();
llvm::DIDumpOptions opts;
opts.ShowAddresses = false;
opts.PrintRegisterOnly = true; // <-- important: suppress DW_OP_... annotations, etc.

entry.expr->DumpLocationWithOptions(&loc_str, eDescriptionLevelBrief, abi, opts);

// Only include if not empty
llvm::StringRef loc_clean = llvm::StringRef(loc_str.GetString()).trim();
if (!loc_clean.empty()) {
annotations.push_back(llvm::formatv("{0} = {1}", name, loc_clean));
}
}
}
}

if (!annotations.empty()) {
ss.FillLastLineToColumn(annotation_column, ' ');
ss.PutCString(" ; ");
for (size_t i = 0; i < annotations.size(); ++i) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@UltimateForce21 UltimateForce21 Jul 20, 2025

Choose a reason for hiding this comment

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

I replaced the manual string concatenation now with llvm::join as suggested in commit
ffefe5f.

if (i > 0)
ss.PutCString(", ");
ss.PutCString(annotations[i]);
}
}
}
}

frame->ChangePC(original_pc);
}
}
}

if (!m_comment.empty()) {
ss.FillLastLineToColumn(
opcode_pos + opcode_column_width + operand_column_width, ' ');
Expand Down
13 changes: 9 additions & 4 deletions lldb/source/Expression/DWARFExpression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ void DWARFExpression::UpdateValue(uint64_t const_value,

void DWARFExpression::DumpLocation(Stream *s, lldb::DescriptionLevel level,
ABI *abi) const {
llvm::DIDumpOptions DumpOpts;
this->DumpLocationWithOptions(s, level, abi, DumpOpts);
}

void DWARFExpression::DumpLocationWithOptions(Stream *s, lldb::DescriptionLevel level,
ABI *abi, llvm::DIDumpOptions options) const {
auto *MCRegInfo = abi ? &abi->GetMCRegisterInfo() : nullptr;
auto GetRegName = [&MCRegInfo](uint64_t DwarfRegNum,
bool IsEH) -> llvm::StringRef {
Expand All @@ -78,10 +84,9 @@ void DWARFExpression::DumpLocation(Stream *s, lldb::DescriptionLevel level,
return llvm::StringRef(RegName);
return {};
};
llvm::DIDumpOptions DumpOpts;
DumpOpts.GetNameForDWARFReg = GetRegName;
llvm::DWARFExpression(m_data.GetAsLLVM(), m_data.GetAddressByteSize())
.print(s->AsRawOstream(), DumpOpts, nullptr);
options.GetNameForDWARFReg = GetRegName;
llvm::DWARFExpression expression (m_data.GetAsLLVM(), m_data.GetAddressByteSize());
expression.print(s->AsRawOstream(), options, nullptr);
}

RegisterKind DWARFExpression::GetRegisterKind() const { return m_reg_kind; }
Expand Down
32 changes: 32 additions & 0 deletions lldb/source/Expression/DWARFExpressionList.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
//
//===----------------------------------------------------------------------===//

#include "lldb/Core/AddressRange.h"
#include "lldb/Expression/DWARFExpressionList.h"
#include "lldb/Symbol/Function.h"
#include "lldb/Target/RegisterContext.h"
Expand Down Expand Up @@ -53,6 +54,37 @@ bool DWARFExpressionList::ContainsAddress(lldb::addr_t func_load_addr,
return GetExpressionAtAddress(func_load_addr, addr) != nullptr;
}

std::optional<DWARFExpressionList::DWARFExpressionEntry>
DWARFExpressionList::GetExpressionEntryAtAddress(lldb::addr_t func_load_addr,
lldb::addr_t load_addr) const {
if (const DWARFExpression *always = GetAlwaysValidExpr()) {
return DWARFExpressionEntry{std::nullopt, always};
}

if (func_load_addr == LLDB_INVALID_ADDRESS)
func_load_addr = m_func_file_addr;

// Guard against underflow when translating a load address back into file space.
if (load_addr < func_load_addr)
return std::nullopt;

// Guard against overflow.
lldb::addr_t delta = load_addr - func_load_addr;
if (delta > std::numeric_limits<lldb::addr_t>::max() - m_func_file_addr)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be better to use the add with overflow methods from MathExtras here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From when I dug into MathExtras last time, I found that its AddOverflow and SubOverflow helpers are only defined for signed types.

I they rely on two’s-complement signed overflow semantics and return the truncated signed result. Since lldb::addr_t is an unsigned address, I think using those helpers would either fail to compile or silently convert to signed semantics.

That's why I tried to implement the signed overflow check into the function.

return std::nullopt;

lldb::addr_t file_pc = (load_addr - func_load_addr) + m_func_file_addr;

if (const auto *entry = m_exprs.FindEntryThatContains(file_pc)) {
AddressRange range_in_file(entry->GetRangeBase(),
entry->GetRangeEnd() - entry->GetRangeBase());
return DWARFExpressionEntry{range_in_file, &entry->data};
}

// No entry covers this PC:
return std::nullopt;
}

const DWARFExpression *
DWARFExpressionList::GetExpressionAtAddress(lldb::addr_t func_load_addr,
lldb::addr_t load_addr) const {
Expand Down
6 changes: 6 additions & 0 deletions lldb/test/API/functionalities/rich-disassembler/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

# CXX_SOURCES := a_loop_with_local_variable.c b_multiple_stack_variables.c c_variable_passed_to_another_function.c d_original_example.c e_control_flow_edge.c
C_SOURCES := a_loop_with_local_variable.c b_multiple_stack_variables.c c_variable_passed_to_another_function.c d_original_example.c e_control_flow_edge.c


include Makefile.rules
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
from lldbsuite.test.lldbtest import *
from lldbsuite.test.decorators import *

class TestRichDisassembler(TestBase):
def test_d_original_example_O1(self):
"""
Tests disassembler output for d_original_example.c built with -O1.
"""
self.build(dictionary={
'C_SOURCES': 'd_original_example.c',
'CFLAGS_EXTRAS': '-g -O1'
})
exe = self.getBuildArtifact("a.out")
target = self.dbg.CreateTarget(exe)
self.assertTrue(target)

breakpoint = target.BreakpointCreateByName("main")
self.assertGreater(breakpoint.GetNumLocations(), 0)

process = target.LaunchSimple(None, None, self.get_process_working_directory())
self.assertTrue(process, "Failed to launch process")
self.assertEqual(process.GetState(), lldb.eStateStopped)

frame = process.GetSelectedThread().GetSelectedFrame()
disasm = frame.Disassemble()
print(disasm)

self.assertIn("argc = ", disasm)
self.assertIn("argv = ", disasm)
self.assertIn("i = ", disasm)
# self.assertIn("DW_OP_reg", disasm)
# self.assertIn("DW_OP_stack_value", disasm)
self.assertNotIn("<decoding error>", disasm)










Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#include <stdio.h>

int main(int argc, char **argv) {
for (int i = 1; i < argc; ++i)
puts(argv[i]);
return 0;
}
1 change: 1 addition & 0 deletions llvm/include/llvm/DebugInfo/DIContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ struct DIDumpOptions {
bool IsEH = false;
bool DumpNonSkeleton = false;
bool ShowAggregateErrors = false;
bool PrintRegisterOnly = false;
std::string JsonErrSummaryFile;
std::function<llvm::StringRef(uint64_t DwarfRegNum, bool IsEH)>
GetNameForDWARFReg;
Expand Down
Loading