Skip to content

Commit 29bff4a

Browse files
authored
[llvm-objdump] Fix coloring with nested WithMarkup
WithMarkup objects may nest, resulting in the `)` in `leaq (%rdx,%rax), %rbx` to be green instead of the default color, mismatching the color of `(`. ``` % llvm-mc -triple=x86_64 -mdis <<< '0x48 0x8d 0x1c 0x02' .text leaq <mem:(<reg:%rdx>,<reg:%rax>)>, <reg:%rbx> ``` To ensure that `(` and `)` get the same color, maintain a color stack within MCInstPrinter. Fix #99661 Pull Request: #113834
1 parent 2c31325 commit 29bff4a

File tree

3 files changed

+44
-13
lines changed

3 files changed

+44
-13
lines changed

llvm/include/llvm/MC/MCInstPrinter.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@
99
#ifndef LLVM_MC_MCINSTPRINTER_H
1010
#define LLVM_MC_MCINSTPRINTER_H
1111

12+
#include "llvm/ADT/SmallVector.h"
1213
#include "llvm/Support/Compiler.h"
1314
#include "llvm/Support/Format.h"
15+
#include "llvm/Support/raw_ostream.h"
1416
#include <cstdint>
1517

1618
namespace llvm {
@@ -24,7 +26,6 @@ class MCRegister;
2426
class MCRegisterInfo;
2527
class MCSubtargetInfo;
2628
class StringRef;
27-
class raw_ostream;
2829

2930
/// Convert `Bytes' to a hex string and output to `OS'
3031
void dumpBytes(ArrayRef<uint8_t> Bytes, raw_ostream &OS);
@@ -76,6 +77,8 @@ class MCInstPrinter {
7677
/// If true, symbolize branch target and memory reference operands.
7778
bool SymbolizeOperands = false;
7879

80+
SmallVector<raw_ostream::Colors, 4> ColorStack{raw_ostream::Colors::RESET};
81+
7982
/// Utility function for printing annotations.
8083
void printAnnotation(raw_ostream &OS, StringRef Annot);
8184

@@ -98,8 +101,8 @@ class MCInstPrinter {
98101

99102
class WithMarkup {
100103
public:
101-
LLVM_CTOR_NODISCARD WithMarkup(raw_ostream &OS, Markup M, bool EnableMarkup,
102-
bool EnableColor);
104+
LLVM_CTOR_NODISCARD WithMarkup(MCInstPrinter &IP, raw_ostream &OS, Markup M,
105+
bool EnableMarkup, bool EnableColor);
103106
~WithMarkup();
104107

105108
template <typename T> WithMarkup &operator<<(T &O) {
@@ -113,6 +116,7 @@ class MCInstPrinter {
113116
}
114117

115118
private:
119+
MCInstPrinter &IP;
116120
raw_ostream &OS;
117121
bool EnableMarkup;
118122
bool EnableColor;

llvm/lib/MC/MCInstPrinter.cpp

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -225,27 +225,31 @@ format_object<uint64_t> MCInstPrinter::formatHex(uint64_t Value) const {
225225
}
226226

227227
MCInstPrinter::WithMarkup MCInstPrinter::markup(raw_ostream &OS, Markup S) {
228-
return WithMarkup(OS, S, getUseMarkup(), getUseColor());
228+
return WithMarkup(*this, OS, S, getUseMarkup(), getUseColor());
229229
}
230230

231-
MCInstPrinter::WithMarkup::WithMarkup(raw_ostream &OS, Markup M,
232-
bool EnableMarkup, bool EnableColor)
233-
: OS(OS), EnableMarkup(EnableMarkup), EnableColor(EnableColor) {
231+
MCInstPrinter::WithMarkup::WithMarkup(MCInstPrinter &IP, raw_ostream &OS,
232+
Markup M, bool EnableMarkup,
233+
bool EnableColor)
234+
: IP(IP), OS(OS), EnableMarkup(EnableMarkup), EnableColor(EnableColor) {
234235
if (EnableColor) {
236+
raw_ostream::Colors Color = raw_ostream::Colors::RESET;
235237
switch (M) {
236238
case Markup::Immediate:
237-
OS.changeColor(raw_ostream::RED);
239+
Color = raw_ostream::RED;
238240
break;
239241
case Markup::Register:
240-
OS.changeColor(raw_ostream::CYAN);
242+
Color = raw_ostream::CYAN;
241243
break;
242244
case Markup::Target:
243-
OS.changeColor(raw_ostream::YELLOW);
245+
Color = raw_ostream::YELLOW;
244246
break;
245247
case Markup::Memory:
246-
OS.changeColor(raw_ostream::GREEN);
248+
Color = raw_ostream::GREEN;
247249
break;
248250
}
251+
IP.ColorStack.push_back(Color);
252+
OS.changeColor(Color);
249253
}
250254

251255
if (EnableMarkup) {
@@ -269,6 +273,8 @@ MCInstPrinter::WithMarkup::WithMarkup(raw_ostream &OS, Markup M,
269273
MCInstPrinter::WithMarkup::~WithMarkup() {
270274
if (EnableMarkup)
271275
OS << '>';
272-
if (EnableColor)
273-
OS.resetColor();
276+
if (!EnableColor)
277+
return;
278+
IP.ColorStack.pop_back();
279+
OS << IP.ColorStack.back();
274280
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# UNSUPPORTED: system-windows
2+
# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t
3+
# RUN: llvm-objdump -d --no-show-raw-insn --disassembler-color=on %t | FileCheck %s --check-prefix=ATT
4+
# RUN: llvm-objdump -d --no-show-raw-insn --disassembler-color=on -M intel %t | FileCheck %s --check-prefix=INTEL
5+
6+
# ATT: <.text>:
7+
# ATT-NEXT: leaq (%rdx,%rax,4), %rbx
8+
# ATT-NEXT: movq (,%rax), %rbx
9+
# ATT-NEXT: leaq 0x3(%rdx,%rax), %rbx
10+
# ATT-NEXT: movq $0x3, %rax
11+
12+
# INTEL: <.text>:
13+
# INTEL-NEXT: lea rbx, [rdx + 4*rax]
14+
# INTEL-NEXT: mov rbx, qword ptr [1*rax]
15+
# INTEL-NEXT: lea rbx, [rdx + rax + 0x3]
16+
# INTEL-NEXT: mov rax, 0x3
17+
18+
leaq (%rdx,%rax,4), %rbx
19+
movq (,%rax), %rbx
20+
leaq 3(%rdx,%rax), %rbx
21+
movq $3, %rax

0 commit comments

Comments
 (0)