From 576ee003d0f53df9a2091741d881d79a05a70ca6 Mon Sep 17 00:00:00 2001 From: Richard Mitton Date: Fri, 30 Aug 2013 21:19:48 +0000 Subject: [PATCH] Fixed a bug where diassembling an instruction that had a prefix would cause LLVM to identify a 1-byte instruction, but then upon querying it for that 1-byte instruction would cause an undefined opcode. llvm-svn: 189698 --- .../X86/Disassembler/X86DisassemblerDecoder.c | 16 ++--- llvm/test/MC/Disassembler/X86/prefixes.txt | 59 +++++++++++++++++++ 2 files changed, 68 insertions(+), 7 deletions(-) create mode 100644 llvm/test/MC/Disassembler/X86/prefixes.txt diff --git a/llvm/lib/Target/X86/Disassembler/X86DisassemblerDecoder.c b/llvm/lib/Target/X86/Disassembler/X86DisassemblerDecoder.c index bb195ee31402..b63fd5aee3c7 100644 --- a/llvm/lib/Target/X86/Disassembler/X86DisassemblerDecoder.c +++ b/llvm/lib/Target/X86/Disassembler/X86DisassemblerDecoder.c @@ -314,20 +314,22 @@ static int readPrefixes(struct InternalInstruction* insn) { while (isPrefix) { prefixLocation = insn->readerCursor; + /* If we fail reading prefixes, just stop here and let the opcode reader deal with it */ if (consumeByte(insn, &byte)) - return -1; + break; /* * If the byte is a LOCK/REP/REPNE prefix and not a part of the opcode, then * break and let it be disassembled as a normal "instruction". */ + if (insn->readerCursor - 1 == insn->startLocation && byte == 0xf0) + break; + + uint8_t nextByte; if (insn->readerCursor - 1 == insn->startLocation - && (byte == 0xf0 || byte == 0xf2 || byte == 0xf3)) { - uint8_t nextByte; - if (byte == 0xf0) - break; - if (lookAtByte(insn, &nextByte)) - return -1; + && (byte == 0xf2 || byte == 0xf3) + && !lookAtByte(insn, &nextByte)) + { /* * If the byte is 0xf2 or 0xf3, and any of the following conditions are * met: diff --git a/llvm/test/MC/Disassembler/X86/prefixes.txt b/llvm/test/MC/Disassembler/X86/prefixes.txt new file mode 100644 index 000000000000..56596e387511 --- /dev/null +++ b/llvm/test/MC/Disassembler/X86/prefixes.txt @@ -0,0 +1,59 @@ +# RUN: llvm-mc --disassemble %s -triple=x86_64 | FileCheck %s + +# CHECK: lock +# CHECK-NEXT: orl $16, %fs:776 +0xf0 0x64 0x83 0x0c 0x25 0x08 0x03 0x00 0x00 0x10 + +# CHECK: movq %fs:768, %rdi +0x64 0x48 0x8b 0x3c 0x25 0x00 0x03 0x00 0x00 + +# CHECK: rep +# CHECK-NEXT: stosq +0xf3 0x48 0xab + +# CHECK: rep +# CHECK-NEXT: stosl +0xf3 0x67 0x48 0xab + +# CHECK: movl 32(%rbp), %eax +0x8b 0x45 0x20 + +# CHECK: movl %es:32(%rbp), %eax +0x26 0x8b 0x45 0x20 + +# CHECK: movl %es:32(%rbp), %eax +0x2e 0x26 0x8b 0x45 0x20 + +# Test that multiple prefixes stack. +# (todo- the correct disassembly is actually more like "es movl %cs:32(%rbp), %eax" +# but we don't support that) +# CHECK: movl %cs:32(%rbp), %eax +0x26 0x2e 0x8b 0x45 0x20 + +# Test that 0xf3 as part of the opcode works. +# CHECK: cvtdq2pd (%rax), %xmm0 +0xf3 0x0f 0xe6 0x00 + +# CHECK: pause +0xf3 0x90 + +# CHECK: nop +0x90 + +# CHECK: lock +# CHECK-NEXT: nop +0xf0 0x90 + +# Test that multiple redundant prefixes work (redundant, but valid x86). +# CHECK: rep +# CHECK-NEXT: rep +# CHECK-NEXT: stosq +0xf3 0xf3 0x48 0xab + +# 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 +# has decided to disassemble prefixes as being separate opcodes, it therefore +# should be capable of re-consuming it's own output. +# CHECK: rep +0xf3 +# ***IMPORTANT ^-- this must be at the end of the file to be a valid test ***