This is an archive of the discontinued LLVM Phabricator instance.

Differential D49001

[X86][Disassembler] Fix LOCK prefix disassembler support
ClosedPublic

Authored by maksfb on Jul 5 2018, 2:57 PM.

Details

Reviewers
craig.topper
Commits
rG89e4abe7b740: [X86][Disassembler] Fix LOCK prefix disassembler support
rL336400: [X86][Disassembler] Fix LOCK prefix disassembler support
Summary

If LOCK prefix is not the first prefix in an instruction, LLVM
disassembler silently drops the prefix.




The fix is to select a proper instruction with a builtin LOCK prefix if

one exists.
Diff Detail
Repository 
rL LLVM 
Event Timeline
maksfb created this revision.Jul 5 2018, 2:57 PM
Herald added a subscriber: llvm-commits.  ·  View Herald TranscriptJul 5 2018, 2:57 PM
craig.topper accepted this revision.Jul 5 2018, 3:17 PM
LGTM
This revision is now accepted and ready to land.Jul 5 2018, 3:17 PM
Closed by commit rL336400: [X86][Disassembler] Fix LOCK prefix disassembler support (authored by maks).  ·  Explain WhyJul 5 2018, 4:37 PM
This revision was automatically updated to reflect the committed changes.
Revision Contents
PathSize
llvm/
trunk/
lib/
Target/
X86/
Disassembler/
2 lines
2 lines
3 lines
test/
MC/
Disassembler/
X86/
4 lines
Diff 154334
llvm/trunk/lib/Target/X86/Disassembler/X86Disassembler.cpp
Show First 20 Lines • Show All 241 Lines • ▼ Show 20 Lines    if (!Ret) {

        if (InternalInstr.hasOpSize)
        if (InternalInstr.hasOpSize)

          Flags |= X86::IP_HAS_OP_SIZE;
          Flags |= X86::IP_HAS_OP_SIZE;

        if (InternalInstr.repeatPrefix == 0xf2)
        if (InternalInstr.repeatPrefix == 0xf2)

          Flags |= X86::IP_HAS_REPEAT_NE;
          Flags |= X86::IP_HAS_REPEAT_NE;

        else if (InternalInstr.repeatPrefix == 0xf3 &&
        else if (InternalInstr.repeatPrefix == 0xf3 &&

                 // It should not be 'pause' f3 90
                 // It should not be 'pause' f3 90

                 InternalInstr.opcode != 0x90)
                 InternalInstr.opcode != 0x90)

          Flags |= X86::IP_HAS_REPEAT;
          Flags |= X86::IP_HAS_REPEAT;

        if (InternalInstr.hasLockPrefix)

          Flags |= X86::IP_HAS_LOCK;

      }
      }

      Instr.setFlags(Flags);
      Instr.setFlags(Flags);

    }
    }

    return (!Ret) ? Success : Fail;
    return (!Ret) ? Success : Fail;

  }
  }

}
}




//
//

▲ Show 20 Lines • Show All 795 Lines • Show Last 20 Lines
llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.h
Show First 20 Lines • Show All 557 Lines • ▼ Show 20 Linesstruct InternalInstruction {

  SegmentOverride segmentOverride;
  SegmentOverride segmentOverride;

  // 1 if the prefix byte, 0xf2 or 0xf3 is xacquire or xrelease
  // 1 if the prefix byte, 0xf2 or 0xf3 is xacquire or xrelease

  bool xAcquireRelease;
  bool xAcquireRelease;




  // Address-size override
  // Address-size override

  bool hasAdSize;
  bool hasAdSize;

  // Operand-size override
  // Operand-size override

  bool hasOpSize;
  bool hasOpSize;

  // Lock prefix

  bool hasLockPrefix;

  // The repeat prefix if any
  // The repeat prefix if any

  uint8_t repeatPrefix;
  uint8_t repeatPrefix;




  // Sizes of various critical pieces of data, in bytes
  // Sizes of various critical pieces of data, in bytes

  uint8_t registerSize;
  uint8_t registerSize;

  uint8_t addressSize;
  uint8_t addressSize;

  uint8_t displacementSize;
  uint8_t displacementSize;

  uint8_t immediateSize;
  uint8_t immediateSize;

▲ Show 20 Lines • Show All 114 Lines • Show Last 20 Lines
llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp
Show First 20 Lines • Show All 292 Lines • ▼ Show 20 Lines
 * setPrefixPresent - Marks that a particular prefix is present as mandatory
 * setPrefixPresent - Marks that a particular prefix is present as mandatory

 *
 *

 * @param insn      - The instruction to be marked as having the prefix.
 * @param insn      - The instruction to be marked as having the prefix.

 * @param prefix    - The prefix that is present.
 * @param prefix    - The prefix that is present.

 */
 */

static void setPrefixPresent(struct InternalInstruction *insn, uint8_t prefix) {
static void setPrefixPresent(struct InternalInstruction *insn, uint8_t prefix) {

  uint8_t nextByte;
  uint8_t nextByte;

  switch (prefix) {
  switch (prefix) {

  case 0xf0:

    insn->hasLockPrefix = true;

    break;

  case 0xf2:
  case 0xf2:

  case 0xf3:
  case 0xf3:

    if (lookAtByte(insn, &nextByte))
    if (lookAtByte(insn, &nextByte))

      break;
      break;

    // TODO:
    // TODO:

    //  1. There could be several 0x66
    //  1. There could be several 0x66

    //  2. if (nextByte == 0x66) and nextNextByte != 0x0f then
    //  2. if (nextByte == 0x66) and nextNextByte != 0x0f then

    //      it's not mandatory prefix
    //      it's not mandatory prefix

▲ Show 20 Lines • Show All 1,648 Lines • Show Last 20 Lines
llvm/trunk/test/MC/Disassembler/X86/prefixes.txt
Show First 20 Lines • Show All 95 Lines • ▼ Show 20 Lines
0x44 0x0f 0x20 0xf8
0x44 0x0f 0x20 0xf8

# CHECK: movq %dr15, %rax
# CHECK: movq %dr15, %rax

0x44 0x0f 0x21 0xf8
0x44 0x0f 0x21 0xf8




# Test that MMX ignore REX.R and REX.B.
# Test that MMX ignore REX.R and REX.B.

# CHECK: movq %mm0, %mm1
# CHECK: movq %mm0, %mm1

0x46 0x0f 0x7f 0xc1
0x46 0x0f 0x7f 0xc1




# Test that lock prefix is not dropped if it's not the first prefix

# CHECK: lock cmpxchgw %di, (%rcx)

0x66 0xf0 0x0f 0xb1 0x39



# Test that a prefix on it's own works. It's debatable as to if this is 
# Test that a prefix on it's own works. It's debatable as to if this is 

# something that is considered valid, but however as LLVM's own disassembler
# something that is considered valid, but however as LLVM's own disassembler

# has decided to disassemble prefixes as being separate opcodes, it therefore 
# has decided to disassemble prefixes as being separate opcodes, it therefore 

# should be capable of re-consuming it's own output.
# should be capable of re-consuming it's own output.

# CHECK: rep
# CHECK: rep

0xf3
0xf3

# ***IMPORTANT ^-- this must be at the end of the file to be a valid test *** 
# ***IMPORTANT ^-- this must be at the end of the file to be a valid test ***