RamNalamothu updated this revision to Diff 546173.
RamNalamothu added a comment.
Herald added subscribers: luke, frasercrmck, sameer.abuasal, Jim, jocewei,
the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27,
niosHD, sabuasal, johnrusso, rbar.
Added RISC-V specific test which overrides MCInstrAnalysis methods and uses the
newly added code path.
Tests in `lldb/unittests/Disassembler/x86/TestGetControlFlowKindx86.cpp` use
MCInstrAnalysis default methods and need old behavior before this patch.
Tests in `lldb/unittests/Disassembler/RISCV/TestMCDisasmInstanceRISCV.cpp` use
RISC-V overridden MCInstrAnalysis methods and need new behavior from this patch.
Does this look good now?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156086/new/
https://reviews.llvm.org/D156086
Files:
lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
lldb/unittests/Disassembler/CMakeLists.txt
lldb/unittests/Disassembler/RISCV/CMakeLists.txt
lldb/unittests/Disassembler/RISCV/TestMCDisasmInstanceRISCV.cpp
lldb/unittests/Disassembler/x86/TestGetControlFlowKindx86.cpp
llvm/include/llvm/MC/MCInstrAnalysis.h
Index: llvm/include/llvm/MC/MCInstrAnalysis.h
===================================================================
--- llvm/include/llvm/MC/MCInstrAnalysis.h
+++ llvm/include/llvm/MC/MCInstrAnalysis.h
@@ -18,6 +18,7 @@
#include "llvm/MC/MCInst.h"
#include "llvm/MC/MCInstrDesc.h"
#include "llvm/MC/MCInstrInfo.h"
+#include "llvm/MC/MCRegisterInfo.h"
#include <cstdint>
#include <vector>
@@ -64,6 +65,19 @@
return Info->get(Inst.getOpcode()).isTerminator();
}
+ virtual bool mayAffectControlFlow(const MCInst &Inst,
+ const MCRegisterInfo &MCRI) const {
+ if (isBranch(Inst) || isCall(Inst) || isReturn(Inst) ||
+ isIndirectBranch(Inst))
+ return true;
+ unsigned PC = MCRI.getProgramCounter();
+ if (PC == 0)
+ return false;
+ if (Info->get(Inst.getOpcode()).hasDefOfPhysReg(Inst, PC, MCRI))
+ return true;
+ return false;
+ }
+
/// Returns true if at least one of the register writes performed by
/// \param Inst implicitly clears the upper portion of all super-registers.
///
Index: lldb/unittests/Disassembler/x86/TestGetControlFlowKindx86.cpp
===================================================================
--- lldb/unittests/Disassembler/x86/TestGetControlFlowKindx86.cpp
+++ lldb/unittests/Disassembler/x86/TestGetControlFlowKindx86.cpp
@@ -140,6 +140,17 @@
ExecutionContext exe_ctx (nullptr, nullptr, nullptr);
InstructionControlFlowKind kind = inst_sp->GetControlFlowKind(&exe_ctx);
EXPECT_EQ(kind, result[i]);
+
+ // Also, test the DisassemblerLLVMC::MCDisasmInstance methods.
+ if (kind == eInstructionControlFlowKindReturn)
+ EXPECT_FALSE(inst_sp->IsCall());
+ if (kind == eInstructionControlFlowKindCall)
+ EXPECT_TRUE(inst_sp->IsCall());
+ if (kind == eInstructionControlFlowKindCall ||
+ kind == eInstructionControlFlowKindJump ||
+ kind == eInstructionControlFlowKindCondJump ||
+ kind == eInstructionControlFlowKindReturn)
+ EXPECT_TRUE(inst_sp->DoesBranch());
}
}
}
Index: lldb/unittests/Disassembler/RISCV/TestMCDisasmInstanceRISCV.cpp
===================================================================
--- /dev/null
+++ lldb/unittests/Disassembler/RISCV/TestMCDisasmInstanceRISCV.cpp
@@ -0,0 +1,100 @@
+//===-- TextX86GetControlFlowKind.cpp ------------------------------------------===//
+
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Support/TargetSelect.h"
+#include "gtest/gtest.h"
+
+#include "lldb/Core/Address.h"
+#include "lldb/Core/Disassembler.h"
+#include "lldb/Target/ExecutionContext.h"
+#include "lldb/Utility/ArchSpec.h"
+
+#include "Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class TestMCDisasmInstanceRISCV : public testing::Test {
+public:
+ static void SetUpTestCase();
+ static void TearDownTestCase();
+
+protected:
+};
+
+void TestMCDisasmInstanceRISCV::SetUpTestCase() {
+ llvm::InitializeAllTargets();
+ llvm::InitializeAllAsmPrinters();
+ llvm::InitializeAllTargetMCs();
+ llvm::InitializeAllDisassemblers();
+ DisassemblerLLVMC::Initialize();
+}
+
+void TestMCDisasmInstanceRISCV::TearDownTestCase() {
+ DisassemblerLLVMC::Terminate();
+}
+
+TEST_F(TestMCDisasmInstanceRISCV, TestRISCV32Instruction) {
+ ArchSpec arch("riscv32-*-linux");
+
+ const unsigned num_of_instructions = 5;
+ uint8_t data[] = {
+ 0xef, 0x00, 0x00, 0x00, // call -- jal x1, 0
+ 0xe7, 0x00, 0x00, 0x00, // call -- jalr x1, x0, 0
+ 0x6f, 0x00, 0x00, 0x00, // jump -- jal x0, 0
+ 0x67, 0x00, 0x00, 0x00, // jump -- jalr x0, x0, 0
+ 0x67, 0x80, 0x00, 0x00 // ret -- jalr x0, x1, 0
+ };
+
+ DisassemblerSP disass_sp;
+ Address start_addr(0x100);
+ disass_sp =
+ Disassembler::DisassembleBytes(arch, nullptr, nullptr, start_addr, &data,
+ sizeof (data), num_of_instructions, false);
+
+ // If we failed to get a disassembler, we can assume it is because
+ // the llvm we linked against was not built with the i386 target,
+ // and we should skip these tests without marking anything as failing.
+
+ if (disass_sp) {
+ const InstructionList inst_list(disass_sp->GetInstructionList());
+ EXPECT_EQ(num_of_instructions, inst_list.GetSize());
+
+ for (size_t i = 0; i < num_of_instructions; ++i) {
+ InstructionSP inst_sp;
+ inst_sp = inst_list.GetInstructionAtIndex(i);
+ }
+ }
+
+ if (disass_sp) {
+ const InstructionList inst_list (disass_sp->GetInstructionList());
+ EXPECT_EQ (num_of_instructions, inst_list.GetSize());
+
+ InstructionSP inst_sp;
+ inst_sp = inst_list.GetInstructionAtIndex (0);
+ EXPECT_TRUE(inst_sp->IsCall());
+ EXPECT_TRUE(inst_sp->DoesBranch());
+
+ inst_sp = inst_list.GetInstructionAtIndex (1);
+ EXPECT_TRUE(inst_sp->IsCall());
+ EXPECT_TRUE(inst_sp->DoesBranch());
+
+ inst_sp = inst_list.GetInstructionAtIndex (2);
+ EXPECT_FALSE(inst_sp->IsCall());
+ EXPECT_TRUE(inst_sp->DoesBranch());
+
+ inst_sp = inst_list.GetInstructionAtIndex (3);
+ EXPECT_FALSE(inst_sp->IsCall());
+ EXPECT_TRUE(inst_sp->DoesBranch());
+
+ inst_sp = inst_list.GetInstructionAtIndex (4);
+ EXPECT_FALSE(inst_sp->IsCall());
+ EXPECT_TRUE(inst_sp->DoesBranch());
+ }
+}
Index: lldb/unittests/Disassembler/RISCV/CMakeLists.txt
===================================================================
--- /dev/null
+++ lldb/unittests/Disassembler/RISCV/CMakeLists.txt
@@ -0,0 +1,12 @@
+add_lldb_unittest(MCDisasmInstanceRISCVTests
+ TestMCDisasmInstanceRISCV.cpp
+ LINK_LIBS
+ lldbCore
+ lldbSymbol
+ lldbTarget
+ lldbPluginDisassemblerLLVMC
+ lldbPluginProcessUtility
+ LINK_COMPONENTS
+ Support
+ ${LLVM_TARGETS_TO_BUILD}
+ )
Index: lldb/unittests/Disassembler/CMakeLists.txt
===================================================================
--- lldb/unittests/Disassembler/CMakeLists.txt
+++ lldb/unittests/Disassembler/CMakeLists.txt
@@ -5,3 +5,7 @@
if("X86" IN_LIST LLVM_TARGETS_TO_BUILD)
add_subdirectory(x86)
endif()
+
+if("RISCV" IN_LIST LLVM_TARGETS_TO_BUILD)
+ add_subdirectory(RISCV)
+endif()
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===================================================================
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -18,6 +18,7 @@
#include "llvm/MC/MCDisassembler/MCRelocationInfo.h"
#include "llvm/MC/MCInst.h"
#include "llvm/MC/MCInstPrinter.h"
+#include "llvm/MC/MCInstrAnalysis.h"
#include "llvm/MC/MCInstrInfo.h"
#include "llvm/MC/MCRegisterInfo.h"
#include "llvm/MC/MCSubtargetInfo.h"
@@ -75,7 +76,8 @@
std::unique_ptr<llvm::MCAsmInfo> &&asm_info_up,
std::unique_ptr<llvm::MCContext> &&context_up,
std::unique_ptr<llvm::MCDisassembler> &&disasm_up,
- std::unique_ptr<llvm::MCInstPrinter> &&instr_printer_up);
+ std::unique_ptr<llvm::MCInstPrinter> &&instr_printer_up,
+ std::unique_ptr<llvm::MCInstrAnalysis> &&instr_analysis_up);
std::unique_ptr<llvm::MCInstrInfo> m_instr_info_up;
std::unique_ptr<llvm::MCRegisterInfo> m_reg_info_up;
@@ -84,6 +86,7 @@
std::unique_ptr<llvm::MCContext> m_context_up;
std::unique_ptr<llvm::MCDisassembler> m_disasm_up;
std::unique_ptr<llvm::MCInstPrinter> m_instr_printer_up;
+ std::unique_ptr<llvm::MCInstrAnalysis> m_instr_analysis_up;
};
namespace x86 {
@@ -1287,11 +1290,15 @@
if (!instr_printer_up)
return Instance();
- return Instance(
- new MCDisasmInstance(std::move(instr_info_up), std::move(reg_info_up),
- std::move(subtarget_info_up), std::move(asm_info_up),
- std::move(context_up), std::move(disasm_up),
- std::move(instr_printer_up)));
+ // Not all targets may have registered createMCInstrAnalysis().
+ std::unique_ptr<llvm::MCInstrAnalysis> instr_analysis_up(
+ curr_target->createMCInstrAnalysis(instr_info_up.get()));
+
+ return Instance(new MCDisasmInstance(
+ std::move(instr_info_up), std::move(reg_info_up),
+ std::move(subtarget_info_up), std::move(asm_info_up),
+ std::move(context_up), std::move(disasm_up), std::move(instr_printer_up),
+ std::move(instr_analysis_up)));
}
DisassemblerLLVMC::MCDisasmInstance::MCDisasmInstance(
@@ -1301,13 +1308,15 @@
std::unique_ptr<llvm::MCAsmInfo> &&asm_info_up,
std::unique_ptr<llvm::MCContext> &&context_up,
std::unique_ptr<llvm::MCDisassembler> &&disasm_up,
- std::unique_ptr<llvm::MCInstPrinter> &&instr_printer_up)
+ std::unique_ptr<llvm::MCInstPrinter> &&instr_printer_up,
+ std::unique_ptr<llvm::MCInstrAnalysis> &&instr_analysis_up)
: m_instr_info_up(std::move(instr_info_up)),
m_reg_info_up(std::move(reg_info_up)),
m_subtarget_info_up(std::move(subtarget_info_up)),
m_asm_info_up(std::move(asm_info_up)),
m_context_up(std::move(context_up)), m_disasm_up(std::move(disasm_up)),
- m_instr_printer_up(std::move(instr_printer_up)) {
+ m_instr_printer_up(std::move(instr_printer_up)),
+ m_instr_analysis_up(std::move(instr_analysis_up)) {
assert(m_instr_info_up && m_reg_info_up && m_subtarget_info_up &&
m_asm_info_up && m_context_up && m_disasm_up && m_instr_printer_up);
}
@@ -1365,6 +1374,8 @@
bool DisassemblerLLVMC::MCDisasmInstance::CanBranch(
llvm::MCInst &mc_inst) const {
+ if (m_instr_analysis_up)
+ return m_instr_analysis_up->mayAffectControlFlow(mc_inst, *m_reg_info_up);
return m_instr_info_up->get(mc_inst.getOpcode())
.mayAffectControlFlow(mc_inst, *m_reg_info_up);
}
@@ -1375,6 +1386,8 @@
}
bool DisassemblerLLVMC::MCDisasmInstance::IsCall(llvm::MCInst &mc_inst) const {
+ if (m_instr_analysis_up)
+ return m_instr_analysis_up->isCall(mc_inst);
return m_instr_info_up->get(mc_inst.getOpcode()).isCall();
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
