[llvm-objdump] Implement highlighting

[seiya]

Summary

As the first step, this patch highlights some elements: registers and immediate values,
and some symbol names.

[seiya]

I've uploaded a series of separate patches: https://reviews.llvm.org/D65191

[seiya]

Oops, I intended to abandon D64311 instead.

[seiya]

• Replace blue with cyan since it may be hard to read on some environments (for example, "Pro" profile of macOS Terminal.app).

[seiya]

Screenshots:

[alexander-shaposhnikov]

i think the title of the diff should be adjusted a bit: 's/llvm-objcopy/llvm-objdump/g'

[MaskRay]

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

336

--color is a more common option name: ls, less, grep, zstd, etc

Can you investigate a bit if it is possible to have both --color and --color=when ?

[JohnReagan]

Do you have a design or spec for what you consider to be "syntax highlighting?" From the early example you show for coloring literals, registers, etc., I don't see the benefit. For the current output, you can tell literals from registers, right? After all, the assembler is able to re-parse them in.

And in general, I dislike color markups in output as they always end up relying on some system/platform screen/color support. For instance, I don't think your highlighting will work on my OpenVMS x86 system.

[jhenderson]

If you haven't already, take a look at WithColor.h. I believe it allows you to stream things in a specific colour properly, including with disabling the feature via --color. From the WithColor class description, you can use it like WithColor(outs(), raw_ostream::CYAN) << "some text"; to print something to the stream in cyan.

llvm/test/tools/llvm-objdump/highlight.test

2 ↗

(On Diff #211432)

REQUIRES usually go first in the output.

4 ↗

(On Diff #211432)

Move this comment to near the top of the file, and make it a bit more verbose, so that it describes the test as a whole. Take a look at recent new tests in various of the LLVM tools for examples.

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

336

LLVM already has a --color option (see for example llvm-dwarfdump output, WithColor::warning() and error() calls etc etc).

I'm inclined to think that syntax highlighting can be on by default and just use --color=false (or whatever it is) to turn it off. If we go that route, the --color option should be documented in the llvm-objdump command guide, and also we should check it is in the tool help text (since it's in a different option category).

[jhenderson]

llvm/test/tools/llvm-objdump/highlight.test

1 ↗

(On Diff #211432)

Try and avoid adding a binary if you can. There's no particular reason this can't use yaml2obj to do things. Just add a .text section with appropriate content.

5 ↗

(On Diff #211432)

I might be missing something, but how does this show that the syntax highlighting works? Is it cross-platform?

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

803–804

How about instead of "HeaderLength" this is called "InstructionCol". The thing you're measuring isn't really a "Header".

1227

Use ArrayRef here.

[grimar]

llvm/test/tools/llvm-objdump/highlight.test

4 ↗

(On Diff #211432)

We usually start comment from ## nowadays.

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

350

It is probably not clear what is Reset. Can it be Default?

354

Make it const?

368

It is generally not a good practice to do a lookup twice.
You can use std::map::find() to avoid that.

(Also I do not think you will be able to use operator[] after making HighlightColors const)

1230

You use BeforeText only once, so you do not need it it seems (just inline it).

1244

Seems this would better look as ifs:

ObjdumpHighlightColor Color;
if (Span.Type == MarkupType::Imm)
  Color = ObjdumpHighlightColor::Immediate;
else if (Span.Type == MarkupType::Reg)
  Color = ObjdumpHighlightColor::Register;
else
  Color = ObjdumpHighlightColor::Reset;

[MaskRay]

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

336

It looks lib/Support/WithColor.cpp#L106 already handles this...

bool WithColor::colorsEnabled() {
  if (DisableColors)
    return false;
  if (UseColor == cl::BOU_UNSET)
    return OS.has_colors();  // e.g. is a terminal
  return UseColor == cl::BOU_TRUE;
}

345

Maybe just ObjdumpColor.

369

Use WithColor::changeColor

[seiya]

• Use --color to enable syntax highlighting.
• Addressed some review comments.

[seiya]

In D65191#1598981, @JohnReagan wrote:

Do you have a design or spec for what you consider to be "syntax highlighting?" From the early example you show for coloring literals, registers, etc., I don't see the benefit. For the current output, you can tell literals from registers, right? After all, the assembler is able to re-parse them in.

The original rough design is described in this document.

In addition to this patch, I plan to implement support for symbol names, relocations, and ascii strings in .rodata, and hopefully, highlight lines by instruction types (for example, highlight jmp instructions with green just like radare2 theme). As a handy (not feature-rich like radare2) disassembler, simple syntax highlighting feature is attractive at least to me.

After all, the assembler is able to re-parse them in.

I'm sorry but I can't see your point. Syntax highlighting is not for programs, but for humans to improve readability by emphasizing contexts.

And in general, I dislike color markups in output as they always end up relying on some system/platform screen/color support. For instance, I don't think your highlighting will work on my OpenVMS x86 system.

I agree with you that platform-independence is important too. I don't have experience with OpenVMS at all so could you explain why this patch won't work on it? This patch uses llvm::sys::Process::OutputColor internally (through raw_fd_ostream::changeColor) to avoid being platform-specific.

[seiya]

llvm/include/llvm/Support/WithColor.h

102

Changes to WithColor.cpp and WithColor.h should be a separate patch, but before creating that I'd like to hear your comments.

llvm/test/tools/llvm-objdump/highlight.test

5 ↗

(On Diff #211432)

Oh I should have added shell to REQUIRES. This test makes sure that some elements in the disassembly are highlighted by checking ANSI escape sequences.

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

336

I'm inclined to think that syntax highlighting can be on by default and just use --color=false (or whatever it is) to turn it off. If we go that route, the --color option should be documented in the llvm-objdump command guide, and also we should check it is in the tool help text (since it's in a different option category).

What do you think about the compatibility with GNU objdump? Making highlighting enabled by default is incompatible behavior with GNU objcopy. Using with objdump in script (e.g., llvm-objdump -d foo | grep bar) won't be broken (it checks if stdout is a tty) so it won't be problem I think.

336

Can you investigate a bit if it is possible to have both --color and --color=when ?

Did you mean whether can we allow both --color and --color=true in command-line options or not and how it is handled in colorsEnabled?

[MaskRay]

llvm/test/tools/llvm-objdump/highlight.test

4 ↗

(On Diff #212532)

# UNSUPPORTED: system-windows
# REQUIRES: shell, x86-registered-target

8 ↗

(On Diff #212532)

llvm-objdump -d --no-show-raw-insn --color %t | tr -d '\033' | FileCheck %s --check-prefix=ENABLED

15 ↗

(On Diff #212532)

# ENABLED-NEXT: 619 movl 2099697([0;36m%rip[0m), [0;36m%edx[0m

[MaskRay]

llvm/lib/Support/WithColor.cpp

103

There is no need to split.

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

2416

WithColor::isColorOptionSet() && outs().has_colors()

[MaskRay]

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

2416

Place extern cl::opt<cl::boolOrDefault> UseColor; at toplevel.

ColorsEnabled = UseColor == cl::BOU_UNSET; may be simpler

[seiya]

• Addressed review comments.

[seiya]

• Update the command guide.

[seiya]

llvm/test/tools/llvm-objdump/highlight.test

8 ↗

(On Diff #212532)

It looks much better. Thanks!

[jhenderson]

llvm/test/tools/llvm-objdump/highlight.test

4 ↗

(On Diff #212532)

Do we need both UNSUPPORTED: system-windows and REQUIRES: shell? I'd think the latter effectively implies the former (but could be wrong).

1 ↗

(On Diff #211432)

I'm being dumb. This should probably just be an assembly file and use llvm-mc to compile it. That'll make the input text more readable too. Also, the test should be in the X86 directory.

1 ↗

(On Diff #212692)

What is "it" here?

Perhaps make this more verbose: "Show that llvm-objdump highlights ... by default."

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

336

What do you think about the compatibility with GNU objdump?

Compatibility is only important for things that can affect parsers (though I have no idea why anybody would be parsing disassembly...). If this can't affect parser behaviour, then I don't think we need to worry about it. If it can, then it should be off by default.

369

Use WithColor::changeColor

This is marked as done, but I don't see it being used?

369–371

Rather than static casting things to int, for the assert, compare the enum values directly:

assert(Color != ObjdumpColor::Sentinel)

The whole point of a enum class is that you can't implicitly convert from/to them. You shouldn't be casting them ever.

Also add a message to the assert i.e. assert(... && "Color cannot be Sentinel");.

This all becomes moot if you follow this suggestion: instead of HighlightColors being an array, it should probably be a std::unordered_map or similar with ObjdumpColor as the key. That way you don't need the cast for the lookup, although you will need to check against HighlightColors.end().

[seiya]

• Addressed review comments.

[seiya]

llvm/test/tools/llvm-objdump/highlight.test

4 ↗

(On Diff #212532)

Basically, REQUIRES: shell implies that the test environment is not Windows, but it looks that setting $LIT_USE_INTERNAL_SHELL forces running tests on Windows that requires shell [1].

Namely, UNSUPPORTED: system-windows disables this test on Windows even if $LIT_USE_INTERNAL_SHELL is set.

I'm not familiar with Windows, but I think this it makes sense because I suppose that SetConsoleTextAttribute API does not emit ANSI escape sequences.

[1]: https://reviews.llvm.org/D31423

1 ↗

(On Diff #211432)

The input file should be an executable to test symbol highlighting because currently only RIP-relative symbols are supported. That said, I think this YAML lacks readability too. Do you have any idea?

1 ↗

(On Diff #212692)

What is "it" here?

"it" referred to llvm-objdump. Fixed the comment.

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

336

I think so too (nobody parse the disassembly output written to a tty not a pipe, I believe).

Do you think syntax highlighting should be enabled by default? This current implementation enables the feature only if --color is explicitly specified (ColorsEnabled = UseColor == cl::BOU_TRUE;).

369

I've replaced outs().changeColor with WithColor.

369–371

I used an array since it should be faster lookup but it was a premature optimization. I've replaced with std::map instead of std::unordered_map here. Enum class as key of std::unordered_map is not supported because of "the specified hash does not meet the Hash requirements" (I must have misunderstood something... it is supported in C++14).

[jhenderson]

llvm/test/tools/llvm-objdump/X86/highlight.test

1 ↗

(On Diff #213841)

Let's rename this test "disassembly-highlighting.test" (or whatever best matches the naming style of other disassembly tests in llvm-objdump). In particular, saying "disassemble" or "disassembly" or similar in the test name is important.

On that note, this comment should say "when disassembling" somewhere.

3 ↗

(On Diff #213841)

You can remove "and the X86 disassembler" now.

44 ↗

(On Diff #213841)

The input file should be an executable to test symbol highlighting because currently only RIP-relative symbols are supported. That said, I think this YAML lacks readability too. Do you have any idea?

I don't I'm afraid. You might be able to simulate it using an assembly file with a single text section still.

If you do stick with the YAML, I think you probably have too much content in your sections. You only check for 3 instructions, so you can reduce the bytes in the content down to those few. You can also significantly simplify much of this object. You probably don't need either .data and .bss. You probably don't need the section alignment fields. You probably can simplify the addresses to round numbers. You may not need all those symbols either (if I'm reading the checks correctly, only 'foo' is important). You don't need the entry label. Basically, you probably are better off starting with an empty YAML and building it up to the bare minimum for the test rather than using obj2yaml on a binary you want to simulate.

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

336

Yes, I think syntax highlighting can be on by default (it is for llvm-dwarfdump for example).

[seiya]

• Enable syntax highlighting by default.
• Added WithColor::colorsAvailable
  • I'll submit this as a separate patch if it's a good approach.
• Replaced the lengthy YAML with a hand-assembled shorter one.

[seiya]

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

2415

@MaskRay I've splitted WithColor::colorsEnabled into colorsEnabled and colorsAvailable again. Do you think I should do (UseColor == cl::BOU_UNSET && OS.has_colors()) || UseColor == cl::BOU_TRUE here instead? I wonder if keeping UseColor static is better.

[jhenderson]

llvm/include/llvm/Support/WithColor.h

97

I think this needs rephrasing. How about "Determine whether the specified stream supports colors."?

llvm/lib/Support/WithColor.cpp

99

How about naming this colorsEnabled too?

102

I'm not sure this clause makes sense to me? If the stream doesn't have colours, saying colours are available for it because somebody has specified --color seems wrong...

Note that colorsEnabled() handles this case via DisableColors, I believe.

llvm/test/tools/llvm-objdump/X86/disassembly-highlighting.test

3

The comment says "by default", but the test doesn't show the default behaviour! It only shows behaviour when --color or --color=false is specified.

[seiya]

llvm/include/llvm/Support/WithColor.h

97

That sounds not sufficient to me. colorsAvailable returns true if the specified stream supports colors or --color=1 is given. As you suggested in another comment, I think adding another colorsEnabled() would be better:

/// Determine whether colors are displayed.
bool colorsEnabled();
static bool colorsEnabled(raw_ostream &OS);

llvm/lib/Support/WithColor.cpp

102

I'm not sure this clause makes sense to me? If the stream doesn't have colours, saying colours are available for it because somebody has specified --color seems wrong...

That makes sense. I'll rename it to colorsEnabled.

Note that colorsEnabled() handles this case via DisableColors, I believe.

DisableColors is not related to the --color option: it disables colors regardless of the option:

/// @param DisableColors Whether to ignore color changes regardless of -color
/// and support in OS
WithColor(raw_ostream &OS, HighlightColor S, bool DisableColors = false)

[seiya]

llvm/test/tools/llvm-objdump/X86/disassembly-highlighting.test

3

Good catch! I'll fix this.

[rupprecht]

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

366

IIUC, using WithColor to do nested coloring doesn't really work -- ~WithColor resets the terminal color to plain, *not* to what the color was before. So if you have some stream of characters like:
AAABBBCCCBBB
where A maps to no coloring (X), B maps to red (R), and C maps to green (G), you should want to see:
XXXRRRGGGRRR
But I think it will actually be:
XXXRRRGGGXXX
because ~WithColor will reset to plain after closing the span of green.

In practice, I'm not sure any assembly actually has anything after nested spans -- the end of any nested span will line up with the span it's nested in. Still, it seems fragile/something that should be checked for somehow. Is this something you're handling somewhere that I'm just missing?

[seiya]

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

366

Good point. As you said, this patch doesn't support such a case. A good example is memory operand:

movq  <imm:$1>,  <mem:(<reg:%rax>, <reg:%rbx>)>
                      ^          ^                        
                Change color.    WithColor resets the color here.

I'll consider implementing a new class WithHighlightColor, which behaves like WithColor, but it saves/restores colors using a stack or something.

[MaskRay]

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

350

= 0 can be deleted. You don't use its integral value.

[seiya]

• Added WithNestedColor to support nested spans.
  • See https://reviews.llvm.org/D65191#inline-594208 about the nested spans.
• Addressed other review comments.

[seiya]

llvm/include/llvm/Support/WithColor.h

97

Updated to "Determine whether colors are displayed..." instead of "... stream supports colors" because even if the stream supports colors, it can be disabled by -color=false.

llvm/test/tools/llvm-objdump/X86/disassembly-highlighting.test

3

I just remembered that I added --color intentionally because stdout is not a tty. I've updated the comment: "... by default" → "... if colors are enabled".

[seiya]

clang-formatted

[jhenderson]

llvm/lib/Support/WithColor.cpp

102

Under what situations can OS.has_colors() return false? I'm not convinced we want to return true if a stream doesn't have colours, even if the switch has been specified.

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

368

a -> A

Would it make sense for this code to exist in WithColor.h/.cpp?

In fact, I wonder if this should be used by WithColor directly, i.e. have WithColor restore the previous state by default, rather than the plain version.

370

Consider replacing the pair with a simple struct e.g. ColorState. That'll make the code using it much clearer.

397

destructed -> destroyed

[seiya]

llvm/lib/Support/WithColor.cpp

102

In UNIX,OS.has_colors() returns false if either the stdout is not a tty (say, pipe) or the it does not support colors. Even if it does not support colors, it's sometimes useful to enable colors with the switch.

For example, in disassembly-highlighting.test, since the stdout of llvm-objdump is a pipe to FileCheck, OS.has_colors() will return false. However, in order to check that the output includes colors, we need a way to force colors by the switch.

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

368

Would it make sense for this code to exist in WithColor.h/.cpp?

I think so too. I'll submit this part as a separate patch.

In fact, I wonder if this should be used by WithColor directly, i.e. have WithColor restore the previous state by default, rather than the plain version.

That sounds good to me. I'll try it.

llvm/docs/CommandGuide/llvm-objdump.rst

 --------
 At least one of the following commands are required, and some commands can be
 combined with other commands:
 
 .. option:: -a, --archive-headers
 
   Display the information contained within an archive's headers.
 
+.. option:: --color
+
+  Enable syntax highlighting.
+
 .. option:: -d, --disassemble
 
   Disassemble all text sections found in the input files.
 
 .. option:: -D, --disassemble-all
 
   Disassemble all sections found in the input files.
   

llvm/include/llvm/Support/WithColor.h

static raw_ostream &warning(raw_ostream &OS, StringRef Prefix = "",
                               bool DisableColors = false);
   /// Convenience method for printing "note: " to the given stream.
   static raw_ostream &note(raw_ostream &OS, StringRef Prefix = "",
                            bool DisableColors = false);
   /// Convenience method for printing "remark: " to the given stream.
   static raw_ostream &remark(raw_ostream &OS, StringRef Prefix = "",
                              bool DisableColors = false);
 
+  /// Determine whether colors are displayed in the given stream.
+  static bool colorsEnabled(raw_ostream &OS);
   /// Determine whether colors are displayed.
   bool colorsEnabled();
 
   /// Change the color of text that will be output from this point forward.
   /// @param Color ANSI color to use, the special SAVEDCOLOR can be used to
   /// change only the bold attribute, and keep colors untouched
   /// @param Bold Bold/brighter text, default false
   /// @param BG If true, change the background, default: change foreground
   WithColor &changeColor(raw_ostream::Colors Color, bool Bold = false,
                          bool BG = false);
 
   /// Reset the colors to terminal defaults. Call this when you are done
   /// outputting colored text, or before program exit.
   WithColor &resetColor();
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_DEBUGINFO_WITHCOLOR_H

llvm/lib/Support/WithColor.cpp

 raw_ostream &WithColor::remark(raw_ostream &OS, StringRef Prefix,
                                bool DisableColors) {
   if (!Prefix.empty())
     OS << Prefix << ": ";
   return WithColor(OS, HighlightColor::Remark, DisableColors).get()
          << "remark: ";
 }
 
-bool WithColor::colorsEnabled() {
-  if (DisableColors)
-    return false;
+bool WithColor::colorsEnabled(raw_ostream &OS) {
   if (UseColor == cl::BOU_UNSET)
     return OS.has_colors();
   return UseColor == cl::BOU_TRUE;
 }
 
+bool WithColor::colorsEnabled() {
+  if (DisableColors)
+    return false;
+  return colorsEnabled(OS);
+}
+
 WithColor &WithColor::changeColor(raw_ostream::Colors Color, bool Bold,
                                   bool BG) {
   if (colorsEnabled())
     OS.changeColor(Color, Bold, BG);
   return *this;
 }
 
 WithColor &WithColor::resetColor() {
   if (colorsEnabled())
     OS.resetColor();
   return *this;
 }
 
 WithColor::~WithColor() { resetColor(); }

llvm/test/tools/llvm-objdump/X86/disassembly-highlighting.test

+## Show that when disassembling, llvm-objdump highlights symbol names,
+## registers, and immediate values if colors are enabled.
+
+## This test expects ANSI escape sequences.
+# UNSUPPORTED: system-windows
+# REQUIRES: shell
+
+# RUN: yaml2obj %s > %t
+# RUN: llvm-objdump -d --no-show-raw-insn --color=false %t | FileCheck %s --check-prefix=DISABLED
+# RUN: llvm-objdump -d --no-show-raw-insn --color %t | tr -d '\033' \
+# RUN:   | FileCheck %s --check-prefix=ENABLED
+
+# DISABLED:      301: movl 249(%rip), %edx
+# ENABLED:       301: movl 249([0;36m%rip[0m)[0m, [0;36m%edx[0m
+# DISABLED-NEXT: 307: callq -12 <foo>
+# ENABLED-NEXT:  307: callq -12[0;1;33m <foo>[0m
+# DISABLED-NEXT: 30c: movl $0, %eax
+# ENABLED-NEXT:  30c: movl [0;31m$0[0m, [0;36m%eax[0m
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name:    .text
+    Type:    SHT_PROGBITS
+    Flags:   [SHF_ALLOC, SHF_EXECINSTR]
+    Address: 0x300
+    Content: 'c38b15f9000000e8f4ffffffb800000000c3'
+  - Name:    .data
+    Type:    SHT_PROGBITS
+    Flags:   [SHF_WRITE, SHF_ALLOC]
+    Address: 0x400
+    Content: '00000000'
+Symbols:
+  - Name:            foo
+    Type:            STT_FUNC
+    Section:         .text
+    Binding:         STB_GLOBAL
+    Value:           0x0000000000000300
+    Size:            0x0000000000000001
+  - Name:            main
+    Type:            STT_FUNC
+    Section:         .text
+    Binding:         STB_GLOBAL
+    Value:           0x0000000000000301
+    Size:            0x0000000000000011

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

static cl::alias UnwindInfoShort("u", cl::desc("Alias for --unwind-info"),
                                  cl::NotHidden, cl::Grouping,
                                  cl::aliasopt(UnwindInfo));
 
 static cl::opt<bool>
     Wide("wide", cl::desc("Ignored for compatibility with GNU objdump"),
          cl::cat(ObjdumpCat));
 static cl::alias WideShort("w", cl::Grouping, cl::aliasopt(Wide));
 
 static cl::extrahelp
     HelpResponse("\nPass @FILE as argument to read options from FILE.\n");
 
 static StringSet<> DisasmFuncsSet;
 static StringSet<> FoundSectionSet;
 static StringRef ToolName;
 
 typedef std::vector<std::tuple<uint64_t, StringRef, uint8_t>> SectionSymbolsTy;
 
+static bool ColorsEnabled;
+
+// Token types in the disassembly. When adding a new element, don't forget to
+// also update HighlightColors.
+enum class ObjdumpColor {
+  Default,
+  Immediate,
+  Register,
+  SymbolName,
+  Memory,
+};
+
+// TODO: Make this configurable (just like $LS_COLORS).
+static const std::map<ObjdumpColor, std::pair<raw_ostream::Colors, bool>>
+    HighlightColors = {
+        // {ObjdumpColor, { raw_ostream::Colors, Bold }}
+        {ObjdumpColor::Default, {raw_ostream::SAVEDCOLOR, false}},
+        {ObjdumpColor::Immediate, {raw_ostream::RED, false}},
+        {ObjdumpColor::Register, {raw_ostream::CYAN, false}},
+        {ObjdumpColor::SymbolName, {raw_ostream::YELLOW, true}},
+        {ObjdumpColor::Memory, {raw_ostream::SAVEDCOLOR, false}},
+};
+
+/// a context for WithNestedColor which holds saved colors.
+class WithNestedColorContext {
+  std::stack<std::pair<raw_ostream::Colors, bool>> NestedColors;
+
+public:
+  void pushColor(raw_ostream::Colors Color, bool Bold) {
+    NestedColors.emplace(Color, Bold);
+  }
+
+  void popColor() { NestedColors.pop(); }
+
+  bool empty() { return NestedColors.empty(); }
+
+  const std::pair<raw_ostream::Colors, bool> &currentColor() {
+    return NestedColors.top();
+  }
+};
+
+/// A RAII object which temporarily changes the color in a specific stream just
+/// like WithColor, but supports restoring the previous color in the given
+/// context.
+///
+/// Usage:
+///
+///   WithNestedColorContext Ctx;
+///   WithNestedColor WNC1(outs(), Ctx, raw_ostream::RED, ...);
+///   {
+///     WithNestedColor WNC2(outs(), Ctx, raw_ostream::BLUE, ...);
+///     outs() << "A blue-colored text.";
+///     // WNC2 is destructed and WNC1's color is restored here.
+///   }
+///   outs() << "A red-colored text.";
+///
+class WithNestedColor {
+  raw_ostream &OS;
+  WithNestedColorContext &Ctx;
+  bool DisableColors;
+
+public:
+  WithNestedColor(raw_ostream &OS, WithNestedColorContext &Ctx,
+                  raw_ostream::Colors Color, bool Bold, bool DisableColors)
+      : OS(OS), Ctx(Ctx), DisableColors(DisableColors) {
+    if (DisableColors)
+      return;
+
+    Ctx.pushColor(Color, Bold);
+    if (Color != raw_ostream::SAVEDCOLOR || Bold)
+      OS.changeColor(Color, Bold);
+  }
+
+  template <typename T> WithNestedColor &operator<<(const T &O) {
+    OS << O;
+    return *this;
+  }
+
+  ~WithNestedColor() {
+    if (DisableColors)
+      return;
+
+    OS.resetColor();
+    Ctx.popColor();
+    // Restore the previous color if it exists.
+    if (!Ctx.empty()) {
+      std::pair<raw_ostream::Colors, bool> Pair = Ctx.currentColor();
+      if (Pair.first != raw_ostream::SAVEDCOLOR || Pair.second)
+        OS.changeColor(Pair.first, Pair.second);
+    }
+  }
+};
+
+static WithColor withHighlightColor(ObjdumpColor Color) {
+  auto Pair = HighlightColors.find(Color);
+  assert(Pair != HighlightColors.end() &&
+         "All token types must be defined in HighlightColors.");
+  return WithColor(outs(), Pair->second.first, Pair->second.second, false,
+                   !ColorsEnabled);
+}
+
+static WithNestedColor withNestedHighlightColor(WithNestedColorContext &Context,
+                                                ObjdumpColor Color) {
+  auto Pair = HighlightColors.find(Color);
+  assert(Pair != HighlightColors.end() &&
+         "All token types must be defined in HighlightColors.");
+  return WithNestedColor(outs(), Context, Pair->second.first,
+                         Pair->second.second, !ColorsEnabled);
+}
+
 static bool shouldKeep(object::SectionRef S) {
   if (FilterSections.empty())
     return true;
 
   Expected<StringRef> SecNameOrErr = S.getName();
   if (!SecNameOrErr) {
     consumeError(SecNameOrErr.takeError());
     return false;

 
 class PrettyPrinter {
 public:
   virtual ~PrettyPrinter() = default;
   virtual void printInst(MCInstPrinter &IP, const MCInst *MI,
                          ArrayRef<uint8_t> Bytes,
                          object::SectionedAddress Address, raw_ostream &OS,
                          StringRef Annot, MCSubtargetInfo const &STI,
                          SourcePrinter *SP, StringRef ObjectFilename,
+                         size_t &InstructionCol,
                          std::vector<RelocationRef> *Rels = nullptr) {
+    uint64_t BeforeOffset = OS.tell();
     if (SP && (PrintSource || PrintLines))
       SP->printSourceLine(OS, Address, ObjectFilename);
 
     size_t Start = OS.tell();
     if (!NoLeadingAddr)
       OS << format("%8" PRIx64 ":", Address.Address);
     if (!NoShowRawInsn) {
       OS << ' ';
       dumpBytes(Bytes, OS);
     }
 
     // The output of printInst starts with a tab. Print some spaces so that
     // the tab has 1 column and advances to the target tab stop.
     unsigned TabStop = NoShowRawInsn ? 16 : 40;
     unsigned Column = OS.tell() - Start;
     OS.indent(Column < TabStop - 1 ? TabStop - 1 - Column : 7 - Column % 8);
 
+    InstructionCol = OS.tell() - BeforeOffset;
     if (MI)
       IP.printInst(MI, OS, "", STI);
     else
       OS << "\t<unknown>";
   }
 };
 PrettyPrinter PrettyPrinterInst;
 
if (!NoShowRawInsn) {
       OS << "\t";
       dumpBytes(Bytes.slice(0, 4), OS);
       OS << format("\t%08" PRIx32, opcode);
     }
   }
   void printInst(MCInstPrinter &IP, const MCInst *MI, ArrayRef<uint8_t> Bytes,
                  object::SectionedAddress Address, raw_ostream &OS,
                  StringRef Annot, MCSubtargetInfo const &STI, SourcePrinter *SP,
-                 StringRef ObjectFilename,
+                 StringRef ObjectFilename, size_t &InstructionCol,
                  std::vector<RelocationRef> *Rels) override {
+    size_t StartOffset = OS.tell();
     if (SP && (PrintSource || PrintLines))
       SP->printSourceLine(OS, Address, ObjectFilename, "");
     if (!MI) {
       printLead(Bytes, Address.Address, OS);
       OS << " <unknown>";
       return;
     }
+
+    InstructionCol = OS.tell() - StartOffset;
     std::string Buffer;
     {
       raw_string_ostream TempStream(Buffer);
       IP.printInst(MI, TempStream, "", STI);
     }
     StringRef Contents(Buffer);
     // Split off bundle attributes
     auto PacketBundle = Contents.rsplit('\n');

 };
 HexagonPrettyPrinter HexagonPrettyPrinterInst;
 
 class AMDGCNPrettyPrinter : public PrettyPrinter {
 public:
   void printInst(MCInstPrinter &IP, const MCInst *MI, ArrayRef<uint8_t> Bytes,
                  object::SectionedAddress Address, raw_ostream &OS,
                  StringRef Annot, MCSubtargetInfo const &STI, SourcePrinter *SP,
-                 StringRef ObjectFilename,
+                 StringRef ObjectFilename, size_t &InstructionCol,
                  std::vector<RelocationRef> *Rels) override {
+    size_t StartOffset = OS.tell();
     if (SP && (PrintSource || PrintLines))
       SP->printSourceLine(OS, Address, ObjectFilename);
 
     if (MI) {
       SmallString<40> InstStr;
       raw_svector_ostream IS(InstStr);
 
+      InstructionCol = OS.tell() - StartOffset;
       IP.printInst(MI, IS, "", STI);
 
       OS << left_justify(IS.str(), 60);
     } else {
       // an unrecognized encoding - this is probably data so represent it
       // using the .long directive, or .byte directive if fewer than 4 bytes
       // remaining
       if (Bytes.size() >= 4) {

 };
 AMDGCNPrettyPrinter AMDGCNPrettyPrinterInst;
 
 class BPFPrettyPrinter : public PrettyPrinter {
 public:
   void printInst(MCInstPrinter &IP, const MCInst *MI, ArrayRef<uint8_t> Bytes,
                  object::SectionedAddress Address, raw_ostream &OS,
                  StringRef Annot, MCSubtargetInfo const &STI, SourcePrinter *SP,
-                 StringRef ObjectFilename,
+                 StringRef ObjectFilename, size_t &InstructionCol,
                  std::vector<RelocationRef> *Rels) override {
+    size_t StartOffset = OS.tell();
     if (SP && (PrintSource || PrintLines))
       SP->printSourceLine(OS, Address, ObjectFilename);
     if (!NoLeadingAddr)
       OS << format("%8" PRId64 ":", Address.Address / 8);
     if (!NoShowRawInsn) {
       OS << "\t";
       dumpBytes(Bytes, OS);
     }
+    InstructionCol = OS.tell() - StartOffset;
     if (MI)
       IP.printInst(MI, OS, "", STI);
     else
       OS << "\t<unknown>";
   }
 };
 BPFPrettyPrinter BPFPrettyPrinterInst;
 
if (NumBytes == 8) {
       outs() << std::string(IndentOffset, ' ') << "         ";
       outs() << reinterpret_cast<char *>(AsciiData);
       outs() << '\n';
       NumBytes = 0;
     }
   }
 }
 
+static void printMarkupSpans(StringRef Text, ArrayRef<MarkupSpan> Spans,
+                             size_t &NextPos, size_t InstOffset,
+                             WithNestedColorContext Context) {
+  for (const MarkupSpan &Span : Spans) {
+    size_t Pos = Span.Pos - InstOffset;
+    size_t InnerPos = Span.InnerPos - InstOffset;
+    outs() << Text.substr(NextPos, Pos - NextPos);
+
+    ObjdumpColor Color;
+    if (Span.Type == MarkupType::Imm)
+      Color = ObjdumpColor::Immediate;
+    else if (Span.Type == MarkupType::Reg)
+      Color = ObjdumpColor::Register;
+    else if (Span.Type == MarkupType::Mem)
+      Color = ObjdumpColor::Memory;
+    else
+      Color = ObjdumpColor::Default;
+
+    if (Span.InnerSpans.empty()) {
+      withNestedHighlightColor(Context, Color)
+          << Text.substr(InnerPos, Span.InnerLength);
+    } else {
+      WithNestedColor WNC = withNestedHighlightColor(Context, Color);
+      NextPos = InnerPos;
+      printMarkupSpans(Text, Span.InnerSpans, NextPos, InstOffset, Context);
+      StringRef AfterText =
+          Text.substr(NextPos, Span.InnerLength - (NextPos - InnerPos));
+      outs() << AfterText;
+    }
+
+    NextPos = Pos + Span.Length;
+  }
+}
+
+static void printMarkedUpInst(StringRef Text,
+                              const std::vector<MarkupSpan> &Spans,
+                              size_t InstructionCol, size_t InstOffset) {
+  if (!ColorsEnabled) {
+    outs() << Text;
+    return;
+  }
+
+  // Print hexdump, etc.
+  outs() << Text.slice(0, InstructionCol);
+
+  StringRef Disasm = Text.slice(InstructionCol, Text.size());
+  size_t NextPos = 0;
+  WithNestedColorContext Context;
+  printMarkupSpans(Disasm, Spans, NextPos, InstOffset, Context);
+
+  // Print the remaining part.
+  outs() << Disasm.substr(NextPos);
+}
+
 static void disassembleObject(const Target *TheTarget, const ObjectFile *Obj,
                               MCContext &Ctx, MCDisassembler *PrimaryDisAsm,
                               MCDisassembler *SecondaryDisAsm,
                               const MCInstrAnalysis *MIA, MCInstPrinter *IP,
                               const MCSubtargetInfo *PrimarySTI,
                               const MCSubtargetInfo *SecondarySTI,
-                              PrettyPrinter &PIP,
-                              SourcePrinter &SP, bool InlineRelocs) {
+                              PrettyPrinter &PIP, SourcePrinter &SP,
+                              bool InlineRelocs) {
   const MCSubtargetInfo *STI = PrimarySTI;
   MCDisassembler *DisAsm = PrimaryDisAsm;
   bool PrimaryIsThumb = false;
   if (isArmElf(Obj))
     PrimaryIsThumb = STI->checkFeatures("+thumb-mode");
 
   std::map<SectionRef, std::vector<RelocationRef>> RelocMap;
   if (InlineRelocs)
if (Symbols.empty() || std::get<0>(Symbols[0]) != 0) {
           Symbols.begin(),
           std::make_tuple(SectionAddr, SectionName,
                           Section.isText() ? ELF::STT_FUNC : ELF::STT_OBJECT));
     }
 
     SmallString<40> Comments;
     raw_svector_ostream CommentStream(Comments);
 
+    std::vector<MarkupSpan> MarkupSpans;
+    IP->setMarkupSpans(MarkupSpans);
+
     ArrayRef<uint8_t> Bytes = arrayRefFromStringRef(
         unwrapOrError(Section.getContents(), Obj->getFileName()));
 
     uint64_t VMAAdjustment = 0;
     if (shouldAdjustVA(Section))
       VMAAdjustment = AdjustVMA;
 
     uint64_t Size;
#endif
         // provided
         MCInst Inst;
         bool Disassembled = DisAsm->getInstruction(
             Inst, Size, Bytes.slice(Index), SectionAddr + Index, DebugOut,
             CommentStream);
         if (Size == 0)
           Size = 1;
 
+        std::string InstText;
+        raw_string_ostream InstTextStream(InstText);
+        size_t InstructionCol;
+        size_t StartOffset = InstTextStream.tell();
         PIP.printInst(*IP, Disassembled ? &Inst : nullptr,
                       Bytes.slice(Index, Size),
                       {SectionAddr + Index + VMAAdjustment, Section.getIndex()},
-                      outs(), "", *STI, &SP, Obj->getFileName(), &Rels);
+                      InstTextStream, "", *STI, &SP, Obj->getFileName(),
+                      InstructionCol, &Rels);
+        printMarkedUpInst(StringRef(InstTextStream.str()), MarkupSpans,
+                          InstructionCol, StartOffset + InstructionCol);
         outs() << CommentStream.str();
         Comments.clear();
+        MarkupSpans.clear();
 
         // Try to resolve the target of a call, tail call, etc. to a specific
         // symbol.
         if (MIA && (MIA->isCall(Inst) || MIA->isUnconditionalBranch(Inst) ||
                     MIA->isConditionalBranch(Inst))) {
           uint64_t Target;
           if (MIA->evaluateBranch(Inst, SectionAddr + Index, Size, Target)) {
             // In a relocatable object, the target's section must reside in
#endif
                   [=](const std::tuple<uint64_t, StringRef, uint8_t> &O) {
                     return std::get<0>(O) <= Target;
                   });
             }
             if (TargetSym != TargetSectionSymbols->begin()) {
               --TargetSym;
               uint64_t TargetAddress = std::get<0>(*TargetSym);
               StringRef TargetName = std::get<1>(*TargetSym);
-              outs() << " <" << TargetName;
+              WithColor Highlight =
+                  withHighlightColor(ObjdumpColor::SymbolName);
+              Highlight << " <" << TargetName;
               uint64_t Disp = Target - TargetAddress;
               if (Disp)
-                outs() << "+0x" << Twine::utohexstr(Disp);
-              outs() << '>';
+                Highlight << "+0x" << Twine::utohexstr(Disp);
+              Highlight << '>';
             }
           }
         }
         outs() << "\n";
 
         // Hexagon does this in pretty printer
         if (Obj->getArch() != Triple::hexagon) {
           // Print relocation for instruction.
static void disassembleObject(const ObjectFile *Obj, bool InlineRelocs) {
 
   int AsmPrinterVariant = AsmInfo->getAssemblerDialect();
   std::unique_ptr<MCInstPrinter> IP(TheTarget->createMCInstPrinter(
       Triple(TripleName), AsmPrinterVariant, *AsmInfo, *MII, *MRI));
   if (!IP)
     report_error(Obj->getFileName(),
                  "no instruction printer for target " + TripleName);
   IP->setPrintImmHex(PrintImmHex);
+  IP->setUseMarkup(ColorsEnabled);
 
   PrettyPrinter &PIP = selectPrettyPrinter(Triple(TripleName));
   SourcePrinter SP(Obj, TheTarget->getName());
 
   for (StringRef Opt : DisassemblerOptions)
     if (!IP->applyTargetSpecificCLOption(Opt))
       error("Unrecognized disassembler option: " + Opt);
 
static void dumpInput(StringRef file) {
   else
     report_error(errorCodeToError(object_error::invalid_file_type), file);
 }
 } // namespace llvm
 
 int main(int argc, char **argv) {
   using namespace llvm;
   InitLLVM X(argc, argv);
-  const cl::OptionCategory *OptionFilters[] = {&ObjdumpCat, &MachOCat};
+  const cl::OptionCategory *OptionFilters[] = {&ObjdumpCat, &MachOCat,
+                                               &ColorCategory};
   cl::HideUnrelatedOptions(OptionFilters);
 
   // Initialize targets and assembly printers/parsers.
   InitializeAllTargetInfos();
   InitializeAllTargetMCs();
   InitializeAllDisassemblers();
 
   // Register the target printer for --version.
   cl::AddExtraVersionPrinter(TargetRegistry::printRegisteredTargetsForVersion);
 
   cl::ParseCommandLineOptions(argc, argv, "llvm object file dumper\n");
 
   if (StartAddress >= StopAddress)
     error("start address should be less than stop address");
 
+  ColorsEnabled = WithColor::colorsEnabled(outs());
+
   ToolName = argv[0];
 
   // Defaults to a.out if no filenames specified.
   if (InputFilenames.empty())
     InputFilenames.push_back("a.out");
 
   if (AllHeaders)
     ArchiveHeaders = FileHeaders = PrivateHeaders = Relocations =