JDevlieghere updated this revision to Diff 555442.
JDevlieghere added a comment.
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.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D159164/new/
https://reviews.llvm.org/D159164
Files:
lldb/include/lldb/Core/Disassembler.h
lldb/source/API/SBInstruction.cpp
lldb/source/API/SBInstructionList.cpp
lldb/source/Core/Disassembler.cpp
lldb/source/Core/DumpDataExtractor.cpp
lldb/source/Expression/IRExecutionUnit.cpp
lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Symbol/Symbol.cpp
lldb/source/Target/ThreadPlanTracer.cpp
lldb/source/Target/TraceDumper.cpp
lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
lldb/test/Shell/Commands/command-disassemble-aarch64-color.s
Index: lldb/test/Shell/Commands/command-disassemble-aarch64-color.s
===================================================================
--- /dev/null
+++ lldb/test/Shell/Commands/command-disassemble-aarch64-color.s
@@ -0,0 +1,32 @@
+# 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
Index: lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
===================================================================
--- lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
+++ lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
@@ -59,9 +59,19 @@
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")
Index: lldb/source/Target/TraceDumper.cpp
===================================================================
--- lldb/source/Target/TraceDumper.cpp
+++ lldb/source/Target/TraceDumper.cpp
@@ -189,7 +189,8 @@
&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);
Index: lldb/source/Target/ThreadPlanTracer.cpp
===================================================================
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -171,13 +171,14 @@
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);
}
}
}
Index: lldb/source/Symbol/Symbol.cpp
===================================================================
--- lldb/source/Symbol/Symbol.cpp
+++ lldb/source/Symbol/Symbol.cpp
@@ -602,8 +602,10 @@
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;
Index: lldb/source/Symbol/Function.cpp
===================================================================
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -440,8 +440,10 @@
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;
Index: lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
===================================================================
--- lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
+++ lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
@@ -84,6 +84,7 @@
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;
@@ -242,8 +243,8 @@
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());
}
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===================================================================
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -9,6 +9,7 @@
#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"
@@ -63,6 +64,8 @@
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;
@@ -565,7 +568,9 @@
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) {
@@ -607,7 +612,14 @@
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);
@@ -672,6 +684,11 @@
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();
+ }
}
}
}
@@ -1344,10 +1361,12 @@
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");
@@ -1374,6 +1393,14 @@
}
}
+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)
Index: lldb/source/Expression/IRExecutionUnit.cpp
===================================================================
--- lldb/source/Expression/IRExecutionUnit.cpp
+++ lldb/source/Expression/IRExecutionUnit.cpp
@@ -202,7 +202,7 @@
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;
}
@@ -318,12 +318,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());
@@ -950,8 +950,8 @@
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);
}
@@ -962,8 +962,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"
@@ -155,7 +156,8 @@
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
Index: lldb/source/Core/Disassembler.cpp
===================================================================
--- lldb/source/Core/Disassembler.cpp
+++ lldb/source/Core/Disassembler.cpp
@@ -182,9 +182,9 @@
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;
}
@@ -275,7 +275,8 @@
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();
@@ -526,8 +527,8 @@
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;
@@ -600,7 +601,7 @@
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,
@@ -646,17 +647,20 @@
}
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(
@@ -985,7 +989,7 @@
}
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;
@@ -1005,7 +1009,7 @@
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);
}
}
Index: lldb/source/API/SBInstructionList.cpp
===================================================================
--- lldb/source/API/SBInstructionList.cpp
+++ lldb/source/API/SBInstructionList.cpp
@@ -166,8 +166,8 @@
}
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;
Index: lldb/source/API/SBInstruction.cpp
===================================================================
--- lldb/source/API/SBInstruction.cpp
+++ lldb/source/API/SBInstruction.cpp
@@ -261,7 +261,7 @@
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;
@@ -296,7 +296,7 @@
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);
}
}
Index: lldb/include/lldb/Core/Disassembler.h
===================================================================
--- lldb/include/lldb/Core/Disassembler.h
+++ lldb/include/lldb/Core/Disassembler.h
@@ -64,14 +64,16 @@
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) {
@@ -153,7 +155,7 @@
/// 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,
@@ -244,7 +246,9 @@
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;
@@ -336,7 +340,8 @@
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;
@@ -449,8 +454,8 @@
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,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
