[Lldb-commits] [lldb] [lldb] Refactor variable annotation logic in Disassembler::PrintInstructions (PR #156118)
via lldb-commits
lldb-commits at lists.llvm.org
Fri Aug 29 15:41:30 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Abdullah Mohammad Amin (UltimateForce21)
<details>
<summary>Changes</summary>
This patch is a follow-up to [#<!-- -->152887](https://github.com/llvm/llvm-project/pull/152887), addressing review comments that came in after the original change was merged.
- Move `VarState` definition out of `PrintInstructions` into a private helper, with member comments placed before fields.
- Introduce a `VariableAnnotator` helper class to encapsulate state and logic for live variable tracking across instructions.
- Replace `seen_this_inst` flag with a map-diff approach: recompute the current variable set per instruction and diff against the previous set.
- Use `nullptr` instead of an empty `ProcessSP` when calling `ABI::FindPlugin`.
- Narrow `Block*` scope with `if (Block *B = ...)`.
- Set `DIDumpOptions::PrintRegisterOnly` directly from `static_cast<bool>(abi_sp)`.
- Prefer `emplace_back` over `push_back` for event strings.
- General cleanup to match LLVM coding style and reviewer feedback.
This makes the annotation code easier to read and consistent with LLVM/LLDB conventions while preserving functionality.
---
Full diff: https://github.com/llvm/llvm-project/pull/156118.diff
2 Files Affected:
- (modified) lldb/include/lldb/Core/Disassembler.h (+20)
- (modified) lldb/source/Core/Disassembler.cpp (+123-142)
``````````diff
diff --git a/lldb/include/lldb/Core/Disassembler.h b/lldb/include/lldb/Core/Disassembler.h
index 6a21470b2472c..db186dd33d774 100644
--- a/lldb/include/lldb/Core/Disassembler.h
+++ b/lldb/include/lldb/Core/Disassembler.h
@@ -566,6 +566,26 @@ class Disassembler : public std::enable_shared_from_this<Disassembler>,
const Disassembler &operator=(const Disassembler &) = delete;
};
+/// Tracks live variable annotations across instructions and produces
+/// per-instruction "events" like `name = RDI` or `name = <undef>`.
+class VariableAnnotator {
+ struct VarState {
+ /// Display name.
+ std::string name;
+ /// Last printed location (empty means <undef>).
+ std::string last_loc;
+ };
+
+ // Live state from the previous instruction, keyed by Variable::GetID().
+ llvm::DenseMap<lldb::user_id_t, VarState> Live_;
+
+public:
+ /// Compute annotation strings for a single instruction and update `Live_`.
+ /// Returns only the events that should be printed *at this instruction*.
+ std::vector<std::string> annotate(Instruction &inst, Target &target,
+ const lldb::ModuleSP &module_sp);
+};
+
} // namespace lldb_private
#endif // LLDB_CORE_DISASSEMBLER_H
diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp
index 9b0b604d6caa0..f2ed1f7395346 100644
--- a/lldb/source/Core/Disassembler.cpp
+++ b/lldb/source/Core/Disassembler.cpp
@@ -286,6 +286,127 @@ bool Disassembler::ElideMixedSourceAndDisassemblyLine(
return false;
}
+// For each instruction, this block attempts to resolve in-scope variables
+// and determine if the current PC falls within their
+// DWARF location entry. If so, it prints a simplified annotation using the
+// variable name and its resolved location (e.g., "var = reg; " ).
+//
+// Annotations are only included if the variable has a valid DWARF location
+// entry, and the location string is non-empty after filtering. Decoding
+// errors and DWARF opcodes are intentionally omitted to keep the output
+// concise and user-friendly.
+//
+// The goal is to give users helpful live variable hints alongside the
+// disassembled instruction stream, similar to how debug information
+// enhances source-level debugging.
+std::vector<std::string>
+VariableAnnotator::annotate(Instruction &inst, Target &target,
+ const lldb::ModuleSP &module_sp) {
+ std::vector<std::string> events;
+
+ // If we lost module context, everything becomes <undef>.
+ if (!module_sp) {
+ for (const auto &KV : Live_)
+ events.emplace_back(llvm::formatv("{0} = <undef>", KV.second.name).str());
+ Live_.clear();
+ return events;
+ }
+
+ // Resolve function/block at this *file* address.
+ SymbolContext sc;
+ const Address &iaddr = inst.GetAddress();
+ const auto mask = eSymbolContextFunction | eSymbolContextBlock;
+ if (!module_sp->ResolveSymbolContextForAddress(iaddr, mask, sc) ||
+ !sc.function) {
+ // No function context: everything dies here.
+ for (const auto &KV : Live_)
+ events.emplace_back(llvm::formatv("{0} = <undef>", KV.second.name).str());
+ Live_.clear();
+ return events;
+ }
+
+ // Collect in-scope variables for this instruction into Current.
+ VariableList var_list;
+ // Innermost block containing iaddr.
+ if (Block *B = sc.block) {
+ auto filter = [](Variable *v) -> bool { return v && !v->IsArtificial(); };
+ B->AppendVariables(/*can_create*/ true,
+ /*get_parent_variables*/ true,
+ /*stop_if_block_is_inlined_function*/ false,
+ /*filter*/ filter,
+ /*variable_list*/ &var_list);
+ }
+
+ const lldb::addr_t pc_file = iaddr.GetFileAddress();
+ const lldb::addr_t func_file = sc.function->GetAddress().GetFileAddress();
+
+ // ABI from Target (pretty reg names if plugin exists). Safe to be null.
+ lldb::ABISP abi_sp = ABI::FindPlugin(nullptr, target.GetArchitecture());
+ ABI *abi = abi_sp.get();
+
+ llvm::DIDumpOptions opts;
+ opts.ShowAddresses = false;
+ // Prefer "register-only" output when we have an ABI.
+ opts.PrintRegisterOnly = static_cast<bool>(abi_sp);
+
+ llvm::DenseMap<lldb::user_id_t, VarState> Current;
+
+ for (size_t i = 0, e = var_list.GetSize(); i != e; ++i) {
+ lldb::VariableSP v = var_list.GetVariableAtIndex(i);
+ if (!v || v->IsArtificial())
+ continue;
+
+ const char *nm = v->GetName().AsCString();
+ llvm::StringRef name = nm ? nm : "<anon>";
+
+ DWARFExpressionList &exprs = v->LocationExpressionList();
+ if (!exprs.IsValid())
+ continue;
+
+ auto entry_or_err = exprs.GetExpressionEntryAtAddress(func_file, pc_file);
+ if (!entry_or_err)
+ continue;
+
+ auto entry = *entry_or_err;
+
+ StreamString loc_ss;
+ entry.expr->DumpLocation(&loc_ss, eDescriptionLevelBrief, abi, opts);
+
+ llvm::StringRef loc = llvm::StringRef(loc_ss.GetString()).trim();
+ if (loc.empty())
+ continue;
+
+ Current.try_emplace(v->GetID(),
+ VarState{std::string(name), std::string(loc)});
+ }
+
+ // Diff Live_ → Current.
+
+ // 1) Starts/changes: iterate Current and compare with Live_.
+ for (const auto &KV : Current) {
+ auto it = Live_.find(KV.first);
+ if (it == Live_.end()) {
+ // Newly live.
+ events.emplace_back(
+ llvm::formatv("{0} = {1}", KV.second.name, KV.second.last_loc).str());
+ } else if (it->second.last_loc != KV.second.last_loc) {
+ // Location changed.
+ events.emplace_back(
+ llvm::formatv("{0} = {1}", KV.second.name, KV.second.last_loc).str());
+ }
+ }
+
+ // 2) Ends: anything that was live but is not in Current becomes <undef>.
+ for (const auto &KV : Live_) {
+ if (!Current.count(KV.first))
+ events.emplace_back(llvm::formatv("{0} = <undef>", KV.second.name).str());
+ }
+
+ // Commit new state.
+ Live_ = std::move(Current);
+ return events;
+}
+
void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch,
const ExecutionContext &exe_ctx,
bool mixed_source_and_assembly,
@@ -382,147 +503,7 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch,
}
}
- // Add variable location annotations to the disassembly output.
- //
- // For each instruction, this block attempts to resolve in-scope variables
- // and determine if the current PC falls within their
- // DWARF location entry. If so, it prints a simplified annotation using the
- // variable name and its resolved location (e.g., "var = reg; " ).
- //
- // Annotations are only included if the variable has a valid DWARF location
- // entry, and the location string is non-empty after filtering. Decoding
- // errors and DWARF opcodes are intentionally omitted to keep the output
- // concise and user-friendly.
- //
- // The goal is to give users helpful live variable hints alongside the
- // disassembled instruction stream, similar to how debug information
- // enhances source-level debugging.
-
- struct VarState {
- std::string name; ///< Display name.
- std::string last_loc; ///< Last printed location (empty means <undef>).
- bool seen_this_inst = false;
- };
-
- // Track live variables across instructions.
- llvm::DenseMap<lldb::user_id_t, VarState> live_vars;
-
- // Stateful annotator: updates live_vars and returns only what should be
- // printed for THIS instruction.
- auto annotate_static = [&](Instruction &inst, Target &target,
- ModuleSP module_sp) -> std::vector<std::string> {
- std::vector<std::string> events;
-
- // Reset per-instruction seen flags.
- for (auto &kv : live_vars)
- kv.second.seen_this_inst = false;
-
- const Address &iaddr = inst.GetAddress();
- if (!module_sp) {
- // Everything previously live becomes <undef>.
- for (auto I = live_vars.begin(), E = live_vars.end(); I != E;) {
- auto Cur = I++;
- events.push_back(
- llvm::formatv("{0} = <undef>", Cur->second.name).str());
- live_vars.erase(Cur);
- }
- return events;
- }
-
- // Resolve innermost block at this *file* address.
- SymbolContext sc;
- const lldb::SymbolContextItem mask =
- eSymbolContextFunction | eSymbolContextBlock;
- if (!module_sp->ResolveSymbolContextForAddress(iaddr, mask, sc) ||
- !sc.function) {
- // No function context: everything dies here.
- for (auto I = live_vars.begin(), E = live_vars.end(); I != E;) {
- auto Cur = I++;
- events.push_back(
- llvm::formatv("{0} = <undef>", Cur->second.name).str());
- live_vars.erase(Cur);
- }
- return events;
- }
-
- Block *B = sc.block; ///< Innermost block containing iaddr.
- VariableList var_list;
- if (B) {
- auto filter = [](Variable *v) -> bool { return v && !v->IsArtificial(); };
-
- B->AppendVariables(/*can_create*/ true,
- /*get_parent_variables*/ true,
- /*stop_if_block_is_inlined_function*/ false,
- /*filter*/ filter,
- /*variable_list*/ &var_list);
- }
-
- const lldb::addr_t pc_file = iaddr.GetFileAddress();
- const lldb::addr_t func_file = sc.function->GetAddress().GetFileAddress();
-
- // ABI from Target (pretty reg names if plugin exists). Safe to be null.
- lldb::ProcessSP no_process;
- lldb::ABISP abi_sp = ABI::FindPlugin(no_process, target.GetArchitecture());
- ABI *abi = abi_sp.get();
-
- llvm::DIDumpOptions opts;
- opts.ShowAddresses = false;
- if (abi)
- opts.PrintRegisterOnly = true;
-
- for (size_t i = 0, e = var_list.GetSize(); i != e; ++i) {
- lldb::VariableSP v = var_list.GetVariableAtIndex(i);
- if (!v || v->IsArtificial())
- continue;
-
- const char *nm = v->GetName().AsCString();
- llvm::StringRef name = nm ? nm : "<anon>";
-
- lldb_private::DWARFExpressionList &exprs = v->LocationExpressionList();
- if (!exprs.IsValid())
- continue;
-
- auto entry_or_err = exprs.GetExpressionEntryAtAddress(func_file, pc_file);
- if (!entry_or_err)
- continue;
-
- auto entry = *entry_or_err;
-
- StreamString loc_ss;
- entry.expr->DumpLocation(&loc_ss, eDescriptionLevelBrief, abi, opts);
- llvm::StringRef loc = llvm::StringRef(loc_ss.GetString()).trim();
- if (loc.empty())
- continue;
-
- auto ins = live_vars.insert(
- {v->GetID(), VarState{name.str(), loc.str(), /*seen*/ true}});
- if (ins.second) {
- // Newly live.
- events.push_back(llvm::formatv("{0} = {1}", name, loc).str());
- } else {
- VarState &vs = ins.first->second;
- vs.seen_this_inst = true;
- if (vs.last_loc != loc) {
- vs.last_loc = loc.str();
- events.push_back(llvm::formatv("{0} = {1}", vs.name, loc).str());
- }
- }
- }
-
- // Anything previously live that we didn't see a location for at this inst
- // is now <undef>.
- for (auto I = live_vars.begin(), E = live_vars.end(); I != E;) {
- auto Cur = I++;
- if (!Cur->second.seen_this_inst) {
- events.push_back(
- llvm::formatv("{0} = <undef>", Cur->second.name).str());
- live_vars.erase(Cur);
- }
- }
-
- return events;
- };
-
+ VariableAnnotator annot;
previous_symbol = nullptr;
SourceLine previous_line;
for (size_t i = 0; i < num_instructions_found; ++i) {
@@ -695,7 +676,7 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch,
address_text_size);
if ((options & eOptionVariableAnnotations) && target_sp) {
- auto annotations = annotate_static(*inst, *target_sp, module_sp);
+ auto annotations = annot.annotate(*inst, *target_sp, module_sp);
if (!annotations.empty()) {
const size_t annotation_column = 100;
inst_line.FillLastLineToColumn(annotation_column, ' ');
``````````
</details>
https://github.com/llvm/llvm-project/pull/156118
More information about the lldb-commits
mailing list