[lldb] Add assembly syntax highlighting

[JDevlieghere]

Summary

Add support for syntax highlighting assembly. The current patch uses a different color to highlight registers, immediate and addresses.

[JDevlieghere]

Similar to the LLVM patch this depends on, this patch is a WIP to get some early feedback. This definitely needs a test. Most of the patch deals with plumbing the value of Debugger::GetUseColor to the Disassembler.

[jasonmolenda]

LGTM this is just mechanical passing around of the user's setting though the layers.

lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h

76 ↗

(On Diff #554569)

Most of this patch uses use_color, but in this class there's a couple use of colors. I don't think it was an intentional difference, might as well make them all the same. In llvm the MCInstPrinter has a setUseColor but llvm::raw_string_ostream has a enable_colors, maybe that's where the plural came in.

[JDevlieghere]

• Fix inconsistencies and use use_color everywhere

[clayborg]

We shouldn't need to pass in "bool use_color" to the Disassembler creation functions as the only reason to pass something to the creation function for any plug-in is if that data would exclude a disassembler if it didn't support color. But we always want to see some disassembly rather than none if a plug-in is the only one that handles a certain architecture and doesn't support color. We should either add accessors to enable colorization to the Disassembler virtual plug-in interface _or_ add "bool use_color" to the needed functions. Looks DisassembleBytes added a "bool use_color" argument to the call already, so we shouldn't need a "bool use_color" for the plug-in selection. Are there other places that need to know about the "use_color" setting that do not results from a direct function call that can have an argument added to it?

Even if colorization is enabled, It would still expect any APIs that return information in individual instructions, like:

const char *SBInstruction::GetMnemonic(lldb::SBTarget target);
const char *SBInstruction::GetOperands(lldb::SBTarget target);
const char *SBInstruction::GetComment(lldb::SBTarget target);

To not have colorization attached to the returned string.

lldb/include/lldb/Core/Disassembler.h

415

We shouldn't need to pass "use_color" in when finding the disassembler plug-in. We should add an accessor so the Disassembler virtual function list like:

bool Disassembler::SetUseColor(bool enable);

And the plug-in can return true if it supports colorization and false if not. The selection of the disassembler plug-in shouldn't be predicated on color support right?

420

remove and use accessor suggested above?

lldb/include/lldb/lldb-private-interfaces.h

33 ↗

(On Diff #554873)

remove per above comments

lldb/source/Commands/CommandObjectDisassemble.cpp

456–457 ↗

(On Diff #554873)

revert

lldb/source/Core/Disassembler.cpp

59

revert

[JDevlieghere]

In D159164#4632017, @clayborg wrote:

We shouldn't need to pass in "bool use_color" to the Disassembler creation functions as the only reason to pass something to the creation function for any plug-in is if that data would exclude a disassembler if it didn't support color. But we always want to see some disassembly rather than none if a plug-in is the only one that handles a certain architecture and doesn't support color. We should either add accessors to enable colorization to the Disassembler virtual plug-in interface _or_ add "bool use_color" to the needed functions. Looks DisassembleBytes added a "bool use_color" argument to the call already, so we shouldn't need a "bool use_color" for the plug-in selection. Are there other places that need to know about the "use_color" setting that do not results from a direct function call that can have an argument added to it?

Thanks for the suggestion, that's definitely much nicer. I made it part of the create function because I thought we needed to know the value during initialization time, but you're correct that we can set this after the fact with the help of an accessor. This simplifies the patch by a lot.

Even if colorization is enabled, It would still expect any APIs that return information in individual instructions, like:

const char *SBInstruction::GetMnemonic(lldb::SBTarget target);
const char *SBInstruction::GetOperands(lldb::SBTarget target);
const char *SBInstruction::GetComment(lldb::SBTarget target);

To not have colorization attached to the returned string.

Yeah, the updated patch accounts for that and adds a test.

[JDevlieghere]

Address @clayborg's code review feedback

[JDevlieghere]

Discard whitespace changes

[clayborg]

Yes, this simplifies the patch a lot. Looks good after we check the mnemonic and comment for no color when used through the SBInstruction APIs.

lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py

70

Can we check both:

inst.GetMnemonic(target)
inst.GetComment(target)

here too to make sure no colorization gets added?

[JDevlieghere]

Test GetMnemonic and GetComment.

[clayborg]

lgtm. Is there any more things you want to add to this patch test wise, or is this complete?

[JDevlieghere]

As I was adding another test I realized an issue with the previous approach due to the fact that we cache the result. The solution is to cache both the plain and the marked up result, and have the different accessors decide whether they want colored output or not. As a result, I've removed the global Disassembler option to enable color, as it's not really meaningful anymore. I've also added a test to make sure this doesn't break.

[JDevlieghere]

@clayborg let me know if you're happy with this. To answer your previous question, I'm not planning anymore changes.

[clayborg]

lldb/include/lldb/Core/Disassembler.h

158

We might not need "bool show_color" here if we can rely on the "exe_ctx->GetTarget()->GetDebugger()"?

343–344

Ditto here. Should we rely on the exe_ctx having a target and thus we can get to the debugger instead of passing "show_color" as a separate arg?

457–458

should "show_color" be put as a bit into the "options" instead?

lldb/source/API/SBInstruction.cpp

263–264

If we rely on the execution context being NULL, then we don't need to pass in color as false here.

299

ditto

lldb/source/API/SBInstructionList.cpp

169–170

ditto

lldb/source/Core/Disassembler.cpp

604–605

Should we just rely on the "exe_ctx" having a target here instead of adding a new parameter here? Cause we can do "exe_ctx->GetTarget()->GetDebugger()" if needed. If no exe_ctx, then assume no color?

658

The color codes bytes should't contribute to the opcode columns width. We should probably use "m_opcode_name", but we will need to guarantee that both are filled in if we ask for color. Can we modify it so if we ask for the opcode name with color that we always fill in the m_opcode_name in as well?

lldb/source/Core/DumpDataExtractor.cpp

160

Here is an example of where we fill the execution context in with the target, but also pass in "target_sp->GetDebugger().GetUseColor()"

[JDevlieghere]

Use the execution context to enable colors.

[clayborg]

Just one last inline comment where we can't use the colorized string to calculate the column width and this is good to go.

lldb/source/Core/Disassembler.cpp

657–659

still need to directly use "m_opcode_name" here instead of "opcode_name" so we don't include color codes as characters.

[JDevlieghere]

Thanks Greg!

[clayborg]

lldb/source/Core/Disassembler.cpp

657–659

lldb/include/lldb/Core/Disassembler.h

 public:
   Instruction(const Address &address,
               AddressClass addr_class = AddressClass::eInvalid);
 
   virtual ~Instruction();
 
   const Address &GetAddress() const { return m_address; }
 
-  const char *GetMnemonic(const ExecutionContext *exe_ctx) {
+  const char *GetMnemonic(const ExecutionContext *exe_ctx,
+                          bool markup = false) {
     CalculateMnemonicOperandsAndCommentIfNeeded(exe_ctx);
-    return m_opcode_name.c_str();
+    return markup ? m_markup_opcode_name.c_str() : m_opcode_name.c_str();
   }
 
-  const char *GetOperands(const ExecutionContext *exe_ctx) {
+  const char *GetOperands(const ExecutionContext *exe_ctx,
+                          bool markup = false) {
     CalculateMnemonicOperandsAndCommentIfNeeded(exe_ctx);
-    return m_mnemonics.c_str();
+    return markup ? m_markup_mnemonics.c_str() : m_mnemonics.c_str();
   }
 
   const char *GetComment(const ExecutionContext *exe_ctx) {
     CalculateMnemonicOperandsAndCommentIfNeeded(exe_ctx);
     return m_comment.c_str();
   }
 
   /// \return
public:
   /// \param[in] max_address_text_size
   ///     The length of the longest address string at the start of the
   ///     disassembly line that will be printed (the
   ///     Debugger::FormatDisassemblerAddress() string)
   ///     so this method can properly align the instruction opcodes.
   ///     May be 0 to indicate no indentation/alignment of the opcodes.
   virtual void Dump(Stream *s, uint32_t max_opcode_byte_size, bool show_address,
                     bool show_bytes, bool show_control_flow_kind,
-                    const ExecutionContext *exe_ctx,
+                    bool show_color, const ExecutionContext *exe_ctx,
                     const SymbolContext *sym_ctx,
                     const SymbolContext *prev_sym_ctx,
                     const FormatEntity::Entry *disassembly_addr_format,
                     size_t max_address_text_size);
 
   virtual bool DoesBranch() = 0;
 
   virtual bool HasDelaySlot();
Address m_address; // The section offset address of this instruction
                      // when disassembling memory, you might run into data.
                      // This can help us to disassemble appropriately.
 private:
   AddressClass m_address_class; // Use GetAddressClass () accessor function!
 
 protected:
   Opcode m_opcode; // The opcode for this instruction
   std::string m_opcode_name;
+  std::string m_markup_opcode_name;
   std::string m_mnemonics;
+  std::string m_markup_mnemonics;
   std::string m_comment;
   bool m_calculated_strings;
 
   void
   CalculateMnemonicOperandsAndCommentIfNeeded(const ExecutionContext *exe_ctx) {
     if (!m_calculated_strings) {
       m_calculated_strings = true;
       CalculateMnemonicOperandsAndComment(exe_ctx);
public:
 
   uint32_t GetIndexOfInstructionAtAddress(const Address &addr);
 
   void Clear();
 
   void Append(lldb::InstructionSP &inst_sp);
 
   void Dump(Stream *s, bool show_address, bool show_bytes,
-            bool show_control_flow_kind, const ExecutionContext *exe_ctx);
+            bool show_control_flow_kind, bool show_color,
+            const ExecutionContext *exe_ctx);
 
 private:
   typedef std::vector<lldb::InstructionSP> collection;
   typedef collection::iterator iterator;
   typedef collection::const_iterator const_iterator;
 
   collection m_instructions;
 };
public:
 
   // FindPlugin should be lax about the flavor string (it is too annoying to
   // have various internal uses of the disassembler fail because the global
   // flavor string gets set wrong. Instead, if you get a flavor string you
   // don't understand, use the default.  Folks who care to check can use the
   // FlavorValidForArchSpec method on the disassembler they got back.
   static lldb::DisassemblerSP
   FindPlugin(const ArchSpec &arch, const char *flavor, const char *plugin_name);
 
   // This version will use the value in the Target settings if flavor is NULL;
   static lldb::DisassemblerSP FindPluginForTarget(const Target &target,
                                                   const ArchSpec &arch,
                                                   const char *flavor,
                                                   const char *plugin_name);
 
   struct Limit {
     enum { Bytes, Instructions } kind;
     lldb::addr_t value;
   };
 
   static lldb::DisassemblerSP DisassembleRange(const ArchSpec &arch,
                                                const char *plugin_name,
public:
 
   // Constructors and Destructors
   Disassembler(const ArchSpec &arch, const char *flavor);
   ~Disassembler() override;
 
   void PrintInstructions(Debugger &debugger, const ArchSpec &arch,
                          const ExecutionContext &exe_ctx,
                          bool mixed_source_and_assembly,
-                         uint32_t num_mixed_context_lines, uint32_t options,
-                         Stream &strm);
+                         uint32_t num_mixed_context_lines, bool show_color,
+                         uint32_t options, Stream &strm);
 
   size_t ParseInstructions(Target &target, Address address, Limit limit,
                            Stream *error_strm_ptr,
                            bool force_live_memory = false);
 
   virtual size_t DecodeInstructions(const Address &base_addr,
                                     const DataExtractor &data,
                                     lldb::offset_t data_offset,

lldb/source/API/SBInstruction.cpp

if (inst_sp) {
     ModuleSP module_sp(addr.GetModule());
     if (module_sp)
       module_sp->ResolveSymbolContextForAddress(addr, eSymbolContextEverything,
                                                 sc);
     // Use the "ref()" instead of the "get()" accessor in case the SBStream
     // didn't have a stream already created, one will get created...
     FormatEntity::Entry format;
     FormatEntity::Parse("${addr}: ", format);
     inst_sp->Dump(&s.ref(), 0, true, false, /*show_control_flow_kind=*/false,
-                  nullptr, &sc, nullptr, &format, 0);
+                  /*show_color=*/false, nullptr, &sc, nullptr, &format, 0);
     return true;
   }
   return false;
 }
 
 void SBInstruction::Print(FILE *outp) {
   LLDB_INSTRUMENT_VA(this, outp);
   FileSP out = std::make_shared<NativeFile>(outp, /*take_ownership=*/false);
if (inst_sp) {
     ModuleSP module_sp(addr.GetModule());
     if (module_sp)
       module_sp->ResolveSymbolContextForAddress(addr, eSymbolContextEverything,
                                                 sc);
     StreamFile out_stream(out_sp);
     FormatEntity::Entry format;
     FormatEntity::Parse("${addr}: ", format);
     inst_sp->Dump(&out_stream, 0, true, false, /*show_control_flow_kind=*/false,
-                  nullptr, &sc, nullptr, &format, 0);
+                  /*show_color=*/false, nullptr, &sc, nullptr, &format, 0);
   }
 }
 
 bool SBInstruction::EmulateWithFrame(lldb::SBFrame &frame,
                                      uint32_t evaluate_options) {
   LLDB_INSTRUMENT_VA(this, frame, evaluate_options);
 
   lldb::InstructionSP inst_sp(GetOpaque());

lldb/source/API/SBInstructionList.cpp

if (num_instructions) {
         prev_sc = sc;
         ModuleSP module_sp(addr.GetModule());
         if (module_sp) {
           module_sp->ResolveSymbolContextForAddress(
               addr, eSymbolContextEverything, sc);
         }
 
         inst->Dump(&sref, max_opcode_byte_size, true, false,
-                   /*show_control_flow_kind=*/false, nullptr, &sc, &prev_sc,
-                   &format, 0);
+                   /*show_control_flow_kind=*/false, /*show_color=*/false,
+                   nullptr, &sc, &prev_sc, &format, 0);
         sref.EOL();
       }
       return true;
     }
   }
   return false;
 }
 

lldb/source/Core/Disassembler.cpp

 #include <cassert>
 
 #define DEFAULT_DISASM_BYTE_SIZE 32
 
 using namespace lldb;
 using namespace lldb_private;
 
 DisassemblerSP Disassembler::FindPlugin(const ArchSpec &arch,
                                         const char *flavor,
                                         const char *plugin_name) {
   LLDB_SCOPED_TIMERF("Disassembler::FindPlugin (arch = %s, plugin_name = %s)",
                      arch.GetArchitectureName(), plugin_name);
 
   DisassemblerCreateInstance create_callback = nullptr;
 
   if (plugin_name) {
     create_callback =
if (!disasm_sp)
     return false;
 
   const bool force_live_memory = true;
   size_t bytes_disassembled = disasm_sp->ParseInstructions(
       exe_ctx.GetTargetRef(), address, limit, &strm, force_live_memory);
   if (bytes_disassembled == 0)
     return false;
 
-  disasm_sp->PrintInstructions(debugger, arch, exe_ctx,
-                               mixed_source_and_assembly,
-                               num_mixed_context_lines, options, strm);
+  disasm_sp->PrintInstructions(
+      debugger, arch, exe_ctx, mixed_source_and_assembly,
+      num_mixed_context_lines, debugger.GetUseColor(), options, strm);
   return true;
 }
 
 Disassembler::SourceLine
 Disassembler::GetFunctionDeclLineEntry(const SymbolContext &sc) {
   if (!sc.function)
     return {};
 
bool Disassembler::ElideMixedSourceAndDisassemblyLine(
   // don't skip this source line
   return false;
 }
 
 void Disassembler::PrintInstructions(Debugger &debugger, const ArchSpec &arch,
                                      const ExecutionContext &exe_ctx,
                                      bool mixed_source_and_assembly,
                                      uint32_t num_mixed_context_lines,
-                                     uint32_t options, Stream &strm) {
+                                     bool show_color, uint32_t options,
+                                     Stream &strm) {
   // We got some things disassembled...
   size_t num_instructions_found = GetInstructionList().GetSize();
 
   const uint32_t max_opcode_byte_size =
       GetInstructionList().GetMaxOpcocdeByteSize();
   SymbolContext sc;
   SymbolContext prev_sc;
   AddressRange current_source_line_range;
if (inst) {
         if (source_lines_to_display.print_source_context_end_eol)
           strm.EOL();
       }
 
       const bool show_bytes = (options & eOptionShowBytes) != 0;
       const bool show_control_flow_kind =
           (options & eOptionShowControlFlowKind) != 0;
       inst->Dump(&strm, max_opcode_byte_size, true, show_bytes,
-                 show_control_flow_kind, &exe_ctx, &sc, &prev_sc, nullptr,
-                 address_text_size);
+                 show_control_flow_kind, show_color, &exe_ctx, &sc, &prev_sc,
+                 nullptr, address_text_size);
       strm.EOL();
     } else {
       break;
     }
   }
 }
 
 bool Disassembler::Disassemble(Debugger &debugger, const ArchSpec &arch,
const char *Instruction::GetNameForInstructionControlFlowKind(
   case eInstructionControlFlowKindFarJump:
     return "far jump";
   }
   llvm_unreachable("Fully covered switch above!");
 }
 
 void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size,
                        bool show_address, bool show_bytes,
-                       bool show_control_flow_kind,
+                       bool show_control_flow_kind, bool show_color,
                        const ExecutionContext *exe_ctx,
                        const SymbolContext *sym_ctx,
                        const SymbolContext *prev_sym_ctx,
                        const FormatEntity::Entry *disassembly_addr_format,
                        size_t max_address_text_size) {
   size_t opcode_column_width = 7;
   const size_t operand_column_width = 25;
 
   CalculateMnemonicOperandsAndCommentIfNeeded(exe_ctx);
void Instruction::Dump(lldb_private::Stream *s, uint32_t max_opcode_byte_size,
   if (show_control_flow_kind) {
     lldb::InstructionControlFlowKind instruction_control_flow_kind =
         GetControlFlowKind(exe_ctx);
     ss.Printf("%-12s", GetNameForInstructionControlFlowKind(
                            instruction_control_flow_kind));
   }
 
   const size_t opcode_pos = ss.GetSizeOfLastLine();
+  const std::string &opcode_name =
+      show_color ? m_markup_opcode_name : m_opcode_name;
+  const std::string &mnemonics = show_color ? m_markup_mnemonics : m_mnemonics;
 
   // The default opcode size of 7 characters is plenty for most architectures
   // but some like arm can pull out the occasional vqrshrun.s16.  We won't get
   // consistent column spacing in these cases, unfortunately.
-  if (m_opcode_name.length() >= opcode_column_width) {
-    opcode_column_width = m_opcode_name.length() + 1;
+  if (opcode_name.length() >= opcode_column_width) {
+    opcode_column_width = opcode_name.length() + 1;
   }
 
-  ss.PutCString(m_opcode_name);
+  ss.PutCString(opcode_name);
   ss.FillLastLineToColumn(opcode_pos + opcode_column_width, ' ');
-  ss.PutCString(m_mnemonics);
+  ss.PutCString(mnemonics);
 
   if (!m_comment.empty()) {
     ss.FillLastLineToColumn(
         opcode_pos + opcode_column_width + operand_column_width, ' ');
     ss.PutCString(" ; ");
     ss.PutCString(m_comment);
   }
   s->PutCString(ss.GetString());

 InstructionSP InstructionList::GetInstructionAtAddress(const Address &address) {
   uint32_t index = GetIndexOfInstructionAtAddress(address);
   if (index != UINT32_MAX)
     return GetInstructionAtIndex(index);
   return nullptr;
 }
 
 void InstructionList::Dump(Stream *s, bool show_address, bool show_bytes,
-                           bool show_control_flow_kind,
+                           bool show_control_flow_kind, bool show_color,
                            const ExecutionContext *exe_ctx) {
   const uint32_t max_opcode_byte_size = GetMaxOpcocdeByteSize();
   collection::const_iterator pos, begin, end;
 
   const FormatEntity::Entry *disassembly_format = nullptr;
   FormatEntity::Entry format;
   if (exe_ctx && exe_ctx->HasTargetScope()) {
     disassembly_format =
         exe_ctx->GetTargetRef().GetDebugger().GetDisassemblyFormat();
   } else {
     FormatEntity::Parse("${addr}: ", format);
     disassembly_format = &format;
   }
 
   for (begin = m_instructions.begin(), end = m_instructions.end(), pos = begin;
        pos != end; ++pos) {
     if (pos != begin)
       s->EOL();
     (*pos)->Dump(s, max_opcode_byte_size, show_address, show_bytes,
-                 show_control_flow_kind, exe_ctx, nullptr, nullptr,
+                 show_control_flow_kind, show_color, exe_ctx, nullptr, nullptr,
                  disassembly_format, 0);
   }
 }
 
 void InstructionList::Clear() { m_instructions.clear(); }
 
 void InstructionList::Append(lldb::InstructionSP &inst_sp) {
   if (inst_sp)

lldb/source/Core/DumpDataExtractor.cpp

 //===-- DumpDataExtractor.cpp ---------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Core/DumpDataExtractor.h"
 
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-forward.h"
 
 #include "lldb/Core/Address.h"
+#include "lldb/Core/Debugger.h"
 #include "lldb/Core/Disassembler.h"
 #include "lldb/Core/ModuleList.h"
 #include "lldb/Target/ABI.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/ExecutionContextScope.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/MemoryTagManager.h"
 #include "lldb/Target/MemoryTagMap.h"
if (disassembler_sp) {
       if (bytes_consumed) {
         offset += bytes_consumed;
         const bool show_address = base_addr != LLDB_INVALID_ADDRESS;
         const bool show_bytes = true;
         const bool show_control_flow_kind = true;
         ExecutionContext exe_ctx;
         exe_scope->CalculateExecutionContext(exe_ctx);
         disassembler_sp->GetInstructionList().Dump(
-            s, show_address, show_bytes, show_control_flow_kind, &exe_ctx);
+            s, show_address, show_bytes, show_control_flow_kind,
+            target_sp->GetDebugger().GetUseColor(), &exe_ctx);
       }
     }
   } else
     s->Printf("invalid target");
 
   return offset;
 }
 

lldb/source/Expression/IRExecutionUnit.cpp

extractor.PutToLog(log, 0, extractor.GetByteSize(), func_remote_addr, 16,
                        DataExtractor::TypeUInt8);
   }
 
   disassembler_sp->DecodeInstructions(Address(func_remote_addr), extractor, 0,
                                       UINT32_MAX, false, false);
 
   InstructionList &instruction_list = disassembler_sp->GetInstructionList();
   instruction_list.Dump(&stream, true, true, /*show_control_flow_kind=*/true,
-                        &exe_ctx);
+                        /*show_color=*/false, &exe_ctx);
 
   return ret;
 }
 
 namespace {
 struct IRExecDiagnosticHandler : public llvm::DiagnosticHandler {
   Status *err;
   IRExecDiagnosticHandler(Status *err) : err(err) {}
void IRExecutionUnit::GetRunnableInfo(Status &error, lldb::addr_t &func_addr,
   public:
     ObjectDumper(FileSpec output_dir)  : m_out_dir(output_dir) {}
     void notifyObjectCompiled(const llvm::Module *module,
                               llvm::MemoryBufferRef object) override {
       int fd = 0;
       llvm::SmallVector<char, 256> result_path;
       std::string object_name_model =
           "jit-object-" + module->getModuleIdentifier() + "-%%%.o";
-      FileSpec model_spec 
-          = m_out_dir.CopyByAppendingPathComponent(object_name_model);
+      FileSpec model_spec =
+          m_out_dir.CopyByAppendingPathComponent(object_name_model);
       std::string model_path = model_spec.GetPath();
 
-      std::error_code result 
-        = llvm::sys::fs::createUniqueFile(model_path, fd, result_path);
+      std::error_code result =
+          llvm::sys::fs::createUniqueFile(model_path, fd, result_path);
       if (!result) {
           llvm::raw_fd_ostream fds(fd, true);
           fds.write(object.getBufferStart(), object.getBufferSize());
       }
     }
     std::unique_ptr<llvm::MemoryBuffer>
     getObject(const llvm::Module *module) override  {
       // Return nothing - we're just abusing the object-cache mechanism to dump

 }
 
 llvm::JITSymbol 
 IRExecutionUnit::MemoryManager::findSymbol(const std::string &Name) {
     bool missing_weak = false;
     uint64_t addr = GetSymbolAddressAndPresence(Name, missing_weak);
     // This is a weak symbol:
     if (missing_weak) 
-      return llvm::JITSymbol(addr, 
-          llvm::JITSymbolFlags::Exported | llvm::JITSymbolFlags::Weak);
+    return llvm::JITSymbol(addr, llvm::JITSymbolFlags::Exported |
+                                     llvm::JITSymbolFlags::Weak);
     else
       return llvm::JITSymbol(addr, llvm::JITSymbolFlags::Exported);
 }
 
 uint64_t
 IRExecutionUnit::MemoryManager::getSymbolAddress(const std::string &Name) {
   bool missing_weak = false;
   return GetSymbolAddressAndPresence(Name, missing_weak);
 }
 
-uint64_t 
-IRExecutionUnit::MemoryManager::GetSymbolAddressAndPresence(
+uint64_t IRExecutionUnit::MemoryManager::GetSymbolAddressAndPresence(
     const std::string &Name, bool &missing_weak) {
   Log *log = GetLog(LLDBLog::Expressions);
 
   ConstString name_cs(Name.c_str());
 
   lldb::addr_t ret = m_parent.FindSymbol(name_cs, missing_weak);
 
   if (ret == LLDB_INVALID_ADDRESS) {

lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp

 //===-- DisassemblerLLVMC.cpp ---------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "DisassemblerLLVMC.h"
 
 #include "llvm-c/Disassembler.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCDisassembler/MCDisassembler.h"
 #include "llvm/MC/MCDisassembler/MCExternalSymbolizer.h"
 #include "llvm/MC/MCDisassembler/MCRelocationInfo.h"
 #include "llvm/MC/MCInst.h"
public:
 
   ~MCDisasmInstance() = default;
 
   uint64_t GetMCInst(const uint8_t *opcode_data, size_t opcode_data_len,
                      lldb::addr_t pc, llvm::MCInst &mc_inst) const;
   void PrintMCInst(llvm::MCInst &mc_inst, lldb::addr_t pc,
                    std::string &inst_string, std::string &comments_string);
   void SetStyle(bool use_hex_immed, HexImmediateStyle hex_style);
+  void SetUseColor(bool use_color);
+  bool GetUseColor() const;
   bool CanBranch(llvm::MCInst &mc_inst) const;
   bool HasDelaySlot(llvm::MCInst &mc_inst) const;
   bool IsCall(llvm::MCInst &mc_inst) const;
   bool IsLoad(llvm::MCInst &mc_inst) const;
   bool IsAuthenticated(llvm::MCInst &mc_inst) const;
 
 private:
   MCDisasmInstance(std::unique_ptr<llvm::MCInstrInfo> &&instr_info_up,
public:
 
   void CalculateMnemonicOperandsAndComment(
       const lldb_private::ExecutionContext *exe_ctx) override {
     DataExtractor data;
     const AddressClass address_class = GetAddressClass();
 
     if (m_opcode.GetData(data)) {
       std::string out_string;
+      std::string markup_out_string;
       std::string comment_string;
+      std::string markup_comment_string;
 
       DisassemblerScope disasm(*this, exe_ctx);
       if (disasm) {
         DisassemblerLLVMC::MCDisasmInstance *mc_disasm_ptr;
 
         if (address_class == AddressClass::eCodeAlternateISA)
           mc_disasm_ptr = disasm->m_alternate_disasm_up.get();
         else
if (m_opcode.GetData(data)) {
         const uint8_t *opcode_data = data.GetDataStart();
         const size_t opcode_data_len = data.GetByteSize();
         llvm::MCInst inst;
         size_t inst_size =
             mc_disasm_ptr->GetMCInst(opcode_data, opcode_data_len, pc, inst);
 
         if (inst_size > 0) {
           mc_disasm_ptr->SetStyle(use_hex_immediates, hex_style);
+
+          const bool saved_use_color = mc_disasm_ptr->GetUseColor();
+          mc_disasm_ptr->SetUseColor(false);
           mc_disasm_ptr->PrintMCInst(inst, pc, out_string, comment_string);
+          mc_disasm_ptr->SetUseColor(true);
+          mc_disasm_ptr->PrintMCInst(inst, pc, markup_out_string,
+                                     markup_comment_string);
+          mc_disasm_ptr->SetUseColor(saved_use_color);
 
           if (!comment_string.empty()) {
             AppendComment(comment_string);
           }
         }
 
         if (inst_size == 0) {
           m_comment.assign("unknown opcode");
if (m_opcode.GetData(data)) {
         static RegularExpression s_regex(
             llvm::StringRef("[ \t]*([^ ^\t]+)[ \t]*([^ ^\t].*)?"));
 
         llvm::SmallVector<llvm::StringRef, 4> matches;
         if (s_regex.Execute(out_string, &matches)) {
           m_opcode_name = matches[1].str();
           m_mnemonics = matches[2].str();
         }
+        matches.clear();
+        if (s_regex.Execute(markup_out_string, &matches)) {
+          m_markup_opcode_name = matches[1].str();
+          m_markup_mnemonics = matches[2].str();
+        }
       }
     }
   }
 
   bool IsValid() const { return m_is_valid; }
 
   bool UsingFileAddress() const { return m_using_file_addr; }
   size_t GetByteSize() const { return m_opcode.GetByteSize(); }

 }
 
 void DisassemblerLLVMC::MCDisasmInstance::PrintMCInst(
     llvm::MCInst &mc_inst, lldb::addr_t pc, std::string &inst_string,
     std::string &comments_string) {
   llvm::raw_string_ostream inst_stream(inst_string);
   llvm::raw_string_ostream comments_stream(comments_string);
 
+  inst_stream.enable_colors(m_instr_printer_up->getUseColor());
   m_instr_printer_up->setCommentStream(comments_stream);
   m_instr_printer_up->printInst(&mc_inst, pc, llvm::StringRef(),
                                 *m_subtarget_info_up, inst_stream);
   m_instr_printer_up->setCommentStream(llvm::nulls());
+
   comments_stream.flush();
 
   static std::string g_newlines("\r\n");
 
   for (size_t newline_pos = 0;
        (newline_pos = comments_string.find_first_of(g_newlines, newline_pos)) !=
        comments_string.npos;
        /**/) {
case eHexStyleC:
     m_instr_printer_up->setPrintHexStyle(llvm::HexStyle::C);
     break;
   case eHexStyleAsm:
     m_instr_printer_up->setPrintHexStyle(llvm::HexStyle::Asm);
     break;
   }
 }
 
+void DisassemblerLLVMC::MCDisasmInstance::SetUseColor(bool use_color) {
+  m_instr_printer_up->setUseColor(use_color);
+}
+
+bool DisassemblerLLVMC::MCDisasmInstance::GetUseColor() const {
+  return m_instr_printer_up->getUseColor();
+}
+
 bool DisassemblerLLVMC::MCDisasmInstance::CanBranch(
     llvm::MCInst &mc_inst) const {
   if (m_instr_analysis_up)
     return m_instr_analysis_up->mayAffectControlFlow(mc_inst, *m_reg_info_up);
   return m_instr_info_up->get(mc_inst.getOpcode())
       .mayAffectControlFlow(mc_inst, *m_reg_info_up);
 }
 

lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp

if (disasm_sp) {
 
       m_range_ptr = ⦥
       m_unwind_plan_ptr = &unwind_plan;
 
       const uint32_t addr_byte_size = m_arch.GetAddressByteSize();
       const bool show_address = true;
       const bool show_bytes = true;
       const bool show_control_flow_kind = true;
+      const bool show_color = false;
       m_cfa_reg_info = *m_inst_emulator_up->GetRegisterInfo(
           unwind_plan.GetRegisterKind(), unwind_plan.GetInitialCFARegister());
       m_fp_is_cfa = false;
       m_register_values.clear();
       m_pushed_regs.clear();
 
       // Initialize the CFA with a known value. In the 32 bit case it will be
       // 0x80000000, and in the 64 bit case 0x8000000000000000. We use the
if (disasm_sp) {
               condition_block_start_offset = current_offset;
             }
 
             if (log && log->GetVerbose()) {
               StreamString strm;
               lldb_private::FormatEntity::Entry format;
               FormatEntity::Parse("${frame.pc}: ", format);
               inst->Dump(&strm, inst_list.GetMaxOpcocdeByteSize(), show_address,
-                         show_bytes, show_control_flow_kind, nullptr, nullptr,
-                         nullptr, &format, 0);
+                         show_bytes, show_control_flow_kind, show_color,
+                         nullptr, nullptr, nullptr, &format, 0);
               log->PutString(strm.GetString());
             }
 
             last_condition = m_inst_emulator_up->GetInstructionCondition();
 
             m_inst_emulator_up->EvaluateInstruction(
                 eEmulateInstructionOptionIgnoreConditions);
 

lldb/source/Symbol/Function.cpp

bool Function::GetDisassembly(const ExecutionContext &exe_ctx,
                               const char *flavor, Stream &strm,
                               bool prefer_file_cache) {
   lldb::DisassemblerSP disassembler_sp =
       GetInstructions(exe_ctx, flavor, prefer_file_cache);
   if (disassembler_sp) {
     const bool show_address = true;
     const bool show_bytes = false;
     const bool show_control_flow_kind = false;
-    disassembler_sp->GetInstructionList().Dump(
-        &strm, show_address, show_bytes, show_control_flow_kind, &exe_ctx);
+    const bool show_color = false;
+    disassembler_sp->GetInstructionList().Dump(&strm, show_address, show_bytes,
+                                               show_control_flow_kind,
+                                               show_color, &exe_ctx);
     return true;
   }
   return false;
 }
 
 // Symbol *
 // Function::CalculateSymbolContextSymbol ()
 //{

lldb/source/Symbol/Symbol.cpp

 bool Symbol::GetDisassembly(const ExecutionContext &exe_ctx, const char *flavor,
                             bool prefer_file_cache, Stream &strm) {
   lldb::DisassemblerSP disassembler_sp =
       GetInstructions(exe_ctx, flavor, prefer_file_cache);
   if (disassembler_sp) {
     const bool show_address = true;
     const bool show_bytes = false;
     const bool show_control_flow_kind = false;
-    disassembler_sp->GetInstructionList().Dump(
-        &strm, show_address, show_bytes, show_control_flow_kind, &exe_ctx);
+    const bool show_color = false;
+    disassembler_sp->GetInstructionList().Dump(&strm, show_address, show_bytes,
+                                               show_control_flow_kind,
+                                               show_color, &exe_ctx);
     return true;
   }
   return false;
 }
 
 bool Symbol::ContainsFileAddress(lldb::addr_t file_addr) const {
   return m_addr_range.ContainsFileAddress(file_addr);
 }

lldb/source/Target/ThreadPlanTracer.cpp

if (err.Success()) {
       InstructionList &instruction_list = disassembler->GetInstructionList();
       const uint32_t max_opcode_byte_size =
           instruction_list.GetMaxOpcocdeByteSize();
 
       if (instruction_list.GetSize()) {
         const bool show_bytes = true;
         const bool show_address = true;
         const bool show_control_flow_kind = true;
+        const bool show_color = false;
         Instruction *instruction =
             instruction_list.GetInstructionAtIndex(0).get();
         const FormatEntity::Entry *disassemble_format =
             m_process.GetTarget().GetDebugger().GetDisassemblyFormat();
         instruction->Dump(stream, max_opcode_byte_size, show_address,
-                          show_bytes, show_control_flow_kind, nullptr, nullptr,
-                          nullptr, disassemble_format, 0);
+                          show_bytes, show_control_flow_kind, show_color,
+                          nullptr, nullptr, nullptr, disassemble_format, 0);
       }
     }
   }
 
   const ABI *abi = m_process.GetABI().get();
   TypeFromUser intptr_type = GetIntPointerType();
 
   if (abi && intptr_type.IsValid()) {

lldb/source/Target/TraceDumper.cpp

void TraceItem(const TraceDumper::TraceItem &item) override {
     } else {
       m_s.Format("{0:x+16}", item.load_address);
       if (item.symbol_info && item.symbol_info->instruction) {
         m_s << "    ";
         item.symbol_info->instruction->Dump(
             &m_s, /*max_opcode_byte_size=*/0,
             /*show_address=*/false,
             /*show_bytes=*/false, m_options.show_control_flow_kind,
-            &item.symbol_info->exe_ctx, &item.symbol_info->sc,
+            /*show_color=*/false, &item.symbol_info->exe_ctx,
+            &item.symbol_info->sc,
             /*prev_sym_ctx=*/nullptr,
             /*disassembly_addr_format=*/nullptr,
             /*max_address_text_size=*/0);
       }
     }
 
     m_was_prev_instruction_an_error = (bool)item.error;
     m_s << "\n";

lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py

def test_disassemble_raw_data(self):
             self.assertEqual(inst.GetMnemonic(target), "li")
             self.assertEqual(inst.GetOperands(target), "4, 0")
             self.assertEqual(
                 inst.GetControlFlowKind(target), lldb.eInstructionControlFlowKindUnknown
             )
         elif arch in ("aarch64", "arm64"):
             self.assertEqual(inst.GetMnemonic(target), "mov")
             self.assertEqual(inst.GetOperands(target), "w0, #0x63")
+            self.assertEqual(inst.GetComment(target), "=99 ")
             self.assertEqual(
                 inst.GetControlFlowKind(target), lldb.eInstructionControlFlowKindUnknown
             )
+            # Make sure that using colors doesn't affect the output here.
+            res = lldb.SBCommandReturnObject()
+            ci = self.dbg.GetCommandInterpreter()
+            ci.HandleCommand("settings set use-color true", res)
+            self.assertEqual(inst.GetOperands(target), "w0, #0x63")
+            self.assertEqual(inst.GetMnemonic(target), "mov")
+            self.assertEqual(inst.GetComment(target), "=99 ")
+            ci.HandleCommand("settings set use-color false", res)
+
         elif arch == "arm":
             self.assertEqual(inst.GetMnemonic(target), "mov")
             self.assertEqual(inst.GetOperands(target), "r3, #99")
             self.assertEqual(
                 inst.GetControlFlowKind(target), lldb.eInstructionControlFlowKindUnknown
             )
         else:
             self.assertEqual(inst.GetMnemonic(target), "movq")
             self.assertEqual(inst.GetOperands(target), "%" + "rsp, " + "%" + "rbp")
             self.assertEqual(
                 inst.GetControlFlowKind(target), lldb.eInstructionControlFlowKindOther
             )

lldb/test/Shell/Commands/command-disassemble-aarch64-color.s

+# REQUIRES: aarch64
+
+# This checks that lldb's disassembler colors AArch64 disassembly.
+
+# RUN: llvm-mc -filetype=obj -triple aarch64-linux-gnueabihf %s -o %t --mattr=+all
+# RUN: %lldb %t -o "settings set use-color true" -o "disassemble -n fn" -o exit 2>&1 | FileCheck %s
+
+.globl  fn
+.type   fn, @function
+fn:
+  // These are in alphabetical order by extension name
+  aesd v0.16b, v0.16b                   // AEK_AES
+  bfadd z23.h, p3/m, z23.h, z13.h       // AEK_B16B16
+  bfdot v2.2s, v3.4h, v4.4h             // AEK_BF16
+  brb iall                              // AEK_BRBE
+  crc32b w0, w0, w0                     // AEK_CRC
+  // AEK_CRYPTO enables a combination of other features
+  smin x0, x0, #0                       // AEK_CSSC
+  sysp  #0, c2, c0, #0, x0, x1          // AEK_D128
+  sdot v0.2s, v1.8b, v2.8b              // AEK_DOTPROD
+  fmmla z0.s, z1.s, z2.s                // AEK_F32MM
+
+# CHECK: `fn:
+# CHECK-NEXT: [0x0] <+0>:    aesd   ␛[0;36mv0␛[0m.16b, ␛[0;36mv0␛[0m.16b
+# CHECK-NEXT: [0x4] <+4>:    bfadd  ␛[0;36mz23␛[0m.h, ␛[0;36mp3␛[0m/m, ␛[0;36mz23␛[0m.h, ␛[0;36mz13␛[0m.h
+# CHECK-NEXT: [0x8] <+8>:    bfdot  ␛[0;36mv2␛[0m.2s, ␛[0;36mv3␛[0m.4h, ␛[0;36mv4␛[0m.4h
+# CHECK-NEXT: [0xc] <+12>:   brb    iall
+# CHECK-NEXT: [0x10] <+16>:  crc32b ␛[0;36mw0␛[0m, ␛[0;36mw0␛[0m, ␛[0;36mw0␛[0m
+# CHECK-NEXT: [0x14] <+20>:  smin   ␛[0;36mx0␛[0m, ␛[0;36mx0␛[0m, ␛[0;31m#0x0␛[0m
+# CHECK-NEXT: [0x18] <+24>:  sysp   ␛[0;31m#0x0␛[0m, c2, c0, ␛[0;31m#0x0␛[0m, ␛[0;36mx0␛[0m, ␛[0;36mx1␛[0m
+# CHECK-NEXT: [0x1c] <+28>:  sdot   ␛[0;36mv0␛[0m.2s, ␛[0;36mv1␛[0m.8b, ␛[0;36mv2␛[0m.8b
+# CHECK-NEXT: [0x20] <+32>:  fmmla  ␛[0;36mz0␛[0m.s, ␛[0;36mz1␛[0m.s, ␛[0;36mz2␛[0m.s