Skip to content

[llvm-objdump] Fix --disassembler-color with --macho flag #163815

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+32 −12
Open

[llvm-objdump] Fix --disassembler-color with --macho flag #163815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f433720
[llvm-objdump] Fix --disassembler-color with --macho flag
rjmansfield Oct 16, 2025
57f1c98
Address reviewer feedback.
rjmansfield Oct 17, 2025
c2f893e
Rename setupMachOInstPrinter to setUpMachOInstPrinter.
rjmansfield Oct 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions llvm/test/tools/llvm-objdump/MachO/arm64-disassembly-color.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
// RUN: llvm-objdump --disassembler-color=off --disassemble %t | FileCheck %s --check-prefix=NOCOLOR
// RUN: llvm-objdump --disassembler-color=terminal --disassemble %t | FileCheck %s --check-prefix=NOCOLOR

//// Test with --macho flag to ensure DisassembleMachO respects color settings.
// RUN: llvm-objdump --disassembler-color=on --macho --disassemble %t | FileCheck %s --check-prefix=COLOR
// RUN: llvm-objdump --disassembler-color=off --macho --disassemble %t | FileCheck %s --check-prefix=NOCOLOR

sub sp, sp, #16
str w0, [sp, #12]
ldr w8, [sp, #12]
Expand Down
23 changes: 19 additions & 4 deletions llvm/tools/llvm-objdump/MachODump.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7235,6 +7235,22 @@ objdump::getMachODSymObject(const MachOObjectFile *MachOOF, StringRef Filename,
return DbgObj;
}

static void setUpMachOInstPrinter(MCInstPrinter *IP) {
IP->setPrintImmHex(PrintImmHex);
switch (DisassemblyColor) {
case ColorOutput::Enable:
IP->setUseColor(true);
break;
case ColorOutput::Auto:
IP->setUseColor(outs().has_colors());
break;
case ColorOutput::Disable:
case ColorOutput::Invalid:
IP->setUseColor(false);
break;
}
}

Comment on lines +7238 to +7253
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a little preoccupied about having a std::unique_ptr and having to break the idea of single ownership with .get() to pass into this helper function. I think the usage of std::unique_ptr<>& as the argument type would have the same semantics and I think it also should be avoided.

If setUseColor just accepts a boolean, this helper can be just to calculate the boolean, and the usages below can be just a call to setUseColor.

static bool shouldInstPrinterUseColor() {
  switch (DisassemblyColor) {
    case ColorOutput::Enable: return true;
    case ColorOutput::Auto: return outs().has_colors();
    case ColorOutput::Disable:
    case ColorOutput::Invalid: return false;
  }
  return false; // default to no color
}

...

IP->setPrintImmHex(PrintImmHex);
IP->setUseColor(shouldInstPrinterUseColor());

static void DisassembleMachO(StringRef Filename, MachOObjectFile *MachOOF,
StringRef DisSegName, StringRef DisSectName) {
const char *McpuDefault = nullptr;
Expand Down Expand Up @@ -7319,8 +7335,8 @@ static void DisassembleMachO(StringRef Filename, MachOObjectFile *MachOOF,
std::unique_ptr<MCInstPrinter> IP(TheTarget->createMCInstPrinter(
TheTriple, AsmPrinterVariant, *AsmInfo, *InstrInfo, *MRI));
CHECK_TARGET_INFO_CREATION(IP);
// Set the display preference for hex vs. decimal immediates.
IP->setPrintImmHex(PrintImmHex);
setUpMachOInstPrinter(IP.get());

// Comment stream and backing vector.
SmallString<128> CommentsToEmit;
raw_svector_ostream CommentStream(CommentsToEmit);
Expand Down Expand Up @@ -7372,8 +7388,7 @@ static void DisassembleMachO(StringRef Filename, MachOObjectFile *MachOOF,
ThumbTriple, ThumbAsmPrinterVariant, *ThumbAsmInfo, *ThumbInstrInfo,
*ThumbMRI));
CHECK_THUMB_TARGET_INFO_CREATION(ThumbIP);
// Set the display preference for hex vs. decimal immediates.
ThumbIP->setPrintImmHex(PrintImmHex);
setUpMachOInstPrinter(ThumbIP.get());
}

#undef CHECK_TARGET_INFO_CREATION
Expand Down
9 changes: 1 addition & 8 deletions llvm/tools/llvm-objdump/llvm-objdump.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,13 +291,6 @@ class BBAddrMapInfo {

#define DEBUG_TYPE "objdump"

enum class ColorOutput {
Auto,
Enable,
Disable,
Invalid,
};

static uint64_t AdjustVMA;
static bool AllHeaders;
static std::string ArchName;
Expand All @@ -310,7 +303,7 @@ bool objdump::SymbolDescription;
bool objdump::TracebackTable;
static std::vector<std::string> DisassembleSymbols;
static bool DisassembleZeroes;
static ColorOutput DisassemblyColor;
ColorOutput objdump::DisassemblyColor;
DIDumpType objdump::DwarfDumpType;
static bool DynamicRelocations;
static bool FaultMapSection;
Expand Down
8 changes: 8 additions & 0 deletions llvm/tools/llvm-objdump/llvm-objdump.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ namespace objdump {

enum DebugFormat { DFASCII, DFDisabled, DFInvalid, DFLimitsOnly, DFUnicode };

enum class ColorOutput {
Auto,
Enable,
Disable,
Invalid,
};

extern bool ArchiveHeaders;
extern int DbgIndent;
extern DebugFormat DbgVariables;
Expand All @@ -51,6 +58,7 @@ extern bool Demangle;
extern bool Disassemble;
extern bool DisassembleAll;
extern std::vector<std::string> DisassemblerOptions;
extern ColorOutput DisassemblyColor;
extern DIDumpType DwarfDumpType;
extern std::vector<std::string> FilterSections;
extern bool LeadingAddr;
Expand Down
Loading