[Lldb-commits] [lldb] af3db4e - [lldb] Reduce duplication in the Disassembler class
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Mon Mar 9 05:51:56 PDT 2020
Author: Pavel Labath
Date: 2020-03-09T13:41:43+01:00
New Revision: af3db4e9aa8fbe7e43f89cdde780c6acc35368be
URL: https://github.com/llvm/llvm-project/commit/af3db4e9aa8fbe7e43f89cdde780c6acc35368be
DIFF: https://github.com/llvm/llvm-project/commit/af3db4e9aa8fbe7e43f89cdde780c6acc35368be.diff
LOG: [lldb] Reduce duplication in the Disassembler class
Summary:
The class has two pairs of functions whose functionalities differ in
only how one specifies how much he wants to disasseble. One limits the
process by the size of the input memory region. The other based on the
total amount of instructions disassembled. They also differ in various
features (like error reporting) that were only added to one of the
versions.
There are various ways in which this could be addressed. This patch
does it by introducing a helper struct called "Limit", which is
effectively a pair specifying the value that you want to limit, and the
actual limit itself.
Reviewers: JDevlieghere
Subscribers: sdardis, jrtc27, atanasyan, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D75730
Added:
Modified:
lldb/include/lldb/Core/Disassembler.h
lldb/source/Commands/CommandObjectDisassemble.cpp
lldb/source/Core/Disassembler.cpp
lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
lldb/source/Target/StackFrame.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Core/Disassembler.h b/lldb/include/lldb/Core/Disassembler.h
index 145bed31a155..7ffdac74ccfc 100644
--- a/lldb/include/lldb/Core/Disassembler.h
+++ b/lldb/include/lldb/Core/Disassembler.h
@@ -384,6 +384,11 @@ class Disassembler : public std::enable_shared_from_this<Disassembler>,
const char *flavor,
const char *plugin_name);
+ struct Limit {
+ enum { Bytes, Instructions } kind;
+ lldb::addr_t value;
+ };
+
static lldb::DisassemblerSP
DisassembleRange(const ArchSpec &arch, const char *plugin_name,
const char *flavor, Target &target,
@@ -395,19 +400,10 @@ class Disassembler : public std::enable_shared_from_this<Disassembler>,
size_t length, uint32_t max_num_instructions,
bool data_from_file);
- static bool Disassemble(Debugger &debugger, const ArchSpec &arch,
- const char *plugin_name, const char *flavor,
- const ExecutionContext &exe_ctx,
- const AddressRange &range, 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,
const char *plugin_name, const char *flavor,
const ExecutionContext &exe_ctx, const Address &start,
- uint32_t num_instructions,
- bool mixed_source_and_assembly,
+ Limit limit, bool mixed_source_and_assembly,
uint32_t num_mixed_context_lines, uint32_t options,
Stream &strm);
@@ -423,17 +419,13 @@ class Disassembler : public std::enable_shared_from_this<Disassembler>,
void PrintInstructions(Debugger &debugger, const ArchSpec &arch,
const ExecutionContext &exe_ctx,
- uint32_t num_instructions,
bool mixed_source_and_assembly,
uint32_t num_mixed_context_lines, uint32_t options,
Stream &strm);
- size_t ParseInstructions(Target &target, AddressRange range,
+ size_t ParseInstructions(Target &target, Address address, Limit limit,
Stream *error_strm_ptr, bool prefer_file_cache);
- size_t ParseInstructions(Target &target, Address address,
- uint32_t num_instructions, bool prefer_file_cache);
-
virtual size_t DecodeInstructions(const Address &base_addr,
const DataExtractor &data,
lldb::offset_t data_offset,
diff --git a/lldb/source/Commands/CommandObjectDisassemble.cpp b/lldb/source/Commands/CommandObjectDisassemble.cpp
index a11af68835d9..bd1c7a43afad 100644
--- a/lldb/source/Commands/CommandObjectDisassemble.cpp
+++ b/lldb/source/Commands/CommandObjectDisassemble.cpp
@@ -459,25 +459,19 @@ bool CommandObjectDisassemble::DoExecute(Args &command,
bool print_sc_header = ranges.size() > 1;
for (AddressRange cur_range : ranges) {
- bool success;
- if (m_options.num_instructions != 0) {
- success = Disassembler::Disassemble(
- GetDebugger(), m_options.arch, plugin_name, flavor_string, m_exe_ctx,
- cur_range.GetBaseAddress(), m_options.num_instructions,
- m_options.show_mixed,
- m_options.show_mixed ? m_options.num_lines_context : 0, options,
- result.GetOutputStream());
+ Disassembler::Limit limit;
+ if (m_options.num_instructions == 0) {
+ limit = {Disassembler::Limit::Bytes, cur_range.GetByteSize()};
+ if (limit.value == 0)
+ limit.value = DEFAULT_DISASM_BYTE_SIZE;
} else {
- if (cur_range.GetByteSize() == 0)
- cur_range.SetByteSize(DEFAULT_DISASM_BYTE_SIZE);
-
- success = Disassembler::Disassemble(
- GetDebugger(), m_options.arch, plugin_name, flavor_string, m_exe_ctx,
- cur_range, m_options.num_instructions, m_options.show_mixed,
- m_options.show_mixed ? m_options.num_lines_context : 0, options,
- result.GetOutputStream());
+ limit = {Disassembler::Limit::Instructions, m_options.num_instructions};
}
- if (success) {
+ if (Disassembler::Disassemble(
+ GetDebugger(), m_options.arch, plugin_name, flavor_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.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 f5220288c8c1..4da823c7a243 100644
--- a/lldb/source/Core/Disassembler.cpp
+++ b/lldb/source/Core/Disassembler.cpp
@@ -137,8 +137,9 @@ lldb::DisassemblerSP Disassembler::DisassembleRange(
if (!disasm_sp)
return {};
- const size_t bytes_disassembled =
- disasm_sp->ParseInstructions(target, range, nullptr, prefer_file_cache);
+ const size_t bytes_disassembled = disasm_sp->ParseInstructions(
+ target, range.GetBaseAddress(), {Limit::Bytes, range.GetByteSize()},
+ nullptr, prefer_file_cache);
if (bytes_disassembled == 0)
return {};
@@ -170,52 +171,25 @@ Disassembler::DisassembleBytes(const ArchSpec &arch, const char *plugin_name,
bool Disassembler::Disassemble(Debugger &debugger, const ArchSpec &arch,
const char *plugin_name, const char *flavor,
const ExecutionContext &exe_ctx,
- const AddressRange &range,
- uint32_t num_instructions,
+ const Address &address, Limit limit,
bool mixed_source_and_assembly,
uint32_t num_mixed_context_lines,
uint32_t options, Stream &strm) {
- if (!range.GetByteSize() || !exe_ctx.GetTargetPtr())
+ if (!exe_ctx.GetTargetPtr())
return false;
lldb::DisassemblerSP disasm_sp(Disassembler::FindPluginForTarget(
exe_ctx.GetTargetRef(), arch, flavor, plugin_name));
-
if (!disasm_sp)
return false;
const bool prefer_file_cache = false;
size_t bytes_disassembled = disasm_sp->ParseInstructions(
- exe_ctx.GetTargetRef(), range, &strm, prefer_file_cache);
+ exe_ctx.GetTargetRef(), address, limit, &strm, prefer_file_cache);
if (bytes_disassembled == 0)
return false;
- disasm_sp->PrintInstructions(debugger, arch, exe_ctx, num_instructions,
- mixed_source_and_assembly,
- num_mixed_context_lines, options, strm);
- return true;
-}
-
-bool Disassembler::Disassemble(
- Debugger &debugger, const ArchSpec &arch, const char *plugin_name,
- const char *flavor, const ExecutionContext &exe_ctx, const Address &address,
- uint32_t num_instructions, bool mixed_source_and_assembly,
- uint32_t num_mixed_context_lines, uint32_t options, Stream &strm) {
- if (num_instructions == 0 || !exe_ctx.GetTargetPtr())
- return false;
-
- lldb::DisassemblerSP disasm_sp(Disassembler::FindPluginForTarget(
- exe_ctx.GetTargetRef(), arch, flavor, plugin_name));
- if (!disasm_sp)
- return false;
-
- const bool prefer_file_cache = false;
- size_t bytes_disassembled = disasm_sp->ParseInstructions(
- exe_ctx.GetTargetRef(), address, num_instructions, prefer_file_cache);
- if (bytes_disassembled == 0)
- return false;
-
- disasm_sp->PrintInstructions(debugger, arch, exe_ctx, num_instructions,
+ disasm_sp->PrintInstructions(debugger, arch, exe_ctx,
mixed_source_and_assembly,
num_mixed_context_lines, options, strm);
return true;
@@ -306,16 +280,12 @@ bool Disassembler::ElideMixedSourceAndDisassemblyLine(
void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch,
const ExecutionContext &exe_ctx,
- uint32_t num_instructions,
bool mixed_source_and_assembly,
uint32_t num_mixed_context_lines,
uint32_t options, Stream &strm) {
// We got some things disassembled...
size_t num_instructions_found = GetInstructionList().GetSize();
- if (num_instructions > 0 && num_instructions < num_instructions_found)
- num_instructions_found = num_instructions;
-
const uint32_t max_opcode_byte_size =
GetInstructionList().GetMaxOpcocdeByteSize();
SymbolContext sc;
@@ -594,9 +564,10 @@ bool Disassembler::Disassemble(Debugger &debugger, const ArchSpec &arch,
range.SetByteSize(DEFAULT_DISASM_BYTE_SIZE);
}
- return Disassemble(debugger, arch, plugin_name, flavor, exe_ctx, range,
- num_instructions, mixed_source_and_assembly,
- num_mixed_context_lines, options, strm);
+ 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);
}
Instruction::Instruction(const Address &address, AddressClass addr_class)
@@ -1101,77 +1072,44 @@ InstructionList::GetIndexOfInstructionAtLoadAddress(lldb::addr_t load_addr,
return GetIndexOfInstructionAtAddress(address);
}
-size_t Disassembler::ParseInstructions(Target &target, AddressRange range,
- Stream *error_strm_ptr,
- bool prefer_file_cache) {
- const addr_t byte_size = range.GetByteSize();
- if (byte_size == 0 || !range.GetBaseAddress().IsValid())
- return 0;
-
- range.GetBaseAddress() = ResolveAddress(target, range.GetBaseAddress());
-
- auto data_sp = std::make_shared<DataBufferHeap>(byte_size, '\0');
-
- Status error;
- lldb::addr_t load_addr = LLDB_INVALID_ADDRESS;
- const size_t bytes_read = target.ReadMemory(
- range.GetBaseAddress(), prefer_file_cache, data_sp->GetBytes(),
- data_sp->GetByteSize(), error, &load_addr);
-
- if (bytes_read > 0) {
- if (bytes_read != data_sp->GetByteSize())
- data_sp->SetByteSize(bytes_read);
- DataExtractor data(data_sp, m_arch.GetByteOrder(),
- m_arch.GetAddressByteSize());
- const bool data_from_file = load_addr == LLDB_INVALID_ADDRESS;
- return DecodeInstructions(range.GetBaseAddress(), data, 0, UINT32_MAX,
- false, data_from_file);
- } else if (error_strm_ptr) {
- const char *error_cstr = error.AsCString();
- if (error_cstr) {
- error_strm_ptr->Printf("error: %s\n", error_cstr);
- }
- }
- return 0;
-}
-
size_t Disassembler::ParseInstructions(Target &target, Address start,
- uint32_t num_instructions,
+ Limit limit, Stream *error_strm_ptr,
bool prefer_file_cache) {
m_instruction_list.Clear();
- if (num_instructions == 0 || !start.IsValid())
+ if (!start.IsValid())
return 0;
start = ResolveAddress(target, start);
- // Calculate the max buffer size we will need in order to disassemble
- const addr_t byte_size = num_instructions * m_arch.GetMaximumOpcodeByteSize();
-
- if (byte_size == 0)
- return 0;
-
- DataBufferHeap *heap_buffer = new DataBufferHeap(byte_size, '\0');
- DataBufferSP data_sp(heap_buffer);
+ addr_t byte_size = limit.value;
+ if (limit.kind == Limit::Instructions)
+ byte_size *= m_arch.GetMaximumOpcodeByteSize();
+ auto data_sp = std::make_shared<DataBufferHeap>(byte_size, '\0');
Status error;
lldb::addr_t load_addr = LLDB_INVALID_ADDRESS;
const size_t bytes_read =
- target.ReadMemory(start, prefer_file_cache, heap_buffer->GetBytes(),
- byte_size, error, &load_addr);
-
+ target.ReadMemory(start, prefer_file_cache, data_sp->GetBytes(),
+ data_sp->GetByteSize(), error, &load_addr);
const bool data_from_file = load_addr == LLDB_INVALID_ADDRESS;
- if (bytes_read == 0)
+ if (bytes_read == 0) {
+ if (error_strm_ptr) {
+ if (const char *error_cstr = error.AsCString())
+ error_strm_ptr->Printf("error: %s\n", error_cstr);
+ }
return 0;
+ }
+
+ if (bytes_read != data_sp->GetByteSize())
+ data_sp->SetByteSize(bytes_read);
DataExtractor data(data_sp, m_arch.GetByteOrder(),
m_arch.GetAddressByteSize());
-
- const bool append_instructions = true;
- DecodeInstructions(start, data, 0, num_instructions, append_instructions,
- data_from_file);
-
- return m_instruction_list.GetSize();
+ return DecodeInstructions(start, data, 0,
+ limit.kind == Limit::Instructions ? limit.value
+ : UINT32_MAX,
+ false, data_from_file);
}
// Disassembler copy constructor
diff --git a/lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp b/lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
index e3d1aa3b11dd..22508969ceed 100644
--- a/lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
+++ b/lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
@@ -168,10 +168,11 @@ Instruction *ArchitectureMips::GetInstructionAtAddress(
for (uint32_t i = 1; i <= loop_count; i++) {
// Adjust the address to read from.
addr.Slide(-2);
- AddressRange range(addr, i * 2);
uint32_t insn_size = 0;
- disasm_sp->ParseInstructions(target, range, nullptr, prefer_file_cache);
+ disasm_sp->ParseInstructions(target, addr,
+ {Disassembler::Limit::Bytes, i * 2}, nullptr,
+ prefer_file_cache);
uint32_t num_insns = disasm_sp->GetInstructionList().GetSize();
if (num_insns) {
diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp
index f9e55cfb7375..a81879476cf2 100644
--- a/lldb/source/Target/StackFrame.cpp
+++ b/lldb/source/Target/StackFrame.cpp
@@ -1940,16 +1940,14 @@ bool StackFrame::GetStatus(Stream &strm, bool show_frame_info, bool show_source,
const uint32_t disasm_lines = debugger.GetDisassemblyLineCount();
if (disasm_lines > 0) {
const ArchSpec &target_arch = target->GetArchitecture();
- AddressRange pc_range;
- pc_range.GetBaseAddress() = GetFrameCodeAddress();
- pc_range.SetByteSize(disasm_lines *
- target_arch.GetMaximumOpcodeByteSize());
const char *plugin_name = nullptr;
const char *flavor = nullptr;
const bool mixed_source_and_assembly = false;
Disassembler::Disassemble(
target->GetDebugger(), target_arch, plugin_name, flavor,
- exe_ctx, pc_range, disasm_lines, mixed_source_and_assembly, 0,
+ exe_ctx, GetFrameCodeAddress(),
+ {Disassembler::Limit::Instructions, disasm_lines},
+ mixed_source_and_assembly, 0,
Disassembler::eOptionMarkPCAddress, strm);
}
}
More information about the lldb-commits
mailing list