[Lldb-commits] [lldb] r272069 - Fix a memory leak in InstructionLLVMC where it held onto a strong reference to the DisassemblerLLVMC which in turn had a vector of InstructionSP causing the strong cycle. This is fixed now.
Greg Clayton via lldb-commits
lldb-commits at lists.llvm.org
Tue Jun 7 15:56:42 PDT 2016
Author: gclayton
Date: Tue Jun 7 17:56:40 2016
New Revision: 272069
URL: http://llvm.org/viewvc/llvm-project?rev=272069&view=rev
Log:
Fix a memory leak in InstructionLLVMC where it held onto a strong reference to the DisassemblerLLVMC which in turn had a vector of InstructionSP causing the strong cycle. This is fixed now.
Rules are as follows for internal code using lldb::DisassemblerSP and lldb::InstructionSP:
1 - The disassembler needs to stay around as long as instructions do as the Instruction subclass now has a weak pointer to the disassembler
2 - The public API has been fixed so that if you get a SBInstruction, it will hold onto a strong reference to the disassembler in a new InstructionImpl class
This will keep code like like:
inst = lldb.target.ReadInstructions(frame.GetPCAddress(), 1).GetInstructionAtIndex(0)
inst.GetMnemonic()
Working as expected (not the SBInstructionList() that was returned by SBTarget.ReadInstructions() is gone, but "inst" has a strong reference inside of it to the disassembler and the instruction.
All code inside the LLDB shared library was verified to correctly hold onto the disassembler instance in all places.
<rdar://problem/24585496>
Modified:
lldb/trunk/include/lldb/API/SBInstruction.h
lldb/trunk/source/API/SBInstruction.cpp
lldb/trunk/source/API/SBInstructionList.cpp
lldb/trunk/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
Modified: lldb/trunk/include/lldb/API/SBInstruction.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBInstruction.h?rev=272069&r1=272068&r2=272069&view=diff
==============================================================================
--- lldb/trunk/include/lldb/API/SBInstruction.h (original)
+++ lldb/trunk/include/lldb/API/SBInstruction.h Tue Jun 7 17:56:40 2016
@@ -18,6 +18,8 @@
// There's a lot to be fixed here, but need to wait for underlying insn implementation
// to be revised & settle down first.
+class InstructionImpl;
+
namespace lldb {
class LLDB_API SBInstruction
@@ -81,14 +83,17 @@ public:
protected:
friend class SBInstructionList;
- SBInstruction (const lldb::InstructionSP &inst_sp);
+ SBInstruction(const lldb::DisassemblerSP &disasm_sp, const lldb::InstructionSP &inst_sp);
void
- SetOpaque (const lldb::InstructionSP &inst_sp);
+ SetOpaque(const lldb::DisassemblerSP &disasm_sp, const lldb::InstructionSP& inst_sp);
+
+ lldb::InstructionSP
+ GetOpaque();
private:
- lldb::InstructionSP m_opaque_sp;
+ std::shared_ptr<InstructionImpl> m_opaque_sp;
};
Modified: lldb/trunk/source/API/SBInstruction.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBInstruction.cpp?rev=272069&r1=272068&r2=272069&view=diff
==============================================================================
--- lldb/trunk/source/API/SBInstruction.cpp (original)
+++ lldb/trunk/source/API/SBInstruction.cpp Tue Jun 7 17:56:40 2016
@@ -26,15 +26,63 @@
#include "lldb/Target/StackFrame.h"
#include "lldb/Target/Target.h"
+//----------------------------------------------------------------------
+// We recently fixed a leak in one of the Instruction subclasses where
+// the instruction will only hold a weak reference to the disassembler
+// to avoid a cycle that was keeping both objects alive (leak) and we
+// need the InstructionImpl class to make sure our public API behaves
+// as users would expect. Calls in our public API allow clients to do
+// things like:
+//
+// 1 lldb::SBInstruction inst;
+// 2 inst = target.ReadInstructions(pc, 1).GetInstructionAtIndex(0)
+// 3 if (inst.DoesBranch())
+// 4 ...
+//
+// There was a temporary lldb::DisassemblerSP object created in the
+// SBInstructionList that was returned by lldb.target.ReadInstructions()
+// that will go away after line 2 but the "inst" object should be able
+// to still answer questions about itself. So we make sure that any
+// SBInstruction objects that are given out have a strong reference to
+// the disassembler and the instruction so that the object can live and
+// successfully respond to all queries.
+//----------------------------------------------------------------------
+class InstructionImpl
+{
+public:
+ InstructionImpl (const lldb::DisassemblerSP &disasm_sp, const lldb::InstructionSP& inst_sp) :
+ m_disasm_sp(disasm_sp),
+ m_inst_sp(inst_sp)
+ {
+ }
+
+ lldb::InstructionSP
+ GetSP() const
+ {
+ return m_inst_sp;
+ }
+
+ bool
+ IsValid() const
+ {
+ return (bool)m_inst_sp;
+ }
+
+protected:
+ lldb::DisassemblerSP m_disasm_sp; // Can be empty/invalid
+ lldb::InstructionSP m_inst_sp;
+};
+
using namespace lldb;
using namespace lldb_private;
-SBInstruction::SBInstruction ()
+SBInstruction::SBInstruction() :
+ m_opaque_sp()
{
}
-SBInstruction::SBInstruction (const lldb::InstructionSP& inst_sp) :
- m_opaque_sp (inst_sp)
+SBInstruction::SBInstruction(const lldb::DisassemblerSP &disasm_sp, const lldb::InstructionSP& inst_sp) :
+ m_opaque_sp(new InstructionImpl(disasm_sp, inst_sp))
{
}
@@ -58,22 +106,24 @@ SBInstruction::~SBInstruction ()
bool
SBInstruction::IsValid()
{
- return (m_opaque_sp.get() != NULL);
+ return m_opaque_sp && m_opaque_sp->IsValid();
}
SBAddress
SBInstruction::GetAddress()
{
SBAddress sb_addr;
- if (m_opaque_sp && m_opaque_sp->GetAddress().IsValid())
- sb_addr.SetAddress(&m_opaque_sp->GetAddress());
+ lldb::InstructionSP inst_sp(GetOpaque());
+ if (inst_sp && inst_sp->GetAddress().IsValid())
+ sb_addr.SetAddress(&inst_sp->GetAddress());
return sb_addr;
}
const char *
SBInstruction::GetMnemonic(SBTarget target)
{
- if (m_opaque_sp)
+ lldb::InstructionSP inst_sp(GetOpaque());
+ if (inst_sp)
{
ExecutionContext exe_ctx;
TargetSP target_sp (target.GetSP());
@@ -85,7 +135,7 @@ SBInstruction::GetMnemonic(SBTarget targ
target_sp->CalculateExecutionContext (exe_ctx);
exe_ctx.SetProcessSP(target_sp->GetProcessSP());
}
- return m_opaque_sp->GetMnemonic(&exe_ctx);
+ return inst_sp->GetMnemonic(&exe_ctx);
}
return NULL;
}
@@ -93,7 +143,8 @@ SBInstruction::GetMnemonic(SBTarget targ
const char *
SBInstruction::GetOperands(SBTarget target)
{
- if (m_opaque_sp)
+ lldb::InstructionSP inst_sp(GetOpaque());
+ if (inst_sp)
{
ExecutionContext exe_ctx;
TargetSP target_sp (target.GetSP());
@@ -105,7 +156,7 @@ SBInstruction::GetOperands(SBTarget targ
target_sp->CalculateExecutionContext (exe_ctx);
exe_ctx.SetProcessSP(target_sp->GetProcessSP());
}
- return m_opaque_sp->GetOperands(&exe_ctx);
+ return inst_sp->GetOperands(&exe_ctx);
}
return NULL;
}
@@ -113,7 +164,8 @@ SBInstruction::GetOperands(SBTarget targ
const char *
SBInstruction::GetComment(SBTarget target)
{
- if (m_opaque_sp)
+ lldb::InstructionSP inst_sp(GetOpaque());
+ if (inst_sp)
{
ExecutionContext exe_ctx;
TargetSP target_sp (target.GetSP());
@@ -125,7 +177,7 @@ SBInstruction::GetComment(SBTarget targe
target_sp->CalculateExecutionContext (exe_ctx);
exe_ctx.SetProcessSP(target_sp->GetProcessSP());
}
- return m_opaque_sp->GetComment(&exe_ctx);
+ return inst_sp->GetComment(&exe_ctx);
}
return NULL;
}
@@ -133,8 +185,9 @@ SBInstruction::GetComment(SBTarget targe
size_t
SBInstruction::GetByteSize ()
{
- if (m_opaque_sp)
- return m_opaque_sp->GetOpcode().GetByteSize();
+ lldb::InstructionSP inst_sp(GetOpaque());
+ if (inst_sp)
+ return inst_sp->GetOpcode().GetByteSize();
return 0;
}
@@ -142,10 +195,11 @@ SBData
SBInstruction::GetData (SBTarget target)
{
lldb::SBData sb_data;
- if (m_opaque_sp)
+ lldb::InstructionSP inst_sp(GetOpaque());
+ if (inst_sp)
{
DataExtractorSP data_extractor_sp (new DataExtractor());
- if (m_opaque_sp->GetData (*data_extractor_sp))
+ if (inst_sp->GetData (*data_extractor_sp))
{
sb_data.SetOpaque (data_extractor_sp);
}
@@ -158,32 +212,44 @@ SBInstruction::GetData (SBTarget target)
bool
SBInstruction::DoesBranch ()
{
- if (m_opaque_sp)
- return m_opaque_sp->DoesBranch ();
+ lldb::InstructionSP inst_sp(GetOpaque());
+ if (inst_sp)
+ return inst_sp->DoesBranch ();
return false;
}
bool
SBInstruction::HasDelaySlot ()
{
- if (m_opaque_sp)
- return m_opaque_sp->HasDelaySlot ();
+ lldb::InstructionSP inst_sp(GetOpaque());
+ if (inst_sp)
+ return inst_sp->HasDelaySlot ();
return false;
}
+lldb::InstructionSP
+SBInstruction::GetOpaque ()
+{
+ if (m_opaque_sp)
+ return m_opaque_sp->GetSP();
+ else
+ return lldb::InstructionSP();
+}
+
void
-SBInstruction::SetOpaque (const lldb::InstructionSP &inst_sp)
+SBInstruction::SetOpaque (const lldb::DisassemblerSP &disasm_sp, const lldb::InstructionSP& inst_sp)
{
- m_opaque_sp = inst_sp;
+ m_opaque_sp.reset(new InstructionImpl(disasm_sp, inst_sp));
}
bool
SBInstruction::GetDescription (lldb::SBStream &s)
{
- if (m_opaque_sp)
+ lldb::InstructionSP inst_sp(GetOpaque());
+ if (inst_sp)
{
SymbolContext sc;
- const Address &addr = m_opaque_sp->GetAddress();
+ const Address &addr = inst_sp->GetAddress();
ModuleSP module_sp (addr.GetModule());
if (module_sp)
module_sp->ResolveSymbolContextForAddress(addr, eSymbolContextEverything, sc);
@@ -191,7 +257,7 @@ SBInstruction::GetDescription (lldb::SBS
// didn't have a stream already created, one will get created...
FormatEntity::Entry format;
FormatEntity::Parse("${addr}: ", format);
- m_opaque_sp->Dump (&s.ref(), 0, true, false, NULL, &sc, NULL, &format, 0);
+ inst_sp->Dump (&s.ref(), 0, true, false, NULL, &sc, NULL, &format, 0);
return true;
}
return false;
@@ -203,24 +269,26 @@ SBInstruction::Print (FILE *out)
if (out == NULL)
return;
- if (m_opaque_sp)
+ lldb::InstructionSP inst_sp(GetOpaque());
+ if (inst_sp)
{
SymbolContext sc;
- const Address &addr = m_opaque_sp->GetAddress();
+ const Address &addr = inst_sp->GetAddress();
ModuleSP module_sp (addr.GetModule());
if (module_sp)
module_sp->ResolveSymbolContextForAddress(addr, eSymbolContextEverything, sc);
StreamFile out_stream (out, false);
FormatEntity::Entry format;
FormatEntity::Parse("${addr}: ", format);
- m_opaque_sp->Dump (&out_stream, 0, true, false, NULL, &sc, NULL, &format, 0);
+ inst_sp->Dump (&out_stream, 0, true, false, NULL, &sc, NULL, &format, 0);
}
}
bool
SBInstruction::EmulateWithFrame (lldb::SBFrame &frame, uint32_t evaluate_options)
{
- if (m_opaque_sp)
+ lldb::InstructionSP inst_sp(GetOpaque());
+ if (inst_sp)
{
lldb::StackFrameSP frame_sp (frame.GetFrameSP());
@@ -231,13 +299,13 @@ SBInstruction::EmulateWithFrame (lldb::S
lldb_private::Target *target = exe_ctx.GetTargetPtr();
lldb_private::ArchSpec arch = target->GetArchitecture();
- return m_opaque_sp->Emulate (arch,
- evaluate_options,
- (void *) frame_sp.get(),
- &lldb_private::EmulateInstruction::ReadMemoryFrame,
- &lldb_private::EmulateInstruction::WriteMemoryFrame,
- &lldb_private::EmulateInstruction::ReadRegisterFrame,
- &lldb_private::EmulateInstruction::WriteRegisterFrame);
+ return inst_sp->Emulate(arch,
+ evaluate_options,
+ (void *) frame_sp.get(),
+ &lldb_private::EmulateInstruction::ReadMemoryFrame,
+ &lldb_private::EmulateInstruction::WriteMemoryFrame,
+ &lldb_private::EmulateInstruction::ReadRegisterFrame,
+ &lldb_private::EmulateInstruction::WriteRegisterFrame);
}
}
return false;
@@ -246,29 +314,32 @@ SBInstruction::EmulateWithFrame (lldb::S
bool
SBInstruction::DumpEmulation (const char *triple)
{
- if (m_opaque_sp && triple)
+ lldb::InstructionSP inst_sp(GetOpaque());
+ if (inst_sp && triple)
{
lldb_private::ArchSpec arch (triple, NULL);
-
- return m_opaque_sp->DumpEmulation (arch);
-
+ return inst_sp->DumpEmulation (arch);
}
return false;
}
bool
-SBInstruction::TestEmulation (lldb::SBStream &output_stream, const char *test_file)
+SBInstruction::TestEmulation (lldb::SBStream &output_stream, const char *test_file)
{
- if (!m_opaque_sp.get())
- m_opaque_sp.reset (new PseudoInstruction());
-
- return m_opaque_sp->TestEmulation (output_stream.get(), test_file);
+ if (!m_opaque_sp)
+ SetOpaque(lldb::DisassemblerSP(), lldb::InstructionSP(new PseudoInstruction()));
+
+ lldb::InstructionSP inst_sp(GetOpaque());
+ if (inst_sp)
+ return inst_sp->TestEmulation (output_stream.get(), test_file);
+ return false;
}
lldb::AddressClass
SBInstruction::GetAddressClass ()
{
- if (m_opaque_sp.get())
- return m_opaque_sp->GetAddressClass();
+ lldb::InstructionSP inst_sp(GetOpaque());
+ if (inst_sp)
+ return inst_sp->GetAddressClass();
return eAddressClassInvalid;
}
Modified: lldb/trunk/source/API/SBInstructionList.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBInstructionList.cpp?rev=272069&r1=272068&r2=272069&view=diff
==============================================================================
--- lldb/trunk/source/API/SBInstructionList.cpp (original)
+++ lldb/trunk/source/API/SBInstructionList.cpp Tue Jun 7 17:56:40 2016
@@ -61,7 +61,7 @@ SBInstructionList::GetInstructionAtIndex
{
SBInstruction inst;
if (m_opaque_sp && idx < m_opaque_sp->GetInstructionList().GetSize())
- inst.SetOpaque (m_opaque_sp->GetInstructionList().GetInstructionAtIndex (idx));
+ inst.SetOpaque (m_opaque_sp, m_opaque_sp->GetInstructionList().GetInstructionAtIndex (idx));
return inst;
}
Modified: lldb/trunk/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp?rev=272069&r1=272068&r2=272069&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp (original)
+++ lldb/trunk/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp Tue Jun 7 17:56:40 2016
@@ -53,7 +53,7 @@ public:
const lldb_private::Address &address,
AddressClass addr_class) :
Instruction (address, addr_class),
- m_disasm_sp (disasm.shared_from_this()),
+ m_disasm_wp (std::static_pointer_cast<DisassemblerLLVMC>(disasm.shared_from_this())),
m_does_branch (eLazyBoolCalculate),
m_has_delay_slot (eLazyBoolCalculate),
m_is_valid (false),
@@ -68,34 +68,38 @@ public:
{
if (m_does_branch == eLazyBoolCalculate)
{
- GetDisassemblerLLVMC().Lock(this, NULL);
- DataExtractor data;
- if (m_opcode.GetData(data))
+ std::shared_ptr<DisassemblerLLVMC> disasm_sp(GetDisassembler());
+ if (disasm_sp)
{
- bool is_alternate_isa;
- lldb::addr_t pc = m_address.GetFileAddress();
-
- DisassemblerLLVMC::LLVMCDisassembler *mc_disasm_ptr = GetDisasmToUse (is_alternate_isa);
- const uint8_t *opcode_data = data.GetDataStart();
- const size_t opcode_data_len = data.GetByteSize();
- llvm::MCInst inst;
- const size_t inst_size = mc_disasm_ptr->GetMCInst (opcode_data,
- opcode_data_len,
- pc,
- inst);
- // Be conservative, if we didn't understand the instruction, say it might branch...
- if (inst_size == 0)
- m_does_branch = eLazyBoolYes;
- else
+ disasm_sp->Lock(this, NULL);
+ DataExtractor data;
+ if (m_opcode.GetData(data))
{
- const bool can_branch = mc_disasm_ptr->CanBranch(inst);
- if (can_branch)
+ bool is_alternate_isa;
+ lldb::addr_t pc = m_address.GetFileAddress();
+
+ DisassemblerLLVMC::LLVMCDisassembler *mc_disasm_ptr = GetDisasmToUse (is_alternate_isa);
+ const uint8_t *opcode_data = data.GetDataStart();
+ const size_t opcode_data_len = data.GetByteSize();
+ llvm::MCInst inst;
+ const size_t inst_size = mc_disasm_ptr->GetMCInst (opcode_data,
+ opcode_data_len,
+ pc,
+ inst);
+ // Be conservative, if we didn't understand the instruction, say it might branch...
+ if (inst_size == 0)
m_does_branch = eLazyBoolYes;
else
- m_does_branch = eLazyBoolNo;
+ {
+ const bool can_branch = mc_disasm_ptr->CanBranch(inst);
+ if (can_branch)
+ m_does_branch = eLazyBoolYes;
+ else
+ m_does_branch = eLazyBoolNo;
+ }
}
+ disasm_sp->Unlock();
}
- GetDisassemblerLLVMC().Unlock();
}
return m_does_branch == eLazyBoolYes;
}
@@ -105,34 +109,38 @@ public:
{
if (m_has_delay_slot == eLazyBoolCalculate)
{
- GetDisassemblerLLVMC().Lock(this, NULL);
- DataExtractor data;
- if (m_opcode.GetData(data))
+ std::shared_ptr<DisassemblerLLVMC> disasm_sp(GetDisassembler());
+ if (disasm_sp)
{
- bool is_alternate_isa;
- lldb::addr_t pc = m_address.GetFileAddress();
-
- DisassemblerLLVMC::LLVMCDisassembler *mc_disasm_ptr = GetDisasmToUse (is_alternate_isa);
- const uint8_t *opcode_data = data.GetDataStart();
- const size_t opcode_data_len = data.GetByteSize();
- llvm::MCInst inst;
- const size_t inst_size = mc_disasm_ptr->GetMCInst (opcode_data,
- opcode_data_len,
- pc,
- inst);
- // if we didn't understand the instruction, say it doesn't have a delay slot...
- if (inst_size == 0)
- m_has_delay_slot = eLazyBoolNo;
- else
+ disasm_sp->Lock(this, NULL);
+ DataExtractor data;
+ if (m_opcode.GetData(data))
{
- const bool has_delay_slot = mc_disasm_ptr->HasDelaySlot(inst);
- if (has_delay_slot)
- m_has_delay_slot = eLazyBoolYes;
- else
+ bool is_alternate_isa;
+ lldb::addr_t pc = m_address.GetFileAddress();
+
+ DisassemblerLLVMC::LLVMCDisassembler *mc_disasm_ptr = GetDisasmToUse (is_alternate_isa);
+ const uint8_t *opcode_data = data.GetDataStart();
+ const size_t opcode_data_len = data.GetByteSize();
+ llvm::MCInst inst;
+ const size_t inst_size = mc_disasm_ptr->GetMCInst (opcode_data,
+ opcode_data_len,
+ pc,
+ inst);
+ // if we didn't understand the instruction, say it doesn't have a delay slot...
+ if (inst_size == 0)
m_has_delay_slot = eLazyBoolNo;
+ else
+ {
+ const bool has_delay_slot = mc_disasm_ptr->HasDelaySlot(inst);
+ if (has_delay_slot)
+ m_has_delay_slot = eLazyBoolYes;
+ else
+ m_has_delay_slot = eLazyBoolNo;
+ }
}
+ disasm_sp->Unlock();
}
- GetDisassemblerLLVMC().Unlock();
}
return m_has_delay_slot == eLazyBoolYes;
}
@@ -141,18 +149,22 @@ public:
GetDisasmToUse (bool &is_alternate_isa)
{
is_alternate_isa = false;
- DisassemblerLLVMC &llvm_disasm = GetDisassemblerLLVMC();
- if (llvm_disasm.m_alternate_disasm_ap.get() != NULL)
+ std::shared_ptr<DisassemblerLLVMC> disasm_sp(GetDisassembler());
+ if (disasm_sp)
{
- const AddressClass address_class = GetAddressClass ();
-
- if (address_class == eAddressClassCodeAlternateISA)
+ if (disasm_sp->m_alternate_disasm_ap.get() != NULL)
{
- is_alternate_isa = true;
- return llvm_disasm.m_alternate_disasm_ap.get();
+ const AddressClass address_class = GetAddressClass ();
+
+ if (address_class == eAddressClassCodeAlternateISA)
+ {
+ is_alternate_isa = true;
+ return disasm_sp->m_alternate_disasm_ap.get();
+ }
}
+ return disasm_sp->m_disasm_ap.get();
}
- return llvm_disasm.m_disasm_ap.get();
+ return nullptr;
}
size_t
@@ -163,101 +175,105 @@ public:
// All we have to do is read the opcode which can be easy for some
// architectures
bool got_op = false;
- DisassemblerLLVMC &llvm_disasm = GetDisassemblerLLVMC();
- const ArchSpec &arch = llvm_disasm.GetArchitecture();
- const lldb::ByteOrder byte_order = data.GetByteOrder();
-
- const uint32_t min_op_byte_size = arch.GetMinimumOpcodeByteSize();
- const uint32_t max_op_byte_size = arch.GetMaximumOpcodeByteSize();
- if (min_op_byte_size == max_op_byte_size)
+ std::shared_ptr<DisassemblerLLVMC> disasm_sp(GetDisassembler());
+ if (disasm_sp)
{
- // Fixed size instructions, just read that amount of data.
- if (!data.ValidOffsetForDataOfSize(data_offset, min_op_byte_size))
- return false;
+ const ArchSpec &arch = disasm_sp->GetArchitecture();
+ const lldb::ByteOrder byte_order = data.GetByteOrder();
- switch (min_op_byte_size)
- {
- case 1:
- m_opcode.SetOpcode8 (data.GetU8 (&data_offset), byte_order);
- got_op = true;
- break;
-
- case 2:
- m_opcode.SetOpcode16 (data.GetU16 (&data_offset), byte_order);
- got_op = true;
- break;
-
- case 4:
- m_opcode.SetOpcode32 (data.GetU32 (&data_offset), byte_order);
- got_op = true;
- break;
-
- case 8:
- m_opcode.SetOpcode64 (data.GetU64 (&data_offset), byte_order);
- got_op = true;
- break;
-
- default:
- m_opcode.SetOpcodeBytes(data.PeekData(data_offset, min_op_byte_size), min_op_byte_size);
- got_op = true;
- break;
- }
- }
- if (!got_op)
- {
- bool is_alternate_isa = false;
- DisassemblerLLVMC::LLVMCDisassembler *mc_disasm_ptr = GetDisasmToUse (is_alternate_isa);
+ const uint32_t min_op_byte_size = arch.GetMinimumOpcodeByteSize();
+ const uint32_t max_op_byte_size = arch.GetMaximumOpcodeByteSize();
+ if (min_op_byte_size == max_op_byte_size)
+ {
+ // Fixed size instructions, just read that amount of data.
+ if (!data.ValidOffsetForDataOfSize(data_offset, min_op_byte_size))
+ return false;
+
+ switch (min_op_byte_size)
+ {
+ case 1:
+ m_opcode.SetOpcode8 (data.GetU8 (&data_offset), byte_order);
+ got_op = true;
+ break;
+
+ case 2:
+ m_opcode.SetOpcode16 (data.GetU16 (&data_offset), byte_order);
+ got_op = true;
+ break;
+
+ case 4:
+ m_opcode.SetOpcode32 (data.GetU32 (&data_offset), byte_order);
+ got_op = true;
+ break;
+
+ case 8:
+ m_opcode.SetOpcode64 (data.GetU64 (&data_offset), byte_order);
+ got_op = true;
+ break;
- const llvm::Triple::ArchType machine = arch.GetMachine();
- if (machine == llvm::Triple::arm || machine == llvm::Triple::thumb)
+ default:
+ m_opcode.SetOpcodeBytes(data.PeekData(data_offset, min_op_byte_size), min_op_byte_size);
+ got_op = true;
+ break;
+ }
+ }
+ if (!got_op)
{
- if (machine == llvm::Triple::thumb || is_alternate_isa)
+ bool is_alternate_isa = false;
+ DisassemblerLLVMC::LLVMCDisassembler *mc_disasm_ptr = GetDisasmToUse (is_alternate_isa);
+
+ const llvm::Triple::ArchType machine = arch.GetMachine();
+ if (machine == llvm::Triple::arm || machine == llvm::Triple::thumb)
{
- uint32_t thumb_opcode = data.GetU16(&data_offset);
- if ((thumb_opcode & 0xe000) != 0xe000 || ((thumb_opcode & 0x1800u) == 0))
+ if (machine == llvm::Triple::thumb || is_alternate_isa)
{
- m_opcode.SetOpcode16 (thumb_opcode, byte_order);
- m_is_valid = true;
+ uint32_t thumb_opcode = data.GetU16(&data_offset);
+ if ((thumb_opcode & 0xe000) != 0xe000 || ((thumb_opcode & 0x1800u) == 0))
+ {
+ m_opcode.SetOpcode16 (thumb_opcode, byte_order);
+ m_is_valid = true;
+ }
+ else
+ {
+ thumb_opcode <<= 16;
+ thumb_opcode |= data.GetU16(&data_offset);
+ m_opcode.SetOpcode16_2 (thumb_opcode, byte_order);
+ m_is_valid = true;
+ }
}
else
{
- thumb_opcode <<= 16;
- thumb_opcode |= data.GetU16(&data_offset);
- m_opcode.SetOpcode16_2 (thumb_opcode, byte_order);
+ m_opcode.SetOpcode32 (data.GetU32(&data_offset), byte_order);
m_is_valid = true;
}
}
else
{
- m_opcode.SetOpcode32 (data.GetU32(&data_offset), byte_order);
- m_is_valid = true;
- }
- }
- else
- {
- // The opcode isn't evenly sized, so we need to actually use the llvm
- // disassembler to parse it and get the size.
- uint8_t *opcode_data = const_cast<uint8_t *>(data.PeekData (data_offset, 1));
- const size_t opcode_data_len = data.BytesLeft(data_offset);
- const addr_t pc = m_address.GetFileAddress();
- llvm::MCInst inst;
-
- llvm_disasm.Lock(this, NULL);
- const size_t inst_size = mc_disasm_ptr->GetMCInst(opcode_data,
- opcode_data_len,
- pc,
- inst);
- llvm_disasm.Unlock();
- if (inst_size == 0)
- m_opcode.Clear();
- else
- {
- m_opcode.SetOpcodeBytes(opcode_data, inst_size);
- m_is_valid = true;
+ // The opcode isn't evenly sized, so we need to actually use the llvm
+ // disassembler to parse it and get the size.
+ uint8_t *opcode_data = const_cast<uint8_t *>(data.PeekData (data_offset, 1));
+ const size_t opcode_data_len = data.BytesLeft(data_offset);
+ const addr_t pc = m_address.GetFileAddress();
+ llvm::MCInst inst;
+
+ disasm_sp->Lock(this, NULL);
+ const size_t inst_size = mc_disasm_ptr->GetMCInst(opcode_data,
+ opcode_data_len,
+ pc,
+ inst);
+ disasm_sp->Unlock();
+ if (inst_size == 0)
+ m_opcode.Clear();
+ else
+ {
+ m_opcode.SetOpcodeBytes(opcode_data, inst_size);
+ m_is_valid = true;
+ }
}
}
+ return m_opcode.GetByteSize();
}
- return m_opcode.GetByteSize();
+ return 0;
}
void
@@ -283,146 +299,148 @@ public:
std::string out_string;
std::string comment_string;
- DisassemblerLLVMC &llvm_disasm = GetDisassemblerLLVMC();
-
- DisassemblerLLVMC::LLVMCDisassembler *mc_disasm_ptr;
+ std::shared_ptr<DisassemblerLLVMC> disasm_sp(GetDisassembler());
+ if (disasm_sp)
+ {
+ DisassemblerLLVMC::LLVMCDisassembler *mc_disasm_ptr;
- if (address_class == eAddressClassCodeAlternateISA)
- mc_disasm_ptr = llvm_disasm.m_alternate_disasm_ap.get();
- else
- mc_disasm_ptr = llvm_disasm.m_disasm_ap.get();
+ if (address_class == eAddressClassCodeAlternateISA)
+ mc_disasm_ptr = disasm_sp->m_alternate_disasm_ap.get();
+ else
+ mc_disasm_ptr = disasm_sp->m_disasm_ap.get();
- lldb::addr_t pc = m_address.GetFileAddress();
- m_using_file_addr = true;
+ lldb::addr_t pc = m_address.GetFileAddress();
+ m_using_file_addr = true;
- const bool data_from_file = GetDisassemblerLLVMC().m_data_from_file;
- bool use_hex_immediates = true;
- Disassembler::HexImmediateStyle hex_style = Disassembler::eHexStyleC;
+ const bool data_from_file = disasm_sp->m_data_from_file;
+ bool use_hex_immediates = true;
+ Disassembler::HexImmediateStyle hex_style = Disassembler::eHexStyleC;
- if (exe_ctx)
- {
- Target *target = exe_ctx->GetTargetPtr();
- if (target)
+ if (exe_ctx)
{
- use_hex_immediates = target->GetUseHexImmediates();
- hex_style = target->GetHexImmediateStyle();
-
- if (!data_from_file)
+ Target *target = exe_ctx->GetTargetPtr();
+ if (target)
{
- const lldb::addr_t load_addr = m_address.GetLoadAddress(target);
- if (load_addr != LLDB_INVALID_ADDRESS)
+ use_hex_immediates = target->GetUseHexImmediates();
+ hex_style = target->GetHexImmediateStyle();
+
+ if (!data_from_file)
{
- pc = load_addr;
- m_using_file_addr = false;
+ const lldb::addr_t load_addr = m_address.GetLoadAddress(target);
+ if (load_addr != LLDB_INVALID_ADDRESS)
+ {
+ pc = load_addr;
+ m_using_file_addr = false;
+ }
}
}
}
- }
- llvm_disasm.Lock(this, exe_ctx);
+ disasm_sp->Lock(this, exe_ctx);
- const uint8_t *opcode_data = data.GetDataStart();
- const size_t opcode_data_len = data.GetByteSize();
- llvm::MCInst inst;
- size_t inst_size = mc_disasm_ptr->GetMCInst (opcode_data,
- opcode_data_len,
- pc,
- inst);
+ const uint8_t *opcode_data = data.GetDataStart();
+ const size_t opcode_data_len = data.GetByteSize();
+ llvm::MCInst inst;
+ size_t inst_size = mc_disasm_ptr->GetMCInst (opcode_data,
+ opcode_data_len,
+ pc,
+ inst);
- if (inst_size > 0)
- {
- mc_disasm_ptr->SetStyle(use_hex_immediates, hex_style);
- mc_disasm_ptr->PrintMCInst(inst, out_string, comment_string);
-
- if (!comment_string.empty())
+ if (inst_size > 0)
{
- AppendComment(comment_string);
+ mc_disasm_ptr->SetStyle(use_hex_immediates, hex_style);
+ mc_disasm_ptr->PrintMCInst(inst, out_string, comment_string);
+
+ if (!comment_string.empty())
+ {
+ AppendComment(comment_string);
+ }
}
- }
- llvm_disasm.Unlock();
+ disasm_sp->Unlock();
- if (inst_size == 0)
- {
- m_comment.assign ("unknown opcode");
- inst_size = m_opcode.GetByteSize();
- StreamString mnemonic_strm;
- lldb::offset_t offset = 0;
- lldb::ByteOrder byte_order = data.GetByteOrder();
- switch (inst_size)
+ if (inst_size == 0)
{
- case 1:
- {
- const uint8_t uval8 = data.GetU8 (&offset);
- m_opcode.SetOpcode8 (uval8, byte_order);
- m_opcode_name.assign (".byte");
- mnemonic_strm.Printf("0x%2.2x", uval8);
- }
- break;
- case 2:
- {
- const uint16_t uval16 = data.GetU16(&offset);
- m_opcode.SetOpcode16(uval16, byte_order);
- m_opcode_name.assign (".short");
- mnemonic_strm.Printf("0x%4.4x", uval16);
- }
- break;
- case 4:
- {
- const uint32_t uval32 = data.GetU32(&offset);
- m_opcode.SetOpcode32(uval32, byte_order);
- m_opcode_name.assign (".long");
- mnemonic_strm.Printf("0x%8.8x", uval32);
- }
- break;
- case 8:
- {
- const uint64_t uval64 = data.GetU64(&offset);
- m_opcode.SetOpcode64(uval64, byte_order);
- m_opcode_name.assign (".quad");
- mnemonic_strm.Printf("0x%16.16" PRIx64, uval64);
- }
- break;
- default:
- if (inst_size == 0)
- return;
- else
- {
- const uint8_t *bytes = data.PeekData(offset, inst_size);
- if (bytes == NULL)
+ m_comment.assign ("unknown opcode");
+ inst_size = m_opcode.GetByteSize();
+ StreamString mnemonic_strm;
+ lldb::offset_t offset = 0;
+ lldb::ByteOrder byte_order = data.GetByteOrder();
+ switch (inst_size)
+ {
+ case 1:
+ {
+ const uint8_t uval8 = data.GetU8 (&offset);
+ m_opcode.SetOpcode8 (uval8, byte_order);
+ m_opcode_name.assign (".byte");
+ mnemonic_strm.Printf("0x%2.2x", uval8);
+ }
+ break;
+ case 2:
+ {
+ const uint16_t uval16 = data.GetU16(&offset);
+ m_opcode.SetOpcode16(uval16, byte_order);
+ m_opcode_name.assign (".short");
+ mnemonic_strm.Printf("0x%4.4x", uval16);
+ }
+ break;
+ case 4:
+ {
+ const uint32_t uval32 = data.GetU32(&offset);
+ m_opcode.SetOpcode32(uval32, byte_order);
+ m_opcode_name.assign (".long");
+ mnemonic_strm.Printf("0x%8.8x", uval32);
+ }
+ break;
+ case 8:
+ {
+ const uint64_t uval64 = data.GetU64(&offset);
+ m_opcode.SetOpcode64(uval64, byte_order);
+ m_opcode_name.assign (".quad");
+ mnemonic_strm.Printf("0x%16.16" PRIx64, uval64);
+ }
+ break;
+ default:
+ if (inst_size == 0)
return;
- m_opcode_name.assign (".byte");
- m_opcode.SetOpcodeBytes(bytes, inst_size);
- mnemonic_strm.Printf("0x%2.2x", bytes[0]);
- for (uint32_t i=1; i<inst_size; ++i)
- mnemonic_strm.Printf(" 0x%2.2x", bytes[i]);
- }
- break;
+ else
+ {
+ const uint8_t *bytes = data.PeekData(offset, inst_size);
+ if (bytes == NULL)
+ return;
+ m_opcode_name.assign (".byte");
+ m_opcode.SetOpcodeBytes(bytes, inst_size);
+ mnemonic_strm.Printf("0x%2.2x", bytes[0]);
+ for (uint32_t i=1; i<inst_size; ++i)
+ mnemonic_strm.Printf(" 0x%2.2x", bytes[i]);
+ }
+ break;
+ }
+ m_mnemonics.swap(mnemonic_strm.GetString());
+ return;
}
- m_mnemonics.swap(mnemonic_strm.GetString());
- return;
- }
- else
- {
- if (m_does_branch == eLazyBoolCalculate)
+ else
{
- const bool can_branch = mc_disasm_ptr->CanBranch(inst);
- if (can_branch)
- m_does_branch = eLazyBoolYes;
- else
- m_does_branch = eLazyBoolNo;
+ if (m_does_branch == eLazyBoolCalculate)
+ {
+ const bool can_branch = mc_disasm_ptr->CanBranch(inst);
+ if (can_branch)
+ m_does_branch = eLazyBoolYes;
+ else
+ m_does_branch = eLazyBoolNo;
+ }
}
- }
- static RegularExpression s_regex("[ \t]*([^ ^\t]+)[ \t]*([^ ^\t].*)?");
+ static RegularExpression s_regex("[ \t]*([^ ^\t]+)[ \t]*([^ ^\t].*)?");
- RegularExpression::Match matches(3);
+ RegularExpression::Match matches(3);
- if (s_regex.Execute(out_string.c_str(), &matches))
- {
- matches.GetMatchAtIndex(out_string.c_str(), 1, m_opcode_name);
- matches.GetMatchAtIndex(out_string.c_str(), 2, m_mnemonics);
+ if (s_regex.Execute(out_string.c_str(), &matches))
+ {
+ matches.GetMatchAtIndex(out_string.c_str(), 1, m_opcode_name);
+ matches.GetMatchAtIndex(out_string.c_str(), 2, m_mnemonics);
+ }
}
}
}
@@ -444,14 +462,14 @@ public:
return m_opcode.GetByteSize();
}
- DisassemblerLLVMC &
- GetDisassemblerLLVMC ()
+ std::shared_ptr<DisassemblerLLVMC>
+ GetDisassembler ()
{
- return *(DisassemblerLLVMC *)m_disasm_sp.get();
+ return m_disasm_wp.lock();
}
protected:
- DisassemblerSP m_disasm_sp; // for ownership
+ std::weak_ptr<DisassemblerLLVMC> m_disasm_wp;
LazyBool m_does_branch;
LazyBool m_has_delay_slot;
bool m_is_valid;
More information about the lldb-commits
mailing list