Skip to content

Commit a385a91

Browse files
nga888tstellar
authored andcommitted
[Support] Fix color handling in formatted_raw_ostream (#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. (cherry picked from commit c9db031)
1 parent 0cd4bab commit a385a91

File tree

4 files changed

+47
-19
lines changed

4 files changed

+47
-19
lines changed

llvm/include/llvm/Support/FormattedStream.h

Lines changed: 44 additions & 7 deletions
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

Lines changed: 3 additions & 0 deletions
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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -547,11 +547,6 @@ int main(int argc, char **argv) {
547547
std::unique_ptr<MCAsmBackend> MAB(
548548
TheTarget->createMCAsmBackend(*STI, *MRI, MCOptions));
549549
auto FOut = std::make_unique<formatted_raw_ostream>(*OS);
550-
// FIXME: Workaround for bug in formatted_raw_ostream. Color escape codes
551-
// are (incorrectly) written directly to the unbuffered raw_ostream wrapped
552-
// by the formatted_raw_ostream.
553-
if (Action == AC_CDisassemble)
554-
FOut->SetUnbuffered();
555550
Str.reset(
556551
TheTarget->createAsmStreamer(Ctx, std::move(FOut), /*asmverbose*/ true,
557552
/*useDwarfDirectory*/ true, IP,

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2032,13 +2032,6 @@ disassembleObject(ObjectFile &Obj, const ObjectFile &DbgObj,
20322032

20332033
formatted_raw_ostream FOS(outs());
20342034

2035-
// FIXME: Workaround for bug in formatted_raw_ostream. Color escape codes
2036-
// are (incorrectly) written directly to the unbuffered raw_ostream
2037-
// wrapped by the formatted_raw_ostream.
2038-
if (DisassemblyColor == ColorOutput::Enable ||
2039-
DisassemblyColor == ColorOutput::Auto)
2040-
FOS.SetUnbuffered();
2041-
20422035
std::unordered_map<uint64_t, std::string> AllLabels;
20432036
std::unordered_map<uint64_t, std::vector<BBAddrMapLabel>> BBAddrMapLabels;
20442037
if (SymbolizeOperands) {

0 commit comments

Comments
 (0)