-
Notifications
You must be signed in to change notification settings - Fork 15k
[lldb] [disassembler] chore: enhance VariableAnnotator to return structured data #165163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
aba4890
199a909
ea1bf46
0331516
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -566,6 +566,21 @@ class Disassembler : public std::enable_shared_from_this<Disassembler>, | |
| const Disassembler &operator=(const Disassembler &) = delete; | ||
| }; | ||
|
|
||
| /// Structured data for a single variable annotation. | ||
| struct VariableAnnotation { | ||
| std::string variable_name; | ||
| std::string location_description; // e.g., "r15", "undef", "const_0" | ||
| lldb::addr_t start_address; // Where this annotation starts being valid | ||
| lldb::addr_t end_address; // Where this annotation ends being valid | ||
|
Comment on lines
+573
to
+574
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be represented by an |
||
| bool is_live; // Whether variable is live at this instruction | ||
| lldb::RegisterKind | ||
| register_kind; // Register numbering scheme for location interpretation | ||
| std::optional<std::string> | ||
| decl_file; // Source file where variable was declared | ||
| std::optional<uint32_t> decl_line; // Line number where variable was declared | ||
|
Comment on lines
+578
to
+580
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be represented by a |
||
| std::optional<std::string> type_name; // Variable's type name | ||
| }; | ||
|
|
||
| /// Tracks live variable annotations across instructions and produces | ||
| /// per-instruction "events" like `name = RDI` or `name = <undef>`. | ||
| class VariableAnnotator { | ||
|
|
@@ -574,16 +589,39 @@ class VariableAnnotator { | |
| std::string name; | ||
| /// Last printed location (empty means <undef>). | ||
| std::string last_loc; | ||
| /// Address range where this variable state is valid. | ||
| lldb::addr_t start_address; | ||
| lldb::addr_t end_address; | ||
| /// Register numbering scheme for location interpretation. | ||
| lldb::RegisterKind register_kind; | ||
|
|
||
| std::optional<std::string> decl_file; | ||
| std::optional<uint32_t> decl_line; | ||
| std::optional<std::string> type_name; | ||
|
Comment on lines
+592
to
+600
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of duplicating these fields, should this hold onto a |
||
| }; | ||
|
|
||
| // Live state from the previous instruction, keyed by Variable::GetID(). | ||
| llvm::DenseMap<lldb::user_id_t, VarState> Live_; | ||
|
|
||
| static constexpr const char *kUndefLocation = "undef"; | ||
|
|
||
| public: | ||
| /// Compute annotation strings for a single instruction and update `Live_`. | ||
| /// Returns only the events that should be printed *at this instruction*. | ||
| std::vector<std::string> annotate(Instruction &inst, Target &target, | ||
| const lldb::ModuleSP &module_sp); | ||
|
|
||
| /// Compute structured annotation data for a single instruction and update | ||
| /// `Live_`. Returns structured data for all variables relevant at this | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment is out of date. This returns a list of |
||
| /// instruction. | ||
| std::vector<VariableAnnotation> | ||
| AnnotateStructured(Instruction &inst, Target &target, | ||
| const lldb::ModuleSP &module_sp); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why take a const ref to a shared pointer? Either take a const ref to the module or take the SP by value and bump the ref count. |
||
|
|
||
| private: | ||
| VariableAnnotation createAnnotation( | ||
| const VarState &var_state, bool is_live, | ||
| const std::optional<std::string> &location_desc = std::nullopt); | ||
| }; | ||
|
|
||
| } // namespace lldb_private | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,10 +10,11 @@ | |
| #include "lldb/Utility/Instrumentation.h" | ||
|
|
||
| #include "lldb/API/SBAddress.h" | ||
| #include "lldb/API/SBFrame.h" | ||
| #include "lldb/API/SBFile.h" | ||
| #include "lldb/API/SBFrame.h" | ||
|
|
||
| #include "lldb/API/SBStream.h" | ||
| #include "lldb/API/SBStructuredData.h" | ||
| #include "lldb/API/SBTarget.h" | ||
| #include "lldb/Core/Disassembler.h" | ||
| #include "lldb/Core/EmulateInstruction.h" | ||
|
|
@@ -26,6 +27,7 @@ | |
| #include "lldb/Utility/ArchSpec.h" | ||
| #include "lldb/Utility/DataBufferHeap.h" | ||
| #include "lldb/Utility/DataExtractor.h" | ||
| #include "lldb/Utility/StructuredData.h" | ||
|
|
||
| #include <memory> | ||
|
|
||
|
|
@@ -163,7 +165,8 @@ const char *SBInstruction::GetComment(SBTarget target) { | |
| return ConstString(inst_sp->GetComment(&exe_ctx)).GetCString(); | ||
| } | ||
|
|
||
| lldb::InstructionControlFlowKind SBInstruction::GetControlFlowKind(lldb::SBTarget target) { | ||
| lldb::InstructionControlFlowKind | ||
| SBInstruction::GetControlFlowKind(lldb::SBTarget target) { | ||
| LLDB_INSTRUMENT_VA(this, target); | ||
|
|
||
| lldb::InstructionSP inst_sp(GetOpaque()); | ||
|
|
@@ -347,3 +350,67 @@ bool SBInstruction::TestEmulation(lldb::SBStream &output_stream, | |
| return inst_sp->TestEmulation(output_stream.ref(), test_file); | ||
| return false; | ||
| } | ||
|
|
||
| lldb::SBStructuredData | ||
| SBInstruction::GetVariableAnnotations(lldb::SBTarget target) { | ||
| LLDB_INSTRUMENT_VA(this, target); | ||
|
|
||
| SBStructuredData result; | ||
|
|
||
| if (!m_opaque_sp || !m_opaque_sp->IsValid() || !target.IsValid()) { | ||
| return result; | ||
| } | ||
|
Comment on lines
+360
to
+362
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No braces around single line ifs. This applies to the whole patch. Match the existing code style of the surrounding code. |
||
|
|
||
| lldb::InstructionSP inst_sp = m_opaque_sp->GetSP(); | ||
| lldb::TargetSP target_sp = target.GetSP(); | ||
|
|
||
| if (!inst_sp || !target_sp) { | ||
| return result; | ||
| } | ||
|
|
||
| const Address &addr = inst_sp->GetAddress(); | ||
| ModuleSP module_sp = addr.GetModule(); | ||
|
|
||
| if (!module_sp) { | ||
| return result; | ||
| } | ||
|
|
||
| VariableAnnotator annotator; | ||
| std::vector<VariableAnnotation> annotations = | ||
| annotator.annotateStructured(*inst_sp, *target_sp, module_sp); | ||
|
|
||
| auto array_sp = std::make_shared<StructuredData::Array>(); | ||
|
|
||
| for (const auto &ann : annotations) { | ||
| auto dict_sp = std::make_shared<StructuredData::Dictionary>(); | ||
|
|
||
| dict_sp->AddStringItem("variable_name", ann.variable_name); | ||
| dict_sp->AddStringItem("location_description", ann.location_description); | ||
| dict_sp->AddBooleanItem("is_live", ann.is_live); | ||
| dict_sp->AddItem( | ||
| "start_address", | ||
| std::make_shared<StructuredData::UnsignedInteger>(ann.start_address)); | ||
| dict_sp->AddItem( | ||
| "end_address", | ||
| std::make_shared<StructuredData::UnsignedInteger>(ann.end_address)); | ||
| dict_sp->AddItem( | ||
| "register_kind", | ||
| std::make_shared<StructuredData::UnsignedInteger>(ann.register_kind)); | ||
| if (ann.decl_file.has_value()) { | ||
| dict_sp->AddStringItem("decl_file", *ann.decl_file); | ||
| } | ||
| if (ann.decl_line.has_value()) { | ||
| dict_sp->AddItem( | ||
| "decl_line", | ||
| std::make_shared<StructuredData::UnsignedInteger>(*ann.decl_line)); | ||
| } | ||
| if (ann.type_name.has_value()) { | ||
| dict_sp->AddStringItem("type_name", *ann.type_name); | ||
| } | ||
|
|
||
| array_sp->AddItem(dict_sp); | ||
| } | ||
|
|
||
| result.m_impl_up->SetObjectSP(array_sp); | ||
| return result; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should be Doxygen-style comments and go on the line before the member.