Skip to content

Commit c9db031

Browse files
authored
[Support] Fix color handling in formatted_raw_ostream (llvm#86700)
The color methods in formatted_raw_ostream were forwarding directly to the underlying stream without considering existing buffered output. This would cause incorrect colored output for buffered uses of formatted_raw_ostream. Fix this issue by applying the color to the formatted_raw_ostream itself and temporarily disabling scanning of any color related output so as not to affect the position tracking. This fix means that workarounds that forced formatted_raw_ostream buffering to be disabled can be removed. In the case of llvm-objdump, this can improve disassembly performance when redirecting to a file by more than an order of magnitude on both Windows and Linux. This improvement restores the disassembly performance when redirecting to a file to a level similar to before color support was added.
1 parent c13556c commit c9db031

File tree

4 files changed

+47
-19
lines changed

4 files changed

+47
-19
lines changed

llvm/include/llvm/Support/FormattedStream.h

+44-7
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,10 @@ class formatted_raw_ostream : public raw_ostream {
5252
/// have the rest of it.
5353
SmallString<4> PartialUTF8Char;
5454

55+
/// DisableScan - Temporarily disable scanning of output. Used to ignore color
56+
/// codes.
57+
bool DisableScan;
58+
5559
void write_impl(const char *Ptr, size_t Size) override;
5660

5761
/// current_pos - Return the current position within the stream,
@@ -89,9 +93,33 @@ class formatted_raw_ostream : public raw_ostream {
8993
SetUnbuffered();
9094
TheStream->SetUnbuffered();
9195

96+
enable_colors(TheStream->colors_enabled());
97+
9298
Scanned = nullptr;
9399
}
94100

101+
void PreDisableScan() {
102+
assert(!DisableScan);
103+
ComputePosition(getBufferStart(), GetNumBytesInBuffer());
104+
assert(PartialUTF8Char.empty());
105+
DisableScan = true;
106+
}
107+
108+
void PostDisableScan() {
109+
assert(DisableScan);
110+
DisableScan = false;
111+
Scanned = getBufferStart() + GetNumBytesInBuffer();
112+
}
113+
114+
struct DisableScanScope {
115+
formatted_raw_ostream *S;
116+
117+
DisableScanScope(formatted_raw_ostream *FRO) : S(FRO) {
118+
S->PreDisableScan();
119+
}
120+
~DisableScanScope() { S->PostDisableScan(); }
121+
};
122+
95123
public:
96124
/// formatted_raw_ostream - Open the specified file for
97125
/// writing. If an error occurs, information about the error is
@@ -104,12 +132,12 @@ class formatted_raw_ostream : public raw_ostream {
104132
/// underneath it.
105133
///
106134
formatted_raw_ostream(raw_ostream &Stream)
107-
: TheStream(nullptr), Position(0, 0) {
135+
: TheStream(nullptr), Position(0, 0), DisableScan(false) {
108136
setStream(Stream);
109137
}
110-
explicit formatted_raw_ostream() : TheStream(nullptr), Position(0, 0) {
111-
Scanned = nullptr;
112-
}
138+
explicit formatted_raw_ostream()
139+
: TheStream(nullptr), Position(0, 0), Scanned(nullptr),
140+
DisableScan(false) {}
113141

114142
~formatted_raw_ostream() override {
115143
flush();
@@ -136,17 +164,26 @@ class formatted_raw_ostream : public raw_ostream {
136164
}
137165

138166
raw_ostream &resetColor() override {
139-
TheStream->resetColor();
167+
if (colors_enabled()) {
168+
DisableScanScope S(this);
169+
raw_ostream::resetColor();
170+
}
140171
return *this;
141172
}
142173

143174
raw_ostream &reverseColor() override {
144-
TheStream->reverseColor();
175+
if (colors_enabled()) {
176+
DisableScanScope S(this);
177+
raw_ostream::reverseColor();
178+
}
145179
return *this;
146180
}
147181

148182
raw_ostream &changeColor(enum Colors Color, bool Bold, bool BG) override {
149-
TheStream->changeColor(Color, Bold, BG);
183+
if (colors_enabled()) {
184+
DisableScanScope S(this);
185+
raw_ostream::changeColor(Color, Bold, BG);
186+
}
150187
return *this;
151188
}
152189

llvm/lib/Support/FormattedStream.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,9 @@ void formatted_raw_ostream::UpdatePosition(const char *Ptr, size_t Size) {
9494
/// ComputePosition - Examine the current output and update line and column
9595
/// counts.
9696
void formatted_raw_ostream::ComputePosition(const char *Ptr, size_t Size) {
97+
if (DisableScan)
98+
return;
99+
97100
// If our previous scan pointer is inside the buffer, assume we already
98101
// scanned those bytes. This depends on raw_ostream to not change our buffer
99102
// in unexpected ways.

llvm/tools/llvm-mc/llvm-mc.cpp

-5
Original file line numberDiff line numberDiff line change
@@ -541,11 +541,6 @@ int main(int argc, char **argv) {
541541
std::unique_ptr<MCAsmBackend> MAB(
542542
TheTarget->createMCAsmBackend(*STI, *MRI, MCOptions));
543543
auto FOut = std::make_unique<formatted_raw_ostream>(*OS);
544-
// FIXME: Workaround for bug in formatted_raw_ostream. Color escape codes
545-
// are (incorrectly) written directly to the unbuffered raw_ostream wrapped
546-
// by the formatted_raw_ostream.
547-
if (Action == AC_CDisassemble)
548-
FOut->SetUnbuffered();
549544
Str.reset(
550545
TheTarget->createAsmStreamer(Ctx, std::move(FOut), /*asmverbose*/ true,
551546
/*useDwarfDirectory*/ true, IP,

llvm/tools/llvm-objdump/llvm-objdump.cpp

-7
Original file line numberDiff line numberDiff line change
@@ -2115,13 +2115,6 @@ disassembleObject(ObjectFile &Obj, const ObjectFile &DbgObj,
21152115

21162116
formatted_raw_ostream FOS(outs());
21172117

2118-
// FIXME: Workaround for bug in formatted_raw_ostream. Color escape codes
2119-
// are (incorrectly) written directly to the unbuffered raw_ostream
2120-
// wrapped by the formatted_raw_ostream.
2121-
if (DisassemblyColor == ColorOutput::Enable ||
2122-
DisassemblyColor == ColorOutput::Auto)
2123-
FOS.SetUnbuffered();
2124-
21252118
std::unordered_map<uint64_t, std::string> AllLabels;
21262119
std::unordered_map<uint64_t, std::vector<BBAddrMapLabel>> BBAddrMapLabels;
21272120
if (SymbolizeOperands) {

0 commit comments

Comments
 (0)