Fixes to the X86 disassembler. The disassembler will now

return an error status in all failure cases, printing
messages to debugs() only when debugging is enabled.


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@100229 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Target/X86/Disassembler/X86Disassembler.cpp

@@ -21,8 +21,8 @@
 #include "llvm/MC/MCDisassembler.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/Target/TargetRegistry.h"
+#include "llvm/Support/Debug.h"
 #include "llvm/Support/MemoryObject.h"
-#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include "X86GenRegisterNames.inc"
@@ -30,6 +30,14 @@
 using namespace llvm;
 using namespace llvm::X86Disassembler;
 
+void x86DisassemblerDebug(const char *file,
+                          unsigned line,
+                          const char *s) {
+  dbgs() << file << ":" << line << ": " << s;
+}
+
+#define debug(s) DEBUG(x86DisassemblerDebug(__FILE__, __LINE__, s));
+
 namespace llvm {  
   
 // Fill-ins to make the compiler happy.  These constants are never actually
@@ -50,7 +58,7 @@ extern Target TheX86_32Target, TheX86_64Target;
 
 }
 
-static void translateInstruction(MCInst &target,
+static bool translateInstruction(MCInst &target,
                                 InternalInstruction &source);
 
 X86GenericDisassembler::X86GenericDisassembler(DisassemblerMode mode) :
@@ -112,8 +120,7 @@ bool X86GenericDisassembler::getInstruction(MCInst &instr,
   }
   else {
     size = internalInstr.length;
-    translateInstruction(instr, internalInstr);
+    return !translateInstruction(instr, internalInstr);
-    return true;
   }
 }
 
@@ -151,29 +158,35 @@ static void translateImmediate(MCInst &mcInst, uint64_t immediate) {
 /// @param mcInst       - The MCInst to append to.
 /// @param insn         - The internal instruction to extract the R/M field
 ///                       from.
-static void translateRMRegister(MCInst &mcInst,
+/// @return             - 0 on success; -1 otherwise
+static bool translateRMRegister(MCInst &mcInst,
                                 InternalInstruction &insn) {
-  assert(insn.eaBase != EA_BASE_sib && insn.eaBase != EA_BASE_sib64 && 
+  if (insn.eaBase == EA_BASE_sib || insn.eaBase == EA_BASE_sib64) {
-         "A R/M register operand may not have a SIB byte");
+    debug("A R/M register operand may not have a SIB byte");
+    return true;
+  }
   
   switch (insn.eaBase) {
+  default:
+    debug("Unexpected EA base register");
+    return true;
   case EA_BASE_NONE:
-    llvm_unreachable("EA_BASE_NONE for ModR/M base");
+    debug("EA_BASE_NONE for ModR/M base");
-    break;
+    return true;
 #define ENTRY(x) case EA_BASE_##x:
   ALL_EA_BASES
 #undef ENTRY
-    llvm_unreachable("A R/M register operand may not have a base; "
+    debug("A R/M register operand may not have a base; "
           "the operand must be a register.");
-    break;
+    return true;
 #define ENTRY(x)                                                      \
   case EA_REG_##x:                                                    \
     mcInst.addOperand(MCOperand::CreateReg(X86::x)); break;
   ALL_REGS
 #undef ENTRY
-  default:
-    llvm_unreachable("Unexpected EA base register");
   }
+  
+  return false;
 }
 
 /// translateRMMemory - Translates a memory operand stored in the Mod and R/M
@@ -186,7 +199,8 @@ static void translateRMRegister(MCInst &mcInst,
 /// @param sr           - Whether or not to emit the segment register.  The
 ///                       LEA instruction does not expect a segment-register
 ///                       operand.
-static void translateRMMemory(MCInst &mcInst,
+/// @return             - 0 on success; nonzero otherwise
+static bool translateRMMemory(MCInst &mcInst,
                               InternalInstruction &insn,
                               bool sr) {
   // Addresses in an MCInst are represented as five operands:
@@ -211,7 +225,8 @@ static void translateRMMemory(MCInst &mcInst,
     if (insn.sibBase != SIB_BASE_NONE) {
       switch (insn.sibBase) {
       default:
-        llvm_unreachable("Unexpected sibBase");
+        debug("Unexpected sibBase");
+        return true;
 #define ENTRY(x)                                          \
       case SIB_BASE_##x:                                  \
         baseReg = MCOperand::CreateReg(X86::x); break;
@@ -225,7 +240,8 @@ static void translateRMMemory(MCInst &mcInst,
     if (insn.sibIndex != SIB_INDEX_NONE) {
       switch (insn.sibIndex) {
       default:
-        llvm_unreachable("Unexpected sibIndex");
+        debug("Unexpected sibIndex");
+        return true;
 #define ENTRY(x)                                          \
       case SIB_INDEX_##x:                                 \
         indexReg = MCOperand::CreateReg(X86::x); break;
@@ -241,9 +257,10 @@ static void translateRMMemory(MCInst &mcInst,
   } else {
     switch (insn.eaBase) {
     case EA_BASE_NONE:
-      assert(insn.eaDisplacement != EA_DISP_NONE && 
+      if (insn.eaDisplacement == EA_DISP_NONE) {
-             "EA_BASE_NONE and EA_DISP_NONE for ModR/M base");
+        debug("EA_BASE_NONE and EA_DISP_NONE for ModR/M base");
-      
+        return true;
+      }
       if (insn.mode == MODE_64BIT)
         baseReg = MCOperand::CreateReg(X86::RIP); // Section 2.2.1.6
       else
@@ -271,8 +288,8 @@ static void translateRMMemory(MCInst &mcInst,
       indexReg = MCOperand::CreateReg(0);
       switch (insn.eaBase) {
       default:
-        llvm_unreachable("Unexpected eaBase");
+        debug("Unexpected eaBase");
-        break;
+        return true;
         // Here, we will use the fill-ins defined above.  However,
         //   BX_SI, BX_DI, BP_SI, and BP_DI are all handled above and
         //   sib and sib64 were handled in the top-level if, so they're only
@@ -285,9 +302,9 @@ static void translateRMMemory(MCInst &mcInst,
 #define ENTRY(x) case EA_REG_##x:
       ALL_REGS
 #undef ENTRY
-        llvm_unreachable("A R/M memory operand may not be a register; "
+        debug("A R/M memory operand may not be a register; "
               "the base field must be a base.");
-            break;
+        return true;
       }
     }
     
@@ -315,6 +332,8 @@ static void translateRMMemory(MCInst &mcInst,
   
   if (sr)
     mcInst.addOperand(segmentReg);
+  
+  return false;
 }
 
 /// translateRM - Translates an operand stored in the R/M (and possibly SIB)
@@ -324,12 +343,14 @@ static void translateRMMemory(MCInst &mcInst,
 /// @param operand      - The operand, as stored in the descriptor table.
 /// @param insn         - The instruction to extract Mod, R/M, and SIB fields
 ///                       from.
-static void translateRM(MCInst &mcInst,
+/// @return             - 0 on success; nonzero otherwise
+static bool translateRM(MCInst &mcInst,
                        OperandSpecifier &operand,
                        InternalInstruction &insn) {
   switch (operand.type) {
   default:
-    llvm_unreachable("Unexpected type for a R/M operand");
+    debug("Unexpected type for a R/M operand");
+    return true;
   case TYPE_R8:
   case TYPE_R16:
   case TYPE_R32:
@@ -345,8 +366,7 @@ static void translateRM(MCInst &mcInst,
   case TYPE_DEBUGREG:
   case TYPE_CR32:
   case TYPE_CR64:
-    translateRMRegister(mcInst, insn);
+    return translateRMRegister(mcInst, insn);
-    break;
   case TYPE_M:
   case TYPE_M8:
   case TYPE_M16:
@@ -364,11 +384,9 @@ static void translateRM(MCInst &mcInst,
   case TYPE_M1616:
   case TYPE_M1632:
   case TYPE_M1664:
-    translateRMMemory(mcInst, insn, true);
+    return translateRMMemory(mcInst, insn, true);
-    break;
   case TYPE_LEA:
-    translateRMMemory(mcInst, insn, false);
+    return translateRMMemory(mcInst, insn, false);
-    break;
   }
 }
   
@@ -377,11 +395,17 @@ static void translateRM(MCInst &mcInst,
 ///
 /// @param mcInst       - The MCInst to append to.
 /// @param stackPos     - The stack position to translate.
-static void translateFPRegister(MCInst &mcInst,
+/// @return             - 0 on success; nonzero otherwise.
+static bool translateFPRegister(MCInst &mcInst,
                                uint8_t stackPos) {
-  assert(stackPos < 8 && "Invalid FP stack position");
+  if (stackPos >= 8) {
+    debug("Invalid FP stack position");
+    return true;
+  }
   
   mcInst.addOperand(MCOperand::CreateReg(X86::ST0 + stackPos));
+
+  return false;
 }
 
 /// translateOperand - Translates an operand stored in an internal instruction 
@@ -390,25 +414,27 @@ static void translateFPRegister(MCInst &mcInst,
 /// @param mcInst       - The MCInst to append to.
 /// @param operand      - The operand, as stored in the descriptor table.
 /// @param insn         - The internal instruction.
-static void translateOperand(MCInst &mcInst,
+/// @return             - false on success; true otherwise.
+static bool translateOperand(MCInst &mcInst,
                             OperandSpecifier &operand,
                             InternalInstruction &insn) {
   switch (operand.encoding) {
   default:
-    llvm_unreachable("Unhandled operand encoding during translation");
+    debug("Unhandled operand encoding during translation");
+    return true;
   case ENCODING_REG:
     translateRegister(mcInst, insn.reg);
-    break;
+    return false;
   case ENCODING_RM:
-    translateRM(mcInst, operand, insn);
+    return translateRM(mcInst, operand, insn);
-    break;
   case ENCODING_CB:
   case ENCODING_CW:
   case ENCODING_CD:
   case ENCODING_CP:
   case ENCODING_CO:
   case ENCODING_CT:
-    llvm_unreachable("Translation of code offsets isn't supported.");
+    debug("Translation of code offsets isn't supported.");
+    return true;
   case ENCODING_IB:
   case ENCODING_IW:
   case ENCODING_ID:
@@ -417,24 +443,22 @@ static void translateOperand(MCInst &mcInst,
   case ENCODING_Ia:
     translateImmediate(mcInst, 
                        insn.immediates[insn.numImmediatesTranslated++]);
-    break;
+    return false;
   case ENCODING_RB:
   case ENCODING_RW:
   case ENCODING_RD:
   case ENCODING_RO:
     translateRegister(mcInst, insn.opcodeRegister);
-    break;
+    return false;
   case ENCODING_I:
-    translateFPRegister(mcInst, insn.opcodeModifier);
+    return translateFPRegister(mcInst, insn.opcodeModifier);
-    break;
   case ENCODING_Rv:
     translateRegister(mcInst, insn.opcodeRegister);
-    break;
+    return false;
   case ENCODING_DUP:
-    translateOperand(mcInst,
+    return translateOperand(mcInst,
                             insn.spec->operands[operand.type - TYPE_DUP0],
                             insn);
-    break;
   }
 }
   
@@ -443,9 +467,13 @@ static void translateOperand(MCInst &mcInst,
 ///
 /// @param mcInst       - The MCInst to populate with the instruction's data.
 /// @param insn         - The internal instruction.
-static void translateInstruction(MCInst &mcInst,
+/// @return             - false on success; true otherwise.
+static bool translateInstruction(MCInst &mcInst,
                                 InternalInstruction &insn) {  
-  assert(insn.spec);
+  if (!insn.spec) {
+    debug("Instruction has no specification");
+    return true;
+  }
   
   mcInst.setOpcode(insn.instructionID);
   
@@ -454,10 +482,15 @@ static void translateInstruction(MCInst &mcInst,
   insn.numImmediatesTranslated = 0;
   
   for (index = 0; index < X86_MAX_OPERANDS; ++index) {
-    if (insn.spec->operands[index].encoding != ENCODING_NONE)                
+    if (insn.spec->operands[index].encoding != ENCODING_NONE) {
-      translateOperand(mcInst, insn.spec->operands[index], insn);
+      if (translateOperand(mcInst, insn.spec->operands[index], insn)) {
+        return true;
       }
     }
+  }
+  
+  return false;
+}
 
 static MCDisassembler *createX86_32Disassembler(const Target &T) {
   return new X86Disassembler::X86_32Disassembler;

lib/Target/X86/Disassembler/X86DisassemblerDecoder.c

@@ -13,7 +13,6 @@
  *
  *===----------------------------------------------------------------------===*/
 
-#include <assert.h>   /* for assert()     */
 #include <stdarg.h>   /* for va_*()       */
 #include <stdio.h>    /* for vsnprintf()  */
 #include <stdlib.h>   /* for exit()       */
@@ -26,17 +25,20 @@
 #define TRUE  1
 #define FALSE 0
 
+typedef int8_t bool;
+
 #ifdef __GNUC__
 #define NORETURN __attribute__((noreturn))
 #else
 #define NORETURN
 #endif
 
-#define unreachable(s)                                      \
+#ifndef NDEBUG
-  do {                                                      \
+#define debug(s) do { x86DisassemblerDebug(__FILE__, __LINE__, s); } while (0)
-    fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, s);  \
+#else
-    exit(-1);                                               \
+#define debug(s) do { } while (0)
-  } while (0);
+#endif
+
 
 /*
  * contextForAttrs - Client for the instruction context table.  Takes a set of
@@ -84,7 +86,6 @@ static int modRMRequired(OpcodeType type,
   return decision->opcodeDecisions[insnContext].modRMDecisions[opcode].
     modrm_type != MODRM_ONEENTRY;
   
-  unreachable("Unknown opcode type");
   return 0;
 }
 
@@ -96,6 +97,7 @@ static int modRMRequired(OpcodeType type,
  * @param insnContext - See modRMRequired().
  * @param opcode      - See modRMRequired().
  * @param modRM       - The ModR/M byte if required, or any value if not.
+ * @return            - The UID of the instruction, or 0 on failure.
  */
 static InstrUID decode(OpcodeType type,
                        InstructionContext insnContext,
@@ -105,7 +107,8 @@ static InstrUID decode(OpcodeType type,
   
   switch (type) {
   default:
-    unreachable("Unknown opcode type");
+    debug("Unknown opcode type");
+    return 0;
   case ONEBYTE:
     dec = &ONEBYTE_SYM.opcodeDecisions[insnContext].modRMDecisions[opcode];
     break;
@@ -122,7 +125,8 @@ static InstrUID decode(OpcodeType type,
   
   switch (dec->modrm_type) {
   default:
-    unreachable("Corrupt table!  Unknown modrm_type");
+    debug("Corrupt table!  Unknown modrm_type");
+    return 0;
   case MODRM_ONEENTRY:
     return dec->instructionIDs[0];
   case MODRM_SPLITRM:
@@ -133,8 +137,6 @@ static InstrUID decode(OpcodeType type,
   case MODRM_FULL:
     return dec->instructionIDs[modRM];
   }
-  
-  return 0;
 }
 
 /*
@@ -342,7 +344,8 @@ static int readPrefixes(struct InternalInstruction* insn) {
         insn->segmentOverride = SEG_OVERRIDE_GS;
         break;
       default:
-        unreachable("Unhandled override");
+        debug("Unhandled override");
+        return -1;
       }
       if (prefixGroups[1])
         dbgprintf(insn, "Redundant Group 2 prefix");
@@ -792,7 +795,8 @@ static int readSIB(struct InternalInstruction* insn) {
                        SIB_BASE_EBP : SIB_BASE_RBP);
       break;
     case 0x3:
-      unreachable("Cannot have Mod = 0b11 and a SIB byte");
+      debug("Cannot have Mod = 0b11 and a SIB byte");
+      return -1;
     }
     break;
   default:
@@ -993,7 +997,9 @@ static int readModRM(struct InternalInstruction* insn) {
     *valid = 1;                                           \
     switch (type) {                                       \
     default:                                              \
-      unreachable("Unhandled register type");             \
+      debug("Unhandled register type");                   \
+      *valid = 0;                                         \
+      return 0;                                           \
     case TYPE_Rv:                                         \
       return base + index;                                \
     case TYPE_R8:                                         \
@@ -1050,6 +1056,7 @@ static int readModRM(struct InternalInstruction* insn) {
  * @param index - The existing value of the field as reported by readModRM().
  * @param valid - The address of a uint8_t.  The target is set to 1 if the
  *                field is valid for the register class; 0 if not.
+ * @return      - The proper value.
  */
 GENERIC_FIXUP_FUNC(fixupRegValue, insn->regBase,    MODRM_REG)
 GENERIC_FIXUP_FUNC(fixupRMValue,  insn->eaRegBase,  EA_REG)
@@ -1071,7 +1078,8 @@ static int fixupReg(struct InternalInstruction *insn,
   
   switch ((OperandEncoding)op->encoding) {
   default:
-    unreachable("Expected a REG or R/M encoding in fixupReg");
+    debug("Expected a REG or R/M encoding in fixupReg");
+    return -1;
   case ENCODING_REG:
     insn->reg = (Reg)fixupRegValue(insn,
                                    (OperandType)op->type,
@@ -1102,26 +1110,29 @@ static int fixupReg(struct InternalInstruction *insn,
  * @param insn    - The instruction whose opcode field is to be read.
  * @param inModRM - Indicates that the opcode field is to be read from the
  *                  ModR/M extension; useful for escape opcodes
+ * @return        - 0 on success; nonzero otherwise.
  */
-static void readOpcodeModifier(struct InternalInstruction* insn) {
+static int readOpcodeModifier(struct InternalInstruction* insn) {
   dbgprintf(insn, "readOpcodeModifier()");
   
   if (insn->consumedOpcodeModifier)
-    return;
+    return 0;
   
   insn->consumedOpcodeModifier = TRUE;
   
   switch (insn->spec->modifierType) {
   default:
-    unreachable("Unknown modifier type.");
+    debug("Unknown modifier type.");
+    return -1;
   case MODIFIER_NONE:
-    unreachable("No modifier but an operand expects one.");
+    debug("No modifier but an operand expects one.");
+    return -1;
   case MODIFIER_OPCODE:
     insn->opcodeModifier = insn->opcode - insn->spec->modifierBase;
-    break;
+    return 0;
   case MODIFIER_MODRM:
     insn->opcodeModifier = insn->modRM - insn->spec->modifierBase;
-    break;
+    return 0;
   }  
 }
 
@@ -1134,11 +1145,13 @@ static void readOpcodeModifier(struct InternalInstruction* insn) {
  * @param size  - The width (in bytes) of the register being specified.
  *                1 means AL and friends, 2 means AX, 4 means EAX, and 8 means
  *                RAX.
+ * @return      - 0 on success; nonzero otherwise.
  */
-static void readOpcodeRegister(struct InternalInstruction* insn, uint8_t size) {
+static int readOpcodeRegister(struct InternalInstruction* insn, uint8_t size) {
   dbgprintf(insn, "readOpcodeRegister()");
 
-  readOpcodeModifier(insn);
+  if (readOpcodeModifier(insn))
+    return -1;
   
   if (size == 0)
     size = insn->registerSize;
@@ -1161,7 +1174,7 @@ static void readOpcodeRegister(struct InternalInstruction* insn, uint8_t size) {
                                     | insn->opcodeModifier));
     break;
   case 4:
-    insn->opcodeRegister = (Reg)(MODRM_REG_EAX +
+    insn->opcodeRegister = (Reg)(MODRM_REG_EAX
                                  + ((bFromREX(insn->rexPrefix) << 3) 
                                     | insn->opcodeModifier));
     break;
@@ -1171,6 +1184,8 @@ static void readOpcodeRegister(struct InternalInstruction* insn, uint8_t size) {
                                     | insn->opcodeModifier));
     break;
   }
+  
+  return 0;
 }
 
 /*
@@ -1190,8 +1205,10 @@ static int readImmediate(struct InternalInstruction* insn, uint8_t size) {
   
   dbgprintf(insn, "readImmediate()");
   
-  if (insn->numImmediatesConsumed == 2)
+  if (insn->numImmediatesConsumed == 2) {
-    unreachable("Already consumed two immediates");
+    debug("Already consumed two immediates");
+    return -1;
+  }
   
   if (size == 0)
     size = insn->immediateSize;
@@ -1274,29 +1291,35 @@ static int readOperands(struct InternalInstruction* insn) {
         return -1;
       break;
     case ENCODING_Iv:
-      readImmediate(insn, insn->immediateSize);
+      if (readImmediate(insn, insn->immediateSize))
-      break;
+        return -1;
     case ENCODING_Ia:
-      readImmediate(insn, insn->addressSize);
+      if (readImmediate(insn, insn->addressSize))
+        return -1;
       break;
     case ENCODING_RB:
-      readOpcodeRegister(insn, 1);
+      if (readOpcodeRegister(insn, 1))
+        return -1;
       break;
     case ENCODING_RW:
-      readOpcodeRegister(insn, 2);
+      if (readOpcodeRegister(insn, 2))
+        return -1;
       break;
     case ENCODING_RD:
-      readOpcodeRegister(insn, 4);
+      if (readOpcodeRegister(insn, 4))
+        return -1;
       break;
     case ENCODING_RO:
-      readOpcodeRegister(insn, 8);
+      if (readOpcodeRegister(insn, 8))
+        return -1;
       break;
     case ENCODING_Rv:
-      readOpcodeRegister(insn, 0);
+      if (readOpcodeRegister(insn, 0))
+        return -1;
       break;
     case ENCODING_I:
-      readOpcodeModifier(insn);
+      if (readOpcodeModifier(insn))
-      break;
+        return -1;
     case ENCODING_DUP:
       break;
     default:

lib/Target/X86/Disassembler/X86DisassemblerDecoder.h

@@ -508,6 +508,17 @@ int decodeInstruction(struct InternalInstruction* insn,
                       uint64_t startLoc,
                       DisassemblerMode mode);
 
+/* x86DisassemblerDebug - C-accessible function for printing a message to
+ *   debugs()
+ * @param file  - The name of the file printing the debug message.
+ * @param line  - The line number that printed the debug message.
+ * @param s     - The message to print.
+ */
+  
+void x86DisassemblerDebug(const char *file,
+                          unsigned line,
+                          const char *s);
+
 #ifdef __cplusplus 
 }
 #endif