[PATCH] Fixed the Intel-syntax X86 disassembler to respect the option for hexadecimal immediates.

[Richard Mitton]

Any chance someone could commit this if it's OK?

Richard Mitton
richard at codersnotes.com

On 07/30/2013 05:33 PM, Richard Mitton wrote:
>    I hate arcanist so much already. I fixed this diff to *only* include the files actually relevant to this single patch.
>
> http://llvm-reviews.chandlerc.com/D1243
>
> CHANGE SINCE LAST DIFF
>    http://llvm-reviews.chandlerc.com/D1243?vs=3091&id=3094#toc
>
> Files:
>    include/llvm/MC/MCInstPrinter.h
>    lib/MC/MCInstPrinter.cpp
>    lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp
>    lib/Target/X86/InstPrinter/X86IntelInstPrinter.cpp
>
> Index: include/llvm/MC/MCInstPrinter.h
> ===================================================================
> --- include/llvm/MC/MCInstPrinter.h
> +++ include/llvm/MC/MCInstPrinter.h
> @@ -21,6 +21,13 @@
>   class MCRegisterInfo;
>   class StringRef;
>   
> +namespace HexStyle {
> +    enum Style {
> +        C,          ///< 0xff
> +        Asm         ///< 0ffh
> +    };
> +}
> +
>   /// MCInstPrinter - This is an instance of a target assembly language printer
>   /// that converts an MCInst to valid target assembly syntax.
>   class MCInstPrinter {
> @@ -42,13 +49,16 @@
>     /// True if we are printing immediates as hex.
>     bool PrintImmHex;
>   
> +  /// Which style to use for printing hexadecimal values.
> +  HexStyle::Style PrintHexStyle;
> +
>     /// Utility function for printing annotations.
>     void printAnnotation(raw_ostream &OS, StringRef Annot);
>   public:
>     MCInstPrinter(const MCAsmInfo &mai, const MCInstrInfo &mii,
>                   const MCRegisterInfo &mri)
>       : CommentStream(0), MAI(mai), MII(mii), MRI(mri), AvailableFeatures(0),
> -      UseMarkup(0), PrintImmHex(0) {}
> +      UseMarkup(0), PrintImmHex(0), PrintHexStyle(HexStyle::C) {}
>   
>     virtual ~MCInstPrinter();
>   
> @@ -80,8 +90,16 @@
>     bool getPrintImmHex() const { return PrintImmHex; }
>     void setPrintImmHex(bool Value) { PrintImmHex = Value; }
>   
> +  HexStyle::Style getPrintHexStyleHex() const { return PrintHexStyle; }
> +  void setPrintImmHex(HexStyle::Style Value) { PrintHexStyle = Value; }
> +
>     /// Utility function to print immediates in decimal or hex.
> -  format_object1<int64_t> formatImm(const int64_t Value) const;
> +  format_object1<int64_t> formatImm(const int64_t Value) const { return PrintImmHex ? formatHex(Value) : formatDec(Value); }
> +
> +  /// Utility functions to print decimal/hexadecimal values.
> +  format_object1<int64_t> formatDec(const int64_t Value) const;
> +  format_object1<int64_t> formatHex(const int64_t Value) const;
> +  format_object1<uint64_t> formatHex(const uint64_t Value) const;
>   };
>   
>   } // namespace llvm
> Index: lib/MC/MCInstPrinter.cpp
> ===================================================================
> --- lib/MC/MCInstPrinter.cpp
> +++ lib/MC/MCInstPrinter.cpp
> @@ -52,10 +52,55 @@
>       return b;
>   }
>   
> -/// Utility function to print immediates in decimal or hex.
> -format_object1<int64_t> MCInstPrinter::formatImm(const int64_t Value) const {
> -  if (getPrintImmHex())
> -    return format("0x%" PRIx64, Value);
> -  else
> -    return format("%" PRId64, Value);
> +// For asm-style hex (e.g. 0ffh) the first digit always has to be a number.
> +static bool needsLeadingZero(uint64_t Value)
> +{
> +  while(Value)
> +  {
> +    uint64_t digit = (Value >> 60) & 0xf;
> +    if (digit != 0)
> +      return (digit >= 0xa);
> +    Value <<= 4;
> +  }
> +  return false;
>   }
> +
> +format_object1<int64_t> MCInstPrinter::formatDec(const int64_t Value) const {
> +  return format("%" PRId64, Value);
> +}
> +
> +format_object1<int64_t> MCInstPrinter::formatHex(const int64_t Value) const {
> +  switch(PrintHexStyle) {
> +  default: // unreachable
> +  case HexStyle::C:
> +    if (Value < 0)
> +      return format("-0x%" PRIx64, -Value);
> +    else
> +      return format("0x%" PRIx64, Value);
> +  case HexStyle::Asm:
> +    if (Value < 0) {
> +      if (needsLeadingZero((uint64_t)(-Value)))
> +        return format("-0%" PRIx64 "h", -Value);
> +      else
> +        return format("-%" PRIx64 "h", -Value);
> +    } else {
> +      if (needsLeadingZero((uint64_t)(Value)))
> +        return format("0%" PRIx64 "h", Value);
> +      else
> +        return format("%" PRIx64 "h", Value);
> +    }
> +  }
> +}
> +
> +format_object1<uint64_t> MCInstPrinter::formatHex(const uint64_t Value) const {
> +  switch(PrintHexStyle) {
> +  default: // unreachable
> +  case HexStyle::C:
> +     return format("0x%" PRIx64, Value);
> +  case HexStyle::Asm:
> +    if (needsLeadingZero(Value))
> +      return format("0%" PRIx64 "h", Value);
> +    else
> +      return format("%" PRIx64 "h", Value);
> +  }
> +}
> Index: lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp
> ===================================================================
> --- lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp
> +++ lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp
> @@ -139,8 +139,7 @@
>       const MCConstantExpr *BranchTarget = dyn_cast<MCConstantExpr>(Op.getExpr());
>       int64_t Address;
>       if (BranchTarget && BranchTarget->EvaluateAsAbsolute(Address)) {
> -      O << "0x";
> -      O.write_hex(Address);
> +      O << formatHex((uint64_t)Address);
>       }
>       else {
>         // Otherwise, just print the expression.
> Index: lib/Target/X86/InstPrinter/X86IntelInstPrinter.cpp
> ===================================================================
> --- lib/Target/X86/InstPrinter/X86IntelInstPrinter.cpp
> +++ lib/Target/X86/InstPrinter/X86IntelInstPrinter.cpp
> @@ -119,16 +119,15 @@
>                                           raw_ostream &O) {
>     const MCOperand &Op = MI->getOperand(OpNo);
>     if (Op.isImm())
> -    O << Op.getImm();
> +    O << formatImm(Op.getImm());
>     else {
>       assert(Op.isExpr() && "unknown pcrel immediate operand");
>       // If a symbolic branch target was added as a constant expression then print
>       // that address in hex.
>       const MCConstantExpr *BranchTarget = dyn_cast<MCConstantExpr>(Op.getExpr());
>       int64_t Address;
>       if (BranchTarget && BranchTarget->EvaluateAsAbsolute(Address)) {
> -      O << "0x";
> -      O.write_hex(Address);
> +      O << formatHex((uint64_t)Address);
>       }
>       else {
>         // Otherwise, just print the expression.
> @@ -143,7 +142,7 @@
>     if (Op.isReg()) {
>       printRegName(O, Op.getReg());
>     } else if (Op.isImm()) {
> -    O << Op.getImm();
> +    O << formatImm((int64_t)Op.getImm());
>     } else {
>       assert(Op.isExpr() && "unknown operand kind in printOperand");
>       O << *Op.getExpr();
> @@ -195,7 +194,7 @@
>             DispVal = -DispVal;
>           }
>         }
> -      O << DispVal;
> +      O << formatImm(DispVal);
>       }
>     }