[Lldb-commits] [lldb] 826997c - [lldb] Fix a regression introduced by D75730
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Thu Oct 22 08:38:11 PDT 2020
Author: Jonas Devlieghere
Date: 2020-10-22T08:38:03-07:00
New Revision: 826997c46280351861be43522d4a022d8fdbc466
URL: https://github.com/llvm/llvm-project/commit/826997c46280351861be43522d4a022d8fdbc466
DIFF: https://github.com/llvm/llvm-project/commit/826997c46280351861be43522d4a022d8fdbc466.diff
LOG: [lldb] Fix a regression introduced by D75730
In a new Range class was introduced to simplify and the Disassembler API
and reduce duplication. It unintentionally broke the
SBFrame::Disassemble functionality because it unconditionally converts
the number of instructions to a Range{Limit::Instructions,
num_instructions}. This is subtly different from the previous behavior,
where now we're passing a Range and assume it's valid in the callee, the
original code would propagate num_instructions and the callee would
compare the value and decided between disassembling instructions or
bytes.
Unfortunately the existing tests was not particularly strict:
disassembly = frame.Disassemble()
self.assertNotEqual(len(disassembly), 0, "Disassembly was empty.")
This would pass because without this patch we'd disassemble zero
instructions, resulting in an error:
(lldb) script print(lldb.frame.Disassemble())
error: error reading data from section __text
Differential revision: https://reviews.llvm.org/D89925
Added:
Modified:
lldb/include/lldb/Core/Disassembler.h
lldb/source/Core/Disassembler.cpp
lldb/source/Target/StackFrame.cpp
lldb/test/API/commands/disassemble/basic/TestFrameDisassemble.py
lldb/test/API/commands/disassemble/basic/main.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Core/Disassembler.h b/lldb/include/lldb/Core/Disassembler.h
index d3b903b83c19..c38f1f7975d5 100644
--- a/lldb/include/lldb/Core/Disassembler.h
+++ b/lldb/include/lldb/Core/Disassembler.h
@@ -48,6 +48,7 @@ class DataExtractor;
class Debugger;
class Disassembler;
class Module;
+class StackFrame;
class Stream;
class SymbolContext;
class SymbolContextList;
@@ -404,11 +405,8 @@ class Disassembler : public std::enable_shared_from_this<Disassembler>,
uint32_t num_mixed_context_lines, uint32_t options,
Stream &strm);
- static bool
- Disassemble(Debugger &debugger, const ArchSpec &arch, const char *plugin_name,
- const char *flavor, const ExecutionContext &exe_ctx,
- uint32_t num_instructions, bool mixed_source_and_assembly,
- uint32_t num_mixed_context_lines, uint32_t options, Stream &strm);
+ static bool Disassemble(Debugger &debugger, const ArchSpec &arch,
+ StackFrame &frame, Stream &strm);
// Constructors and Destructors
Disassembler(const ArchSpec &arch, const char *flavor);
diff --git a/lldb/source/Core/Disassembler.cpp b/lldb/source/Core/Disassembler.cpp
index cc98b7309d6d..111cec6e2968 100644
--- a/lldb/source/Core/Disassembler.cpp
+++ b/lldb/source/Core/Disassembler.cpp
@@ -540,34 +540,29 @@ void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch,
}
bool Disassembler::Disassemble(Debugger &debugger, const ArchSpec &arch,
- const char *plugin_name, const char *flavor,
- const ExecutionContext &exe_ctx,
- uint32_t num_instructions,
- bool mixed_source_and_assembly,
- uint32_t num_mixed_context_lines,
- uint32_t options, Stream &strm) {
+ StackFrame &frame, Stream &strm) {
AddressRange range;
- StackFrame *frame = exe_ctx.GetFramePtr();
- if (frame) {
- SymbolContext sc(
- frame->GetSymbolContext(eSymbolContextFunction | eSymbolContextSymbol));
- if (sc.function) {
- range = sc.function->GetAddressRange();
- } else if (sc.symbol && sc.symbol->ValueIsAddress()) {
- range.GetBaseAddress() = sc.symbol->GetAddressRef();
- range.SetByteSize(sc.symbol->GetByteSize());
- } else {
- range.GetBaseAddress() = frame->GetFrameCodeAddress();
- }
+ SymbolContext sc(
+ frame.GetSymbolContext(eSymbolContextFunction | eSymbolContextSymbol));
+ if (sc.function) {
+ range = sc.function->GetAddressRange();
+ } else if (sc.symbol && sc.symbol->ValueIsAddress()) {
+ range.GetBaseAddress() = sc.symbol->GetAddressRef();
+ range.SetByteSize(sc.symbol->GetByteSize());
+ } else {
+ range.GetBaseAddress() = frame.GetFrameCodeAddress();
+ }
if (range.GetBaseAddress().IsValid() && range.GetByteSize() == 0)
range.SetByteSize(DEFAULT_DISASM_BYTE_SIZE);
- }
- return Disassemble(
- debugger, arch, plugin_name, flavor, exe_ctx, range.GetBaseAddress(),
- {Limit::Instructions, num_instructions}, mixed_source_and_assembly,
- num_mixed_context_lines, options, strm);
+ Disassembler::Limit limit = {Disassembler::Limit::Bytes,
+ range.GetByteSize()};
+ if (limit.value == 0)
+ limit.value = DEFAULT_DISASM_BYTE_SIZE;
+
+ return Disassemble(debugger, arch, nullptr, nullptr, frame,
+ range.GetBaseAddress(), limit, false, 0, 0, strm);
}
Instruction::Instruction(const Address &address, AddressClass addr_class)
diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp
index 22bca52d7f98..131581567d73 100644
--- a/lldb/source/Target/StackFrame.cpp
+++ b/lldb/source/Target/StackFrame.cpp
@@ -229,21 +229,16 @@ bool StackFrame::ChangePC(addr_t pc) {
const char *StackFrame::Disassemble() {
std::lock_guard<std::recursive_mutex> guard(m_mutex);
- if (m_disassembly.Empty()) {
- ExecutionContext exe_ctx(shared_from_this());
- Target *target = exe_ctx.GetTargetPtr();
- if (target) {
- const char *plugin_name = nullptr;
- const char *flavor = nullptr;
- Disassembler::Disassemble(target->GetDebugger(),
- target->GetArchitecture(), plugin_name, flavor,
- exe_ctx, 0, false, 0, 0, m_disassembly);
- }
- if (m_disassembly.Empty())
- return nullptr;
+ 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);
}
- return m_disassembly.GetData();
+ return m_disassembly.Empty() ? nullptr : m_disassembly.GetData();
}
Block *StackFrame::GetFrameBlock() {
diff --git a/lldb/test/API/commands/disassemble/basic/TestFrameDisassemble.py b/lldb/test/API/commands/disassemble/basic/TestFrameDisassemble.py
index 156458234a16..5c573f064e1e 100644
--- a/lldb/test/API/commands/disassemble/basic/TestFrameDisassemble.py
+++ b/lldb/test/API/commands/disassemble/basic/TestFrameDisassemble.py
@@ -57,4 +57,6 @@ def frame_disassemble_test(self):
frame = threads[0].GetFrameAtIndex(0)
disassembly = frame.Disassemble()
- self.assertNotEqual(len(disassembly), 0, "Disassembly was empty.")
+ self.assertNotEqual(disassembly, "")
+ self.assertNotIn("error", disassembly)
+ self.assertIn(": nop", disassembly)
diff --git a/lldb/test/API/commands/disassemble/basic/main.cpp b/lldb/test/API/commands/disassemble/basic/main.cpp
index cfa2498fc020..4a2d9c106e21 100644
--- a/lldb/test/API/commands/disassemble/basic/main.cpp
+++ b/lldb/test/API/commands/disassemble/basic/main.cpp
@@ -2,6 +2,7 @@ int
sum (int a, int b)
{
int result = a + b; // Set a breakpoint here
+ asm("nop");
return result;
}
More information about the lldb-commits
mailing list