labath created this revision.
labath added a reviewer: JDevlieghere.
Herald added subscribers: atanasyan, jrtc27, sdardis.
Herald added a project: LLDB.
labath marked an inline comment as done.
labath added inline comments.
================
Comment at: lldb/source/Commands/CommandObjectDisassemble.cpp:462-469
+ 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 {
+ limit = {Disassembler::Limit::Instructions, m_options.num_instructions};
----------------
The idea is that further refactorings will change `ranges` from a
`vector<AddressRange>` to a `vector<pair<Address, Limit>>` and this will become
a simple ranged for loop.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D75730
Files:
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
Index: lldb/source/Target/StackFrame.cpp
===================================================================
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -1940,16 +1940,14 @@
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);
}
}
Index: lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
===================================================================
--- lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
+++ lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
@@ -168,10 +168,11 @@
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) {
Index: lldb/source/Core/Disassembler.cpp
===================================================================
--- lldb/source/Core/Disassembler.cpp
+++ lldb/source/Core/Disassembler.cpp
@@ -137,8 +137,9 @@
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 @@
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 @@
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 @@
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 @@
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
Index: lldb/source/Commands/CommandObjectDisassemble.cpp
===================================================================
--- lldb/source/Commands/CommandObjectDisassemble.cpp
+++ lldb/source/Commands/CommandObjectDisassemble.cpp
@@ -459,25 +459,19 @@
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) {
Index: lldb/include/lldb/Core/Disassembler.h
===================================================================
--- lldb/include/lldb/Core/Disassembler.h
+++ lldb/include/lldb/Core/Disassembler.h
@@ -384,6 +384,11 @@
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 @@
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 @@
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,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
