[llvm] cf9b3ef - Revert "[X86] Fix MCSymbolizer interface for X86Disassembler"

Maksim Panchenko via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 7 10:41:20 PST 2022


Author: Maksim Panchenko
Date: 2022-03-07T10:40:48-08:00
New Revision: cf9b3ef941d4b459943790e11971266e1d45db3a

URL: https://github.com/llvm/llvm-project/commit/cf9b3ef941d4b459943790e11971266e1d45db3a
DIFF: https://github.com/llvm/llvm-project/commit/cf9b3ef941d4b459943790e11971266e1d45db3a.diff

LOG: Revert "[X86] Fix MCSymbolizer interface for X86Disassembler"

This reverts commit 0c2b43ab8cb1067dd1c7899094b824890803a7d2.

Added: 
    

Modified: 
    llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp

Removed: 
    llvm/unittests/MC/X86/CMakeLists.txt
    llvm/unittests/MC/X86/X86MCDisassemblerTest.cpp


################################################################################
diff  --git a/llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp b/llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp
index f2f79003b255e..908eb6d1fab13 100644
--- a/llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp
+++ b/llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp
@@ -1809,11 +1809,11 @@ static void translateRegister(MCInst &mcInst, Reg reg) {
 /// @param isBranch   - If the instruction is a branch instruction
 /// @param Address    - The starting address of the instruction
 /// @param Offset     - The byte offset to this immediate in the instruction
-/// @param InstSize   - Size of the instruction in bytes
+/// @param Width      - The byte width of this immediate in the instruction
 ///
 /// If the getOpInfo() function was set when setupForSymbolicDisassembly() was
 /// called then that function is called to get any symbolic information for the
-/// immediate in the instruction using the Address, Offset and InstSize. If that
+/// immediate in the instruction using the Address, Offset and Width.  If that
 /// returns non-zero then the symbolic information it returns is used to create
 /// an MCExpr and that is added as an operand to the MCInst.  If getOpInfo()
 /// returns zero and isBranch is true then a symbol look up for immediate Value
@@ -1822,10 +1822,10 @@ static void translateRegister(MCInst &mcInst, Reg reg) {
 /// if it adds an operand to the MCInst and false otherwise.
 static bool tryAddingSymbolicOperand(int64_t Value, bool isBranch,
                                      uint64_t Address, uint64_t Offset,
-                                     uint64_t InstSize, MCInst &MI,
+                                     uint64_t Width, MCInst &MI,
                                      const MCDisassembler *Dis) {
-  return Dis->tryAddingSymbolicOperand(MI, Value, Address, isBranch, Offset,
-                                       InstSize);
+  return Dis->tryAddingSymbolicOperand(MI, Value, Address, isBranch,
+                                       Offset, Width);
 }
 
 /// tryAddingPcLoadReferenceComment - trys to add a comment as to what is being
@@ -1914,7 +1914,8 @@ static void translateImmediate(MCInst &mcInst, uint64_t immediate,
   uint64_t pcrel = 0;
   if (type == TYPE_REL) {
     isBranch = true;
-    pcrel = insn.startLocation + insn.length;
+    pcrel = insn.startLocation +
+            insn.immediateOffset + insn.immediateSize;
     switch (operand.encoding) {
     default:
       break;
@@ -1989,8 +1990,9 @@ static void translateImmediate(MCInst &mcInst, uint64_t immediate,
     break;
   }
 
-  if (!tryAddingSymbolicOperand(immediate + pcrel, isBranch, insn.startLocation,
-                                insn.immediateOffset, insn.length, mcInst, Dis))
+  if(!tryAddingSymbolicOperand(immediate + pcrel, isBranch, insn.startLocation,
+                               insn.immediateOffset, insn.immediateSize,
+                               mcInst, Dis))
     mcInst.addOperand(MCOperand::createImm(immediate));
 
   if (type == TYPE_MOFFS) {
@@ -2127,7 +2129,8 @@ static bool translateRMMemory(MCInst &mcInst, InternalInstruction &insn,
         return true;
       }
       if (insn.mode == MODE_64BIT){
-        pcrel = insn.startLocation + insn.length;
+        pcrel = insn.startLocation +
+                insn.displacementOffset + insn.displacementSize;
         tryAddingPcLoadReferenceComment(insn.startLocation +
                                         insn.displacementOffset,
                                         insn.displacement + pcrel, Dis);
@@ -2190,13 +2193,9 @@ static bool translateRMMemory(MCInst &mcInst, InternalInstruction &insn,
   mcInst.addOperand(baseReg);
   mcInst.addOperand(scaleAmount);
   mcInst.addOperand(indexReg);
-
-  const uint8_t dispOffset =
-      (insn.eaDisplacement == EA_DISP_NONE) ? 0 : insn.displacementOffset;
-
-  if (!tryAddingSymbolicOperand(insn.displacement + pcrel, false,
-                                insn.startLocation, dispOffset, insn.length,
-                                mcInst, Dis))
+  if(!tryAddingSymbolicOperand(insn.displacement + pcrel, false,
+                               insn.startLocation, insn.displacementOffset,
+                               insn.displacementSize, mcInst, Dis))
     mcInst.addOperand(displacement);
   mcInst.addOperand(segmentReg);
   return false;

diff  --git a/llvm/unittests/MC/X86/CMakeLists.txt b/llvm/unittests/MC/X86/CMakeLists.txt
deleted file mode 100644
index a68aac868fa20..0000000000000
--- a/llvm/unittests/MC/X86/CMakeLists.txt
+++ /dev/null
@@ -1,16 +0,0 @@
-include_directories(
-  ${CMAKE_SOURCE_DIR}/lib/Target/X86
-  ${CMAKE_BINARY_DIR}/lib/Target/X86
-  )
-
-set(LLVM_LINK_COMPONENTS
-  MC
-  MCDisassembler
-  Target
-  X86Desc
-  X86Disassembler
-  )
-
-add_llvm_unittest(X86MCTests
-  X86MCDisassemblerTest.cpp
-  )

diff  --git a/llvm/unittests/MC/X86/X86MCDisassemblerTest.cpp b/llvm/unittests/MC/X86/X86MCDisassemblerTest.cpp
deleted file mode 100644
index 8d677638ebda4..0000000000000
--- a/llvm/unittests/MC/X86/X86MCDisassemblerTest.cpp
+++ /dev/null
@@ -1,137 +0,0 @@
-//===- X86MCDisassemblerTest.cpp - Tests for X86 MCDisassembler -----------===//
-//
-// 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/MC/MCAsmInfo.h"
-#include "llvm/MC/MCContext.h"
-#include "llvm/MC/MCDisassembler/MCDisassembler.h"
-#include "llvm/MC/MCDisassembler/MCSymbolizer.h"
-#include "llvm/MC/MCInst.h"
-#include "llvm/MC/MCRegisterInfo.h"
-#include "llvm/MC/MCSubtargetInfo.h"
-#include "llvm/MC/MCTargetOptions.h"
-#include "llvm/MC/TargetRegistry.h"
-#include "llvm/Support/TargetSelect.h"
-#include "gtest/gtest.h"
-
-using namespace llvm;
-
-namespace {
-struct Context {
-  const char *TripleName = "x86_64-pc-linux";
-  std::unique_ptr<MCRegisterInfo> MRI;
-  std::unique_ptr<MCAsmInfo> MAI;
-  std::unique_ptr<MCContext> Ctx;
-  std::unique_ptr<MCSubtargetInfo> STI;
-  std::unique_ptr<MCDisassembler> DisAsm;
-
-  Context() {
-    LLVMInitializeX86TargetInfo();
-    LLVMInitializeX86TargetMC();
-    LLVMInitializeX86Disassembler();
-
-    // If we didn't build x86, do not run the test.
-    std::string Error;
-    const Target *TheTarget = TargetRegistry::lookupTarget(TripleName, Error);
-    if (!TheTarget)
-      return;
-
-    MRI.reset(TheTarget->createMCRegInfo(TripleName));
-    MAI.reset(TheTarget->createMCAsmInfo(*MRI, TripleName, MCTargetOptions()));
-    STI.reset(TheTarget->createMCSubtargetInfo(TripleName, "", ""));
-    Ctx = std::make_unique<MCContext>(Triple(TripleName), MAI.get(), MRI.get(),
-                                      STI.get());
-
-    DisAsm.reset(TheTarget->createMCDisassembler(*STI, *Ctx));
-  }
-
-  operator bool() { return Ctx.get(); }
-  operator MCContext &() { return *Ctx; };
-};
-
-Context &getContext() {
-  static Context Ctxt;
-  return Ctxt;
-}
-} // namespace
-
-class X86MCSymbolizerTest : public MCSymbolizer {
-public:
-  X86MCSymbolizerTest(MCContext &MC) : MCSymbolizer(MC, nullptr) {}
-  ~X86MCSymbolizerTest() {}
-
-  struct OpInfo {
-    int64_t Value = 0;
-    uint64_t Offset = 0;
-  };
-  std::vector<OpInfo> Operands;
-  uint64_t InstructionSize = 0;
-
-  void reset() {
-    Operands.clear();
-    InstructionSize = 0;
-  }
-
-  bool tryAddingSymbolicOperand(MCInst &Inst, raw_ostream &CStream,
-                                int64_t Value, uint64_t Address, bool IsBranch,
-                                uint64_t Offset, uint64_t InstSize) override {
-    Operands.push_back({Value, Offset});
-    InstructionSize = InstSize;
-    return false;
-  }
-
-  void tryAddingPcLoadReferenceComment(raw_ostream &cStream, int64_t Value,
-                                       uint64_t Address) override {}
-};
-
-TEST(X86Disassembler, X86MCSymbolizerTest) {
-  if (!getContext())
-    return;
-
-  X86MCSymbolizerTest *TestSymbolizer = new X86MCSymbolizerTest(getContext());
-  getContext().DisAsm->setSymbolizer(
-      std::unique_ptr<MCSymbolizer>(TestSymbolizer));
-
-  MCDisassembler::DecodeStatus Status;
-  MCInst Inst;
-  uint64_t InstSize;
-
-  auto checkBytes = [&](ArrayRef<uint8_t> Bytes) {
-    TestSymbolizer->reset();
-    Status =
-        getContext().DisAsm->getInstruction(Inst, InstSize, Bytes, 0, nulls());
-    ASSERT_TRUE(Status == MCDisassembler::Success);
-    EXPECT_EQ(TestSymbolizer->InstructionSize, InstSize);
-  };
-
-  auto checkOperand = [&](size_t OpNo, int64_t Value, uint64_t Offset) {
-    ASSERT_TRUE(TestSymbolizer->Operands.size() > OpNo);
-    EXPECT_EQ(TestSymbolizer->Operands[OpNo].Value, Value);
-    EXPECT_EQ(TestSymbolizer->Operands[OpNo].Offset, Offset);
-  };
-
-  // movq   $0xffffffffffffefe8,-0x1(%rip)
-  // Test that the value of the rip-relative operand is set correctly.
-  // The instruction address is 0 and the size is 12 bytes.
-  checkBytes(
-      {0x48, 0xc7, 0x05, 0xff, 0xff, 0xff, 0xff, 0xe8, 0xef, 0xff, 0xff});
-  checkOperand(0, /*next instr address*/ 11 - /*disp*/ 1, 3);
-  checkOperand(1, 0xffffffffffffefe8, 7);
-
-  // movq   $0xfffffffffffffef5,(%r12)
-  // Test that the displacement operand has an offset of 0, since it is not
-  // explicitly specified in the instruction.
-  checkBytes({0x49, 0xc7, 0x04, 0x24, 0xf5, 0xfe, 0xff, 0xff});
-  checkOperand(0, 0, 0);
-  checkOperand(1, 0xfffffffffffffef5, 4);
-
-  // movq    $0x80000, 0x80000
-  checkBytes(
-      {0x48, 0xc7, 0x04, 0x25, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x08, 0x00});
-  checkOperand(0, 0x80000, 4);
-  checkOperand(1, 0x80000, 8);
-}


        


More information about the llvm-commits mailing list