-
Notifications
You must be signed in to change notification settings - Fork 15.2k
Annotate disassembly with register‐resident variable locations #147460
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 26 commits
8ed8c54
1db5002
a26010b
72237b7
94e4951
e8142da
7e8741e
c4cd77f
62c02a9
d015971
60898ea
3462165
2ed8443
8c6b22d
842a9e5
2fa6d24
6bbc8aa
cbbc924
b887db2
912ba6d
09c4d04
6e17f77
31431c0
9c5cb8f
ca8510c
ffefe5f
fae745a
dcddf16
7bac074
79c0a9e
3d19b02
6ca4bb6
4bf584e
b1f13e7
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 |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #include "lldb/Core/AddressRange.h" | ||
| #include "lldb/Expression/DWARFExpressionList.h" | ||
| #include "lldb/Symbol/Function.h" | ||
| #include "lldb/Target/RegisterContext.h" | ||
|
|
@@ -53,6 +54,37 @@ bool DWARFExpressionList::ContainsAddress(lldb::addr_t func_load_addr, | |
| return GetExpressionAtAddress(func_load_addr, addr) != nullptr; | ||
| } | ||
|
|
||
| std::optional<DWARFExpressionList::DWARFExpressionEntry> | ||
| DWARFExpressionList::GetExpressionEntryAtAddress(lldb::addr_t func_load_addr, | ||
| lldb::addr_t load_addr) const { | ||
| if (const DWARFExpression *always = GetAlwaysValidExpr()) { | ||
| return DWARFExpressionEntry{std::nullopt, always}; | ||
| } | ||
|
|
||
| if (func_load_addr == LLDB_INVALID_ADDRESS) | ||
| func_load_addr = m_func_file_addr; | ||
|
|
||
| // Guard against underflow when translating a load address back into file space. | ||
| if (load_addr < func_load_addr) | ||
| return std::nullopt; | ||
|
|
||
| // Guard against overflow. | ||
| lldb::addr_t delta = load_addr - func_load_addr; | ||
| if (delta > std::numeric_limits<lldb::addr_t>::max() - m_func_file_addr) | ||
|
Collaborator
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. I think it would be better to use the add with overflow methods from MathExtras here.
Contributor
Author
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. From when I dug into MathExtras last time, I found that its I they rely on two’s-complement signed overflow semantics and return the truncated signed result. Since That's why I tried to implement the signed overflow check into the function. |
||
| return std::nullopt; | ||
|
|
||
| lldb::addr_t file_pc = (load_addr - func_load_addr) + m_func_file_addr; | ||
|
|
||
| if (const auto *entry = m_exprs.FindEntryThatContains(file_pc)) { | ||
| AddressRange range_in_file(entry->GetRangeBase(), | ||
| entry->GetRangeEnd() - entry->GetRangeBase()); | ||
| return DWARFExpressionEntry{range_in_file, &entry->data}; | ||
| } | ||
|
|
||
| // No entry covers this PC: | ||
| return std::nullopt; | ||
| } | ||
|
|
||
| const DWARFExpression * | ||
| DWARFExpressionList::GetExpressionAtAddress(lldb::addr_t func_load_addr, | ||
| lldb::addr_t load_addr) const { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
|
|
||
| # CXX_SOURCES := a_loop_with_local_variable.c b_multiple_stack_variables.c c_variable_passed_to_another_function.c d_original_example.c e_control_flow_edge.c | ||
| C_SOURCES := a_loop_with_local_variable.c b_multiple_stack_variables.c c_variable_passed_to_another_function.c d_original_example.c e_control_flow_edge.c | ||
|
|
||
|
|
||
| include Makefile.rules |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| from lldbsuite.test.lldbtest import * | ||
| from lldbsuite.test.decorators import * | ||
|
|
||
| class TestRichDisassembler(TestBase): | ||
| def test_d_original_example_O1(self): | ||
| """ | ||
| Tests disassembler output for d_original_example.c built with -O1. | ||
| """ | ||
| self.build(dictionary={ | ||
| 'C_SOURCES': 'd_original_example.c', | ||
| 'CFLAGS_EXTRAS': '-g -O1' | ||
| }) | ||
| exe = self.getBuildArtifact("a.out") | ||
| target = self.dbg.CreateTarget(exe) | ||
| self.assertTrue(target) | ||
|
|
||
| breakpoint = target.BreakpointCreateByName("main") | ||
| self.assertGreater(breakpoint.GetNumLocations(), 0) | ||
|
|
||
| process = target.LaunchSimple(None, None, self.get_process_working_directory()) | ||
| self.assertTrue(process, "Failed to launch process") | ||
| self.assertEqual(process.GetState(), lldb.eStateStopped) | ||
|
|
||
| frame = process.GetSelectedThread().GetSelectedFrame() | ||
| disasm = frame.Disassemble() | ||
| print(disasm) | ||
|
|
||
| self.assertIn("argc = ", disasm) | ||
| self.assertIn("argv = ", disasm) | ||
| self.assertIn("i = ", disasm) | ||
| # self.assertIn("DW_OP_reg", disasm) | ||
| # self.assertIn("DW_OP_stack_value", disasm) | ||
| self.assertNotIn("<decoding error>", disasm) | ||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| #include <stdio.h> | ||
|
|
||
| int main(int argc, char **argv) { | ||
| for (int i = 1; i < argc; ++i) | ||
| puts(argv[i]); | ||
| return 0; | ||
| } |
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.
@JDevlieghere Question for a future PR: Should we consider threading through and checking the actual width of the Terminal to see if the annotations would fit?