[llvm-objdump] Enable assembly highlighting in llvm-objdump

[JDevlieghere]

Summary

Enable assembly highlighting in llvm-objdump.

[JDevlieghere]

Move test

[MaskRay]

GNU objdump from 2.39 onwards provides --disassembler-color=on. I wonder whether we want similar option names, but I can see that --disassembler-color=on is long and inconvenient..

[JDevlieghere]

In D159224#4630031, @MaskRay wrote:

GNU objdump from 2.39 onwards provides --disassembler-color=on. I wonder whether we want similar option names, but I can see that --disassembler-color=on is long and inconvenient..

I expect this flag to be mostly used for writing tests so I'm not too worried about being verbose. FWIW I ended up going for the two separate flags because no other objdump options take a predefined set of options (there's a few ones that take arbitrary strings such as --debug-vars= or --arch-name=) but if we feel that something like --disassembler-color={on, off, auto} is the right thing to do, then I'm happy to go that route.

[MaskRay]

In D159224#4630057, @JDevlieghere wrote:

In D159224#4630031, @MaskRay wrote:

GNU objdump from 2.39 onwards provides --disassembler-color=on. I wonder whether we want similar option names, but I can see that --disassembler-color=on is long and inconvenient..

I expect this flag to be mostly used for writing tests so I'm not too worried about being verbose. FWIW I ended up going for the two separate flags because no other objdump options take a predefined set of options (there's a few ones that take arbitrary strings such as --debug-vars= or --arch-name=) but if we feel that something like --disassembler-color={on, off, auto} is the right thing to do, then I'm happy to go that route.

Sounds good. objdump uses terminal instead of auto.

% objdump --help
...
      --disassembler-color=off       Disable disassembler color output. (default)
      --disassembler-color=terminal  Enable disassembler color output if displaying on a terminal.
      --disassembler-color=on        Enable disassembler color output.
      --disassembler-color=extended  Use 8-bit colors in disassembler output.

[JDevlieghere]

In D159224#4630068, @MaskRay wrote:

In D159224#4630057, @JDevlieghere wrote:

In D159224#4630031, @MaskRay wrote:

GNU objdump from 2.39 onwards provides --disassembler-color=on. I wonder whether we want similar option names, but I can see that --disassembler-color=on is long and inconvenient..

I expect this flag to be mostly used for writing tests so I'm not too worried about being verbose. FWIW I ended up going for the two separate flags because no other objdump options take a predefined set of options (there's a few ones that take arbitrary strings such as --debug-vars= or --arch-name=) but if we feel that something like --disassembler-color={on, off, auto} is the right thing to do, then I'm happy to go that route.

Sounds good. objdump uses terminal instead of auto.

% objdump --help
...
      --disassembler-color=off       Disable disassembler color output. (default)
      --disassembler-color=terminal  Enable disassembler color output if displaying on a terminal.
      --disassembler-color=on        Enable disassembler color output.
      --disassembler-color=extended  Use 8-bit colors in disassembler output.

Perfect, I'll mirror that. Thanks!

[jhenderson]

Don't forget to add any new options to the llvm-objdump command guide.

[jhenderson]

llvm/docs/CommandGuide/llvm-objdump.rst

183

Is this really a -M option, given that that is the option for --disassembler-options?

187–189

It's probably worth highlighting which of these is the default.

[JDevlieghere]

Address @jhenderson's feedback.

llvm/docs/CommandGuide/llvm-objdump.rst

183

No, good catch. That's a copy/paste error.

[jhenderson]

LGTM. Please wait for @MaskRay too.

[MaskRay]

Thanks!

[thakis]

This breaks tests on windows: http://45.33.8.238/win/83747/step_11.txt

Please take a look and revert for now if it takes a while to fix.

[JDevlieghere]

In D159224#4634766, @thakis wrote:

This breaks tests on windows: http://45.33.8.238/win/83747/step_11.txt

Please take a look and revert for now if it takes a while to fix.

Should be fixed by 7678a322196b.

[dyung]

The newly added test tools/llvm-objdump/MachO/arm64-disassembly-color.s is failing if AArch64 backend is not available, sounds like a REQUIRES is needed?

https://lab.llvm.org/buildbot/#/builders/139/builds/48856

******************** TEST 'LLVM :: tools/llvm-objdump/MachO/arm64-disassembly-color.s' FAILED ********************
Exit Code: 1
Command Output (stderr):
--
+ : 'RUN: at line 1'
+ /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/bin/llvm-mc -triple arm64-apple-macosx /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/llvm/test/tools/llvm-objdump/MachO/arm64-disassembly-color.s -filetype=obj -o /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/test/tools/llvm-objdump/MachO/Output/arm64-disassembly-color.s.tmp
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/bin/llvm-mc: error: unable to get target for 'arm64-apple-macosx', see --version and --triple.
--
********************

[thakis]

Should be fixed by 7678a322196b.

Yup, thanks!

[JDevlieghere]

In D159224#4634865, @dyung wrote:

The newly added test tools/llvm-objdump/MachO/arm64-disassembly-color.s is failing if AArch64 backend is not available, sounds like a REQUIRES is needed?

https://lab.llvm.org/buildbot/#/builders/139/builds/48856

******************** TEST 'LLVM :: tools/llvm-objdump/MachO/arm64-disassembly-color.s' FAILED ********************
Exit Code: 1
Command Output (stderr):
--
+ : 'RUN: at line 1'
+ /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/bin/llvm-mc -triple arm64-apple-macosx /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/llvm/test/tools/llvm-objdump/MachO/arm64-disassembly-color.s -filetype=obj -o /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/test/tools/llvm-objdump/MachO/Output/arm64-disassembly-color.s.tmp
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/bin/llvm-mc: error: unable to get target for 'arm64-apple-macosx', see --version and --triple.
--
********************

Yup, let me add that!

edit: Should be fixed by a23b9b7ef134.

[nickdesaulniers]

This is a very welcomed change! Have I really not run the disassembler since this landed?

llvm/docs/CommandGuide/llvm-objdump.rst

.. option:: -M, --disassembler-options=<opt1[,opt2,...]>
 
   * ``reg-names-std``: ARM only (default). Print in ARM 's instruction set documentation, with r13/r14/r15 replaced by sp/lr/pc.
   * ``reg-names-raw``: ARM only. Use r followed by the register number.
   * ``no-aliases``: AArch64 and RISC-V only. Print raw instruction mnemonic instead of pseudo instruction mnemonic.
   * ``numeric``: RISC-V only. Print raw register names instead of ABI mnemonic. (e.g. print x1 instead of ra)
   * ``att``: x86 only (default). Print in the AT&T syntax.
   * ``intel``: x86 only. Print in the intel syntax.
 
+
+.. option::  --disassembler-color=<mode>
+
+  Enable or disable disassembler color output.
+
+  * ``off``: Disable disassembler color output.
+  * ``on``: Enable disassembler color output.
+  * ``terminal``: Enable disassembler color output if the terminal supports it (default).
+
 .. option:: --mcpu=<cpu-name>
 
   Target a specific CPU type for disassembly. Specify ``--mcpu=help`` to display
   available CPUs.
 
 .. option:: --mattr=<a1,+a2,-a3,...>
 
   Enable/disable target-specific attributes. Specify ``--mattr=help`` to display

llvm/llvm/llvm/test/tools/llvm-objdump/MachO/arm64-disassembly-color.s

+// RUN: llvm-mc -triple arm64-apple-macosx %s -filetype=obj -o %t
+// RUN: llvm-objdump --disassembler-color=on --disassemble %t | FileCheck %s --check-prefix=COLOR
+// RUN: llvm-objdump --disassembler-color=off --disassemble %t | FileCheck %s --check-prefix=NOCOLOR
+// RUN: llvm-objdump --disassembler-color=terminal --disassemble %t | FileCheck %s --check-prefix=NOCOLOR
+
+sub sp, sp, #16
+str w0, [sp, #12]
+ldr w8, [sp, #12]
+ldr w9, [sp, #12]
+mul w0, w8, w9
+add sp, sp, #16
+
+// NOCOLOR: sub sp, sp, #0x10
+// NOCOLOR: str w0, [sp, #0xc]
+// NOCOLOR: ldr w8, [sp, #0xc]
+// NOCOLOR: ldr w9, [sp, #0xc]
+// NOCOLOR: mul w0, w8, w9
+// NOCOLOR: add sp, sp, #0x10
+
+// COLOR: sub ␛[0;36msp␛[0m, ␛[0;36msp␛[0m, ␛[0;31m#0x10␛[0m
+// COLOR: str ␛[0;36mw0␛[0m, [␛[0;36msp␛[0m, ␛[0;31m#0xc␛[0m]
+// COLOR: ldr ␛[0;36mw8␛[0m, [␛[0;36msp␛[0m, ␛[0;31m#0xc␛[0m]
+// COLOR: ldr ␛[0;36mw9␛[0m, [␛[0;36msp␛[0m, ␛[0;31m#0xc␛[0m]
+// COLOR: mul ␛[0;36mw0␛[0m, ␛[0;36mw8␛[0m, ␛[0;36mw9␛[0m
+// COLOR: add ␛[0;36msp␛[0m, ␛[0;36msp␛[0m, ␛[0;31m#0x10␛[0m

llvm/llvm/test/tools/llvm-objdump/MachO/arm64-disassembly-color.s

+// RUN: llvm-mc -triple arm64-apple-macosx %s -filetype=obj -o %t
+// RUN: llvm-objdump --disassembler-color=on --disassemble %t | FileCheck %s --check-prefix=COLOR
+// RUN: llvm-objdump --disassembler-color=off --disassemble %t | FileCheck %s --check-prefix=NOCOLOR
+// RUN: llvm-objdump --disassembler-color=terminal --disassemble %t | FileCheck %s --check-prefix=NOCOLOR
+
+sub sp, sp, #16
+str w0, [sp, #12]
+ldr w8, [sp, #12]
+ldr w9, [sp, #12]
+mul w0, w8, w9
+add sp, sp, #16
+
+// NOCOLOR: sub sp, sp, #0x10
+// NOCOLOR: str w0, [sp, #0xc]
+// NOCOLOR: ldr w8, [sp, #0xc]
+// NOCOLOR: ldr w9, [sp, #0xc]
+// NOCOLOR: mul w0, w8, w9
+// NOCOLOR: add sp, sp, #0x10
+
+// COLOR: sub ␛[0;36msp␛[0m, ␛[0;36msp␛[0m, ␛[0;31m#0x10␛[0m
+// COLOR: str ␛[0;36mw0␛[0m, [␛[0;36msp␛[0m, ␛[0;31m#0xc␛[0m]
+// COLOR: ldr ␛[0;36mw8␛[0m, [␛[0;36msp␛[0m, ␛[0;31m#0xc␛[0m]
+// COLOR: ldr ␛[0;36mw9␛[0m, [␛[0;36msp␛[0m, ␛[0;31m#0xc␛[0m]
+// COLOR: mul ␛[0;36mw0␛[0m, ␛[0;36mw8␛[0m, ␛[0;36mw9␛[0m
+// COLOR: add ␛[0;36msp␛[0m, ␛[0;36msp␛[0m, ␛[0;31m#0x10␛[0m

llvm/test/tools/llvm-objdump/MachO/arm64-disassembly-color.s

+// RUN: llvm-mc -triple arm64-apple-macosx %s -filetype=obj -o %t
+// RUN: llvm-objdump --disassembler-color=on --disassemble %t | FileCheck %s --check-prefix=COLOR
+// RUN: llvm-objdump --disassembler-color=off --disassemble %t | FileCheck %s --check-prefix=NOCOLOR
+// RUN: llvm-objdump --disassembler-color=terminal --disassemble %t | FileCheck %s --check-prefix=NOCOLOR
+
+sub sp, sp, #16
+str w0, [sp, #12]
+ldr w8, [sp, #12]
+ldr w9, [sp, #12]
+mul w0, w8, w9
+add sp, sp, #16
+
+// NOCOLOR: sub sp, sp, #0x10
+// NOCOLOR: str w0, [sp, #0xc]
+// NOCOLOR: ldr w8, [sp, #0xc]
+// NOCOLOR: ldr w9, [sp, #0xc]
+// NOCOLOR: mul w0, w8, w9
+// NOCOLOR: add sp, sp, #0x10
+
+// COLOR: sub ␛[0;36msp␛[0m, ␛[0;36msp␛[0m, ␛[0;31m#0x10␛[0m
+// COLOR: str ␛[0;36mw0␛[0m, [␛[0;36msp␛[0m, ␛[0;31m#0xc␛[0m]
+// COLOR: ldr ␛[0;36mw8␛[0m, [␛[0;36msp␛[0m, ␛[0;31m#0xc␛[0m]
+// COLOR: ldr ␛[0;36mw9␛[0m, [␛[0;36msp␛[0m, ␛[0;31m#0xc␛[0m]
+// COLOR: mul ␛[0;36mw0␛[0m, ␛[0;36mw8␛[0m, ␛[0;36mw9␛[0m
+// COLOR: add ␛[0;36msp␛[0m, ␛[0;36msp␛[0m, ␛[0;31m#0x10␛[0m

llvm/tools/llvm-objdump/ObjdumpOpts.td

def : Flag<["-"], "z">, Alias<disassemble_zeroes>,
   HelpText<"Alias for --disassemble-zeroes">;
 
 def disassembler_options_EQ : Joined<["--"], "disassembler-options=">,
   MetaVarName<"options">,
   HelpText<"Pass target specific disassembler options">;
 def : JoinedOrSeparate<["-"], "M">, Alias<disassembler_options_EQ>,
   HelpText<"Alias for --disassembler-options=">;
 
+def disassembler_color_EQ : Joined<["--"], "disassembler-color=">,
+  MetaVarName<"mode">,
+  HelpText<"Enable or disable disassembler color output. "
+           "Valid options are \"on\", \"off\" and \"terminal\" (default)">;
+
 def dynamic_reloc : Flag<["--"], "dynamic-reloc">,
   HelpText<"Display the dynamic relocation entries in the file">;
 def : Flag<["-"], "R">, Alias<dynamic_reloc>,
   HelpText<"Alias for --dynamic-reloc">;
 
 def dwarf_EQ : Joined<["--"], "dwarf=">,
   HelpText<"Dump the specified DWARF debug sections. The "
            "only supported value is 'frames'">,

llvm/tools/llvm-objdump/llvm-objdump.cpp

OtoolOptTable()
       : CommonOptTable(otool::OtoolInfoTable, " [option...] [file...]",
                        "Mach-O object file displaying tool") {}
 };
 
 } // namespace
 
 #define DEBUG_TYPE "objdump"
 
+enum class ColorOutput {
+  Auto,
+  Enable,
+  Disable,
+  Invalid,
+};
+
 static uint64_t AdjustVMA;
 static bool AllHeaders;
 static std::string ArchName;
 bool objdump::ArchiveHeaders;
 bool objdump::Demangle;
 bool objdump::Disassemble;
 bool objdump::DisassembleAll;
 bool objdump::SymbolDescription;
 bool objdump::TracebackTable;
 static std::vector<std::string> DisassembleSymbols;
 static bool DisassembleZeroes;
 static std::vector<std::string> DisassemblerOptions;
+static ColorOutput DisassemblyColor;
 DIDumpType objdump::DwarfDumpType;
 static bool DynamicRelocations;
 static bool FaultMapSection;
 static bool FileHeaders;
 bool objdump::SectionContents;
 static std::vector<std::string> InputFilenames;
 bool objdump::PrintLines;
 static bool MachOOpt;
InstPrinter.reset(TheTarget->createMCInstPrinter(Triple(TripleName),
                                                    *InstrInfo, *RegisterInfo));
   if (!InstPrinter)
     reportError(Obj.getFileName(),
                 "no instruction printer for target " + TripleName);
   InstPrinter->setPrintImmHex(PrintImmHex);
   InstPrinter->setPrintBranchImmAsAddress(true);
   InstPrinter->setSymbolizeOperands(SymbolizeOperands);
   InstPrinter->setMCInstrAnalysis(InstrAnalysis.get());
+
+  switch (DisassemblyColor) {
+  case ColorOutput::Enable:
+    InstPrinter->setUseColor(true);
+    break;
+  case ColorOutput::Auto:
+    InstPrinter->setUseColor(outs().has_colors());
+    break;
+  case ColorOutput::Disable:
+  case ColorOutput::Invalid:
+    InstPrinter->setUseColor(false);
+    break;
+  };
 }
 
 DisassemblerTarget::DisassemblerTarget(DisassemblerTarget &Other,
                                        SubtargetFeatures &Features)
     : TheTarget(Other.TheTarget),
       SubtargetInfo(TheTarget->createMCSubtargetInfo(TripleName, MCPU,
                                                      Features.getString())),
       Context(Other.Context),
for (size_t SI = 0, SE = Symbols.size(); SI != SE;) {
       bool DumpARMELFData = false;
       bool DumpTracebackTableForXCOFFFunction =
           Obj.isXCOFF() && Section.isText() && TracebackTable &&
           Symbols[SI - 1].XCOFFSymInfo.StorageMappingClass &&
           (*Symbols[SI - 1].XCOFFSymInfo.StorageMappingClass == XCOFF::XMC_PR);
 
       formatted_raw_ostream FOS(outs());
 
+      // FIXME: Workaround for bug in formatted_raw_ostream. Color escape codes
+      // are (incorrectly) written directly to the unbuffered raw_ostream
+      // wrapped by the formatted_raw_ostream.
+      if (DisassemblyColor == ColorOutput::Enable ||
+          DisassemblyColor == ColorOutput::Auto)
+        FOS.SetUnbuffered();
+
       std::unordered_map<uint64_t, std::string> AllLabels;
       std::unordered_map<uint64_t, std::vector<std::string>> BBAddrMapLabels;
       if (SymbolizeOperands) {
         collectLocalBranchTargets(Bytes, DT->InstrAnalysis.get(),
                                   DT->DisAsm.get(), DT->InstPrinter.get(),
                                   PrimaryTarget.SubtargetInfo.get(),
                                   SectionAddr, Index, End, AllLabels);
         collectBBAddrMapLabels(AddrToBBAddrMap, SectionAddr, Index, End,
static void parseObjdumpOptions(const llvm::opt::InputArgList &InputArgs) {
   if (const opt::Arg *A = InputArgs.getLastArg(OBJDUMP_debug_vars_EQ)) {
     DbgVariables = StringSwitch<DebugVarsFormat>(A->getValue())
                        .Case("ascii", DVASCII)
                        .Case("unicode", DVUnicode)
                        .Default(DVInvalid);
     if (DbgVariables == DVInvalid)
       invalidArgValue(A);
   }
+  if (const opt::Arg *A = InputArgs.getLastArg(OBJDUMP_disassembler_color_EQ)) {
+    DisassemblyColor = StringSwitch<ColorOutput>(A->getValue())
+                           .Case("on", ColorOutput::Enable)
+                           .Case("off", ColorOutput::Disable)
+                           .Case("terminal", ColorOutput::Auto)
+                           .Default(ColorOutput::Invalid);
+    if (DisassemblyColor == ColorOutput::Invalid)
+      invalidArgValue(A);
+  }
+
   parseIntArg(InputArgs, OBJDUMP_debug_vars_indent_EQ, DbgIndent);
 
   parseMachOOptions(InputArgs);
 
   // Parse -M (--disassembler-options) and deprecated
   // --x86-asm-syntax={att,intel}.
   //
   // Note, for x86, the asm dialect (AssemblerDialect) is initialized when the