From 8ed8c540e7600d720a63bc2882a81a2c65c11d41 Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Wed, 11 Jun 2025 00:11:09 -0400 Subject: [PATCH 01/39] [lldb] Add DWARFExpressionEntry and GetExpressionEntryAtAddress() to DWARFExpressionList This introduces a new API for retrieving DWARF expression metadata associated with variable location entries at a given PC address. It provides the base, end, and expression pointer for downstream consumers such as disassembler annotations. Intended for use in richer instruction annotations in Instruction::Dump(). --- .../lldb/Expression/DWARFExpressionList.h | 12 +++++++++++ .../source/Expression/DWARFExpressionList.cpp | 21 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/lldb/include/lldb/Expression/DWARFExpressionList.h b/lldb/include/lldb/Expression/DWARFExpressionList.h index d8f8ec247ed56..a329b37393018 100644 --- a/lldb/include/lldb/Expression/DWARFExpressionList.h +++ b/lldb/include/lldb/Expression/DWARFExpressionList.h @@ -59,6 +59,18 @@ class DWARFExpressionList { lldb::addr_t GetFuncFileAddress() { return m_func_file_addr; } + /// Represents an entry in the DWARFExpressionList with all needed metadata + struct DWARFExpressionEntry { + lldb::addr_t base; + lldb::addr_t end; + const DWARFExpression *expr; + }; + + /// Returns the entry (base, end, data) for a given PC address + llvm::Expected + 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; diff --git a/lldb/source/Expression/DWARFExpressionList.cpp b/lldb/source/Expression/DWARFExpressionList.cpp index 04592a1eb7ff4..b55bc7120c4af 100644 --- a/lldb/source/Expression/DWARFExpressionList.cpp +++ b/lldb/source/Expression/DWARFExpressionList.cpp @@ -53,6 +53,27 @@ bool DWARFExpressionList::ContainsAddress(lldb::addr_t func_load_addr, return GetExpressionAtAddress(func_load_addr, addr) != nullptr; } +llvm::Expected +DWARFExpressionList::GetExpressionEntryAtAddress(lldb::addr_t func_load_addr, + lldb::addr_t load_addr) const { + if (const DWARFExpression *expr = GetAlwaysValidExpr()) { + return DWARFExpressionEntry{0, LLDB_INVALID_ADDRESS, expr}; + } + + if (func_load_addr == LLDB_INVALID_ADDRESS) + func_load_addr = m_func_file_addr; + + addr_t addr = load_addr - func_load_addr + m_func_file_addr; + uint32_t index = m_exprs.FindEntryIndexThatContains(addr); + if (index == UINT32_MAX) { + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "No DWARF expression found for address 0x%llx", addr); + } + + const Entry &entry = *m_exprs.GetEntryAtIndex(index); + return DWARFExpressionEntry{entry.base, entry.GetRangeEnd(), &entry.data}; +} + const DWARFExpression * DWARFExpressionList::GetExpressionAtAddress(lldb::addr_t func_load_addr, lldb::addr_t load_addr) const { From 1db5002a69dba4f88aaac56d61520b7b4b214b01 Mon Sep 17 00:00:00 2001 From: Abdullah Mohammad Amin <67847674+UltimateForce21@users.noreply.github.com> Date: Thu, 19 Jun 2025 11:55:35 -0400 Subject: [PATCH 02/39] Update lldb/include/lldb/Expression/DWARFExpressionList.h Co-authored-by: Jonas Devlieghere --- lldb/include/lldb/Expression/DWARFExpressionList.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/include/lldb/Expression/DWARFExpressionList.h b/lldb/include/lldb/Expression/DWARFExpressionList.h index a329b37393018..89e55ffc07659 100644 --- a/lldb/include/lldb/Expression/DWARFExpressionList.h +++ b/lldb/include/lldb/Expression/DWARFExpressionList.h @@ -59,7 +59,7 @@ class DWARFExpressionList { lldb::addr_t GetFuncFileAddress() { return m_func_file_addr; } - /// Represents an entry in the DWARFExpressionList with all needed metadata + /// Represents an entry in the DWARFExpressionList with all needed metadata. struct DWARFExpressionEntry { lldb::addr_t base; lldb::addr_t end; From a26010b06e5067b8b3b223cbd76e8848ecb9a289 Mon Sep 17 00:00:00 2001 From: Abdullah Mohammad Amin <67847674+UltimateForce21@users.noreply.github.com> Date: Thu, 19 Jun 2025 11:58:28 -0400 Subject: [PATCH 03/39] Update lldb/include/lldb/Expression/DWARFExpressionList.h Updated comment for GetExpressionEntryAtAddress to directly refer to struct DWARFExpressionEntry Co-authored-by: Jonas Devlieghere --- lldb/include/lldb/Expression/DWARFExpressionList.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/include/lldb/Expression/DWARFExpressionList.h b/lldb/include/lldb/Expression/DWARFExpressionList.h index 89e55ffc07659..f6a269809decc 100644 --- a/lldb/include/lldb/Expression/DWARFExpressionList.h +++ b/lldb/include/lldb/Expression/DWARFExpressionList.h @@ -66,7 +66,7 @@ class DWARFExpressionList { const DWARFExpression *expr; }; - /// Returns the entry (base, end, data) for a given PC address + /// Returns the DWARFExpressionEntry for a given PC address. llvm::Expected GetExpressionEntryAtAddress(lldb::addr_t func_load_addr, lldb::addr_t load_addr) const; From 72237b75a12daa94f887f7492b2dfc141519b8a8 Mon Sep 17 00:00:00 2001 From: Abdullah Mohammad Amin <67847674+UltimateForce21@users.noreply.github.com> Date: Thu, 19 Jun 2025 11:59:35 -0400 Subject: [PATCH 04/39] Update lldb/source/Expression/DWARFExpressionList.cpp updating code style for function GetExpressionEntryAtAddress Co-authored-by: Jonas Devlieghere --- lldb/source/Expression/DWARFExpressionList.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lldb/source/Expression/DWARFExpressionList.cpp b/lldb/source/Expression/DWARFExpressionList.cpp index b55bc7120c4af..ebf57dd457769 100644 --- a/lldb/source/Expression/DWARFExpressionList.cpp +++ b/lldb/source/Expression/DWARFExpressionList.cpp @@ -56,9 +56,8 @@ bool DWARFExpressionList::ContainsAddress(lldb::addr_t func_load_addr, llvm::Expected DWARFExpressionList::GetExpressionEntryAtAddress(lldb::addr_t func_load_addr, lldb::addr_t load_addr) const { - if (const DWARFExpression *expr = GetAlwaysValidExpr()) { + if (const DWARFExpression *expr = GetAlwaysValidExpr()) return DWARFExpressionEntry{0, LLDB_INVALID_ADDRESS, expr}; - } if (func_load_addr == LLDB_INVALID_ADDRESS) func_load_addr = m_func_file_addr; From 94e4951ac8eb39f078b783c2d3a7006c395ae4b2 Mon Sep 17 00:00:00 2001 From: Abdullah Mohammad Amin <67847674+UltimateForce21@users.noreply.github.com> Date: Tue, 24 Jun 2025 16:28:14 -0400 Subject: [PATCH 05/39] Update DWARFExpressionList.h Replace raw base/end with `AddressRange` in `DWARFExpressionEntry` and cleans up helper comments to follow Doxygen convention. Using `AddressRange` makes the intent clearer, avoids duplication of basic `AddressRange` logic usage --- lldb/include/lldb/Expression/DWARFExpressionList.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lldb/include/lldb/Expression/DWARFExpressionList.h b/lldb/include/lldb/Expression/DWARFExpressionList.h index f6a269809decc..4af6f99b9c23a 100644 --- a/lldb/include/lldb/Expression/DWARFExpressionList.h +++ b/lldb/include/lldb/Expression/DWARFExpressionList.h @@ -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" @@ -58,15 +59,16 @@ class DWARFExpressionList { } lldb::addr_t GetFuncFileAddress() { return m_func_file_addr; } - + /// Represents an entry in the DWARFExpressionList with all needed metadata. struct DWARFExpressionEntry { - lldb::addr_t base; - lldb::addr_t end; + AddressRange file_range; /// Represents a DWARF location range in the DWARF unit’s file‐address space const DWARFExpression *expr; }; - /// Returns the DWARFExpressionEntry for a given PC address. + /// 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. llvm::Expected GetExpressionEntryAtAddress(lldb::addr_t func_load_addr, lldb::addr_t load_addr) const; From e8142dab5a1c90f05deb659a57059313c055b99d Mon Sep 17 00:00:00 2001 From: Abdullah Mohammad Amin <67847674+UltimateForce21@users.noreply.github.com> Date: Tue, 24 Jun 2025 16:36:41 -0400 Subject: [PATCH 06/39] Update DWARFExpressionList.cpp Converts `GetExpressionEntryAtAddress` to return `llvm::Expected` using the updated `DWARFExpressionEntry`. Updates the implementation to compute a single `AddressRange file_range` for each DWARF location interval. --- .../source/Expression/DWARFExpressionList.cpp | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/lldb/source/Expression/DWARFExpressionList.cpp b/lldb/source/Expression/DWARFExpressionList.cpp index ebf57dd457769..8b8378eb895d3 100644 --- a/lldb/source/Expression/DWARFExpressionList.cpp +++ b/lldb/source/Expression/DWARFExpressionList.cpp @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// +#include "lldb/Core/AddressRange.h" #include "lldb/Expression/DWARFExpressionList.h" #include "lldb/Symbol/Function.h" #include "lldb/Target/RegisterContext.h" @@ -55,22 +56,25 @@ bool DWARFExpressionList::ContainsAddress(lldb::addr_t func_load_addr, llvm::Expected DWARFExpressionList::GetExpressionEntryAtAddress(lldb::addr_t func_load_addr, - lldb::addr_t load_addr) const { - if (const DWARFExpression *expr = GetAlwaysValidExpr()) - return DWARFExpressionEntry{0, LLDB_INVALID_ADDRESS, expr}; + lldb::addr_t load_addr) const { + if (const DWARFExpression *always = GetAlwaysValidExpr()) { + AddressRange full_range(m_func_file_addr, /*size=*/LLDB_INVALID_ADDRESS); + return DWARFExpressionEntry{full_range, always}; + } if (func_load_addr == LLDB_INVALID_ADDRESS) func_load_addr = m_func_file_addr; + lldb::addr_t file_pc = load_addr - func_load_addr + m_func_file_addr; - addr_t addr = load_addr - func_load_addr + m_func_file_addr; - uint32_t index = m_exprs.FindEntryIndexThatContains(addr); - if (index == UINT32_MAX) { - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "No DWARF expression found for address 0x%llx", addr); - } + uint32_t idx = m_exprs.FindEntryIndexThatContains(file_pc); + if (idx == UINT32_MAX) + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + "no DWARF location list entry for PC 0x%" PRIx64, load_addr); - const Entry &entry = *m_exprs.GetEntryAtIndex(index); - return DWARFExpressionEntry{entry.base, entry.GetRangeEnd(), &entry.data}; + const auto &entry = *m_exprs.GetEntryAtIndex(idx); + AddressRange range_in_file(entry.base, entry.GetRangeEnd() - entry.base); + return DWARFExpressionEntry{range_in_file, &entry.data}; } const DWARFExpression * From 7e8741edfefa6989d06b4e50e11dfd4a47d57d28 Mon Sep 17 00:00:00 2001 From: Abdullah Mohammad Amin <67847674+UltimateForce21@users.noreply.github.com> Date: Sat, 28 Jun 2025 12:59:08 -0400 Subject: [PATCH 07/39] Update DWARFExpressionList.h Updated commenting style for struct DWARFExpressionEntry --- lldb/include/lldb/Expression/DWARFExpressionList.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lldb/include/lldb/Expression/DWARFExpressionList.h b/lldb/include/lldb/Expression/DWARFExpressionList.h index 4af6f99b9c23a..31b852eb1ec80 100644 --- a/lldb/include/lldb/Expression/DWARFExpressionList.h +++ b/lldb/include/lldb/Expression/DWARFExpressionList.h @@ -62,7 +62,8 @@ class DWARFExpressionList { /// Represents an entry in the DWARFExpressionList with all needed metadata. struct DWARFExpressionEntry { - AddressRange file_range; /// Represents a DWARF location range in the DWARF unit’s file‐address space + /// Represents a DWARF location range in the DWARF unit’s file‐address space + AddressRange file_range; const DWARFExpression *expr; }; From c4cd77fa3605b3f1653b3987ed0a65b4c264f655 Mon Sep 17 00:00:00 2001 From: Abdullah Mohammad Amin <67847674+UltimateForce21@users.noreply.github.com> Date: Sat, 28 Jun 2025 13:24:57 -0400 Subject: [PATCH 08/39] Update DWARFExpressionList.cpp Updated function `llvm::Expected DWARFExpressionList::GetExpressionEntryAtAddress` to use `FindEntryThatContains` instead of `FindEntryIndexThatContains` --- lldb/source/Expression/DWARFExpressionList.cpp | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lldb/source/Expression/DWARFExpressionList.cpp b/lldb/source/Expression/DWARFExpressionList.cpp index 8b8378eb895d3..9a6cccf445028 100644 --- a/lldb/source/Expression/DWARFExpressionList.cpp +++ b/lldb/source/Expression/DWARFExpressionList.cpp @@ -64,17 +64,19 @@ DWARFExpressionList::GetExpressionEntryAtAddress(lldb::addr_t func_load_addr, if (func_load_addr == LLDB_INVALID_ADDRESS) func_load_addr = m_func_file_addr; + + // translate to file-relative PC lldb::addr_t file_pc = load_addr - func_load_addr + m_func_file_addr; - uint32_t idx = m_exprs.FindEntryIndexThatContains(file_pc); - if (idx == UINT32_MAX) - return llvm::createStringError( - llvm::inconvertibleErrorCode(), - "no DWARF location list entry for PC 0x%" PRIx64, load_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}; + } - const auto &entry = *m_exprs.GetEntryAtIndex(idx); - AddressRange range_in_file(entry.base, entry.GetRangeEnd() - entry.base); - return DWARFExpressionEntry{range_in_file, &entry.data}; + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + "no DWARF location list entry for PC 0x%" PRIx64, load_addr); } const DWARFExpression * From 62c02a9f1afd2018e0bd11541bf001047e4f7917 Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Sat, 28 Jun 2025 21:46:46 -0400 Subject: [PATCH 09/39] Change GetExpressionEntryAtAddress to return std::optional instead of llvm::Expected --- lldb/include/lldb/Expression/DWARFExpressionList.h | 2 +- lldb/source/Expression/DWARFExpressionList.cpp | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lldb/include/lldb/Expression/DWARFExpressionList.h b/lldb/include/lldb/Expression/DWARFExpressionList.h index 31b852eb1ec80..b21b694694ddc 100644 --- a/lldb/include/lldb/Expression/DWARFExpressionList.h +++ b/lldb/include/lldb/Expression/DWARFExpressionList.h @@ -70,7 +70,7 @@ class DWARFExpressionList { /// 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. - llvm::Expected + std::optional GetExpressionEntryAtAddress(lldb::addr_t func_load_addr, lldb::addr_t load_addr) const; diff --git a/lldb/source/Expression/DWARFExpressionList.cpp b/lldb/source/Expression/DWARFExpressionList.cpp index 9a6cccf445028..763dcb568d2ec 100644 --- a/lldb/source/Expression/DWARFExpressionList.cpp +++ b/lldb/source/Expression/DWARFExpressionList.cpp @@ -54,7 +54,7 @@ bool DWARFExpressionList::ContainsAddress(lldb::addr_t func_load_addr, return GetExpressionAtAddress(func_load_addr, addr) != nullptr; } -llvm::Expected +std::optional DWARFExpressionList::GetExpressionEntryAtAddress(lldb::addr_t func_load_addr, lldb::addr_t load_addr) const { if (const DWARFExpression *always = GetAlwaysValidExpr()) { @@ -74,9 +74,8 @@ DWARFExpressionList::GetExpressionEntryAtAddress(lldb::addr_t func_load_addr, return DWARFExpressionEntry{range_in_file, &entry->data}; } - return llvm::createStringError( - llvm::inconvertibleErrorCode(), - "no DWARF location list entry for PC 0x%" PRIx64, load_addr); + // No entry covers this PC: + return std::nullopt; } const DWARFExpression * From d015971a280d41a68e17530b00258c193ea46a82 Mon Sep 17 00:00:00 2001 From: Abdullah Mohammad Amin <67847674+UltimateForce21@users.noreply.github.com> Date: Wed, 2 Jul 2025 11:45:48 -0400 Subject: [PATCH 10/39] Update DWARFExpressionList.cpp Co-authored-by: Adrian Prantl --- lldb/source/Expression/DWARFExpressionList.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Expression/DWARFExpressionList.cpp b/lldb/source/Expression/DWARFExpressionList.cpp index 763dcb568d2ec..2f0f034b613db 100644 --- a/lldb/source/Expression/DWARFExpressionList.cpp +++ b/lldb/source/Expression/DWARFExpressionList.cpp @@ -65,7 +65,7 @@ DWARFExpressionList::GetExpressionEntryAtAddress(lldb::addr_t func_load_addr, if (func_load_addr == LLDB_INVALID_ADDRESS) func_load_addr = m_func_file_addr; - // translate to file-relative PC + // Translate to file-relative PC. lldb::addr_t file_pc = load_addr - func_load_addr + m_func_file_addr; if (const auto *entry = m_exprs.FindEntryThatContains(file_pc)) { From 60898ea58bf178a2aa87f86b44ddfe7fd8b5d6da Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Thu, 3 Jul 2025 00:05:26 -0400 Subject: [PATCH 11/39] Add underflow/overflow checks to GetExpressionEntryAtAddressi This patch adds explicit checks: - ensure `load_addr >= func_load_addr` to avoid underflow, - compute and verify a temporary delta variable, then verify `delta + m_func_file_addr` does not exceed `addr_t` max to avoid overflow. --- lldb/source/Expression/DWARFExpressionList.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/lldb/source/Expression/DWARFExpressionList.cpp b/lldb/source/Expression/DWARFExpressionList.cpp index 2f0f034b613db..772c23fd96d0f 100644 --- a/lldb/source/Expression/DWARFExpressionList.cpp +++ b/lldb/source/Expression/DWARFExpressionList.cpp @@ -64,10 +64,18 @@ DWARFExpressionList::GetExpressionEntryAtAddress(lldb::addr_t func_load_addr, if (func_load_addr == LLDB_INVALID_ADDRESS) func_load_addr = m_func_file_addr; - - // Translate to file-relative PC. - lldb::addr_t file_pc = load_addr - 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::max() - m_func_file_addr) + 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()); From 3462165da7811d190d1896ed41372af25548abd5 Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Mon, 7 Jul 2025 22:39:45 -0400 Subject: [PATCH 12/39] Make file_range optional in DWARFExpressionEntry for always-valid expr --- lldb/include/lldb/Expression/DWARFExpressionList.h | 2 +- lldb/source/Expression/DWARFExpressionList.cpp | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lldb/include/lldb/Expression/DWARFExpressionList.h b/lldb/include/lldb/Expression/DWARFExpressionList.h index b21b694694ddc..1bd762a9836e8 100644 --- a/lldb/include/lldb/Expression/DWARFExpressionList.h +++ b/lldb/include/lldb/Expression/DWARFExpressionList.h @@ -63,7 +63,7 @@ class DWARFExpressionList { /// 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 - AddressRange file_range; + std::optional file_range; ///< None = always-valid single expr const DWARFExpression *expr; }; diff --git a/lldb/source/Expression/DWARFExpressionList.cpp b/lldb/source/Expression/DWARFExpressionList.cpp index 772c23fd96d0f..b51317a98365c 100644 --- a/lldb/source/Expression/DWARFExpressionList.cpp +++ b/lldb/source/Expression/DWARFExpressionList.cpp @@ -58,8 +58,7 @@ std::optional DWARFExpressionList::GetExpressionEntryAtAddress(lldb::addr_t func_load_addr, lldb::addr_t load_addr) const { if (const DWARFExpression *always = GetAlwaysValidExpr()) { - AddressRange full_range(m_func_file_addr, /*size=*/LLDB_INVALID_ADDRESS); - return DWARFExpressionEntry{full_range, always}; + return DWARFExpressionEntry{std::nullopt, always}; } if (func_load_addr == LLDB_INVALID_ADDRESS) From 2ed84430163412eec217f8cabc653191f5be4766 Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Thu, 3 Jul 2025 19:54:27 -0400 Subject: [PATCH 13/39] Annotate Instruction::Dump() with DWARF variable locations --- lldb/source/Core/Disassembler.cpp | 49 +++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index dce3d59457c0e..f818eb20bdad5 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -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" @@ -702,6 +705,52 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, ss.FillLastLineToColumn(opcode_pos + opcode_column_width, ' '); ss.PutCString(mnemonics); + if (exe_ctx && exe_ctx->GetFramePtr()) { + 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)) { + 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 (var_list_sp) { + for (size_t i = 0; i < var_list_sp->GetSize(); ++i) { + 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(); + + // Handle std::optional. + if (auto entryOrErr = expr_list.GetExpressionEntryAtAddress(func_load_addr, current_pc)) { + auto entry = *entryOrErr; + + // Translate file-range to load-space start. + addr_t file_base = entry.file_range.GetBaseAddress().GetFileAddress(); + addr_t start_load_addr = file_base + (func_load_addr - expr_list.GetFuncFileAddress()); + + if (current_pc == start_load_addr) { + StreamString loc_str; + ABI *abi = exe_ctx->GetProcessPtr()->GetABI().get(); + entry.expr->DumpLocation(&loc_str, eDescriptionLevelBrief, abi); + ss.FillLastLineToColumn(opcode_pos + opcode_column_width + operand_column_width, ' '); + ss.Printf(" ; %s = %s", name, loc_str.GetString().str().c_str()); + } + } + } + } + + frame->ChangePC(original_pc); + } + } + } + if (!m_comment.empty()) { ss.FillLastLineToColumn( opcode_pos + opcode_column_width + operand_column_width, ' '); From 8c6b22dab6ef744742ae46e948f204d6b27f8145 Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Sat, 5 Jul 2025 01:22:33 -0400 Subject: [PATCH 14/39] Added Initial Basic API test for rich variable annotation in disassembly output. Right now just checks if DW annotations show up for a basic program and that a variable location is annotated (i.e 'a = DW_OP_reg...'). --- .../rich-disassembler/Makefile | 4 ++ .../rich-disassembler/TestRichDisassembler.py | 38 ++++++++++++++++++ .../rich-disassembler/main.cpp | 15 +++++++ .../rich-disassembler/unoptimized_output | Bin 0 -> 17184 bytes 4 files changed, 57 insertions(+) create mode 100644 lldb/test/API/functionalities/rich-disassembler/Makefile create mode 100644 lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py create mode 100644 lldb/test/API/functionalities/rich-disassembler/main.cpp create mode 100755 lldb/test/API/functionalities/rich-disassembler/unoptimized_output diff --git a/lldb/test/API/functionalities/rich-disassembler/Makefile b/lldb/test/API/functionalities/rich-disassembler/Makefile new file mode 100644 index 0000000000000..5fcbc722e7559 --- /dev/null +++ b/lldb/test/API/functionalities/rich-disassembler/Makefile @@ -0,0 +1,4 @@ +CXX := clang++ +CXX_SOURCES := main.cpp +CXXFLAGS_EXTRAS := -g -O0 -fno-inline +include Makefile.rules diff --git a/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py b/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py new file mode 100644 index 0000000000000..2a8b7df1947c1 --- /dev/null +++ b/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py @@ -0,0 +1,38 @@ +from lldbsuite.test.lldbtest import * +from lldbsuite.test.decorators import * + +class TestRichDisassembler(TestBase): + """ + Tests that the disassembler includes DWARF variable annotations in output. + Specifically checks that variables like 'a' and 'temp' are shown with DW_OP locations. + """ + def test_variable_annotation(self): + print("Building with:", self.getCompiler()) + self.build() + exe = self.getBuildArtifact("a.out") + target = self.dbg.CreateTarget(exe) + self.assertTrue(target) + + src_file = lldb.SBFileSpec("main.cpp") + breakpoint = target.BreakpointCreateByName("test") + print("Breakpoint locations:", breakpoint.GetNumLocations()) + self.assertGreater(breakpoint.GetNumLocations(), 0) + + process = target.LaunchSimple(None, None, self.get_process_working_directory()) + print("Process state:", process.GetState()) + print("Exit status:", process.GetExitStatus()) + print("Exit description:", process.GetExitDescription()) + + self.assertTrue(process, "Failed to launch process") + self.assertEqual(process.GetState(), lldb.eStateStopped, "Process did not stop") + + frame = process.GetSelectedThread().GetSelectedFrame() + disasm = frame.Disassemble() + print(disasm) + + # Check that at least one DWARF annotation is shown. + self.assertIn("DW_OP_", disasm) + + # Check that at least one variable name is annotated. + self.assertRegex(disasm, r'[a-zA-Z_]\w*\s*=\s*DW_OP_') + diff --git a/lldb/test/API/functionalities/rich-disassembler/main.cpp b/lldb/test/API/functionalities/rich-disassembler/main.cpp new file mode 100644 index 0000000000000..4fa182b9fa0de --- /dev/null +++ b/lldb/test/API/functionalities/rich-disassembler/main.cpp @@ -0,0 +1,15 @@ +// main.cpp + + +__attribute__((noinline)) +void test(int a) { + for (int i = 0; i < 3; ++i) { // i should be in a register + int temp = i + 1; // temp also likely register + } +} + +int main() { + test(5); + return 0; +} + diff --git a/lldb/test/API/functionalities/rich-disassembler/unoptimized_output b/lldb/test/API/functionalities/rich-disassembler/unoptimized_output new file mode 100755 index 0000000000000000000000000000000000000000..3ead1df962fa45b59eb04f5957d72195cfbe1332 GIT binary patch literal 17184 zcmeHOYiu0V6~43UUE4VEBamVe6mKA=;o-5@aY7Ⓢuj*MhTC^fwUzIBHoQ&6_wnK@^? znXXY=sr|!TYv#M>yXU+2&bfD9-nm~H+_fX5X-ZO9-K5A$w7N`!3ubQM3?QL4s-@U> zsx>MKyUyX<@`MX0b<-Q63$cOuFpzZZl&OK2yR;IN9uksnvD82vLK0Mb9Hfg-0o#5& z%?$`jxz}Ed#7Hq`@8P>_3Rdf=E3;j$!|av|F~M<3GK!8&(#^AOo^^sV+)qJ?C&h%e zjT}!u#!yBn=Ijw4M$zqOogcTepCI{D+WVHBro6u$v18MpxV^>3p`GI|$l+Dp+04j>zV2+gE1SueC%Yy$Zs^+3r3IiAbFv(|5N4zb9NYF$>NjQSp{!Bac6`0EzocYwbF+rK2k7|Rv% zM#(mdwqYn^Xn41gwu;tRres^i;oSq-Lf#rSN3xb%R=dPVO`67NCU0glhv8^oR8m}L z&dlT$?2?^MU3)E!%my-ZUT!P}SF+;7r(AyLuAyxMMz6lEPf4~{Ur)UcBgXXSMxXE- zU}YY?aPmonKdhc&uDl}CNTc&Zk|N3ZE$3CN&E@C#{1twND~u)Fa^b~(W2?*4Skt&m znyKPxj3mug@ovxElDR6LPi?1QzKW;$7o;EpK?H&b1Q7@#5JVt|KoEh?Rs=q1S@n

mGaHaYcj!x<+KmG$R9S32LszNI69+z*f&Jt0vdef+(lV`uM9 z9y^zudUNi!;lcQH{MX6jTiWoPT3P-8+&>)ETRI+bB;bDrCV4#a6M`G&?d53tZpx$S z#>z}f$5C=T!wCO}9sl)*39de$JT{kn>Can}FU^OO+AGPk=j}^SK^IrhP?;HR>Da!Y zr~Vz?vIWJeeC@vE)RuEPvdLp_*-Mg-ZCM3=_UcNdGMmQkmB@0~+5xofwNLuD??)l` zXCI`;j(qa!;5_wm>d1V!<@lw@JvKN$HLV>R{7^FUZ~qah17{Wv-RT$U5%1Pl2S1?- zenrptv(LaYK8>Ka9-yduW8#ehxdthSKoEf-0zm|V2m}!bA`nC%h(Hj5AOb-If(ZQg z5uo3>w3XpaJ*a}-MK7PLRMNoffR6(=19y@Aex>pwU>5i?kbc{wq#fR)w8>U&<&ye_ z6L{e#o_@2R{Y#~C6*+9LcYg6&gDm}~`#gAuXl~un+6TrZF#3#!M1!KN$+ozr2MbvD+RJo@yCnuE#&?>V&z*{rC#Imi(Fbg$JoT! z&bXXWfMrT0T&A82lH8E=W14wcT; zUO%}0|2N2Y<%_i|-=*#x7`QREcHc-jZBn=xJ&-=Rk&(i9PWlMX&nW9pO3^35QNTSg#^^5OvyG z>?5I*lMO2B5Fr{K5O$-gCju!NUX8tWM?^IdQy-1gDXmTG?7TP^&KsLHHEqT04M$X{-8F|UMw?BJ4_fKFfRF0R)ru==>({7o=S@vb zGzpJji{NOkr`~AV$IVL#Xj))Hrh~F7taXH1BCE8$WZZk_I}f})_TAxozV+Ptr_Xm> zg(=?{>fsaXbCD$%l_Sfeb|idrq%}GgSr*+KY1|xH5#1CCM^8kw=$FvkC6Vw&Z+$=1 zfENDE;h@i{_0+_K>K-rTtnPBw&g4wn8Z8u4R&TsJn?0E8nkW{&Y^7`%>5*>RD%sur zw+(fVmh&kaV~lUK?Tl6GE@o2WUFl58ES0PrzV9w70(BfOrEG|4!@_&?~FchJ{4eW2hg^?7Xc@31Lsk7GND?MZCk!S;1*Ph(p&(P-AG z_bYw}nxGm=9^?}~#txWTHT_qC52gAwv`wsuxfSbN(wV$bE?H?*SiKM?QN47w%SMw;nbSP(=BS5?8;<)5+Kd_1u zO3xQ;OCQUZb=+$vtfGCWiW(_rupTRuW~zVNP#4V$$9>!^jVnEUD32yww2N*DKY?6e z7$_@RS(5}xr%_ejB=$}W^GAT4# zu&HKriAGgNM3s&ynZry2XN{D{FchN&$(ti1MeCqgSbAnj9)XgqnWj#-VsgP4c{Z{} zVRW=)*`*q1;eSrib30B^+!Nf?&i5}td2Z)o$5--%S7M`QYthU4K*1Qws+03LSVe3= zzFNPZ+ZUvVhw3EPW8vujSFM-(uV73Z8U6Y@!T$}<>e7FC9}$%HX}uODfB#Kz^cX05 zd0!DcK_VpTBgI+t@}7|aM~5tWdH)fV_Z_%6iR!3M_raid5z)*0hoHR2N&m%9@FD2w z-9~JAzY*-=#{B)4J{(7eVk&xhzY?5gM`_=G{NF}79g~!M_cOk)$$KSNU$fhR|Tkv~6eGeB1N`IvSu?2tZ(E(HMt_viR__@}CE&Mwupt&dQ%k!qJ&z|A&qxnxsbTW>o!I6vT zWqs!x-Q=y*S{J;%=mkGONwr=j*tN$+xlSMa z-~AlrXGDmwNhH?{iaAy)LuXS0Fl}ceCE#{b;%1 literal 0 HcmV?d00001 From 842a9e51bd89282277e067a66cd6f1651fa61a74 Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Sun, 6 Jul 2025 01:09:36 -0400 Subject: [PATCH 15/39] Improved DWARF variable annotation printing and alignment - Fixed an issue where variable location annotations were not shown if the current instruction address did not exactly match the DWARF base address. Now, annotations are shown as long as the PC is within the valid range. - Improved alignment of annotation comments in Instruction::Dump(). While `FillLastLineToColumn` can sometimes overcompensate due to internal formatting or byte-width mismatches, the overall alignment is now significantly more consistent and readable. --- lldb/source/Core/Disassembler.cpp | 64 ++++++++++++++++++++----------- 1 file changed, 41 insertions(+), 23 deletions(-) diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index f818eb20bdad5..a3f8efe3487ef 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -705,6 +705,8 @@ 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; + if (exe_ctx && exe_ctx->GetFramePtr()) { StackFrame *frame = exe_ctx->GetFramePtr(); TargetSP target_sp = exe_ctx->GetTargetSP(); @@ -718,29 +720,45 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, if (sc.function) func_load_addr = sc.function->GetAddress().GetLoadAddress(target_sp.get()); - if (var_list_sp) { - for (size_t i = 0; i < var_list_sp->GetSize(); ++i) { - 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(); - - // Handle std::optional. - if (auto entryOrErr = expr_list.GetExpressionEntryAtAddress(func_load_addr, current_pc)) { - auto entry = *entryOrErr; - - // Translate file-range to load-space start. - addr_t file_base = entry.file_range.GetBaseAddress().GetFileAddress(); - addr_t start_load_addr = file_base + (func_load_addr - expr_list.GetFuncFileAddress()); - - if (current_pc == start_load_addr) { - StreamString loc_str; - ABI *abi = exe_ctx->GetProcessPtr()->GetABI().get(); - entry.expr->DumpLocation(&loc_str, eDescriptionLevelBrief, abi); - ss.FillLastLineToColumn(opcode_pos + opcode_column_width + operand_column_width, ' '); - ss.Printf(" ; %s = %s", name, loc_str.GetString().str().c_str()); + + if(ss.GetSizeOfLastLine() < annotation_column) { + + std::vector annotations; + + if (var_list_sp) { + for (size_t i = 0; i < var_list_sp->GetSize(); ++i) { + 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. + if (auto entryOrErr = expr_list.GetExpressionEntryAtAddress(func_load_addr, current_pc)) { + auto entry = *entryOrErr; + + // Translate file-range to load-space start. + addr_t file_base = entry.file_range.GetBaseAddress().GetFileAddress(); + addr_t start_load_addr = file_base + (func_load_addr - expr_list.GetFuncFileAddress()); + + if (current_pc >= start_load_addr) { + StreamString loc_str; + ABI *abi = exe_ctx->GetProcessPtr()->GetABI().get(); + entry.expr->DumpLocation(&loc_str, eDescriptionLevelBrief, abi); + annotations.push_back(llvm::formatv("{0} = {1}", name, loc_str.GetString())); + } + } + } + + if (!annotations.empty()) { + ss.FillLastLineToColumn(annotation_column, ' '); + ss.PutCString(" ; "); + for (size_t i = 0; i < annotations.size(); ++i) { + if (i > 0) + ss.PutCString(", "); + ss.PutCString(annotations[i]); } } } From 2fa6d243b24927510af426cfbd57c112c7289b0e Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Sun, 6 Jul 2025 11:10:46 -0400 Subject: [PATCH 16/39] Filter out partial DWARF decoding errors from disassembly annotations Previously, when a DWARF expression contained any decoding error, the entire variable location annotation was printed with the error, e.g. `c = DW_OP_addr 0x0, 00 00 00`. This was misleading and cluttered the disassembly view. This patch improves the formatting by stripping out the `` fragments while preserving the valid portions of the expression, so that partial information like `c = DW_OP_addr 0x0` can still be shown. This allows the rich disassembler to give more meaningful variable annotations, especially in optimized (-O1/-O2) builds where partial DWARF corruption or unsupported expressions may occur. --- lldb/source/Core/Disassembler.cpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index a3f8efe3487ef..0982c98fd80c0 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -49,6 +49,7 @@ #include #include +#include #include #include @@ -747,7 +748,19 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, StreamString loc_str; ABI *abi = exe_ctx->GetProcessPtr()->GetABI().get(); entry.expr->DumpLocation(&loc_str, eDescriptionLevelBrief, abi); - annotations.push_back(llvm::formatv("{0} = {1}", name, loc_str.GetString())); + + std::string loc_output = loc_str.GetString().str(); + + // Remove ", ..." segments. + std::regex decoding_err_re(", [^,]*"); + loc_output = std::regex_replace(loc_output, decoding_err_re, ""); + + llvm::StringRef cleaned_output = llvm::StringRef(loc_output).trim(); + + // Only keep this annotation if there is still something useful left. + if (!cleaned_output.empty()) { + annotations.push_back(llvm::formatv("{0} = {1}", name, cleaned_output)); + } } } } From 6bbc8aaa45e8df045ccb5be4db52ab23fa169aa2 Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Sun, 6 Jul 2025 11:45:19 -0400 Subject: [PATCH 17/39] Ignore annotations with only decoding errors Handled edge case where the entire DWARF expression is a ``, ensuring no misleading or empty annotations are printed for such variables. --- lldb/source/Core/Disassembler.cpp | 35 +++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 0982c98fd80c0..710856404b67a 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -751,15 +751,38 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, std::string loc_output = loc_str.GetString().str(); - // Remove ", ..." segments. - std::regex decoding_err_re(", [^,]*"); - loc_output = std::regex_replace(loc_output, decoding_err_re, ""); + // // Remove ", ..." segments. + // std::regex decoding_err_re(", [^,]*"); + // loc_output = std::regex_replace(loc_output, decoding_err_re, ""); - llvm::StringRef cleaned_output = llvm::StringRef(loc_output).trim(); + // llvm::StringRef cleaned_output = llvm::StringRef(loc_output).trim(); + + // // Only keep this annotation if there is still something useful left. + // if (!cleaned_output.empty()) { + // annotations.push_back(llvm::formatv("{0} = {1}", name, cleaned_output)); + // } + + llvm::SmallVector parts; + llvm::StringRef(loc_str.GetString()).split(parts, ", "); + + // Reconstruct the string without the decoding error chunks + std::string cleaned_output; + bool first = true; + + for (const auto &part : parts) { + if (part.contains("")) + continue; + + if (!first) + cleaned_output += ", "; + cleaned_output += part.str(); + first = false; + } // Only keep this annotation if there is still something useful left. - if (!cleaned_output.empty()) { - annotations.push_back(llvm::formatv("{0} = {1}", name, cleaned_output)); + llvm::StringRef cleaned_ref = llvm::StringRef(cleaned_output).trim(); + if (!cleaned_ref.empty()) { + annotations.push_back(llvm::formatv("{0} = {1}", name, cleaned_ref)); } } } From cbbc9241e0b620f7cc8e925fd83fe36dd94536a4 Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Sun, 6 Jul 2025 14:19:12 -0400 Subject: [PATCH 18/39] Add tests for disassembly variable annotations and decoding edge cases This patch adds API tests to verify that DWARF variable location annotations are correctly displayed in the disassembly output. The tests cover: - Local variables in loops and functions - Multiple stack variables - Control flow edge cases - Different optimization levels (-O0, -O1, -O2) - Ensuring decoding errors are excluded from output --- .../rich-disassembler/Makefile | 8 +- .../rich-disassembler/TestRichDisassembler.py | 295 +++++++++++++++++- .../a_loop_with_local_variable.c | 8 + .../b_multiple_stack_variables.c | 9 + .../c_variable_passed_to_another_function.c | 11 + .../rich-disassembler/d_original_example.c | 7 + .../rich-disassembler/e_control_flow_edge.c | 10 + .../rich-disassembler/main.cpp | 15 - .../rich-disassembler/unoptimized_output | Bin 17184 -> 0 bytes 9 files changed, 328 insertions(+), 35 deletions(-) create mode 100644 lldb/test/API/functionalities/rich-disassembler/a_loop_with_local_variable.c create mode 100644 lldb/test/API/functionalities/rich-disassembler/b_multiple_stack_variables.c create mode 100644 lldb/test/API/functionalities/rich-disassembler/c_variable_passed_to_another_function.c create mode 100644 lldb/test/API/functionalities/rich-disassembler/d_original_example.c create mode 100644 lldb/test/API/functionalities/rich-disassembler/e_control_flow_edge.c delete mode 100644 lldb/test/API/functionalities/rich-disassembler/main.cpp delete mode 100755 lldb/test/API/functionalities/rich-disassembler/unoptimized_output diff --git a/lldb/test/API/functionalities/rich-disassembler/Makefile b/lldb/test/API/functionalities/rich-disassembler/Makefile index 5fcbc722e7559..ae3330e632a0e 100644 --- a/lldb/test/API/functionalities/rich-disassembler/Makefile +++ b/lldb/test/API/functionalities/rich-disassembler/Makefile @@ -1,4 +1,6 @@ -CXX := clang++ -CXX_SOURCES := main.cpp -CXXFLAGS_EXTRAS := -g -O0 -fno-inline + +# 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 diff --git a/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py b/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py index 2a8b7df1947c1..d85e6488575b7 100644 --- a/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py +++ b/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py @@ -2,37 +2,298 @@ from lldbsuite.test.decorators import * class TestRichDisassembler(TestBase): - """ - Tests that the disassembler includes DWARF variable annotations in output. - Specifically checks that variables like 'a' and 'temp' are shown with DW_OP locations. - """ - def test_variable_annotation(self): - print("Building with:", self.getCompiler()) - self.build() + + @no_debug_info_test + def test_a_loop_with_local_variable(self): + """ + Tests that the disassembler includes basic DWARF variable annotation in output. + Specifically checks that local variables in a loop are shown with DW_OP locations. + Additionally, it verifies that the disassembly does not contain decoding errors. + """ + self.build(dictionary={ + 'C_SOURCES': 'a_loop_with_local_variable.c', + 'CFLAGS_EXTRAS': '-g -O0' + }) + exe = self.getBuildArtifact("a.out") + target = self.dbg.CreateTarget(exe) + self.assertTrue(target) + + # Set a breakpoint inside main's loop + src_file = lldb.SBFileSpec("test_loop_function_call.c") + 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) + + # Check that we have DWARF annotations for variables + self.assertIn("i = ", disasm) + self.assertIn("DW_OP", disasm) + self.assertNotIn("", disasm) + + + @no_debug_info_test + def test_b_multiple_stack_variables_O0(self): + """ + Tests disassembler output for b_multiple_stack_variables.c built with -O0. + This test checks that multiple local variables are annotated with DWARF + and that their locations are distinct. It also ensures that no decoding errors appear. + """ + self.build(dictionary={ + 'C_SOURCES': 'b_multiple_stack_variables.c', + 'CFLAGS_EXTRAS': '-g -O0' + }) + exe = self.getBuildArtifact("a.out") + target = self.dbg.CreateTarget(exe) + self.assertTrue(target) + + # Set a breakpoint inside main's loop + src_file = lldb.SBFileSpec("test_loop_function_call.c") + 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) + + # Check that we have DWARF annotations for variables + self.assertIn("a = ", disasm) + self.assertIn("b = ", disasm) + self.assertIn("c = ", disasm) + self.assertIn("DW_OP", disasm) + self.assertNotIn("", disasm) + + + @no_debug_info_test + def test_b_multiple_stack_variables_O1(self): + """ + Tests disassembler output for b_multiple_stack_variables.c built with -O1. + Due to optimizations, some variables may be optimized out. + We only check for 'c' and ensure no decoding errors appear. + """ + self.build(dictionary={ + 'C_SOURCES': 'b_multiple_stack_variables.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("c = ", disasm) + self.assertIn("DW_OP", disasm) + self.assertNotIn("", disasm) + + + @no_debug_info_test + def test_c_variable_passed_to_another_function(self): + """ + Tests disassembler output for c_variable_passed_to_another_function.c. + This test checks that a variable passed to another function is annotated + with DWARF and that its location is distinct. It also ensures that no decoding errors appear. + """ + self.build(dictionary={ + 'C_SOURCES': 'c_variable_passed_to_another_function.c', + 'CFLAGS_EXTRAS': '-g -O0' + }) + 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("x = ", disasm) + self.assertIn("DW_OP", disasm) + self.assertNotIn("", disasm) + + + @no_debug_info_test + def test_c_variable_passed_to_another_function_O1(self): + """ + Tests disassembler output for c_variable_passed_to_another_function.c built with -O1. + """ + self.build(dictionary={ + 'C_SOURCES': 'c_variable_passed_to_another_function.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("x = ", disasm) + self.assertIn("arg = ", disasm) + self.assertIn("DW_OP", disasm) + self.assertNotIn("", disasm) + + @no_debug_info_test + def test_d_original_example(self): + """ + Tests disassembler output for d_original_example.c. + This test checks that the disassembly includes basic DWARF variable annotations + and that local variables in the main function are shown with DW_OP locations. + Additionally, it verifies that the disassembly does not contain decoding errors. + """ + self.build(dictionary={ + 'C_SOURCES': 'd_original_example.c', + 'CFLAGS_EXTRAS': '-g -O0' + }) exe = self.getBuildArtifact("a.out") target = self.dbg.CreateTarget(exe) self.assertTrue(target) - src_file = lldb.SBFileSpec("main.cpp") - breakpoint = target.BreakpointCreateByName("test") - print("Breakpoint locations:", breakpoint.GetNumLocations()) + breakpoint = target.BreakpointCreateByName("main") self.assertGreater(breakpoint.GetNumLocations(), 0) process = target.LaunchSimple(None, None, self.get_process_working_directory()) - print("Process state:", process.GetState()) - print("Exit status:", process.GetExitStatus()) - print("Exit description:", process.GetExitDescription()) + 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", disasm) + self.assertNotIn("", disasm) + + @no_debug_info_test + 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("", disasm) + + + @no_debug_info_test + def test_e_control_flow_edge(self): + """ + Tests disassembler output for e_control_flow_edge.c with a focus on control flow edges. + This test checks that the disassembly includes basic DWARF variable annotations + and that local variables in the main function are shown with DW_OP locations. + Additionally, it verifies that the disassembly does not contain decoding errors. + """ + self.build(dictionary={ + 'C_SOURCES': 'e_control_flow_edge.c', + 'CFLAGS_EXTRAS': '-g -O0' + }) + 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, "Process did not stop") + self.assertEqual(process.GetState(), lldb.eStateStopped) frame = process.GetSelectedThread().GetSelectedFrame() disasm = frame.Disassemble() print(disasm) - # Check that at least one DWARF annotation is shown. + self.assertIn("a = ", disasm) + self.assertIn("b = ", disasm) self.assertIn("DW_OP_", disasm) + self.assertNotIn("", disasm) + + @no_debug_info_test + def test_e_control_flow_edge_O1(self): + """ + Tests disassembler output for e_control_flow_edge.c built with -O1. + This test checks that the disassembly annotation does not contain decoding errors. + """ + self.build(dictionary={ + 'C_SOURCES': 'e_control_flow_edge.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.assertNotIn("", disasm) + + + + + + + + - # Check that at least one variable name is annotated. - self.assertRegex(disasm, r'[a-zA-Z_]\w*\s*=\s*DW_OP_') diff --git a/lldb/test/API/functionalities/rich-disassembler/a_loop_with_local_variable.c b/lldb/test/API/functionalities/rich-disassembler/a_loop_with_local_variable.c new file mode 100644 index 0000000000000..6555f3822187f --- /dev/null +++ b/lldb/test/API/functionalities/rich-disassembler/a_loop_with_local_variable.c @@ -0,0 +1,8 @@ +#include + +int main() { + for (int i = 0; i < 3; ++i) { + puts("Hi"); + } + return 0; +} diff --git a/lldb/test/API/functionalities/rich-disassembler/b_multiple_stack_variables.c b/lldb/test/API/functionalities/rich-disassembler/b_multiple_stack_variables.c new file mode 100644 index 0000000000000..d3cd447b43d65 --- /dev/null +++ b/lldb/test/API/functionalities/rich-disassembler/b_multiple_stack_variables.c @@ -0,0 +1,9 @@ +#include + +int main() { + int a = 1; + int b = 2; + int c = a + b; + return c; +} + diff --git a/lldb/test/API/functionalities/rich-disassembler/c_variable_passed_to_another_function.c b/lldb/test/API/functionalities/rich-disassembler/c_variable_passed_to_another_function.c new file mode 100644 index 0000000000000..9603bdc636043 --- /dev/null +++ b/lldb/test/API/functionalities/rich-disassembler/c_variable_passed_to_another_function.c @@ -0,0 +1,11 @@ +#include + +void foo(int arg) { + printf("%d\n", arg); +} + +int main() { + int x = 10; + foo(x); + return 0; +} diff --git a/lldb/test/API/functionalities/rich-disassembler/d_original_example.c b/lldb/test/API/functionalities/rich-disassembler/d_original_example.c new file mode 100644 index 0000000000000..4f245f518a182 --- /dev/null +++ b/lldb/test/API/functionalities/rich-disassembler/d_original_example.c @@ -0,0 +1,7 @@ +#include + +int main(int argc, char **argv) { + for (int i = 1; i < argc; ++i) + puts(argv[i]); + return 0; +} diff --git a/lldb/test/API/functionalities/rich-disassembler/e_control_flow_edge.c b/lldb/test/API/functionalities/rich-disassembler/e_control_flow_edge.c new file mode 100644 index 0000000000000..d8d69f501eb4f --- /dev/null +++ b/lldb/test/API/functionalities/rich-disassembler/e_control_flow_edge.c @@ -0,0 +1,10 @@ +#include + +int main() { + int a = 1; + if (a > 0) { + int b = 2; + return b; + } + return a; +} diff --git a/lldb/test/API/functionalities/rich-disassembler/main.cpp b/lldb/test/API/functionalities/rich-disassembler/main.cpp deleted file mode 100644 index 4fa182b9fa0de..0000000000000 --- a/lldb/test/API/functionalities/rich-disassembler/main.cpp +++ /dev/null @@ -1,15 +0,0 @@ -// main.cpp - - -__attribute__((noinline)) -void test(int a) { - for (int i = 0; i < 3; ++i) { // i should be in a register - int temp = i + 1; // temp also likely register - } -} - -int main() { - test(5); - return 0; -} - diff --git a/lldb/test/API/functionalities/rich-disassembler/unoptimized_output b/lldb/test/API/functionalities/rich-disassembler/unoptimized_output deleted file mode 100755 index 3ead1df962fa45b59eb04f5957d72195cfbe1332..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 17184 zcmeHOYiu0V6~43UUE4VEBamVe6mKA=;o-5@aY7Ⓢuj*MhTC^fwUzIBHoQ&6_wnK@^? znXXY=sr|!TYv#M>yXU+2&bfD9-nm~H+_fX5X-ZO9-K5A$w7N`!3ubQM3?QL4s-@U> zsx>MKyUyX<@`MX0b<-Q63$cOuFpzZZl&OK2yR;IN9uksnvD82vLK0Mb9Hfg-0o#5& z%?$`jxz}Ed#7Hq`@8P>_3Rdf=E3;j$!|av|F~M<3GK!8&(#^AOo^^sV+)qJ?C&h%e zjT}!u#!yBn=Ijw4M$zqOogcTepCI{D+WVHBro6u$v18MpxV^>3p`GI|$l+Dp+04j>zV2+gE1SueC%Yy$Zs^+3r3IiAbFv(|5N4zb9NYF$>NjQSp{!Bac6`0EzocYwbF+rK2k7|Rv% zM#(mdwqYn^Xn41gwu;tRres^i;oSq-Lf#rSN3xb%R=dPVO`67NCU0glhv8^oR8m}L z&dlT$?2?^MU3)E!%my-ZUT!P}SF+;7r(AyLuAyxMMz6lEPf4~{Ur)UcBgXXSMxXE- zU}YY?aPmonKdhc&uDl}CNTc&Zk|N3ZE$3CN&E@C#{1twND~u)Fa^b~(W2?*4Skt&m znyKPxj3mug@ovxElDR6LPi?1QzKW;$7o;EpK?H&b1Q7@#5JVt|KoEh?Rs=q1S@n

mGaHaYcj!x<+KmG$R9S32LszNI69+z*f&Jt0vdef+(lV`uM9 z9y^zudUNi!;lcQH{MX6jTiWoPT3P-8+&>)ETRI+bB;bDrCV4#a6M`G&?d53tZpx$S z#>z}f$5C=T!wCO}9sl)*39de$JT{kn>Can}FU^OO+AGPk=j}^SK^IrhP?;HR>Da!Y zr~Vz?vIWJeeC@vE)RuEPvdLp_*-Mg-ZCM3=_UcNdGMmQkmB@0~+5xofwNLuD??)l` zXCI`;j(qa!;5_wm>d1V!<@lw@JvKN$HLV>R{7^FUZ~qah17{Wv-RT$U5%1Pl2S1?- zenrptv(LaYK8>Ka9-yduW8#ehxdthSKoEf-0zm|V2m}!bA`nC%h(Hj5AOb-If(ZQg z5uo3>w3XpaJ*a}-MK7PLRMNoffR6(=19y@Aex>pwU>5i?kbc{wq#fR)w8>U&<&ye_ z6L{e#o_@2R{Y#~C6*+9LcYg6&gDm}~`#gAuXl~un+6TrZF#3#!M1!KN$+ozr2MbvD+RJo@yCnuE#&?>V&z*{rC#Imi(Fbg$JoT! z&bXXWfMrT0T&A82lH8E=W14wcT; zUO%}0|2N2Y<%_i|-=*#x7`QREcHc-jZBn=xJ&-=Rk&(i9PWlMX&nW9pO3^35QNTSg#^^5OvyG z>?5I*lMO2B5Fr{K5O$-gCju!NUX8tWM?^IdQy-1gDXmTG?7TP^&KsLHHEqT04M$X{-8F|UMw?BJ4_fKFfRF0R)ru==>({7o=S@vb zGzpJji{NOkr`~AV$IVL#Xj))Hrh~F7taXH1BCE8$WZZk_I}f})_TAxozV+Ptr_Xm> zg(=?{>fsaXbCD$%l_Sfeb|idrq%}GgSr*+KY1|xH5#1CCM^8kw=$FvkC6Vw&Z+$=1 zfENDE;h@i{_0+_K>K-rTtnPBw&g4wn8Z8u4R&TsJn?0E8nkW{&Y^7`%>5*>RD%sur zw+(fVmh&kaV~lUK?Tl6GE@o2WUFl58ES0PrzV9w70(BfOrEG|4!@_&?~FchJ{4eW2hg^?7Xc@31Lsk7GND?MZCk!S;1*Ph(p&(P-AG z_bYw}nxGm=9^?}~#txWTHT_qC52gAwv`wsuxfSbN(wV$bE?H?*SiKM?QN47w%SMw;nbSP(=BS5?8;<)5+Kd_1u zO3xQ;OCQUZb=+$vtfGCWiW(_rupTRuW~zVNP#4V$$9>!^jVnEUD32yww2N*DKY?6e z7$_@RS(5}xr%_ejB=$}W^GAT4# zu&HKriAGgNM3s&ynZry2XN{D{FchN&$(ti1MeCqgSbAnj9)XgqnWj#-VsgP4c{Z{} zVRW=)*`*q1;eSrib30B^+!Nf?&i5}td2Z)o$5--%S7M`QYthU4K*1Qws+03LSVe3= zzFNPZ+ZUvVhw3EPW8vujSFM-(uV73Z8U6Y@!T$}<>e7FC9}$%HX}uODfB#Kz^cX05 zd0!DcK_VpTBgI+t@}7|aM~5tWdH)fV_Z_%6iR!3M_raid5z)*0hoHR2N&m%9@FD2w z-9~JAzY*-=#{B)4J{(7eVk&xhzY?5gM`_=G{NF}79g~!M_cOk)$$KSNU$fhR|Tkv~6eGeB1N`IvSu?2tZ(E(HMt_viR__@}CE&Mwupt&dQ%k!qJ&z|A&qxnxsbTW>o!I6vT zWqs!x-Q=y*S{J;%=mkGONwr=j*tN$+xlSMa z-~AlrXGDmwNhH?{iaAy)LuXS0Fl}ceCE#{b;%1 From b887db2cb51c170ec8c6b0159768344d919cab10 Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Mon, 7 Jul 2025 23:52:24 -0400 Subject: [PATCH 19/39] Rebase disassembler annotations branch onto updated DWARFExpressionEntry API This rebases the `add-disassembler-annotations` work onto the latest `add-dwarfexprentry-api` branch so that the instruction annotation patches sit cleanly atop the new DWARFExpressionEntry struct and helper API. All conflicts have been resolved and the annotation code now integrates with the updated std::optional-based GetExpressionEntryAtAddress signature. --- lldb/source/Core/Disassembler.cpp | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 710856404b67a..30a028d005dc1 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -49,7 +49,6 @@ #include #include -#include #include #include @@ -740,28 +739,16 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, if (auto entryOrErr = expr_list.GetExpressionEntryAtAddress(func_load_addr, current_pc)) { auto entry = *entryOrErr; - // Translate file-range to load-space start. - addr_t file_base = entry.file_range.GetBaseAddress().GetFileAddress(); - addr_t start_load_addr = file_base + (func_load_addr - expr_list.GetFuncFileAddress()); + // 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())) { - if (current_pc >= start_load_addr) { StreamString loc_str; ABI *abi = exe_ctx->GetProcessPtr()->GetABI().get(); entry.expr->DumpLocation(&loc_str, eDescriptionLevelBrief, abi); std::string loc_output = loc_str.GetString().str(); - // // Remove ", ..." segments. - // std::regex decoding_err_re(", [^,]*"); - // loc_output = std::regex_replace(loc_output, decoding_err_re, ""); - - // llvm::StringRef cleaned_output = llvm::StringRef(loc_output).trim(); - - // // Only keep this annotation if there is still something useful left. - // if (!cleaned_output.empty()) { - // annotations.push_back(llvm::formatv("{0} = {1}", name, cleaned_output)); - // } - llvm::SmallVector parts; llvm::StringRef(loc_str.GetString()).split(parts, ", "); From 912ba6d5a7a00c9111aae12a89ef84ccf0de00b4 Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Wed, 9 Jul 2025 11:48:09 -0400 Subject: [PATCH 20/39] Add `PrintRegisterOnly` flag in `struct DIDumpOptions` and created new `DWARFExpression::DumpLocationWithOptions` for simplified expression printing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch introduces a PrintRegisterOnly flag to the DIDumpOptions struct, enabling concise rendering of DWARF expressions for disassembler annotations. Key changes: - Added DumpLocationWithOptions to DWARFExpression for flexible dumping with DIDumpOptions. - Updated DWARFExpression::print and Operation::print to respect PrintRegisterOnly, rendering registers like RDI without DW_OP_ or llvm: prefixes. - Suppressed output when PrintRegisterOnly is true to avoid clutter during register-only disassembly output. These changes are motivated by LLDB’s rich disassembler feature, where annotations should match user-facing register names without DWARF-level detail. Test impact: Some rich-disassembler tests that relied on DW_OP_ for validation were deprecated. Updated tests aligned with the new formatting will be added next. --- .../include/lldb/Expression/DWARFExpression.h | 1 + lldb/source/Core/Disassembler.cpp | 33 +-- lldb/source/Expression/DWARFExpression.cpp | 13 +- .../rich-disassembler/TestRichDisassembler.py | 260 +----------------- .../a_loop_with_local_variable.c | 8 - .../b_multiple_stack_variables.c | 9 - .../c_variable_passed_to_another_function.c | 11 - .../rich-disassembler/e_control_flow_edge.c | 10 - llvm/include/llvm/DebugInfo/DIContext.h | 1 + llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp | 118 ++++---- 10 files changed, 84 insertions(+), 380 deletions(-) delete mode 100644 lldb/test/API/functionalities/rich-disassembler/a_loop_with_local_variable.c delete mode 100644 lldb/test/API/functionalities/rich-disassembler/b_multiple_stack_variables.c delete mode 100644 lldb/test/API/functionalities/rich-disassembler/c_variable_passed_to_another_function.c delete mode 100644 lldb/test/API/functionalities/rich-disassembler/e_control_flow_edge.c diff --git a/lldb/include/lldb/Expression/DWARFExpression.h b/lldb/include/lldb/Expression/DWARFExpression.h index 0adbe3e8df2ee..6eb421bc94e90 100644 --- a/lldb/include/lldb/Expression/DWARFExpression.h +++ b/lldb/include/lldb/Expression/DWARFExpression.h @@ -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; bool MatchesOperand(StackFrame &frame, const Instruction::Operand &op) const; diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 30a028d005dc1..683db0f541389 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -742,34 +742,19 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, // 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(); - entry.expr->DumpLocation(&loc_str, eDescriptionLevelBrief, abi); - - std::string loc_output = loc_str.GetString().str(); - - llvm::SmallVector parts; - llvm::StringRef(loc_str.GetString()).split(parts, ", "); + llvm::DIDumpOptions opts; + opts.ShowAddresses = false; + opts.PrintRegisterOnly = true; // <-- important: suppress DW_OP_... annotations, etc. - // Reconstruct the string without the decoding error chunks - std::string cleaned_output; - bool first = true; - - for (const auto &part : parts) { - if (part.contains("")) - continue; - - if (!first) - cleaned_output += ", "; - cleaned_output += part.str(); - first = false; - } + entry.expr->DumpLocationWithOptions(&loc_str, eDescriptionLevelBrief, abi, opts); - // Only keep this annotation if there is still something useful left. - llvm::StringRef cleaned_ref = llvm::StringRef(cleaned_output).trim(); - if (!cleaned_ref.empty()) { - annotations.push_back(llvm::formatv("{0} = {1}", name, cleaned_ref)); + // 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)); } } } diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp index 4b2b111e08e6d..d0d4278c98ae5 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.cpp @@ -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 { @@ -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; } diff --git a/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py b/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py index d85e6488575b7..0164402dfa587 100644 --- a/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py +++ b/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py @@ -2,202 +2,6 @@ from lldbsuite.test.decorators import * class TestRichDisassembler(TestBase): - - @no_debug_info_test - def test_a_loop_with_local_variable(self): - """ - Tests that the disassembler includes basic DWARF variable annotation in output. - Specifically checks that local variables in a loop are shown with DW_OP locations. - Additionally, it verifies that the disassembly does not contain decoding errors. - """ - self.build(dictionary={ - 'C_SOURCES': 'a_loop_with_local_variable.c', - 'CFLAGS_EXTRAS': '-g -O0' - }) - exe = self.getBuildArtifact("a.out") - target = self.dbg.CreateTarget(exe) - self.assertTrue(target) - - # Set a breakpoint inside main's loop - src_file = lldb.SBFileSpec("test_loop_function_call.c") - 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) - - # Check that we have DWARF annotations for variables - self.assertIn("i = ", disasm) - self.assertIn("DW_OP", disasm) - self.assertNotIn("", disasm) - - - @no_debug_info_test - def test_b_multiple_stack_variables_O0(self): - """ - Tests disassembler output for b_multiple_stack_variables.c built with -O0. - This test checks that multiple local variables are annotated with DWARF - and that their locations are distinct. It also ensures that no decoding errors appear. - """ - self.build(dictionary={ - 'C_SOURCES': 'b_multiple_stack_variables.c', - 'CFLAGS_EXTRAS': '-g -O0' - }) - exe = self.getBuildArtifact("a.out") - target = self.dbg.CreateTarget(exe) - self.assertTrue(target) - - # Set a breakpoint inside main's loop - src_file = lldb.SBFileSpec("test_loop_function_call.c") - 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) - - # Check that we have DWARF annotations for variables - self.assertIn("a = ", disasm) - self.assertIn("b = ", disasm) - self.assertIn("c = ", disasm) - self.assertIn("DW_OP", disasm) - self.assertNotIn("", disasm) - - - @no_debug_info_test - def test_b_multiple_stack_variables_O1(self): - """ - Tests disassembler output for b_multiple_stack_variables.c built with -O1. - Due to optimizations, some variables may be optimized out. - We only check for 'c' and ensure no decoding errors appear. - """ - self.build(dictionary={ - 'C_SOURCES': 'b_multiple_stack_variables.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("c = ", disasm) - self.assertIn("DW_OP", disasm) - self.assertNotIn("", disasm) - - - @no_debug_info_test - def test_c_variable_passed_to_another_function(self): - """ - Tests disassembler output for c_variable_passed_to_another_function.c. - This test checks that a variable passed to another function is annotated - with DWARF and that its location is distinct. It also ensures that no decoding errors appear. - """ - self.build(dictionary={ - 'C_SOURCES': 'c_variable_passed_to_another_function.c', - 'CFLAGS_EXTRAS': '-g -O0' - }) - 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("x = ", disasm) - self.assertIn("DW_OP", disasm) - self.assertNotIn("", disasm) - - - @no_debug_info_test - def test_c_variable_passed_to_another_function_O1(self): - """ - Tests disassembler output for c_variable_passed_to_another_function.c built with -O1. - """ - self.build(dictionary={ - 'C_SOURCES': 'c_variable_passed_to_another_function.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("x = ", disasm) - self.assertIn("arg = ", disasm) - self.assertIn("DW_OP", disasm) - self.assertNotIn("", disasm) - - @no_debug_info_test - def test_d_original_example(self): - """ - Tests disassembler output for d_original_example.c. - This test checks that the disassembly includes basic DWARF variable annotations - and that local variables in the main function are shown with DW_OP locations. - Additionally, it verifies that the disassembly does not contain decoding errors. - """ - self.build(dictionary={ - 'C_SOURCES': 'd_original_example.c', - 'CFLAGS_EXTRAS': '-g -O0' - }) - 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", disasm) - self.assertNotIn("", disasm) - - @no_debug_info_test def test_d_original_example_O1(self): """ Tests disassembler output for d_original_example.c built with -O1. @@ -224,68 +28,8 @@ def test_d_original_example_O1(self): 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("", disasm) - - - @no_debug_info_test - def test_e_control_flow_edge(self): - """ - Tests disassembler output for e_control_flow_edge.c with a focus on control flow edges. - This test checks that the disassembly includes basic DWARF variable annotations - and that local variables in the main function are shown with DW_OP locations. - Additionally, it verifies that the disassembly does not contain decoding errors. - """ - self.build(dictionary={ - 'C_SOURCES': 'e_control_flow_edge.c', - 'CFLAGS_EXTRAS': '-g -O0' - }) - 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("a = ", disasm) - self.assertIn("b = ", disasm) - self.assertIn("DW_OP_", disasm) - self.assertNotIn("", disasm) - - @no_debug_info_test - def test_e_control_flow_edge_O1(self): - """ - Tests disassembler output for e_control_flow_edge.c built with -O1. - This test checks that the disassembly annotation does not contain decoding errors. - """ - self.build(dictionary={ - 'C_SOURCES': 'e_control_flow_edge.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("DW_OP_reg", disasm) + # self.assertIn("DW_OP_stack_value", disasm) self.assertNotIn("", disasm) diff --git a/lldb/test/API/functionalities/rich-disassembler/a_loop_with_local_variable.c b/lldb/test/API/functionalities/rich-disassembler/a_loop_with_local_variable.c deleted file mode 100644 index 6555f3822187f..0000000000000 --- a/lldb/test/API/functionalities/rich-disassembler/a_loop_with_local_variable.c +++ /dev/null @@ -1,8 +0,0 @@ -#include - -int main() { - for (int i = 0; i < 3; ++i) { - puts("Hi"); - } - return 0; -} diff --git a/lldb/test/API/functionalities/rich-disassembler/b_multiple_stack_variables.c b/lldb/test/API/functionalities/rich-disassembler/b_multiple_stack_variables.c deleted file mode 100644 index d3cd447b43d65..0000000000000 --- a/lldb/test/API/functionalities/rich-disassembler/b_multiple_stack_variables.c +++ /dev/null @@ -1,9 +0,0 @@ -#include - -int main() { - int a = 1; - int b = 2; - int c = a + b; - return c; -} - diff --git a/lldb/test/API/functionalities/rich-disassembler/c_variable_passed_to_another_function.c b/lldb/test/API/functionalities/rich-disassembler/c_variable_passed_to_another_function.c deleted file mode 100644 index 9603bdc636043..0000000000000 --- a/lldb/test/API/functionalities/rich-disassembler/c_variable_passed_to_another_function.c +++ /dev/null @@ -1,11 +0,0 @@ -#include - -void foo(int arg) { - printf("%d\n", arg); -} - -int main() { - int x = 10; - foo(x); - return 0; -} diff --git a/lldb/test/API/functionalities/rich-disassembler/e_control_flow_edge.c b/lldb/test/API/functionalities/rich-disassembler/e_control_flow_edge.c deleted file mode 100644 index d8d69f501eb4f..0000000000000 --- a/lldb/test/API/functionalities/rich-disassembler/e_control_flow_edge.c +++ /dev/null @@ -1,10 +0,0 @@ -#include - -int main() { - int a = 1; - if (a > 0) { - int b = 2; - return b; - } - return a; -} diff --git a/llvm/include/llvm/DebugInfo/DIContext.h b/llvm/include/llvm/DebugInfo/DIContext.h index 0347f90c236d1..e7e87bbfebf38 100644 --- a/llvm/include/llvm/DebugInfo/DIContext.h +++ b/llvm/include/llvm/DebugInfo/DIContext.h @@ -209,6 +209,7 @@ struct DIDumpOptions { bool IsEH = false; bool DumpNonSkeleton = false; bool ShowAggregateErrors = false; + bool PrintRegisterOnly = false; std::string JsonErrSummaryFile; std::function GetNameForDWARFReg; diff --git a/llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp b/llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp index 2ae5ff3efc8c5..fbf6fbf2cd368 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp @@ -302,14 +302,16 @@ std::optional DWARFExpression::Operation::getSubCode() const { bool DWARFExpression::Operation::print(raw_ostream &OS, DIDumpOptions DumpOpts, const DWARFExpression *Expr, DWARFUnit *U) const { - if (Error) { + if (Error && !DumpOpts.PrintRegisterOnly) { OS << ""; return false; } - StringRef Name = OperationEncodingString(Opcode); - assert(!Name.empty() && "DW_OP has no name!"); - OS << Name; + if (!DumpOpts.PrintRegisterOnly) { + StringRef Name = OperationEncodingString(Opcode); + assert(!Name.empty() && "DW_OP has no name!"); + OS << Name; + } if ((Opcode >= DW_OP_breg0 && Opcode <= DW_OP_breg31) || (Opcode >= DW_OP_reg0 && Opcode <= DW_OP_reg31) || @@ -318,44 +320,46 @@ bool DWARFExpression::Operation::print(raw_ostream &OS, DIDumpOptions DumpOpts, if (prettyPrintRegisterOp(U, OS, DumpOpts, Opcode, Operands)) return true; - for (unsigned Operand = 0; Operand < Desc.Op.size(); ++Operand) { - unsigned Size = Desc.Op[Operand]; - unsigned Signed = Size & Operation::SignBit; - - if (Size == Operation::SizeSubOpLEB) { - StringRef SubName = SubOperationEncodingString(Opcode, Operands[Operand]); - assert(!SubName.empty() && "DW_OP SubOp has no name!"); - OS << " " << SubName; - } else if (Size == Operation::BaseTypeRef && U) { - // For DW_OP_convert the operand may be 0 to indicate that conversion to - // the generic type should be done. The same holds for DW_OP_reinterpret, - // which is currently not supported. - if (Opcode == DW_OP_convert && Operands[Operand] == 0) - OS << " 0x0"; - else - prettyPrintBaseTypeRef(U, OS, DumpOpts, Operands, Operand); - } else if (Size == Operation::WasmLocationArg) { - assert(Operand == 1); - switch (Operands[0]) { - case 0: - case 1: - case 2: - case 3: // global as uint32 - case 4: - OS << format(" 0x%" PRIx64, Operands[Operand]); - break; - default: assert(false); + if (!DumpOpts.PrintRegisterOnly) { + for (unsigned Operand = 0; Operand < Desc.Op.size(); ++Operand) { + unsigned Size = Desc.Op[Operand]; + unsigned Signed = Size & Operation::SignBit; + + if (Size == Operation::SizeSubOpLEB) { + StringRef SubName = SubOperationEncodingString(Opcode, Operands[Operand]); + assert(!SubName.empty() && "DW_OP SubOp has no name!"); + OS << " " << SubName; + } else if (Size == Operation::BaseTypeRef && U) { + // For DW_OP_convert the operand may be 0 to indicate that conversion to + // the generic type should be done. The same holds for DW_OP_reinterpret, + // which is currently not supported. + if (Opcode == DW_OP_convert && Operands[Operand] == 0) + OS << " 0x0"; + else + prettyPrintBaseTypeRef(U, OS, DumpOpts, Operands, Operand); + } else if (Size == Operation::WasmLocationArg) { + assert(Operand == 1); + switch (Operands[0]) { + case 0: + case 1: + case 2: + case 3: // global as uint32 + case 4: + OS << format(" 0x%" PRIx64, Operands[Operand]); + break; + default: assert(false); + } + } else if (Size == Operation::SizeBlock) { + uint64_t Offset = Operands[Operand]; + for (unsigned i = 0; i < Operands[Operand - 1]; ++i) + OS << format(" 0x%02x", Expr->Data.getU8(&Offset)); + } else { + if (Signed) + OS << format(" %+" PRId64, (int64_t)Operands[Operand]); + else if (Opcode != DW_OP_entry_value && + Opcode != DW_OP_GNU_entry_value) + OS << format(" 0x%" PRIx64, Operands[Operand]); } - } else if (Size == Operation::SizeBlock) { - uint64_t Offset = Operands[Operand]; - for (unsigned i = 0; i < Operands[Operand - 1]; ++i) - OS << format(" 0x%02x", Expr->Data.getU8(&Offset)); - } else { - if (Signed) - OS << format(" %+" PRId64, (int64_t)Operands[Operand]); - else if (Opcode != DW_OP_entry_value && - Opcode != DW_OP_GNU_entry_value) - OS << format(" 0x%" PRIx64, Operands[Operand]); } } return true; @@ -370,29 +374,31 @@ void DWARFExpression::print(raw_ostream &OS, DIDumpOptions DumpOpts, for (auto &Op : *this) { DumpOpts.IsEH = IsEH; - if (!Op.print(OS, DumpOpts, this, U)) { + if (!Op.print(OS, DumpOpts, this, U) && !DumpOpts.PrintRegisterOnly) { uint64_t FailOffset = Op.getEndOffset(); while (FailOffset < Data.getData().size()) OS << format(" %02x", Data.getU8(&FailOffset)); return; } - if (Op.getCode() == DW_OP_entry_value || - Op.getCode() == DW_OP_GNU_entry_value) { - OS << "("; - EntryValExprSize = Op.getRawOperand(0); - EntryValStartOffset = Op.getEndOffset(); - continue; - } + if (!DumpOpts.PrintRegisterOnly){ + if (Op.getCode() == DW_OP_entry_value || + Op.getCode() == DW_OP_GNU_entry_value) { + OS << "("; + EntryValExprSize = Op.getRawOperand(0); + EntryValStartOffset = Op.getEndOffset(); + continue; + } - if (EntryValExprSize) { - EntryValExprSize -= Op.getEndOffset() - EntryValStartOffset; - if (EntryValExprSize == 0) - OS << ")"; + if (EntryValExprSize) { + EntryValExprSize -= Op.getEndOffset() - EntryValStartOffset; + if (EntryValExprSize == 0) + OS << ")"; + } + + if (Op.getEndOffset() < Data.getData().size()) + OS << ", "; } - - if (Op.getEndOffset() < Data.getData().size()) - OS << ", "; } } From 09c4d04b33e16e25da3fbfb826c609c8bb42264e Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Sun, 20 Jul 2025 15:24:40 -0400 Subject: [PATCH 21/39] Add high-level comment explaining rich disassembly annotation logic in Instruction::Dump --- lldb/source/Core/Disassembler.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 683db0f541389..af1c7209fbce7 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -705,6 +705,22 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, ss.FillLastLineToColumn(opcode_pos + opcode_column_width, ' '); ss.PutCString(mnemonics); + // Add rich 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. + const size_t annotation_column = 150; if (exe_ctx && exe_ctx->GetFramePtr()) { From 6e17f77c4300654c7664b52efc045983bb59721d Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Sun, 20 Jul 2025 15:38:01 -0400 Subject: [PATCH 22/39] Add comment clarifying annotation column length check in Instruction::Dump --- lldb/source/Core/Disassembler.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index af1c7209fbce7..7661cd9a77cb5 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -736,7 +736,8 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, if (sc.function) func_load_addr = sc.function->GetAddress().GetLoadAddress(target_sp.get()); - + // Only annotate if the current disassembly line is short enough + // to keep annotations aligned past the desired annotation_column. if(ss.GetSizeOfLastLine() < annotation_column) { std::vector annotations; From 31431c0c1e8348dec7761dc7052ca355bfc950dd Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Sun, 20 Jul 2025 16:03:50 -0400 Subject: [PATCH 23/39] Refactor variable annotation logic in `Instruction::Dump` using `annotate_variable` lambda function --- lldb/source/Core/Disassembler.cpp | 131 ++++++++++++++++-------------- 1 file changed, 70 insertions(+), 61 deletions(-) diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 7661cd9a77cb5..053e41f9d9aa2 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -723,75 +723,83 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, const size_t annotation_column = 150; - if (exe_ctx && exe_ctx->GetFramePtr()) { + auto annotate_variables = [&]() { 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)) { - 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()); - - // Only annotate if the current disassembly line is short enough - // to keep annotations aligned past the desired annotation_column. - if(ss.GetSizeOfLastLine() < annotation_column) { - - std::vector annotations; - - if (var_list_sp) { - for (size_t i = 0; i < var_list_sp->GetSize(); ++i) { - 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. - 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 (!frame || !target_sp) + return; - if (!annotations.empty()) { - ss.FillLastLineToColumn(annotation_column, ' '); - ss.PutCString(" ; "); - for (size_t i = 0; i < annotations.size(); ++i) { - if (i > 0) - ss.PutCString(", "); - ss.PutCString(annotations[i]); - } - } + 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)) + return; + + VariableListSP var_list_sp = frame->GetInScopeVariableList(true); + if (!var_list_sp) + return; + + 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()); + + // Only annotate if the current disassembly line is short enough + // to keep annotations aligned past the desired annotation_column. + if (ss.GetSizeOfLastLine() >= annotation_column) + return; + + std::vector annotations; + + for (size_t i = 0; i < var_list_sp->GetSize(); ++i) { + 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. + 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)); } } + } + } - frame->ChangePC(original_pc); + if (!annotations.empty()) { + ss.FillLastLineToColumn(annotation_column, ' '); + ss.PutCString(" ; "); + for (size_t i = 0; i < annotations.size(); ++i) { + if (i > 0) + ss.PutCString(", "); + ss.PutCString(annotations[i]); } } + + frame->ChangePC(original_pc); + }; + + if (exe_ctx && exe_ctx->GetFramePtr()) { + annotate_variables(); } if (!m_comment.empty()) { @@ -803,6 +811,7 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, s->PutCString(ss.GetString()); } + bool Instruction::DumpEmulation(const ArchSpec &arch) { std::unique_ptr insn_emulator_up( EmulateInstruction::FindPlugin(arch, eInstructionTypeAny, nullptr)); From 9c5cb8fa6d622cdd12db4c6a291265a991d3eb5e Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Sun, 20 Jul 2025 16:19:12 -0400 Subject: [PATCH 24/39] Use range-based for loop for variable list iteration in Instruction::Dump --- lldb/source/Core/Disassembler.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 053e41f9d9aa2..74112faa44af1 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -751,8 +751,7 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, std::vector annotations; - for (size_t i = 0; i < var_list_sp->GetSize(); ++i) { - VariableSP var_sp = var_list_sp->GetVariableAtIndex(i); + for (const VariableSP &var_sp : *var_list_sp) { if (!var_sp) continue; From ca8510c41add8d34a03eef9f8fce57fd42759b79 Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Sun, 20 Jul 2025 16:52:45 -0400 Subject: [PATCH 25/39] Consolidated DumpLocation and DumpLocationWithOptions using default DIDumpOptions instead of having to introduce new function --- lldb/include/lldb/Expression/DWARFExpression.h | 4 ++-- lldb/source/Core/Disassembler.cpp | 2 +- lldb/source/Expression/DWARFExpression.cpp | 6 ------ 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/lldb/include/lldb/Expression/DWARFExpression.h b/lldb/include/lldb/Expression/DWARFExpression.h index 6eb421bc94e90..07c6325254907 100644 --- a/lldb/include/lldb/Expression/DWARFExpression.h +++ b/lldb/include/lldb/Expression/DWARFExpression.h @@ -157,8 +157,8 @@ class DWARFExpression { return data.GetByteSize() > 0; } - void DumpLocation(Stream *s, lldb::DescriptionLevel level, ABI *abi) const; - void DumpLocationWithOptions(Stream *s, lldb::DescriptionLevel level, ABI *abi, llvm::DIDumpOptions options) const; + void DumpLocation(Stream *s, lldb::DescriptionLevel level, ABI *abi, + llvm::DIDumpOptions options = {}) const; bool MatchesOperand(StackFrame &frame, const Instruction::Operand &op) const; diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 74112faa44af1..e14468ad5bcd2 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -773,7 +773,7 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, opts.ShowAddresses = false; opts.PrintRegisterOnly = true; // <-- important: suppress DW_OP_... annotations, etc. - entry.expr->DumpLocationWithOptions(&loc_str, eDescriptionLevelBrief, abi, opts); + entry.expr->DumpLocation(&loc_str, eDescriptionLevelBrief, abi, opts); // Only include if not empty. llvm::StringRef loc_clean = llvm::StringRef(loc_str.GetString()).trim(); diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp index d0d4278c98ae5..3db9cad1fd260 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.cpp @@ -66,12 +66,6 @@ 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, From ffefe5f08ba1f62c8ebea4e28f421e50aa193f66 Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Sun, 20 Jul 2025 17:54:23 -0400 Subject: [PATCH 26/39] Use `llvm::join` to simplify annotation output formatting --- lldb/source/Core/Disassembler.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index e14468ad5bcd2..cb3241da8b1a3 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -787,11 +787,7 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, if (!annotations.empty()) { ss.FillLastLineToColumn(annotation_column, ' '); ss.PutCString(" ; "); - for (size_t i = 0; i < annotations.size(); ++i) { - if (i > 0) - ss.PutCString(", "); - ss.PutCString(annotations[i]); - } + ss.PutCString(llvm::join(annotations, ", ")); } frame->ChangePC(original_pc); From dcddf1618c9841246a315f99fb44c9f5e8a2f9e5 Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Mon, 4 Aug 2025 16:01:05 -0400 Subject: [PATCH 27/39] Fix formatting to match LLVM style --- .../lldb/Expression/DWARFExpressionList.h | 5 +++-- lldb/source/Expression/DWARFExpression.cpp | 3 ++- lldb/source/Expression/DWARFExpressionList.cpp | 12 ++++++------ .../DebugInfo/DWARF/DWARFExpressionPrinter.cpp | 17 +++++++++-------- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/lldb/include/lldb/Expression/DWARFExpressionList.h b/lldb/include/lldb/Expression/DWARFExpressionList.h index 1bd762a9836e8..d303ad834b354 100644 --- a/lldb/include/lldb/Expression/DWARFExpressionList.h +++ b/lldb/include/lldb/Expression/DWARFExpressionList.h @@ -59,7 +59,7 @@ 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 @@ -69,7 +69,8 @@ class DWARFExpressionList { /// 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. + /// start; `load_addr` is the full runtime PC. On success, `expr` is + /// non-null. std::optional GetExpressionEntryAtAddress(lldb::addr_t func_load_addr, lldb::addr_t load_addr) const; diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp index c2c9f11642273..df56bcf5eb43e 100644 --- a/lldb/source/Expression/DWARFExpression.cpp +++ b/lldb/source/Expression/DWARFExpression.cpp @@ -67,7 +67,8 @@ void DWARFExpression::UpdateValue(uint64_t const_value, } void DWARFExpression::DumpLocation(Stream *s, lldb::DescriptionLevel level, - ABI *abi, llvm::DIDumpOptions options) const { + ABI *abi, + llvm::DIDumpOptions options) const { auto *MCRegInfo = abi ? &abi->GetMCRegisterInfo() : nullptr; auto GetRegName = [&MCRegInfo](uint64_t DwarfRegNum, bool IsEH) -> llvm::StringRef { diff --git a/lldb/source/Expression/DWARFExpressionList.cpp b/lldb/source/Expression/DWARFExpressionList.cpp index 88305fc17d1f0..ef7333518f008 100644 --- a/lldb/source/Expression/DWARFExpressionList.cpp +++ b/lldb/source/Expression/DWARFExpressionList.cpp @@ -6,7 +6,6 @@ // //===----------------------------------------------------------------------===// -#include "lldb/Core/AddressRange.h" #include "lldb/Expression/DWARFExpressionList.h" #include "lldb/Core/AddressRange.h" #include "lldb/Symbol/Function.h" @@ -57,7 +56,7 @@ bool DWARFExpressionList::ContainsAddress(lldb::addr_t func_load_addr, std::optional DWARFExpressionList::GetExpressionEntryAtAddress(lldb::addr_t func_load_addr, - lldb::addr_t load_addr) const { + lldb::addr_t load_addr) const { if (const DWARFExpression *always = GetAlwaysValidExpr()) { return DWARFExpressionEntry{std::nullopt, always}; } @@ -65,9 +64,10 @@ DWARFExpressionList::GetExpressionEntryAtAddress(lldb::addr_t func_load_addr, 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. + // Guard against underflow when translating a load address back into file + // space. if (load_addr < func_load_addr) - return std::nullopt; + return std::nullopt; // Guard against overflow. lldb::addr_t delta = load_addr - func_load_addr; @@ -75,10 +75,10 @@ DWARFExpressionList::GetExpressionEntryAtAddress(lldb::addr_t func_load_addr, 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()); + entry->GetRangeEnd() - entry->GetRangeBase()); return DWARFExpressionEntry{range_in_file, &entry->data}; } diff --git a/llvm/lib/DebugInfo/DWARF/DWARFExpressionPrinter.cpp b/llvm/lib/DebugInfo/DWARF/DWARFExpressionPrinter.cpp index 2e22f40094ea6..3622ff3a886a1 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFExpressionPrinter.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFExpressionPrinter.cpp @@ -68,23 +68,24 @@ static bool printOp(const DWARFExpression::Operation *Op, raw_ostream &OS, if (!DumpOpts.PrintRegisterOnly) { for (unsigned Operand = 0; Operand < Op->getDescription().Op.size(); - ++Operand) { + ++Operand) { unsigned Size = Op->getDescription().Op[Operand]; unsigned Signed = Size & DWARFExpression::Operation::SignBit; if (Size == DWARFExpression::Operation::SizeSubOpLEB) { - StringRef SubName = - SubOperationEncodingString(Op->getCode(), Op->getRawOperand(Operand)); + StringRef SubName = SubOperationEncodingString( + Op->getCode(), Op->getRawOperand(Operand)); assert(!SubName.empty() && "DW_OP SubOp has no name!"); OS << " " << SubName; } else if (Size == DWARFExpression::Operation::BaseTypeRef && U) { // For DW_OP_convert the operand may be 0 to indicate that conversion to - // the generic type should be done. The same holds for DW_OP_reinterpret, - // which is currently not supported. + // the generic type should be done. The same holds for + // DW_OP_reinterpret, which is currently not supported. if (Op->getCode() == DW_OP_convert && Op->getRawOperand(Operand) == 0) OS << " 0x0"; else - prettyPrintBaseTypeRef(U, OS, DumpOpts, Op->getRawOperands(), Operand); + prettyPrintBaseTypeRef(U, OS, DumpOpts, Op->getRawOperands(), + Operand); } else if (Size == DWARFExpression::Operation::WasmLocationArg) { assert(Operand == 1); switch (Op->getRawOperand(0)) { @@ -102,12 +103,12 @@ static bool printOp(const DWARFExpression::Operation *Op, raw_ostream &OS, uint64_t Offset = Op->getRawOperand(Operand); for (unsigned i = 0; i < Op->getRawOperand(Operand - 1); ++i) OS << format(" 0x%02x", - static_cast(Expr->getData()[Offset++])); + static_cast(Expr->getData()[Offset++])); } else { if (Signed) OS << format(" %+" PRId64, (int64_t)Op->getRawOperand(Operand)); else if (Op->getCode() != DW_OP_entry_value && - Op->getCode() != DW_OP_GNU_entry_value) + Op->getCode() != DW_OP_GNU_entry_value) OS << format(" 0x%" PRIx64, Op->getRawOperand(Operand)); } } From 7bac074eca37d88d64eefec5cdc86f5dd440b39c Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Mon, 4 Aug 2025 16:10:24 -0400 Subject: [PATCH 28/39] More formatting fixes --- .../rich-disassembler/TestRichDisassembler.py | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py b/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py index 0164402dfa587..7d29c3a9b6119 100644 --- a/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py +++ b/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py @@ -6,10 +6,9 @@ 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' - }) + 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) @@ -30,14 +29,4 @@ def test_d_original_example_O1(self): self.assertIn("i = ", disasm) # self.assertIn("DW_OP_reg", disasm) # self.assertIn("DW_OP_stack_value", disasm) - self.assertNotIn("", disasm) - - - - - - - - - - + self.assertNotIn("", disasm) \ No newline at end of file From 79c0a9e1e7da0f727c41d27c9c6ff8a28bb7d06f Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Mon, 4 Aug 2025 22:22:59 -0400 Subject: [PATCH 29/39] Fix formatting for code and tests --- lldb/source/Core/Disassembler.cpp | 31 ++++++++++--------- .../rich-disassembler/TestRichDisassembler.py | 3 +- .../rich-disassembler/d_original_example.c | 6 ++-- 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index d8924ea444ebd..0f68d38e07bed 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -732,7 +732,8 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, return; addr_t current_pc = m_address.GetLoadAddress(target_sp.get()); - addr_t original_pc = frame->GetFrameCodeAddress().GetLoadAddress(target_sp.get()); + addr_t original_pc = + frame->GetFrameCodeAddress().GetLoadAddress(target_sp.get()); if (!frame->ChangePC(current_pc)) return; @@ -744,7 +745,8 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, 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()); + func_load_addr = + sc.function->GetAddress().GetLoadAddress(target_sp.get()); // Only annotate if the current disassembly line is short enough // to keep annotations aligned past the desired annotation_column. @@ -763,22 +765,26 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, continue; // Handle std::optional. - if (auto entryOrErr = expr_list.GetExpressionEntryAtAddress(func_load_addr, current_pc)) { + 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())) { + (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. + opts.PrintRegisterOnly = + true; // <-- important: suppress DW_OP_... annotations, etc. entry.expr->DumpLocation(&loc_str, eDescriptionLevelBrief, abi, opts); - + // Only include if not empty. - llvm::StringRef loc_clean = llvm::StringRef(loc_str.GetString()).trim(); + llvm::StringRef loc_clean = + llvm::StringRef(loc_str.GetString()).trim(); if (!loc_clean.empty()) { annotations.push_back(llvm::formatv("{0} = {1}", name, loc_clean)); } @@ -808,7 +814,6 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, s->PutCString(ss.GetString()); } - bool Instruction::DumpEmulation(const ArchSpec &arch) { std::unique_ptr insn_emulator_up( EmulateInstruction::FindPlugin(arch, eInstructionTypeAny, nullptr)); @@ -820,9 +825,7 @@ bool Instruction::DumpEmulation(const ArchSpec &arch) { return false; } -bool Instruction::CanSetBreakpoint () { - return !HasDelaySlot(); -} +bool Instruction::CanSetBreakpoint() { return !HasDelaySlot(); } bool Instruction::HasDelaySlot() { // Default is false. @@ -1159,10 +1162,8 @@ void InstructionList::Append(lldb::InstructionSP &inst_sp) { m_instructions.push_back(inst_sp); } -uint32_t -InstructionList::GetIndexOfNextBranchInstruction(uint32_t start, - bool ignore_calls, - bool *found_calls) const { +uint32_t InstructionList::GetIndexOfNextBranchInstruction( + uint32_t start, bool ignore_calls, bool *found_calls) const { size_t num_instructions = m_instructions.size(); uint32_t next_branch = UINT32_MAX; diff --git a/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py b/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py index 7d29c3a9b6119..6b08eb3b43804 100644 --- a/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py +++ b/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py @@ -1,13 +1,14 @@ 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'} + dictionary={"C_SOURCES": "d_original_example.c", "CFLAGS_EXTRAS": "-g -O1"} ) exe = self.getBuildArtifact("a.out") target = self.dbg.CreateTarget(exe) diff --git a/lldb/test/API/functionalities/rich-disassembler/d_original_example.c b/lldb/test/API/functionalities/rich-disassembler/d_original_example.c index 4f245f518a182..1c864753a0220 100644 --- a/lldb/test/API/functionalities/rich-disassembler/d_original_example.c +++ b/lldb/test/API/functionalities/rich-disassembler/d_original_example.c @@ -1,7 +1,7 @@ #include int main(int argc, char **argv) { - for (int i = 1; i < argc; ++i) - puts(argv[i]); - return 0; + for (int i = 1; i < argc; ++i) + puts(argv[i]); + return 0; } From c7f1b30ba586db5cd1b3ff19a9a8f713ffdf4d9d Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Wed, 6 Aug 2025 20:41:37 -0400 Subject: [PATCH 30/39] Ported annotations from Instruction::Dump to Disassembler::PrintInstructions --- lldb/source/Core/Disassembler.cpp | 195 ++++++++++++++---------------- 1 file changed, 94 insertions(+), 101 deletions(-) diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 0f68d38e07bed..2edb2ee8fd9c4 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -379,6 +379,82 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, } } + // Add rich 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. + + auto annotate_variables = [&](Instruction &inst) -> std::vector { + std::vector annotations; + + StackFrame *frame = exe_ctx.GetFramePtr(); + TargetSP target_sp = exe_ctx.GetTargetSP(); + if (!frame || !target_sp) + return annotations; + + addr_t current_pc = inst.GetAddress().GetLoadAddress(target_sp.get()); + addr_t original_pc = frame->GetFrameCodeAddress().GetLoadAddress(target_sp.get()); + + if (!frame->ChangePC(current_pc)) + return annotations; + + VariableListSP var_list_sp = frame->GetInScopeVariableList(true); + if (!var_list_sp) + return annotations; + + SymbolContext sc = frame->GetSymbolContext(eSymbolContextFunction); + addr_t func_load_addr = sc.function + ? sc.function->GetAddress().GetLoadAddress(target_sp.get()) + : LLDB_INVALID_ADDRESS; + + for (const VariableSP &var_sp : *var_list_sp) { + if (!var_sp) + continue; + + const char *name = var_sp->GetName().AsCString(); + auto &expr_list = var_sp->LocationExpressionList(); + if (!expr_list.IsValid()) + continue; + + if (auto entryOrErr = expr_list.GetExpressionEntryAtAddress(func_load_addr, current_pc)) { + auto entry = *entryOrErr; + + 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; + + entry.expr->DumpLocation(&loc_str, eDescriptionLevelBrief, abi, opts); + + llvm::StringRef loc_clean = llvm::StringRef(loc_str.GetString()).trim(); + if (!loc_clean.empty()) { + annotations.push_back(llvm::formatv("{0} = {1}", name, loc_clean)); + } + } + } + } + + frame->ChangePC(original_pc); + return annotations; + }; + + previous_symbol = nullptr; SourceLine previous_line; for (size_t i = 0; i < num_instructions_found; ++i) { @@ -543,10 +619,25 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, const bool show_bytes = (options & eOptionShowBytes) != 0; const bool show_control_flow_kind = (options & eOptionShowControlFlowKind) != 0; - inst->Dump(&strm, max_opcode_byte_size, true, show_bytes, - show_control_flow_kind, &exe_ctx, &sc, &prev_sc, nullptr, - address_text_size); + + StreamString inst_line; + + inst->Dump(&inst_line, max_opcode_byte_size, true, show_bytes, + show_control_flow_kind, &exe_ctx, &sc, &prev_sc, nullptr, + address_text_size); + + std::vector annotations = annotate_variables(*inst); + if (!annotations.empty()) { + const size_t annotation_column = 100; + inst_line.FillLastLineToColumn(annotation_column, ' '); + inst_line.PutCString("; "); + inst_line.PutCString(llvm::join(annotations, ", ")); + } + + strm.PutCString(inst_line.GetString()); strm.EOL(); + + } else { break; } @@ -707,104 +798,6 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, ss.FillLastLineToColumn(opcode_pos + opcode_column_width, ' '); ss.PutCString(mnemonics); - // Add rich 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. - - const size_t annotation_column = 150; - - auto annotate_variables = [&]() { - StackFrame *frame = exe_ctx->GetFramePtr(); - TargetSP target_sp = exe_ctx->GetTargetSP(); - if (!frame || !target_sp) - return; - - 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)) - return; - - VariableListSP var_list_sp = frame->GetInScopeVariableList(true); - if (!var_list_sp) - return; - - 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()); - - // Only annotate if the current disassembly line is short enough - // to keep annotations aligned past the desired annotation_column. - if (ss.GetSizeOfLastLine() >= annotation_column) - return; - - std::vector annotations; - - for (const VariableSP &var_sp : *var_list_sp) { - 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. - 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->DumpLocation(&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(" ; "); - ss.PutCString(llvm::join(annotations, ", ")); - } - - frame->ChangePC(original_pc); - }; - - if (exe_ctx && exe_ctx->GetFramePtr()) { - annotate_variables(); - } - if (!m_comment.empty()) { ss.FillLastLineToColumn( opcode_pos + opcode_column_width + operand_column_width, ' '); From 3d19b0294eafb14a550acb99b9b72c37895016fa Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Fri, 8 Aug 2025 11:13:21 -0400 Subject: [PATCH 31/39] Added `--rich` option for disassembler annotations and updated SBFrame path** * Added a new `--rich` (`-R`) command-line option to `CommandObjectDisassemble` to enable rich disassembly annotations for the current invocation. * Plumbed a new `enable_rich_annotations` flag through: * `Disassembler::Disassemble` overloads * `Disassembler::PrintInstructions` * `Instruction::Dump` * `StackFrame::Disassemble` * Updated `StackFrame::Disassemble` to take an optional `bool enable_rich_annotations` (default `false`) so the SB API can request annotated output without CLI involvement. * Ensured annotations are only added when `enable_rich_annotations` is `true`; preserved caching for the non-rich path. * Modified `Options.td` to define the new `--rich` option. * Added/updated API test `TestRichDisassembler.py` to run `disassemble --rich -f` and check annotated output. * Kept default behavior unchanged so existing scripts and IDE integrations are unaffected. --- lldb/include/lldb/Core/Disassembler.h | 9 ++++--- lldb/include/lldb/Target/StackFrame.h | 3 ++- .../Commands/CommandObjectDisassemble.cpp | 7 ++++- .../Commands/CommandObjectDisassemble.h | 1 + lldb/source/Commands/Options.td | 2 ++ lldb/source/Core/Disassembler.cpp | 22 ++++++++++------ lldb/source/Target/StackFrame.cpp | 17 ++++++++---- .../rich-disassembler/TestRichDisassembler.py | 26 +++++++++---------- 8 files changed, 55 insertions(+), 32 deletions(-) diff --git a/lldb/include/lldb/Core/Disassembler.h b/lldb/include/lldb/Core/Disassembler.h index 21bacb14f9b25..94550bf9c9556 100644 --- a/lldb/include/lldb/Core/Disassembler.h +++ b/lldb/include/lldb/Core/Disassembler.h @@ -159,7 +159,8 @@ class Instruction { const SymbolContext *sym_ctx, const SymbolContext *prev_sym_ctx, const FormatEntity::Entry *disassembly_addr_format, - size_t max_address_text_size); + size_t max_address_text_size, + bool enable_rich_annotations = false); virtual bool DoesBranch() = 0; @@ -443,10 +444,10 @@ class Disassembler : public std::enable_shared_from_this, const ExecutionContext &exe_ctx, const Address &start, Limit limit, bool mixed_source_and_assembly, uint32_t num_mixed_context_lines, uint32_t options, - Stream &strm); + Stream &strm, bool enable_rich_annotations = false); static bool Disassemble(Debugger &debugger, const ArchSpec &arch, - StackFrame &frame, Stream &strm); + StackFrame &frame, Stream &strm, bool enable_rich_annotations = false); // Constructors and Destructors Disassembler(const ArchSpec &arch, const char *flavor); @@ -456,7 +457,7 @@ class Disassembler : public std::enable_shared_from_this, const ExecutionContext &exe_ctx, bool mixed_source_and_assembly, uint32_t num_mixed_context_lines, uint32_t options, - Stream &strm); + Stream &strm, bool enable_rich_annotations = false); size_t ParseInstructions(Target &target, Address address, Limit limit, Stream *error_strm_ptr, diff --git a/lldb/include/lldb/Target/StackFrame.h b/lldb/include/lldb/Target/StackFrame.h index 3f51c9a7f22f0..3a1490b5c575c 100644 --- a/lldb/include/lldb/Target/StackFrame.h +++ b/lldb/include/lldb/Target/StackFrame.h @@ -321,7 +321,8 @@ class StackFrame : public ExecutionContextScope, /// /// \return /// C string with the assembly instructions for this function. - const char *Disassemble(); + const char *Disassemble(bool enable_rich_annotations = false); + /// Print a description of this frame using the provided frame format. /// diff --git a/lldb/source/Commands/CommandObjectDisassemble.cpp b/lldb/source/Commands/CommandObjectDisassemble.cpp index 70e687e19ac6d..7b7ab3314f5d6 100644 --- a/lldb/source/Commands/CommandObjectDisassemble.cpp +++ b/lldb/source/Commands/CommandObjectDisassemble.cpp @@ -154,6 +154,10 @@ Status CommandObjectDisassemble::CommandOptions::SetOptionValue( } } break; + case 'R': // --rich + enable_rich_annotations = true; + break; + case '\x01': force = true; break; @@ -180,6 +184,7 @@ void CommandObjectDisassemble::CommandOptions::OptionParsingStarting( end_addr = LLDB_INVALID_ADDRESS; symbol_containing_addr = LLDB_INVALID_ADDRESS; raw = false; + enable_rich_annotations = false; plugin_name.clear(); Target *target = @@ -550,7 +555,7 @@ void CommandObjectDisassemble::DoExecute(Args &command, cpu_string, features_string, m_exe_ctx, cur_range.GetBaseAddress(), limit, m_options.show_mixed, m_options.show_mixed ? m_options.num_lines_context : 0, options, - result.GetOutputStream())) { + result.GetOutputStream(), /*enable_rich_annotations=*/m_options.enable_rich_annotations)) { result.SetStatus(eReturnStatusSuccessFinishResult); } else { if (m_options.symbol_containing_addr != LLDB_INVALID_ADDRESS) { diff --git a/lldb/source/Commands/CommandObjectDisassemble.h b/lldb/source/Commands/CommandObjectDisassemble.h index 4fbcd72d1c042..caaeabc15593d 100644 --- a/lldb/source/Commands/CommandObjectDisassemble.h +++ b/lldb/source/Commands/CommandObjectDisassemble.h @@ -78,6 +78,7 @@ class CommandObjectDisassemble : public CommandObjectParsed { // in SetOptionValue if anything the selects a location is set. lldb::addr_t symbol_containing_addr = 0; bool force = false; + bool enable_rich_annotations = false; }; CommandObjectDisassemble(CommandInterpreter &interpreter); diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index acb741081cac3..15e03a2a3056b 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -361,6 +361,8 @@ let Command = "disassemble" in { Desc<"Disassemble function containing this address.">; def disassemble_options_force : Option<"force", "\\x01">, Groups<[2,3,4,5,7]>, Desc<"Force disassembly of large functions.">; + def disassemble_options_rich : Option<"rich", "R">, + Desc<"Enable rich disassembly annotations for this invocation.">; } let Command = "diagnostics dump" in { diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 0f68d38e07bed..956db5eb8604c 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -175,7 +175,8 @@ bool Disassembler::Disassemble(Debugger &debugger, const ArchSpec &arch, const Address &address, Limit limit, bool mixed_source_and_assembly, uint32_t num_mixed_context_lines, - uint32_t options, Stream &strm) { + uint32_t options, Stream &strm, + bool enable_rich_annotations) { if (!exe_ctx.GetTargetPtr()) return false; @@ -191,8 +192,9 @@ bool Disassembler::Disassemble(Debugger &debugger, const ArchSpec &arch, return false; disasm_sp->PrintInstructions(debugger, arch, exe_ctx, - mixed_source_and_assembly, - num_mixed_context_lines, options, strm); + mixed_source_and_assembly, + num_mixed_context_lines, options, strm, + /*enable_rich_annotations=*/enable_rich_annotations); return true; } @@ -287,7 +289,8 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, const ExecutionContext &exe_ctx, bool mixed_source_and_assembly, uint32_t num_mixed_context_lines, - uint32_t options, Stream &strm) { + uint32_t options, Stream &strm, + bool enable_rich_annotations) { // We got some things disassembled... size_t num_instructions_found = GetInstructionList().GetSize(); @@ -545,7 +548,8 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, (options & eOptionShowControlFlowKind) != 0; inst->Dump(&strm, max_opcode_byte_size, true, show_bytes, show_control_flow_kind, &exe_ctx, &sc, &prev_sc, nullptr, - address_text_size); + address_text_size, + enable_rich_annotations); strm.EOL(); } else { break; @@ -554,7 +558,8 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, } bool Disassembler::Disassemble(Debugger &debugger, const ArchSpec &arch, - StackFrame &frame, Stream &strm) { + StackFrame &frame, Stream &strm, + bool enable_rich_annotations) { constexpr const char *plugin_name = nullptr; constexpr const char *flavor = nullptr; constexpr const char *cpu = nullptr; @@ -641,7 +646,8 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, const SymbolContext *sym_ctx, const SymbolContext *prev_sym_ctx, const FormatEntity::Entry *disassembly_addr_format, - size_t max_address_text_size) { + size_t max_address_text_size, + bool enable_rich_annotations) { size_t opcode_column_width = 7; const size_t operand_column_width = 25; @@ -801,7 +807,7 @@ void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size, frame->ChangePC(original_pc); }; - if (exe_ctx && exe_ctx->GetFramePtr()) { + if (enable_rich_annotations && exe_ctx && exe_ctx->GetFramePtr()) { annotate_variables(); } diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp index d97a814952186..67a38c2e405a4 100644 --- a/lldb/source/Target/StackFrame.cpp +++ b/lldb/source/Target/StackFrame.cpp @@ -262,15 +262,22 @@ bool StackFrame::ChangePC(addr_t pc) { return true; } -const char *StackFrame::Disassemble() { +const char *StackFrame::Disassemble(bool enable_rich_annotations) { std::lock_guard guard(m_mutex); - if (!m_disassembly.Empty()) - return m_disassembly.GetData(); + + // Keep the existing cache only for the plain (non-rich) path. + if (!enable_rich_annotations) { + if (!m_disassembly.Empty()) + return m_disassembly.GetData(); + } ExecutionContext exe_ctx(shared_from_this()); if (Target *target = exe_ctx.GetTargetPtr()) { - Disassembler::Disassemble(target->GetDebugger(), target->GetArchitecture(), - *this, m_disassembly); + Disassembler::Disassemble(target->GetDebugger(), + target->GetArchitecture(), + *this, + m_disassembly, + /*enable_rich_annotations=*/enable_rich_annotations); } return m_disassembly.Empty() ? nullptr : m_disassembly.GetData(); diff --git a/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py b/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py index 6b08eb3b43804..86fc46b41c6e5 100644 --- a/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py +++ b/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py @@ -1,11 +1,12 @@ from lldbsuite.test.lldbtest import * from lldbsuite.test.decorators import * - +import lldb class TestRichDisassembler(TestBase): def test_d_original_example_O1(self): """ - Tests disassembler output for d_original_example.c built with -O1. + Tests disassembler output for d_original_example.c built with -O1, + using the CLI with --rich to enable annotations. """ self.build( dictionary={"C_SOURCES": "d_original_example.c", "CFLAGS_EXTRAS": "-g -O1"} @@ -14,20 +15,19 @@ def test_d_original_example_O1(self): target = self.dbg.CreateTarget(exe) self.assertTrue(target) - breakpoint = target.BreakpointCreateByName("main") - self.assertGreater(breakpoint.GetNumLocations(), 0) + bp = target.BreakpointCreateByName("main") + self.assertGreater(bp.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) + # Run the CLI command and read output from self.res + self.runCmd("disassemble --rich -f", check=True) + out = self.res.GetOutput() + print(out) - 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("", disasm) \ No newline at end of file + self.assertIn("argc = ", out) + self.assertIn("argv = ", out) + self.assertIn("i = ", out) + self.assertNotIn("", out) From 6ca4bb602ca76011af7da9ffd77f977f9314d60c Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Fri, 8 Aug 2025 11:33:18 -0400 Subject: [PATCH 32/39] Formatting changes. --- lldb/include/lldb/Core/Disassembler.h | 20 +++++------ lldb/include/lldb/Target/StackFrame.h | 1 - .../Commands/CommandObjectDisassemble.cpp | 3 +- lldb/source/Core/Disassembler.cpp | 26 +++++++-------- lldb/source/Target/StackFrame.cpp | 33 +++++++++---------- .../rich-disassembler/TestRichDisassembler.py | 1 + 6 files changed, 39 insertions(+), 45 deletions(-) diff --git a/lldb/include/lldb/Core/Disassembler.h b/lldb/include/lldb/Core/Disassembler.h index 94550bf9c9556..a35c04bd3c574 100644 --- a/lldb/include/lldb/Core/Disassembler.h +++ b/lldb/include/lldb/Core/Disassembler.h @@ -170,7 +170,7 @@ class Instruction { virtual bool IsAuthenticated() = 0; - bool CanSetBreakpoint (); + bool CanSetBreakpoint(); virtual size_t Decode(const Disassembler &disassembler, const DataExtractor &data, @@ -283,7 +283,7 @@ std::function FetchImmOp(int64_t &imm); std::function MatchOpType(Instruction::Operand::Type type); -} +} // namespace OperandMatchers class InstructionList { public: @@ -315,20 +315,19 @@ class InstructionList { /// @param[in] ignore_calls /// It true, then fine the first branch instruction that isn't /// a function call (a branch that calls and returns to the next - /// instruction). If false, find the instruction index of any + /// instruction). If false, find the instruction index of any /// branch in the list. - /// + /// /// @param[out] found_calls - /// If non-null, this will be set to true if any calls were found in + /// If non-null, this will be set to true if any calls were found in /// extending the range. - /// + /// /// @return /// The instruction index of the first branch that is at or past - /// \a start. Returns UINT32_MAX if no matching branches are + /// \a start. Returns UINT32_MAX if no matching branches are /// found. //------------------------------------------------------------------ - uint32_t GetIndexOfNextBranchInstruction(uint32_t start, - bool ignore_calls, + uint32_t GetIndexOfNextBranchInstruction(uint32_t start, bool ignore_calls, bool *found_calls) const; uint32_t GetIndexOfInstructionAtLoadAddress(lldb::addr_t load_addr, @@ -447,7 +446,8 @@ class Disassembler : public std::enable_shared_from_this, Stream &strm, bool enable_rich_annotations = false); static bool Disassemble(Debugger &debugger, const ArchSpec &arch, - StackFrame &frame, Stream &strm, bool enable_rich_annotations = false); + StackFrame &frame, Stream &strm, + bool enable_rich_annotations = false); // Constructors and Destructors Disassembler(const ArchSpec &arch, const char *flavor); diff --git a/lldb/include/lldb/Target/StackFrame.h b/lldb/include/lldb/Target/StackFrame.h index 3a1490b5c575c..a970743f0fa5b 100644 --- a/lldb/include/lldb/Target/StackFrame.h +++ b/lldb/include/lldb/Target/StackFrame.h @@ -323,7 +323,6 @@ class StackFrame : public ExecutionContextScope, /// C string with the assembly instructions for this function. const char *Disassemble(bool enable_rich_annotations = false); - /// Print a description of this frame using the provided frame format. /// /// \param[out] strm diff --git a/lldb/source/Commands/CommandObjectDisassemble.cpp b/lldb/source/Commands/CommandObjectDisassemble.cpp index 7b7ab3314f5d6..5b7249be80a3e 100644 --- a/lldb/source/Commands/CommandObjectDisassemble.cpp +++ b/lldb/source/Commands/CommandObjectDisassemble.cpp @@ -555,7 +555,8 @@ void CommandObjectDisassemble::DoExecute(Args &command, cpu_string, features_string, m_exe_ctx, cur_range.GetBaseAddress(), limit, m_options.show_mixed, m_options.show_mixed ? m_options.num_lines_context : 0, options, - result.GetOutputStream(), /*enable_rich_annotations=*/m_options.enable_rich_annotations)) { + result.GetOutputStream(), + /*enable_rich_annotations=*/m_options.enable_rich_annotations)) { result.SetStatus(eReturnStatusSuccessFinishResult); } else { if (m_options.symbol_containing_addr != LLDB_INVALID_ADDRESS) { diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 956db5eb8604c..3e94fa11c4ad3 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -168,15 +168,12 @@ Disassembler::DisassembleBytes(const ArchSpec &arch, const char *plugin_name, return disasm_sp; } -bool Disassembler::Disassemble(Debugger &debugger, const ArchSpec &arch, - const char *plugin_name, const char *flavor, - const char *cpu, const char *features, - const ExecutionContext &exe_ctx, - const Address &address, Limit limit, - bool mixed_source_and_assembly, - uint32_t num_mixed_context_lines, - uint32_t options, Stream &strm, - bool enable_rich_annotations) { +bool Disassembler::Disassemble( + Debugger &debugger, const ArchSpec &arch, const char *plugin_name, + const char *flavor, const char *cpu, const char *features, + const ExecutionContext &exe_ctx, const Address &address, Limit limit, + bool mixed_source_and_assembly, uint32_t num_mixed_context_lines, + uint32_t options, Stream &strm, bool enable_rich_annotations) { if (!exe_ctx.GetTargetPtr()) return false; @@ -191,10 +188,10 @@ bool Disassembler::Disassemble(Debugger &debugger, const ArchSpec &arch, if (bytes_disassembled == 0) return false; - disasm_sp->PrintInstructions(debugger, arch, exe_ctx, - mixed_source_and_assembly, - num_mixed_context_lines, options, strm, - /*enable_rich_annotations=*/enable_rich_annotations); + disasm_sp->PrintInstructions( + debugger, arch, exe_ctx, mixed_source_and_assembly, + num_mixed_context_lines, options, strm, + /*enable_rich_annotations=*/enable_rich_annotations); return true; } @@ -548,8 +545,7 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, (options & eOptionShowControlFlowKind) != 0; inst->Dump(&strm, max_opcode_byte_size, true, show_bytes, show_control_flow_kind, &exe_ctx, &sc, &prev_sc, nullptr, - address_text_size, - enable_rich_annotations); + address_text_size, enable_rich_annotations); strm.EOL(); } else { break; diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp index 67a38c2e405a4..0aa93e635b866 100644 --- a/lldb/source/Target/StackFrame.cpp +++ b/lldb/source/Target/StackFrame.cpp @@ -273,11 +273,9 @@ const char *StackFrame::Disassemble(bool enable_rich_annotations) { ExecutionContext exe_ctx(shared_from_this()); if (Target *target = exe_ctx.GetTargetPtr()) { - Disassembler::Disassemble(target->GetDebugger(), - target->GetArchitecture(), - *this, - m_disassembly, - /*enable_rich_annotations=*/enable_rich_annotations); + Disassembler::Disassemble( + target->GetDebugger(), target->GetArchitecture(), *this, m_disassembly, + /*enable_rich_annotations=*/enable_rich_annotations); } return m_disassembly.Empty() ? nullptr : m_disassembly.GetData(); @@ -445,10 +443,10 @@ VariableList *StackFrame::GetVariableList(bool get_file_globals, const bool get_child_variables = true; const bool can_create = true; const bool stop_if_child_block_is_inlined_function = true; - frame_block->AppendBlockVariables(can_create, get_child_variables, - stop_if_child_block_is_inlined_function, - [](Variable *v) { return true; }, - m_variable_list_sp.get()); + frame_block->AppendBlockVariables( + can_create, get_child_variables, + stop_if_child_block_is_inlined_function, + [](Variable *v) { return true; }, m_variable_list_sp.get()); } } @@ -1232,10 +1230,12 @@ StackFrame::GetValueObjectForFrameVariable(const VariableSP &variable_sp, VariableList *var_list = GetVariableList(true, nullptr); if (var_list) { // Make sure the variable is a frame variable - const uint32_t var_idx = var_list->FindIndexForVariable(variable_sp.get()); + const uint32_t var_idx = + var_list->FindIndexForVariable(variable_sp.get()); const uint32_t num_variables = var_list->GetSize(); if (var_idx < num_variables) { - valobj_sp = m_variable_list_value_objects.GetValueObjectAtIndex(var_idx); + valobj_sp = + m_variable_list_value_objects.GetValueObjectAtIndex(var_idx); if (!valobj_sp) { if (m_variable_list_value_objects.GetSize() < num_variables) m_variable_list_value_objects.Resize(num_variables); @@ -1769,11 +1769,9 @@ lldb::ValueObjectSP DoGuessValueAt(StackFrame &frame, ConstString reg, if (clobbered_reg_matcher(operands[0])) { origin_operand = &operands[1]; - } - else if (clobbered_reg_matcher(operands[1])) { + } else if (clobbered_reg_matcher(operands[1])) { origin_operand = &operands[0]; - } - else { + } else { continue; } @@ -1799,8 +1797,7 @@ lldb::ValueObjectSP DoGuessValueAt(StackFrame &frame, ConstString reg, if (!source_path) { continue; } - source_path = - GetValueForDereferincingOffset(frame, source_path, offset); + source_path = GetValueForDereferincingOffset(frame, source_path, offset); } if (source_path) { @@ -1810,7 +1807,7 @@ lldb::ValueObjectSP DoGuessValueAt(StackFrame &frame, ConstString reg, return ValueObjectSP(); } -} +} // namespace lldb::ValueObjectSP StackFrame::GuessValueForRegisterAndOffset(ConstString reg, int64_t offset) { diff --git a/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py b/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py index 86fc46b41c6e5..d54c305f30761 100644 --- a/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py +++ b/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py @@ -2,6 +2,7 @@ from lldbsuite.test.decorators import * import lldb + class TestRichDisassembler(TestBase): def test_d_original_example_O1(self): """ From b1f13e7078e805c9807f83ebed2e8b9e208df9d4 Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Sat, 9 Aug 2025 01:37:15 -0400 Subject: [PATCH 33/39] Redo Workflow tests --- .../functionalities/rich-disassembler/TestRichDisassembler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py b/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py index d54c305f30761..4bad4a4f4986b 100644 --- a/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py +++ b/lldb/test/API/functionalities/rich-disassembler/TestRichDisassembler.py @@ -7,7 +7,7 @@ class TestRichDisassembler(TestBase): def test_d_original_example_O1(self): """ Tests disassembler output for d_original_example.c built with -O1, - using the CLI with --rich to enable annotations. + using the CLI with --rich for enabled annotations. """ self.build( dictionary={"C_SOURCES": "d_original_example.c", "CFLAGS_EXTRAS": "-g -O1"} From 10fddc43855bdf154cc2cf24ced0452b03debced Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Sat, 9 Aug 2025 15:39:07 -0400 Subject: [PATCH 34/39] Added basic stateful variable location annotations to disassembly output This change introduces a simple live-variable tracking system for annotated disassembly. While iterating over instructions, we now maintain an unordered_map keyed by `lldb::user_id_t` to remember each in-scope variable's last known location string. For each instruction: * If a variable is new, print `name = location` and add it to the map. * If a variable's location has changed, print the updated mapping. * If a previously tracked variable is no longer found, print `name = ` and remove it. This produces concise, stateful annotations that only update when needed, reducing noise in the disassembly while still showing variable lifetimes. --- lldb/source/Core/Disassembler.cpp | 111 ++++++++++++++++++++++-------- 1 file changed, 83 insertions(+), 28 deletions(-) diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index f90c2f3b21115..315cd54c70e1d 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -49,6 +49,7 @@ #include #include +#include #include #include @@ -394,67 +395,121 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, // 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 ) + bool seen_this_inst = false; + }; + + // Track live variables across instructions (keyed by stable LLDB user_id_t) + std::unordered_map live_vars; + + // Stateful annotator: updates live_vars and returns only what should be printed for THIS instruction. auto annotate_variables = [&](Instruction &inst) -> std::vector { - std::vector annotations; + std::vector events; StackFrame *frame = exe_ctx.GetFramePtr(); TargetSP target_sp = exe_ctx.GetTargetSP(); - if (!frame || !target_sp) - return annotations; + ProcessSP process_sp = exe_ctx.GetProcessSP(); + if (!frame || !target_sp || !process_sp) + return events; + + // Reset "seen" flags for this instruction + for (auto &kv : live_vars) + kv.second.seen_this_inst = false; addr_t current_pc = inst.GetAddress().GetLoadAddress(target_sp.get()); addr_t original_pc = frame->GetFrameCodeAddress().GetLoadAddress(target_sp.get()); + // We temporarily move the frame PC so variable locations resolve at this inst if (!frame->ChangePC(current_pc)) - return annotations; + return events; VariableListSP var_list_sp = frame->GetInScopeVariableList(true); - if (!var_list_sp) - return annotations; + if (!var_list_sp) { + // No variables in scope: everything previously live becomes + for (auto it = live_vars.begin(); it != live_vars.end(); ) { + events.push_back(llvm::formatv("{0} = ", it->second.name).str()); + it = live_vars.erase(it); + } + frame->ChangePC(original_pc); + return events; + } SymbolContext sc = frame->GetSymbolContext(eSymbolContextFunction); addr_t func_load_addr = sc.function - ? sc.function->GetAddress().GetLoadAddress(target_sp.get()) - : LLDB_INVALID_ADDRESS; + ? sc.function->GetAddress().GetLoadAddress(target_sp.get()) + : LLDB_INVALID_ADDRESS; + // Walk all in-scope variables and try to resolve a location for (const VariableSP &var_sp : *var_list_sp) { if (!var_sp) continue; - const char *name = var_sp->GetName().AsCString(); + const auto var_id = var_sp->GetID(); // lldb::user_id_t – stable key + const char *name_cstr = var_sp->GetName().AsCString(); + llvm::StringRef name = name_cstr ? name_cstr : ""; + auto &expr_list = var_sp->LocationExpressionList(); if (!expr_list.IsValid()) continue; - if (auto entryOrErr = expr_list.GetExpressionEntryAtAddress(func_load_addr, current_pc)) { - auto entry = *entryOrErr; + // Try to get the expression entry for this PC + auto entry_or_err = expr_list.GetExpressionEntryAtAddress(func_load_addr, current_pc); + if (!entry_or_err) + continue; - if (!entry.file_range || - entry.file_range->ContainsFileAddress( - (current_pc - func_load_addr) + expr_list.GetFuncFileAddress())) { + auto entry = *entry_or_err; - StreamString loc_str; - ABI *abi = exe_ctx.GetProcessPtr()->GetABI().get(); - llvm::DIDumpOptions opts; - opts.ShowAddresses = false; - opts.PrintRegisterOnly = true; + // Check range if present + if (entry.file_range && + !entry.file_range->ContainsFileAddress( + (current_pc - func_load_addr) + expr_list.GetFuncFileAddress())) + continue; - entry.expr->DumpLocation(&loc_str, eDescriptionLevelBrief, abi, opts); + // Render a compact location string + ABI *abi = process_sp->GetABI().get(); + llvm::DIDumpOptions opts; + opts.ShowAddresses = false; + opts.PrintRegisterOnly = true; - llvm::StringRef loc_clean = llvm::StringRef(loc_str.GetString()).trim(); - if (!loc_clean.empty()) { - annotations.push_back(llvm::formatv("{0} = {1}", name, loc_clean)); - } + StreamString loc_str; + entry.expr->DumpLocation(&loc_str, eDescriptionLevelBrief, abi, opts); + llvm::StringRef loc_clean = llvm::StringRef(loc_str.GetString()).trim(); + if (loc_clean.empty()) + continue; + + // Update map + decide if we print + auto it = live_vars.find(var_id); + if (it == live_vars.end()) { + // New var → print + live_vars.emplace(var_id, VarState{std::string(name), loc_clean.str(), true}); + events.push_back(llvm::formatv("{0} = {1}", name, loc_clean).str()); + } else { + it->second.seen_this_inst = true; + if (it->second.last_loc != loc_clean) { + it->second.last_loc = loc_clean.str(); + events.push_back(llvm::formatv("{0} = {1}", it->second.name, loc_clean).str()); } } } + // Anything previously live that we didn't see a location for at this inst is now + for (auto it = live_vars.begin(); it != live_vars.end(); ) { + if (!it->second.seen_this_inst) { + events.push_back(llvm::formatv("{0} = ", it->second.name).str()); + it = live_vars.erase(it); + } else { + ++it; + } + } + + // Restore PC frame->ChangePC(original_pc); - return annotations; + return events; }; - previous_symbol = nullptr; SourceLine previous_line; for (size_t i = 0; i < num_instructions_found; ++i) { @@ -626,7 +681,7 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, show_control_flow_kind, &exe_ctx, &sc, &prev_sc, nullptr, address_text_size); - if(enable_rich_annotations){ + if (enable_rich_annotations){ std::vector annotations = annotate_variables(*inst); if (!annotations.empty()) { const size_t annotation_column = 100; From b7848681c1d827b5068e002c95697116a140dd8e Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Sun, 10 Aug 2025 16:25:11 -0400 Subject: [PATCH 35/39] Formatting changes. --- lldb/source/Core/Disassembler.cpp | 44 ++++++++++++++++++------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 315cd54c70e1d..8894ebe94264d 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -397,15 +397,16 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, // enhances source-level debugging. struct VarState { - std::string name; // display name - std::string last_loc; // last printed location (empty means ) + std::string name; // display name + std::string last_loc; // last printed location (empty means ) bool seen_this_inst = false; }; // Track live variables across instructions (keyed by stable LLDB user_id_t) std::unordered_map live_vars; - // Stateful annotator: updates live_vars and returns only what should be printed for THIS instruction. + // Stateful annotator: updates live_vars and returns only what should be + // printed for THIS instruction. auto annotate_variables = [&](Instruction &inst) -> std::vector { std::vector events; @@ -420,16 +421,18 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, kv.second.seen_this_inst = false; addr_t current_pc = inst.GetAddress().GetLoadAddress(target_sp.get()); - addr_t original_pc = frame->GetFrameCodeAddress().GetLoadAddress(target_sp.get()); + addr_t original_pc = + frame->GetFrameCodeAddress().GetLoadAddress(target_sp.get()); - // We temporarily move the frame PC so variable locations resolve at this inst + // We temporarily move the frame PC so variable locations resolve at this + // inst if (!frame->ChangePC(current_pc)) return events; VariableListSP var_list_sp = frame->GetInScopeVariableList(true); if (!var_list_sp) { // No variables in scope: everything previously live becomes - for (auto it = live_vars.begin(); it != live_vars.end(); ) { + for (auto it = live_vars.begin(); it != live_vars.end();) { events.push_back(llvm::formatv("{0} = ", it->second.name).str()); it = live_vars.erase(it); } @@ -438,16 +441,16 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, } SymbolContext sc = frame->GetSymbolContext(eSymbolContextFunction); - addr_t func_load_addr = sc.function - ? sc.function->GetAddress().GetLoadAddress(target_sp.get()) - : LLDB_INVALID_ADDRESS; + addr_t func_load_addr = + sc.function ? sc.function->GetAddress().GetLoadAddress(target_sp.get()) + : LLDB_INVALID_ADDRESS; // Walk all in-scope variables and try to resolve a location for (const VariableSP &var_sp : *var_list_sp) { if (!var_sp) continue; - const auto var_id = var_sp->GetID(); // lldb::user_id_t – stable key + const auto var_id = var_sp->GetID(); // lldb::user_id_t – stable key const char *name_cstr = var_sp->GetName().AsCString(); llvm::StringRef name = name_cstr ? name_cstr : ""; @@ -456,7 +459,8 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, continue; // Try to get the expression entry for this PC - auto entry_or_err = expr_list.GetExpressionEntryAtAddress(func_load_addr, current_pc); + auto entry_or_err = + expr_list.GetExpressionEntryAtAddress(func_load_addr, current_pc); if (!entry_or_err) continue; @@ -484,19 +488,22 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, auto it = live_vars.find(var_id); if (it == live_vars.end()) { // New var → print - live_vars.emplace(var_id, VarState{std::string(name), loc_clean.str(), true}); + live_vars.emplace(var_id, + VarState{std::string(name), loc_clean.str(), true}); events.push_back(llvm::formatv("{0} = {1}", name, loc_clean).str()); } else { it->second.seen_this_inst = true; if (it->second.last_loc != loc_clean) { it->second.last_loc = loc_clean.str(); - events.push_back(llvm::formatv("{0} = {1}", it->second.name, loc_clean).str()); + events.push_back( + llvm::formatv("{0} = {1}", it->second.name, loc_clean).str()); } } } - // Anything previously live that we didn't see a location for at this inst is now - for (auto it = live_vars.begin(); it != live_vars.end(); ) { + // Anything previously live that we didn't see a location for at this inst + // is now + for (auto it = live_vars.begin(); it != live_vars.end();) { if (!it->second.seen_this_inst) { events.push_back(llvm::formatv("{0} = ", it->second.name).str()); it = live_vars.erase(it); @@ -678,10 +685,10 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, StreamString inst_line; inst->Dump(&inst_line, max_opcode_byte_size, true, show_bytes, - show_control_flow_kind, &exe_ctx, &sc, &prev_sc, nullptr, - address_text_size); + show_control_flow_kind, &exe_ctx, &sc, &prev_sc, nullptr, + address_text_size); - if (enable_rich_annotations){ + if (enable_rich_annotations) { std::vector annotations = annotate_variables(*inst); if (!annotations.empty()) { const size_t annotation_column = 100; @@ -694,7 +701,6 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, strm.PutCString(inst_line.GetString()); strm.EOL(); - } else { break; } From cb0cd3abd5aa591d1062e865e6136a6ad23a7810 Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Mon, 11 Aug 2025 19:28:16 -0400 Subject: [PATCH 36/39] Moved rich annotations flag into Disassembler options This change refactors how the `--rich` flag is handled, based on code review feedback. - Removed the `enable_rich_annotations` boolean from the API signatures of: - Disassembler::Disassemble(...) - Disassembler::PrintInstructions(...) - StackFrame::Disassemble(...) - Added a new Disassembler::Option enum value: eOptionRichAnnotations. - The `--rich` CLI flag now sets the new option bit in CommandObjectDisassemble::DoExecute: options |= Disassembler::eOptionRichAnnotations; - Disassembler::PrintInstructions checks the bit to determine whether to enable rich annotations: const bool enable_rich = (options & eOptionRichAnnotations) != 0; The SB API remains unchanged and defaults to non-rich output. Tested via the existing test using `disassemble --rich -f`. --- lldb/include/lldb/Core/Disassembler.h | 8 ++--- lldb/include/lldb/Target/StackFrame.h | 2 +- .../Commands/CommandObjectDisassemble.cpp | 6 ++-- lldb/source/Core/Disassembler.cpp | 29 +++++++++---------- lldb/source/Target/StackFrame.cpp | 14 ++++----- 5 files changed, 28 insertions(+), 31 deletions(-) diff --git a/lldb/include/lldb/Core/Disassembler.h b/lldb/include/lldb/Core/Disassembler.h index fc83e747bb2b7..00a04678e3202 100644 --- a/lldb/include/lldb/Core/Disassembler.h +++ b/lldb/include/lldb/Core/Disassembler.h @@ -398,6 +398,7 @@ class Disassembler : public std::enable_shared_from_this, eOptionMarkPCAddress = (1u << 3), // Mark the disassembly line the contains the PC eOptionShowControlFlowKind = (1u << 4), + eOptionRichAnnotations = (1u << 5), }; enum HexImmediateStyle { @@ -444,11 +445,10 @@ class Disassembler : public std::enable_shared_from_this, const ExecutionContext &exe_ctx, const Address &start, Limit limit, bool mixed_source_and_assembly, uint32_t num_mixed_context_lines, uint32_t options, - Stream &strm, bool enable_rich_annotations = false); + Stream &strm); static bool Disassemble(Debugger &debugger, const ArchSpec &arch, - StackFrame &frame, Stream &strm, - bool enable_rich_annotations = false); + StackFrame &frame, Stream &strm); // Constructors and Destructors Disassembler(const ArchSpec &arch, const char *flavor); @@ -458,7 +458,7 @@ class Disassembler : public std::enable_shared_from_this, const ExecutionContext &exe_ctx, bool mixed_source_and_assembly, uint32_t num_mixed_context_lines, uint32_t options, - Stream &strm, bool enable_rich_annotations = false); + Stream &strm); size_t ParseInstructions(Target &target, Address address, Limit limit, Stream *error_strm_ptr, diff --git a/lldb/include/lldb/Target/StackFrame.h b/lldb/include/lldb/Target/StackFrame.h index a970743f0fa5b..3f51c9a7f22f0 100644 --- a/lldb/include/lldb/Target/StackFrame.h +++ b/lldb/include/lldb/Target/StackFrame.h @@ -321,7 +321,7 @@ class StackFrame : public ExecutionContextScope, /// /// \return /// C string with the assembly instructions for this function. - const char *Disassemble(bool enable_rich_annotations = false); + const char *Disassemble(); /// Print a description of this frame using the provided frame format. /// diff --git a/lldb/source/Commands/CommandObjectDisassemble.cpp b/lldb/source/Commands/CommandObjectDisassemble.cpp index 5b7249be80a3e..ebd77123f60f9 100644 --- a/lldb/source/Commands/CommandObjectDisassemble.cpp +++ b/lldb/source/Commands/CommandObjectDisassemble.cpp @@ -533,6 +533,9 @@ void CommandObjectDisassemble::DoExecute(Args &command, if (m_options.raw) options |= Disassembler::eOptionRawOuput; + if (m_options.enable_rich_annotations) + options |= Disassembler::eOptionRichAnnotations; + llvm::Expected> ranges = GetRangesForSelectedMode(result); if (!ranges) { @@ -555,8 +558,7 @@ void CommandObjectDisassemble::DoExecute(Args &command, cpu_string, features_string, m_exe_ctx, cur_range.GetBaseAddress(), limit, m_options.show_mixed, m_options.show_mixed ? m_options.num_lines_context : 0, options, - result.GetOutputStream(), - /*enable_rich_annotations=*/m_options.enable_rich_annotations)) { + result.GetOutputStream())) { result.SetStatus(eReturnStatusSuccessFinishResult); } else { if (m_options.symbol_containing_addr != LLDB_INVALID_ADDRESS) { diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 8894ebe94264d..70b0ccc2ffc7c 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -169,12 +169,14 @@ Disassembler::DisassembleBytes(const ArchSpec &arch, const char *plugin_name, return disasm_sp; } -bool Disassembler::Disassemble( - Debugger &debugger, const ArchSpec &arch, const char *plugin_name, - const char *flavor, const char *cpu, const char *features, - const ExecutionContext &exe_ctx, const Address &address, Limit limit, - bool mixed_source_and_assembly, uint32_t num_mixed_context_lines, - uint32_t options, Stream &strm, bool enable_rich_annotations) { +bool Disassembler::Disassemble(Debugger &debugger, const ArchSpec &arch, + const char *plugin_name, const char *flavor, + const char *cpu, const char *features, + const ExecutionContext &exe_ctx, + const Address &address, Limit limit, + bool mixed_source_and_assembly, + uint32_t num_mixed_context_lines, + uint32_t options, Stream &strm) { if (!exe_ctx.GetTargetPtr()) return false; @@ -189,10 +191,9 @@ bool Disassembler::Disassemble( if (bytes_disassembled == 0) return false; - disasm_sp->PrintInstructions( - debugger, arch, exe_ctx, mixed_source_and_assembly, - num_mixed_context_lines, options, strm, - /*enable_rich_annotations=*/enable_rich_annotations); + disasm_sp->PrintInstructions(debugger, arch, exe_ctx, + mixed_source_and_assembly, + num_mixed_context_lines, options, strm); return true; } @@ -287,8 +288,7 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, const ExecutionContext &exe_ctx, bool mixed_source_and_assembly, uint32_t num_mixed_context_lines, - uint32_t options, Stream &strm, - bool enable_rich_annotations) { + uint32_t options, Stream &strm) { // We got some things disassembled... size_t num_instructions_found = GetInstructionList().GetSize(); @@ -688,7 +688,7 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, show_control_flow_kind, &exe_ctx, &sc, &prev_sc, nullptr, address_text_size); - if (enable_rich_annotations) { + if (options & eOptionRichAnnotations) { std::vector annotations = annotate_variables(*inst); if (!annotations.empty()) { const size_t annotation_column = 100; @@ -708,8 +708,7 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, } bool Disassembler::Disassemble(Debugger &debugger, const ArchSpec &arch, - StackFrame &frame, Stream &strm, - bool enable_rich_annotations) { + StackFrame &frame, Stream &strm) { constexpr const char *plugin_name = nullptr; constexpr const char *flavor = nullptr; constexpr const char *cpu = nullptr; diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp index 0aa93e635b866..c84e99c17c72e 100644 --- a/lldb/source/Target/StackFrame.cpp +++ b/lldb/source/Target/StackFrame.cpp @@ -262,20 +262,16 @@ bool StackFrame::ChangePC(addr_t pc) { return true; } -const char *StackFrame::Disassemble(bool enable_rich_annotations) { +const char *StackFrame::Disassemble() { std::lock_guard guard(m_mutex); - // Keep the existing cache only for the plain (non-rich) path. - if (!enable_rich_annotations) { - if (!m_disassembly.Empty()) - return m_disassembly.GetData(); - } + if (!m_disassembly.Empty()) + return m_disassembly.GetData(); ExecutionContext exe_ctx(shared_from_this()); if (Target *target = exe_ctx.GetTargetPtr()) { - Disassembler::Disassemble( - target->GetDebugger(), target->GetArchitecture(), *this, m_disassembly, - /*enable_rich_annotations=*/enable_rich_annotations); + Disassembler::Disassemble(target->GetDebugger(), target->GetArchitecture(), + *this, m_disassembly); } return m_disassembly.Empty() ? nullptr : m_disassembly.GetData(); From fbd4e65fad4f40733ba43a461f7f1f7af9c0ffe9 Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Mon, 11 Aug 2025 19:53:47 -0400 Subject: [PATCH 37/39] Switched to llvm::SmallDenseMap for live_vars in PrintInstructions Address code review feedback suggesting the use of LLVM's DenseMap family over std::unordered_map for consistency and potential performance benefits within LLDB. Replaced: std::unordered_map with: llvm::SmallDenseMap The small buffer size of 8 is a heuristic for typical numbers of live variables in scope, reducing allocations for common cases. --- lldb/source/Core/Disassembler.cpp | 46 ++++++++++++++++--------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 70b0ccc2ffc7c..4e3cd0668efaf 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -44,12 +44,13 @@ #include "lldb/lldb-private-enumerations.h" #include "lldb/lldb-private-interfaces.h" #include "lldb/lldb-private-types.h" +#include "llvm/ADT/DenseMap.h" #include "llvm/Support/Compiler.h" #include "llvm/TargetParser/Triple.h" + #include #include -#include #include #include @@ -403,7 +404,7 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, }; // Track live variables across instructions (keyed by stable LLDB user_id_t) - std::unordered_map live_vars; + llvm::SmallDenseMap live_vars; // 8 is a good small-buffer guess // Stateful annotator: updates live_vars and returns only what should be // printed for THIS instruction. @@ -432,9 +433,10 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, VariableListSP var_list_sp = frame->GetInScopeVariableList(true); if (!var_list_sp) { // No variables in scope: everything previously live becomes - for (auto it = live_vars.begin(); it != live_vars.end();) { - events.push_back(llvm::formatv("{0} = ", it->second.name).str()); - it = live_vars.erase(it); + for (auto I = live_vars.begin(), E = live_vars.end(); I != E; ) { + auto Cur = I++; + events.push_back(llvm::formatv("{0} = ", Cur->second.name).str()); + live_vars.erase(Cur); } frame->ChangePC(original_pc); return events; @@ -484,31 +486,31 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, if (loc_clean.empty()) continue; - // Update map + decide if we print - auto it = live_vars.find(var_id); - if (it == live_vars.end()) { - // New var → print - live_vars.emplace(var_id, - VarState{std::string(name), loc_clean.str(), true}); + auto insert_res = live_vars.insert({var_id, + VarState{std::string(name), + loc_clean.str(), + /*seen_this_inst*/ true}}); + if (insert_res.second) { + // Newly inserted → print events.push_back(llvm::formatv("{0} = {1}", name, loc_clean).str()); } else { - it->second.seen_this_inst = true; - if (it->second.last_loc != loc_clean) { - it->second.last_loc = loc_clean.str(); - events.push_back( - llvm::formatv("{0} = {1}", it->second.name, loc_clean).str()); + // Already present + VarState &vs = insert_res.first->second; + vs.seen_this_inst = true; + if (vs.last_loc != loc_clean) { + vs.last_loc = loc_clean.str(); + events.push_back(llvm::formatv("{0} = {1}", vs.name, loc_clean).str()); } } } // Anything previously live that we didn't see a location for at this inst // is now - for (auto it = live_vars.begin(); it != live_vars.end();) { - if (!it->second.seen_this_inst) { - events.push_back(llvm::formatv("{0} = ", it->second.name).str()); - it = live_vars.erase(it); - } else { - ++it; + 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} = ", Cur->second.name).str()); + live_vars.erase(Cur); } } From 77fa1ed32148ff474719af9dd37109e4df23b115 Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Mon, 11 Aug 2025 20:11:49 -0400 Subject: [PATCH 38/39] Fixed code style to match LLVM convention --- .../Commands/CommandObjectDisassemble.cpp | 2 +- lldb/source/Core/Disassembler.cpp | 32 +++++++++---------- .../rich-disassembler/Makefile | 7 +--- 3 files changed, 18 insertions(+), 23 deletions(-) diff --git a/lldb/source/Commands/CommandObjectDisassemble.cpp b/lldb/source/Commands/CommandObjectDisassemble.cpp index ebd77123f60f9..35315dc144669 100644 --- a/lldb/source/Commands/CommandObjectDisassemble.cpp +++ b/lldb/source/Commands/CommandObjectDisassemble.cpp @@ -154,7 +154,7 @@ Status CommandObjectDisassemble::CommandOptions::SetOptionValue( } } break; - case 'R': // --rich + case 'R': //< --rich enable_rich_annotations = true; break; diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 4e3cd0668efaf..64c52913574a3 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -398,13 +398,13 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, // enhances source-level debugging. struct VarState { - std::string name; // display name - std::string last_loc; // last printed location (empty means ) + std::string name; //< Display name. + std::string last_loc; //< Last printed location (empty means ). bool seen_this_inst = false; }; - // Track live variables across instructions (keyed by stable LLDB user_id_t) - llvm::SmallDenseMap live_vars; // 8 is a good small-buffer guess + // Track live variables across instructions (keyed by stable LLDB user_id_t. 8 is a good small-buffer guess + llvm::SmallDenseMap live_vars; // Stateful annotator: updates live_vars and returns only what should be // printed for THIS instruction. @@ -417,7 +417,7 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, if (!frame || !target_sp || !process_sp) return events; - // Reset "seen" flags for this instruction + // Reset "seen" flags for this instruction. for (auto &kv : live_vars) kv.second.seen_this_inst = false; @@ -426,13 +426,13 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, frame->GetFrameCodeAddress().GetLoadAddress(target_sp.get()); // We temporarily move the frame PC so variable locations resolve at this - // inst + // instruction. if (!frame->ChangePC(current_pc)) return events; VariableListSP var_list_sp = frame->GetInScopeVariableList(true); if (!var_list_sp) { - // No variables in scope: everything previously live becomes + // No variables in scope: everything previously live becomes . for (auto I = live_vars.begin(), E = live_vars.end(); I != E; ) { auto Cur = I++; events.push_back(llvm::formatv("{0} = ", Cur->second.name).str()); @@ -447,12 +447,13 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, sc.function ? sc.function->GetAddress().GetLoadAddress(target_sp.get()) : LLDB_INVALID_ADDRESS; - // Walk all in-scope variables and try to resolve a location + // Walk all in-scope variables and try to resolve a location. for (const VariableSP &var_sp : *var_list_sp) { if (!var_sp) continue; - const auto var_id = var_sp->GetID(); // lldb::user_id_t – stable key + // The var_id is a lldb::user_id_t – stable key. + const auto var_id = var_sp->GetID(); const char *name_cstr = var_sp->GetName().AsCString(); llvm::StringRef name = name_cstr ? name_cstr : ""; @@ -460,7 +461,6 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, if (!expr_list.IsValid()) continue; - // Try to get the expression entry for this PC auto entry_or_err = expr_list.GetExpressionEntryAtAddress(func_load_addr, current_pc); if (!entry_or_err) @@ -468,13 +468,13 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, auto entry = *entry_or_err; - // Check range if present + // Check range if present. if (entry.file_range && !entry.file_range->ContainsFileAddress( (current_pc - func_load_addr) + expr_list.GetFuncFileAddress())) continue; - // Render a compact location string + // Render a compact location string. ABI *abi = process_sp->GetABI().get(); llvm::DIDumpOptions opts; opts.ShowAddresses = false; @@ -491,10 +491,10 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, loc_clean.str(), /*seen_this_inst*/ true}}); if (insert_res.second) { - // Newly inserted → print + // Newly inserted → print. events.push_back(llvm::formatv("{0} = {1}", name, loc_clean).str()); } else { - // Already present + // Already present. VarState &vs = insert_res.first->second; vs.seen_this_inst = true; if (vs.last_loc != loc_clean) { @@ -505,7 +505,7 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, } // Anything previously live that we didn't see a location for at this inst - // is now + // is now . for (auto I = live_vars.begin(), E = live_vars.end(); I != E; ) { auto Cur = I++; if (!Cur->second.seen_this_inst) { @@ -514,7 +514,7 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, } } - // Restore PC + // Restore PC. frame->ChangePC(original_pc); return events; }; diff --git a/lldb/test/API/functionalities/rich-disassembler/Makefile b/lldb/test/API/functionalities/rich-disassembler/Makefile index ae3330e632a0e..be53a34f7e265 100644 --- a/lldb/test/API/functionalities/rich-disassembler/Makefile +++ b/lldb/test/API/functionalities/rich-disassembler/Makefile @@ -1,6 +1 @@ - -# 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 +include Makefile.rules \ No newline at end of file From 7069b6a8ae67aa0e22d10c1a2433269e3a399dc1 Mon Sep 17 00:00:00 2001 From: ultimateforce21 Date: Mon, 11 Aug 2025 20:23:34 -0400 Subject: [PATCH 39/39] Formatting changes. --- lldb/source/Core/Disassembler.cpp | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp index 64c52913574a3..08190d05db3c6 100644 --- a/lldb/source/Core/Disassembler.cpp +++ b/lldb/source/Core/Disassembler.cpp @@ -48,7 +48,6 @@ #include "llvm/Support/Compiler.h" #include "llvm/TargetParser/Triple.h" - #include #include #include @@ -403,7 +402,8 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, bool seen_this_inst = false; }; - // Track live variables across instructions (keyed by stable LLDB user_id_t. 8 is a good small-buffer guess + // Track live variables across instructions (keyed by stable LLDB user_id_t. 8 + // is a good small-buffer guess. llvm::SmallDenseMap live_vars; // Stateful annotator: updates live_vars and returns only what should be @@ -433,9 +433,10 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, VariableListSP var_list_sp = frame->GetInScopeVariableList(true); if (!var_list_sp) { // No variables in scope: everything previously live becomes . - for (auto I = live_vars.begin(), E = live_vars.end(); I != E; ) { + for (auto I = live_vars.begin(), E = live_vars.end(); I != E;) { auto Cur = I++; - events.push_back(llvm::formatv("{0} = ", Cur->second.name).str()); + events.push_back( + llvm::formatv("{0} = ", Cur->second.name).str()); live_vars.erase(Cur); } frame->ChangePC(original_pc); @@ -486,9 +487,8 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, if (loc_clean.empty()) continue; - auto insert_res = live_vars.insert({var_id, - VarState{std::string(name), - loc_clean.str(), + auto insert_res = + live_vars.insert({var_id, VarState{std::string(name), loc_clean.str(), /*seen_this_inst*/ true}}); if (insert_res.second) { // Newly inserted → print. @@ -499,17 +499,19 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch, vs.seen_this_inst = true; if (vs.last_loc != loc_clean) { vs.last_loc = loc_clean.str(); - events.push_back(llvm::formatv("{0} = {1}", vs.name, loc_clean).str()); + events.push_back( + llvm::formatv("{0} = {1}", vs.name, loc_clean).str()); } } } // Anything previously live that we didn't see a location for at this inst // is now . - for (auto I = live_vars.begin(), E = live_vars.end(); I != E; ) { + 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} = ", Cur->second.name).str()); + events.push_back( + llvm::formatv("{0} = ", Cur->second.name).str()); live_vars.erase(Cur); } }