tatyana-krasnukha updated this revision to Diff 129127.
tatyana-krasnukha added a comment.
Added function Create that creates an instance of LLVMCDisassembler only if
pass all constraints.
Moved LLVMCDisassembler declaration to .cpp file, renamed to MCDisasmToolset
(is this name ok?).
Added const qualifier to some functions of the class.
I also had the courage to remove ‘_ap’ suffixes from unique pointers.
https://reviews.llvm.org/D41584
Files:
source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h
Index: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h
===================================================================
--- source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h
+++ source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h
@@ -16,62 +16,14 @@
#include <mutex>
#include <string>
-// Other libraries and framework includes
-#include "llvm-c/Disassembler.h"
-
// Project includes
#include "lldb/Core/Address.h"
#include "lldb/Core/Disassembler.h"
#include "lldb/Core/PluginManager.h"
-// Opaque references to C++ Objects in LLVM's MC.
-namespace llvm {
-class MCContext;
-class MCInst;
-class MCInstrInfo;
-class MCRegisterInfo;
-class MCDisassembler;
-class MCInstPrinter;
-class MCAsmInfo;
-class MCSubtargetInfo;
-} // namespace llvm
-
class InstructionLLVMC;
class DisassemblerLLVMC : public lldb_private::Disassembler {
- // Since we need to make two actual MC Disassemblers for ARM (ARM & THUMB),
- // and there's a bit of goo to set up and own
- // in the MC disassembler world, I added this class to manage the actual
- // disassemblers.
- class LLVMCDisassembler {
- public:
- LLVMCDisassembler(const char *triple, const char *cpu,
- const char *features_str, unsigned flavor,
- DisassemblerLLVMC &owner);
-
- ~LLVMCDisassembler();
-
- uint64_t GetMCInst(const uint8_t *opcode_data, size_t opcode_data_len,
- lldb::addr_t pc, llvm::MCInst &mc_inst);
- void PrintMCInst(llvm::MCInst &mc_inst, std::string &inst_string,
- std::string &comments_string);
- void SetStyle(bool use_hex_immed, HexImmediateStyle hex_style);
- bool CanBranch(llvm::MCInst &mc_inst);
- bool HasDelaySlot(llvm::MCInst &mc_inst);
- bool IsCall(llvm::MCInst &mc_inst);
- bool IsValid() { return m_is_valid; }
-
- private:
- bool m_is_valid;
- std::unique_ptr<llvm::MCContext> m_context_ap;
- std::unique_ptr<llvm::MCAsmInfo> m_asm_info_ap;
- std::unique_ptr<llvm::MCSubtargetInfo> m_subtarget_info_ap;
- std::unique_ptr<llvm::MCInstrInfo> m_instr_info_ap;
- std::unique_ptr<llvm::MCRegisterInfo> m_reg_info_ap;
- std::unique_ptr<llvm::MCInstPrinter> m_instr_printer_ap;
- std::unique_ptr<llvm::MCDisassembler> m_disasm_ap;
- };
-
public:
DisassemblerLLVMC(const lldb_private::ArchSpec &arch,
const char *flavor /* = NULL */);
@@ -108,9 +60,7 @@
bool FlavorValidForArchSpec(const lldb_private::ArchSpec &arch,
const char *flavor) override;
- bool IsValid() {
- return (m_disasm_ap.get() != NULL && m_disasm_ap->IsValid());
- }
+ bool IsValid() const;
int OpInfo(uint64_t PC, uint64_t Offset, uint64_t Size, int TagType,
void *TagBug);
@@ -145,8 +95,13 @@
std::mutex m_mutex;
bool m_data_from_file;
- std::unique_ptr<LLVMCDisassembler> m_disasm_ap;
- std::unique_ptr<LLVMCDisassembler> m_alternate_disasm_ap;
+ // Since we need to make two actual MC Disassemblers for ARM (ARM & THUMB),
+ // and there's a bit of goo to set up and own
+ // in the MC disassembler world, I added this class to manage the actual
+ // disassemblers.
+ class MCDisasmToolset;
+ std::unique_ptr<MCDisasmToolset> m_disasm;
+ std::unique_ptr<MCDisasmToolset> m_alternate_disasm;
};
#endif // liblldb_DisassemblerLLVM_h_
Index: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
===================================================================
--- source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
+++ source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp
@@ -48,6 +48,41 @@
using namespace lldb;
using namespace lldb_private;
+class DisassemblerLLVMC::MCDisasmToolset {
+public:
+ static std::unique_ptr<MCDisasmToolset>
+ Create(const char *triple, const char *cpu, const char *features_str,
+ unsigned flavor, DisassemblerLLVMC &owner);
+
+ ~MCDisasmToolset() = default;
+
+ uint64_t GetMCInst(const uint8_t *opcode_data, size_t opcode_data_len,
+ lldb::addr_t pc, llvm::MCInst &mc_inst) const;
+ void PrintMCInst(llvm::MCInst &mc_inst, std::string &inst_string,
+ std::string &comments_string);
+ void SetStyle(bool use_hex_immed, HexImmediateStyle hex_style);
+ bool CanBranch(llvm::MCInst &mc_inst) const;
+ bool HasDelaySlot(llvm::MCInst &mc_inst) const;
+ bool IsCall(llvm::MCInst &mc_inst) const;
+
+private:
+ MCDisasmToolset(std::unique_ptr<llvm::MCInstrInfo> &&instr_info,
+ std::unique_ptr<llvm::MCRegisterInfo> &®_info,
+ std::unique_ptr<llvm::MCSubtargetInfo> &&subtarget_info,
+ std::unique_ptr<llvm::MCAsmInfo> &&asm_info,
+ std::unique_ptr<llvm::MCContext> &&context,
+ std::unique_ptr<llvm::MCDisassembler> &&disasm,
+ std::unique_ptr<llvm::MCInstPrinter> &&instr_printer);
+
+ std::unique_ptr<llvm::MCInstrInfo> m_instr_info;
+ std::unique_ptr<llvm::MCRegisterInfo> m_reg_info;
+ std::unique_ptr<llvm::MCSubtargetInfo> m_subtarget_info;
+ std::unique_ptr<llvm::MCAsmInfo> m_asm_info;
+ std::unique_ptr<llvm::MCContext> m_context;
+ std::unique_ptr<llvm::MCDisassembler> m_disasm;
+ std::unique_ptr<llvm::MCInstPrinter> m_instr_printer;
+};
+
class InstructionLLVMC : public lldb_private::Instruction {
public:
InstructionLLVMC(DisassemblerLLVMC &disasm,
@@ -72,7 +107,7 @@
bool is_alternate_isa;
lldb::addr_t pc = m_address.GetFileAddress();
- DisassemblerLLVMC::LLVMCDisassembler *mc_disasm_ptr =
+ DisassemblerLLVMC::MCDisasmToolset *mc_disasm_ptr =
GetDisasmToUse(is_alternate_isa);
const uint8_t *opcode_data = data.GetDataStart();
const size_t opcode_data_len = data.GetByteSize();
@@ -107,7 +142,7 @@
bool is_alternate_isa;
lldb::addr_t pc = m_address.GetFileAddress();
- DisassemblerLLVMC::LLVMCDisassembler *mc_disasm_ptr =
+ DisassemblerLLVMC::MCDisasmToolset *mc_disasm_ptr =
GetDisasmToUse(is_alternate_isa);
const uint8_t *opcode_data = data.GetDataStart();
const size_t opcode_data_len = data.GetByteSize();
@@ -132,19 +167,19 @@
return m_has_delay_slot == eLazyBoolYes;
}
- DisassemblerLLVMC::LLVMCDisassembler *GetDisasmToUse(bool &is_alternate_isa) {
+ DisassemblerLLVMC::MCDisasmToolset *GetDisasmToUse(bool &is_alternate_isa) {
is_alternate_isa = false;
std::shared_ptr<DisassemblerLLVMC> disasm_sp(GetDisassembler());
if (disasm_sp) {
- if (disasm_sp->m_alternate_disasm_ap.get() != NULL) {
+ if (disasm_sp->m_alternate_disasm) {
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_alternate_disasm.get();
}
}
- return disasm_sp->m_disasm_ap.get();
+ return disasm_sp->m_disasm.get();
}
return nullptr;
}
@@ -197,7 +232,7 @@
}
if (!got_op) {
bool is_alternate_isa = false;
- DisassemblerLLVMC::LLVMCDisassembler *mc_disasm_ptr =
+ DisassemblerLLVMC::MCDisasmToolset *mc_disasm_ptr =
GetDisasmToUse(is_alternate_isa);
const llvm::Triple::ArchType machine = arch.GetMachine();
@@ -264,12 +299,12 @@
std::shared_ptr<DisassemblerLLVMC> disasm_sp(GetDisassembler());
if (disasm_sp) {
- DisassemblerLLVMC::LLVMCDisassembler *mc_disasm_ptr;
+ DisassemblerLLVMC::MCDisasmToolset *mc_disasm_ptr;
if (address_class == eAddressClassCodeAlternateISA)
- mc_disasm_ptr = disasm_sp->m_alternate_disasm_ap.get();
+ mc_disasm_ptr = disasm_sp->m_alternate_disasm.get();
else
- mc_disasm_ptr = disasm_sp->m_disasm_ap.get();
+ mc_disasm_ptr = disasm_sp->m_disasm.get();
lldb::addr_t pc = m_address.GetFileAddress();
m_using_file_addr = true;
@@ -850,7 +885,7 @@
bool is_alternate_isa;
lldb::addr_t pc = m_address.GetFileAddress();
- DisassemblerLLVMC::LLVMCDisassembler *mc_disasm_ptr =
+ DisassemblerLLVMC::MCDisasmToolset *mc_disasm_ptr =
GetDisasmToUse(is_alternate_isa);
const uint8_t *opcode_data = data.GetDataStart();
const size_t opcode_data_len = data.GetByteSize();
@@ -881,81 +916,101 @@
bool m_using_file_addr;
};
-DisassemblerLLVMC::LLVMCDisassembler::LLVMCDisassembler(
- const char *triple, const char *cpu, const char *features_str,
- unsigned flavor, DisassemblerLLVMC &owner)
- : m_is_valid(true) {
+std::unique_ptr<DisassemblerLLVMC::MCDisasmToolset>
+DisassemblerLLVMC::MCDisasmToolset::Create(const char *triple, const char *cpu,
+ const char *features_str,
+ unsigned flavor,
+ DisassemblerLLVMC &owner) {
+ using Instance = std::unique_ptr<DisassemblerLLVMC::MCDisasmToolset>;
+
std::string Status;
const llvm::Target *curr_target =
llvm::TargetRegistry::lookupTarget(triple, Status);
- if (!curr_target) {
- m_is_valid = false;
- return;
- }
+ if (!curr_target)
+ return Instance();
- m_instr_info_ap.reset(curr_target->createMCInstrInfo());
- m_reg_info_ap.reset(curr_target->createMCRegInfo(triple));
+ std::unique_ptr<llvm::MCInstrInfo> instr_info(
+ curr_target->createMCInstrInfo());
+ if (!instr_info)
+ return Instance();
- m_subtarget_info_ap.reset(
- curr_target->createMCSubtargetInfo(triple, cpu, features_str));
-
std::unique_ptr<llvm::MCRegisterInfo> reg_info(
curr_target->createMCRegInfo(triple));
- m_asm_info_ap.reset(curr_target->createMCAsmInfo(*reg_info, triple));
+ if (!reg_info)
+ return Instance();
- if (m_instr_info_ap.get() == NULL || m_reg_info_ap.get() == NULL ||
- m_subtarget_info_ap.get() == NULL || m_asm_info_ap.get() == NULL) {
- m_is_valid = false;
- return;
- }
+ std::unique_ptr<llvm::MCSubtargetInfo> subtarget_info(
+ curr_target->createMCSubtargetInfo(triple, cpu, features_str));
+ if (!subtarget_info)
+ return Instance();
- m_context_ap.reset(
- new llvm::MCContext(m_asm_info_ap.get(), m_reg_info_ap.get(), 0));
+ std::unique_ptr<llvm::MCAsmInfo> asm_info(
+ curr_target->createMCAsmInfo(*reg_info, triple));
+ if (!asm_info)
+ return Instance();
- m_disasm_ap.reset(curr_target->createMCDisassembler(
- *m_subtarget_info_ap.get(), *m_context_ap.get()));
- if (m_disasm_ap.get() && m_context_ap.get()) {
- std::unique_ptr<llvm::MCRelocationInfo> RelInfo(
- curr_target->createMCRelocationInfo(triple, *m_context_ap.get()));
- if (!RelInfo) {
- m_is_valid = false;
- return;
- }
- std::unique_ptr<llvm::MCSymbolizer> symbolizer_up(
- curr_target->createMCSymbolizer(
- triple, NULL, DisassemblerLLVMC::SymbolLookupCallback,
- (void *)&owner, m_context_ap.get(), std::move(RelInfo)));
- m_disasm_ap->setSymbolizer(std::move(symbolizer_up));
+ std::unique_ptr<llvm::MCContext> context(
+ new llvm::MCContext(asm_info.get(), reg_info.get(), 0));
+ if (!context)
+ return Instance();
- unsigned asm_printer_variant;
- if (flavor == ~0U)
- asm_printer_variant = m_asm_info_ap->getAssemblerDialect();
- else {
- asm_printer_variant = flavor;
- }
+ std::unique_ptr<llvm::MCDisassembler> disasm(
+ curr_target->createMCDisassembler(*subtarget_info, *context));
+ if (!disasm)
+ return Instance();
- m_instr_printer_ap.reset(curr_target->createMCInstPrinter(
- llvm::Triple{triple}, asm_printer_variant, *m_asm_info_ap.get(),
- *m_instr_info_ap.get(), *m_reg_info_ap.get()));
- if (m_instr_printer_ap.get() == NULL) {
- m_disasm_ap.reset();
- m_is_valid = false;
- }
- } else
- m_is_valid = false;
+ std::unique_ptr<llvm::MCRelocationInfo> rel_info(
+ curr_target->createMCRelocationInfo(triple, *context));
+ if (!rel_info)
+ return Instance();
+
+ std::unique_ptr<llvm::MCSymbolizer> symbolizer(
+ curr_target->createMCSymbolizer(
+ triple, NULL, DisassemblerLLVMC::SymbolLookupCallback, (void *)&owner,
+ context.get(), std::move(rel_info)));
+ disasm->setSymbolizer(std::move(symbolizer));
+
+ unsigned asm_printer_variant =
+ flavor == ~0U ? asm_info->getAssemblerDialect() : flavor;
+
+ std::unique_ptr<llvm::MCInstPrinter> instr_printer(
+ curr_target->createMCInstPrinter(llvm::Triple{triple},
+ asm_printer_variant, *asm_info,
+ *instr_info, *reg_info));
+ if (!instr_printer)
+ return Instance();
+
+ return Instance(new MCDisasmToolset(
+ std::move(instr_info), std::move(reg_info), std::move(subtarget_info),
+ std::move(asm_info), std::move(context), std::move(disasm),
+ std::move(instr_printer)));
}
-DisassemblerLLVMC::LLVMCDisassembler::~LLVMCDisassembler() = default;
+DisassemblerLLVMC::MCDisasmToolset::MCDisasmToolset(
+ std::unique_ptr<llvm::MCInstrInfo> &&instr_info,
+ std::unique_ptr<llvm::MCRegisterInfo> &®_info,
+ std::unique_ptr<llvm::MCSubtargetInfo> &&subtarget_info,
+ std::unique_ptr<llvm::MCAsmInfo> &&asm_info,
+ std::unique_ptr<llvm::MCContext> &&context,
+ std::unique_ptr<llvm::MCDisassembler> &&disasm,
+ std::unique_ptr<llvm::MCInstPrinter> &&instr_printer)
+ : m_instr_info(std::move(instr_info)), m_reg_info(std::move(reg_info)),
+ m_subtarget_info(std::move(subtarget_info)),
+ m_asm_info(std::move(asm_info)), m_context(std::move(context)),
+ m_disasm(std::move(disasm)), m_instr_printer(std::move(instr_printer)) {
+ assert(m_instr_info && m_reg_info && m_subtarget_info && m_asm_info &&
+ m_context && disasm && instr_printer);
+}
-uint64_t DisassemblerLLVMC::LLVMCDisassembler::GetMCInst(
+uint64_t DisassemblerLLVMC::MCDisasmToolset::GetMCInst(
const uint8_t *opcode_data, size_t opcode_data_len, lldb::addr_t pc,
- llvm::MCInst &mc_inst) {
+ llvm::MCInst &mc_inst) const {
llvm::ArrayRef<uint8_t> data(opcode_data, opcode_data_len);
llvm::MCDisassembler::DecodeStatus status;
uint64_t new_inst_size;
- status = m_disasm_ap->getInstruction(mc_inst, new_inst_size, data, pc,
- llvm::nulls(), llvm::nulls());
+ status = m_disasm->getInstruction(mc_inst, new_inst_size, data, pc,
+ llvm::nulls(), llvm::nulls());
if (status == llvm::MCDisassembler::Success)
return new_inst_size;
else
@@ -962,16 +1017,16 @@
return 0;
}
-void DisassemblerLLVMC::LLVMCDisassembler::PrintMCInst(
+void DisassemblerLLVMC::MCDisasmToolset::PrintMCInst(
llvm::MCInst &mc_inst, std::string &inst_string,
std::string &comments_string) {
llvm::raw_string_ostream inst_stream(inst_string);
llvm::raw_string_ostream comments_stream(comments_string);
- m_instr_printer_ap->setCommentStream(comments_stream);
- m_instr_printer_ap->printInst(&mc_inst, inst_stream, llvm::StringRef(),
- *m_subtarget_info_ap);
- m_instr_printer_ap->setCommentStream(llvm::nulls());
+ m_instr_printer->setCommentStream(comments_stream);
+ m_instr_printer->printInst(&mc_inst, inst_stream, llvm::StringRef(),
+ *m_subtarget_info);
+ m_instr_printer->setCommentStream(llvm::nulls());
comments_stream.flush();
static std::string g_newlines("\r\n");
@@ -985,30 +1040,32 @@
}
}
-void DisassemblerLLVMC::LLVMCDisassembler::SetStyle(
- bool use_hex_immed, HexImmediateStyle hex_style) {
- m_instr_printer_ap->setPrintImmHex(use_hex_immed);
+void DisassemblerLLVMC::MCDisasmToolset::SetStyle(bool use_hex_immed,
+ HexImmediateStyle hex_style) {
+ m_instr_printer->setPrintImmHex(use_hex_immed);
switch (hex_style) {
case eHexStyleC:
- m_instr_printer_ap->setPrintHexStyle(llvm::HexStyle::C);
+ m_instr_printer->setPrintHexStyle(llvm::HexStyle::C);
break;
case eHexStyleAsm:
- m_instr_printer_ap->setPrintHexStyle(llvm::HexStyle::Asm);
+ m_instr_printer->setPrintHexStyle(llvm::HexStyle::Asm);
break;
}
}
-bool DisassemblerLLVMC::LLVMCDisassembler::CanBranch(llvm::MCInst &mc_inst) {
- return m_instr_info_ap->get(mc_inst.getOpcode())
- .mayAffectControlFlow(mc_inst, *m_reg_info_ap.get());
+bool DisassemblerLLVMC::MCDisasmToolset::CanBranch(
+ llvm::MCInst &mc_inst) const {
+ return m_instr_info->get(mc_inst.getOpcode())
+ .mayAffectControlFlow(mc_inst, *m_reg_info);
}
-bool DisassemblerLLVMC::LLVMCDisassembler::HasDelaySlot(llvm::MCInst &mc_inst) {
- return m_instr_info_ap->get(mc_inst.getOpcode()).hasDelaySlot();
+bool DisassemblerLLVMC::MCDisasmToolset::HasDelaySlot(
+ llvm::MCInst &mc_inst) const {
+ return m_instr_info->get(mc_inst.getOpcode()).hasDelaySlot();
}
-bool DisassemblerLLVMC::LLVMCDisassembler::IsCall(llvm::MCInst &mc_inst) {
- return m_instr_info_ap->get(mc_inst.getOpcode()).isCall();
+bool DisassemblerLLVMC::MCDisasmToolset::IsCall(llvm::MCInst &mc_inst) const {
+ return m_instr_info->get(mc_inst.getOpcode()).isCall();
}
DisassemblerLLVMC::DisassemblerLLVMC(const ArchSpec &arch,
@@ -1132,15 +1189,11 @@
if (triple.getArch() == llvm::Triple::aarch64)
features_str += "+v8.2a";
- m_disasm_ap.reset(new LLVMCDisassembler(triple_str, cpu, features_str.c_str(),
- flavor, *this));
- if (!m_disasm_ap->IsValid()) {
- // We use m_disasm_ap.get() to tell whether we are valid or not, so if this
- // isn't good for some reason,
- // we reset it, and then we won't be valid and FindPlugin will fail and we
- // won't get used.
- m_disasm_ap.reset();
- }
+ // We use m_disasm_ap.get() to tell whether we are valid or not,
+ // so if this isn't good for some reason,
+ // we won't be valid and FindPlugin will fail and we won't get used.
+ m_disasm = MCDisasmToolset::Create(triple_str, cpu, features_str.c_str(),
+ flavor, *this);
llvm::Triple::ArchType llvm_arch = triple.getArch();
@@ -1148,12 +1201,11 @@
// thumb instruction disassembler.
if (llvm_arch == llvm::Triple::arm) {
std::string thumb_triple(thumb_arch.GetTriple().getTriple());
- m_alternate_disasm_ap.reset(
- new LLVMCDisassembler(thumb_triple.c_str(), "", "", flavor, *this));
- if (!m_alternate_disasm_ap->IsValid()) {
- m_disasm_ap.reset();
- m_alternate_disasm_ap.reset();
- }
+ m_alternate_disasm =
+ MCDisasmToolset::Create(thumb_triple.c_str(), "", "", flavor, *this);
+ if (!m_alternate_disasm)
+ m_disasm.reset();
+
} else if (llvm_arch == llvm::Triple::mips ||
llvm_arch == llvm::Triple::mipsel ||
llvm_arch == llvm::Triple::mips64 ||
@@ -1165,12 +1217,10 @@
else if (arch_flags & ArchSpec::eMIPSAse_micromips)
features_str += "+micromips,";
- m_alternate_disasm_ap.reset(new LLVMCDisassembler(
- triple_str, cpu, features_str.c_str(), flavor, *this));
- if (!m_alternate_disasm_ap->IsValid()) {
- m_disasm_ap.reset();
- m_alternate_disasm_ap.reset();
- }
+ m_alternate_disasm = MCDisasmToolset::Create(
+ triple_str, cpu, features_str.c_str(), flavor, *this);
+ if (!m_alternate_disasm)
+ m_disasm.reset();
}
}
@@ -1210,7 +1260,7 @@
AddressClass address_class = eAddressClassCode;
- if (m_alternate_disasm_ap.get() != NULL)
+ if (m_alternate_disasm)
address_class = inst_addr.GetAddressClass();
InstructionSP inst_sp(
@@ -1285,6 +1335,8 @@
return false;
}
+bool DisassemblerLLVMC::IsValid() const { return m_disasm.operator bool(); }
+
int DisassemblerLLVMC::OpInfo(uint64_t PC, uint64_t Offset, uint64_t Size,
int tag_type, void *tag_bug) {
switch (tag_type) {
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
