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