JDevlieghere updated this revision to Diff 554873.
JDevlieghere added a comment.

- Fix inconsistencies and use `use_color` everywhere


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159164/new/

https://reviews.llvm.org/D159164

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectDisassemble.cpp
  lldb/source/Core/Disassembler.cpp
  lldb/source/Core/DumpDataExtractor.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
  
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
  lldb/source/Target/ThreadPlanTracer.cpp

Index: lldb/source/Target/ThreadPlanTracer.cpp
===================================================================
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -96,7 +96,8 @@
 Disassembler *ThreadPlanAssemblyTracer::GetDisassembler() {
   if (!m_disassembler_sp)
     m_disassembler_sp = Disassembler::FindPlugin(
-        m_process.GetTarget().GetArchitecture(), nullptr, nullptr);
+        m_process.GetTarget().GetArchitecture(), nullptr,
+        m_process.GetTarget().GetDebugger().GetUseColor(), nullptr);
   return m_disassembler_sp.get();
 }
 
Index: lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
===================================================================
--- lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
+++ lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
@@ -70,7 +70,7 @@
 
     const bool prefer_file_cache = true;
     DisassemblerSP disasm_sp(Disassembler::DisassembleBytes(
-        m_arch, nullptr, nullptr, range.GetBaseAddress(), opcode_data,
+        m_arch, nullptr, nullptr, false, range.GetBaseAddress(), opcode_data,
         opcode_size, 99999, prefer_file_cache));
 
     Log *log = GetLog(LLDBLog::Unwind);
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
===================================================================
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
@@ -22,8 +22,8 @@
 
 class DisassemblerLLVMC : public lldb_private::Disassembler {
 public:
-  DisassemblerLLVMC(const lldb_private::ArchSpec &arch,
-                    const char *flavor /* = NULL */);
+  DisassemblerLLVMC(const lldb_private::ArchSpec &arch, const char *flavor,
+                    bool use_color = false);
 
   ~DisassemblerLLVMC() override;
 
@@ -35,7 +35,8 @@
   static llvm::StringRef GetPluginNameStatic() { return "llvm-mc"; }
 
   static lldb::DisassemblerSP CreateInstance(const lldb_private::ArchSpec &arch,
-                                             const char *flavor);
+                                             const char *flavor,
+                                             bool use_color);
 
   size_t DecodeInstructions(const lldb_private::Address &base_addr,
                             const lldb_private::DataExtractor &data,
@@ -72,6 +73,7 @@
   InstructionLLVMC *m_inst;
   std::mutex m_mutex;
   bool m_data_from_file;
+  bool m_use_color;
   // Save the AArch64 ADRP instruction word and address it was at,
   // in case the next instruction is an ADD to the same register;
   // this is a pc-relative address calculation and we need both
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===================================================================
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -54,7 +54,7 @@
 public:
   static std::unique_ptr<MCDisasmInstance>
   Create(const char *triple, const char *cpu, const char *features_str,
-         unsigned flavor, DisassemblerLLVMC &owner);
+         unsigned flavor, bool use_color, DisassemblerLLVMC &owner);
 
   ~MCDisasmInstance() = default;
 
@@ -1227,7 +1227,7 @@
 std::unique_ptr<DisassemblerLLVMC::MCDisasmInstance>
 DisassemblerLLVMC::MCDisasmInstance::Create(const char *triple, const char *cpu,
                                             const char *features_str,
-                                            unsigned flavor,
+                                            unsigned flavor, bool use_color,
                                             DisassemblerLLVMC &owner) {
   using Instance = std::unique_ptr<DisassemblerLLVMC::MCDisasmInstance>;
 
@@ -1291,6 +1291,7 @@
     return Instance();
 
   instr_printer_up->setPrintBranchImmAsAddress(true);
+  instr_printer_up->setUseColor(use_color);
 
   // Not all targets may have registered createMCInstrAnalysis().
   std::unique_ptr<llvm::MCInstrAnalysis> instr_analysis_up(
@@ -1344,6 +1345,8 @@
   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);
@@ -1415,10 +1418,10 @@
 }
 
 DisassemblerLLVMC::DisassemblerLLVMC(const ArchSpec &arch,
-                                     const char *flavor_string)
+                                     const char *flavor_string, bool use_color)
     : Disassembler(arch, flavor_string), m_exe_ctx(nullptr), m_inst(nullptr),
-      m_data_from_file(false), m_adrp_address(LLDB_INVALID_ADDRESS),
-      m_adrp_insn() {
+      m_data_from_file(false), m_use_color(use_color),
+      m_adrp_address(LLDB_INVALID_ADDRESS), m_adrp_insn() {
   if (!FlavorValidForArchSpec(arch, m_flavor.c_str())) {
     m_flavor.assign("default");
   }
@@ -1556,7 +1559,7 @@
   // isn't good for some reason, we won't be valid and FindPlugin will fail and
   // we won't get used.
   m_disasm_up = MCDisasmInstance::Create(triple_str, cpu, features_str.c_str(),
-                                         flavor, *this);
+                                         flavor, use_color, *this);
 
   llvm::Triple::ArchType llvm_arch = triple.getArch();
 
@@ -1566,7 +1569,7 @@
     std::string thumb_triple(thumb_arch.GetTriple().getTriple());
     m_alternate_disasm_up =
         MCDisasmInstance::Create(thumb_triple.c_str(), "", features_str.c_str(),
-                                 flavor, *this);
+                                 flavor, m_use_color, *this);
     if (!m_alternate_disasm_up)
       m_disasm_up.reset();
 
@@ -1579,7 +1582,7 @@
       features_str += "+micromips,";
 
     m_alternate_disasm_up = MCDisasmInstance::Create(
-        triple_str, cpu, features_str.c_str(), flavor, *this);
+        triple_str, cpu, features_str.c_str(), flavor, m_use_color, *this);
     if (!m_alternate_disasm_up)
       m_disasm_up.reset();
   }
@@ -1588,9 +1591,11 @@
 DisassemblerLLVMC::~DisassemblerLLVMC() = default;
 
 lldb::DisassemblerSP DisassemblerLLVMC::CreateInstance(const ArchSpec &arch,
-                                                       const char *flavor) {
+                                                       const char *flavor,
+                                                       bool use_color) {
   if (arch.GetTriple().getArch() != llvm::Triple::UnknownArch) {
-    auto disasm_sp = std::make_shared<DisassemblerLLVMC>(arch, flavor);
+    auto disasm_sp =
+        std::make_shared<DisassemblerLLVMC>(arch, flavor, use_color);
     if (disasm_sp && disasm_sp->IsValid())
       return disasm_sp;
   }
Index: lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
===================================================================
--- lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
+++ lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
@@ -8,6 +8,7 @@
 
 #include "Plugins/Architecture/Mips/ArchitectureMips.h"
 #include "lldb/Core/Address.h"
+#include "lldb/Core/Debugger.h"
 #include "lldb/Core/Disassembler.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
@@ -149,8 +150,8 @@
   }
 
   // Create Disassembler Instance
-  lldb::DisassemblerSP disasm_sp(
-    Disassembler::FindPlugin(m_arch, nullptr, nullptr));
+  lldb::DisassemblerSP disasm_sp(Disassembler::FindPlugin(
+      m_arch, nullptr, target.GetDebugger().GetUseColor(), nullptr));
 
   InstructionList instruction_list;
   InstructionSP prev_insn;
Index: lldb/source/Expression/IRExecutionUnit.cpp
===================================================================
--- lldb/source/Expression/IRExecutionUnit.cpp
+++ lldb/source/Expression/IRExecutionUnit.cpp
@@ -171,8 +171,9 @@
 
   const char *plugin_name = nullptr;
   const char *flavor_string = nullptr;
+  const bool use_color = target->GetDebugger().GetUseColor();
   lldb::DisassemblerSP disassembler_sp =
-      Disassembler::FindPlugin(arch, flavor_string, plugin_name);
+      Disassembler::FindPlugin(arch, flavor_string, use_color, plugin_name);
 
   if (!disassembler_sp) {
     ret.SetErrorToGenericError();
@@ -318,12 +319,12 @@
       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());
@@ -944,16 +945,16 @@
   }
 }
 
-llvm::JITSymbol 
+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);
-    else
-      return llvm::JITSymbol(addr, llvm::JITSymbolFlags::Exported);
+  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);
+  else
+    return llvm::JITSymbol(addr, llvm::JITSymbolFlags::Exported);
 }
 
 uint64_t
@@ -962,8 +963,7 @@
   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);
 
Index: lldb/source/Core/DumpDataExtractor.cpp
===================================================================
--- lldb/source/Core/DumpDataExtractor.cpp
+++ lldb/source/Core/DumpDataExtractor.cpp
@@ -12,6 +12,7 @@
 #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"
@@ -128,9 +129,9 @@
   if (exe_scope)
     target_sp = exe_scope->CalculateTarget();
   if (target_sp) {
-    DisassemblerSP disassembler_sp(
-        Disassembler::FindPlugin(target_sp->GetArchitecture(),
-                                 target_sp->GetDisassemblyFlavor(), nullptr));
+    DisassemblerSP disassembler_sp(Disassembler::FindPlugin(
+        target_sp->GetArchitecture(), target_sp->GetDisassemblyFlavor(),
+        target_sp->GetDebugger().GetUseColor(), nullptr));
     if (disassembler_sp) {
       lldb::addr_t addr = base_addr + start_offset;
       lldb_private::Address so_addr;
Index: lldb/source/Core/Disassembler.cpp
===================================================================
--- lldb/source/Core/Disassembler.cpp
+++ lldb/source/Core/Disassembler.cpp
@@ -56,7 +56,7 @@
 using namespace lldb_private;
 
 DisassemblerSP Disassembler::FindPlugin(const ArchSpec &arch,
-                                        const char *flavor,
+                                        const char *flavor, bool use_color,
                                         const char *plugin_name) {
   LLDB_SCOPED_TIMERF("Disassembler::FindPlugin (arch = %s, plugin_name = %s)",
                      arch.GetArchitectureName(), plugin_name);
@@ -67,7 +67,7 @@
     create_callback =
         PluginManager::GetDisassemblerCreateCallbackForPluginName(plugin_name);
     if (create_callback) {
-      if (auto disasm_sp = create_callback(arch, flavor))
+      if (auto disasm_sp = create_callback(arch, flavor, use_color))
         return disasm_sp;
     }
   } else {
@@ -75,7 +75,7 @@
          (create_callback = PluginManager::GetDisassemblerCreateCallbackAtIndex(
               idx)) != nullptr;
          ++idx) {
-      if (auto disasm_sp = create_callback(arch, flavor))
+      if (auto disasm_sp = create_callback(arch, flavor, use_color))
         return disasm_sp;
     }
   }
@@ -85,6 +85,7 @@
 DisassemblerSP Disassembler::FindPluginForTarget(const Target &target,
                                                  const ArchSpec &arch,
                                                  const char *flavor,
+                                                 bool use_color,
                                                  const char *plugin_name) {
   if (flavor == nullptr) {
     // FIXME - we don't have the mechanism in place to do per-architecture
@@ -94,7 +95,7 @@
         arch.GetTriple().getArch() == llvm::Triple::x86_64)
       flavor = target.GetDisassemblyFlavor();
   }
-  return FindPlugin(arch, flavor, plugin_name);
+  return FindPlugin(arch, flavor, use_color, plugin_name);
 }
 
 static Address ResolveAddress(Target &target, const Address &addr) {
@@ -124,8 +125,8 @@
   if (!range.GetBaseAddress().IsValid())
     return {};
 
-  lldb::DisassemblerSP disasm_sp =
-      Disassembler::FindPluginForTarget(target, arch, flavor, plugin_name);
+  lldb::DisassemblerSP disasm_sp = Disassembler::FindPluginForTarget(
+      target, arch, flavor, target.GetDebugger().GetUseColor(), plugin_name);
 
   if (!disasm_sp)
     return {};
@@ -139,16 +140,15 @@
   return disasm_sp;
 }
 
-lldb::DisassemblerSP
-Disassembler::DisassembleBytes(const ArchSpec &arch, const char *plugin_name,
-                               const char *flavor, const Address &start,
-                               const void *src, size_t src_len,
-                               uint32_t num_instructions, bool data_from_file) {
+lldb::DisassemblerSP Disassembler::DisassembleBytes(
+    const ArchSpec &arch, const char *plugin_name, const char *flavor,
+    bool use_color, const Address &start, const void *src, size_t src_len,
+    uint32_t num_instructions, bool data_from_file) {
   if (!src)
     return {};
 
   lldb::DisassemblerSP disasm_sp =
-      Disassembler::FindPlugin(arch, flavor, plugin_name);
+      Disassembler::FindPlugin(arch, flavor, use_color, plugin_name);
 
   if (!disasm_sp)
     return {};
@@ -171,8 +171,9 @@
   if (!exe_ctx.GetTargetPtr())
     return false;
 
-  lldb::DisassemblerSP disasm_sp(Disassembler::FindPluginForTarget(
-      exe_ctx.GetTargetRef(), arch, flavor, plugin_name));
+  lldb::DisassemblerSP disasm_sp(
+      Disassembler::FindPluginForTarget(exe_ctx.GetTargetRef(), arch, flavor,
+                                        debugger.GetUseColor(), plugin_name));
   if (!disasm_sp)
     return false;
 
Index: lldb/source/Commands/CommandObjectDisassemble.cpp
===================================================================
--- lldb/source/Commands/CommandObjectDisassemble.cpp
+++ lldb/source/Commands/CommandObjectDisassemble.cpp
@@ -453,8 +453,8 @@
   const char *plugin_name = m_options.GetPluginName();
   const char *flavor_string = m_options.GetFlavorString();
 
-  DisassemblerSP disassembler =
-      Disassembler::FindPlugin(m_options.arch, flavor_string, plugin_name);
+  DisassemblerSP disassembler = Disassembler::FindPlugin(
+      m_options.arch, flavor_string, GetDebugger().GetUseColor(), plugin_name);
 
   if (!disassembler) {
     if (plugin_name) {
Index: lldb/source/API/SBTarget.cpp
===================================================================
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -1997,8 +1997,9 @@
                                 error, force_live_memory, &load_addr);
       const bool data_from_file = load_addr == LLDB_INVALID_ADDRESS;
       sb_instructions.SetDisassembler(Disassembler::DisassembleBytes(
-          target_sp->GetArchitecture(), nullptr, flavor_string, *addr_ptr,
-          data.GetBytes(), bytes_read, count, data_from_file));
+          target_sp->GetArchitecture(), nullptr, flavor_string,
+          target_sp->GetDebugger().GetUseColor(), *addr_ptr, data.GetBytes(),
+          bytes_read, count, data_from_file));
     }
   }
 
@@ -2031,8 +2032,9 @@
     const bool data_from_file = true;
 
     sb_instructions.SetDisassembler(Disassembler::DisassembleBytes(
-        target_sp->GetArchitecture(), nullptr, flavor_string, addr, buf, size,
-        UINT32_MAX, data_from_file));
+        target_sp->GetArchitecture(), nullptr, flavor_string,
+        target_sp->GetDebugger().GetUseColor(), addr, buf, size, UINT32_MAX,
+        data_from_file));
   }
 
   return sb_instructions;
Index: lldb/include/lldb/lldb-private-interfaces.h
===================================================================
--- lldb/include/lldb/lldb-private-interfaces.h
+++ lldb/include/lldb/lldb-private-interfaces.h
@@ -29,7 +29,8 @@
 typedef std::unique_ptr<Architecture> (*ArchitectureCreateInstance)(
     const ArchSpec &arch);
 typedef lldb::DisassemblerSP (*DisassemblerCreateInstance)(const ArchSpec &arch,
-                                                           const char *flavor);
+                                                           const char *flavor,
+                                                           bool use_color);
 typedef DynamicLoader *(*DynamicLoaderCreateInstance)(Process *process,
                                                       bool force);
 typedef lldb::JITLoaderSP (*JITLoaderCreateInstance)(Process *process,
Index: lldb/include/lldb/Core/Disassembler.h
===================================================================
--- lldb/include/lldb/Core/Disassembler.h
+++ lldb/include/lldb/Core/Disassembler.h
@@ -310,16 +310,16 @@
   /// @param[in] ignore_calls
   ///     It true, then fine the first branch instruction that isn't
   ///     a function call (a branch that calls and returns to the next
-  ///     instruction). If false, find the instruction index of any 
+  ///     instruction). If false, find the instruction index of any
   ///     branch in the list.
-  ///     
+  ///
   /// @param[out] found_calls
-  ///     If non-null, this will be set to true if any calls were found in 
+  ///     If non-null, this will be set to true if any calls were found in
   ///     extending the range.
-  ///    
+  ///
   /// @return
   ///     The instruction index of the first branch that is at or past
-  ///     \a start. Returns UINT32_MAX if no matching branches are 
+  ///     \a start. Returns UINT32_MAX if no matching branches are
   ///     found.
   //------------------------------------------------------------------
   uint32_t GetIndexOfNextBranchInstruction(uint32_t start,
@@ -405,13 +405,15 @@
   // 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);
+  static lldb::DisassemblerSP FindPlugin(const ArchSpec &arch,
+                                         const char *flavor, bool use_color,
+                                         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,
+                                                  bool use_color,
                                                   const char *plugin_name);
 
   struct Limit {
@@ -428,9 +430,9 @@
 
   static lldb::DisassemblerSP
   DisassembleBytes(const ArchSpec &arch, const char *plugin_name,
-                   const char *flavor, const Address &start, const void *bytes,
-                   size_t length, uint32_t max_num_instructions,
-                   bool data_from_file);
+                   const char *flavor, bool use_color, const Address &start,
+                   const void *bytes, size_t length,
+                   uint32_t max_num_instructions, bool data_from_file);
 
   static bool Disassemble(Debugger &debugger, const ArchSpec &arch,
                           const char *plugin_name, const char *flavor,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to