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; } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
