dexonsmith created this revision.
dexonsmith added reviewers: akyrtzi, steven_wu, benlangmuir.
Herald added a project: All.
dexonsmith requested review of this revision.
Herald added a project: clang.
Delete the output streams coming from
CompilerInstance::createOutputFile() and friends once writes are
finished. Concretely, replacing `OS->flush()` with `OS = nullptr` in:
- `PrecompiledPreambleAction::setEmittedPreamblePCH()`
- `ExtractAPIAction::EndSourceFileAction()`
- `cc1_main()'s support for `-ftime-trace`
This fixes theoretical bugs related to proxy streams, which may have
cleanups to run in their destructor. E.g., `buffer_ostream` supports
`pwrite()` by holding all content in a buffer until destruction time.
This also protects against some logic bugs, triggering a null
dereference on a latter attempt to write to the stream.
No tests, since in practice these particular code paths don't seem to
ever use `buffer_ostream`; you need to be writing a binary file to a
pipe (such as stdout) to hit it; but I have some other patches in the
works that add some safety, crashing if the stream hasn't been
destructed by the time the CompilerInstance is told to keep the output
file.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D124635
Files:
clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
clang/lib/Frontend/PrecompiledPreamble.cpp
clang/tools/driver/cc1_main.cpp
Index: clang/tools/driver/cc1_main.cpp
===================================================================
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -260,8 +260,7 @@
Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
/*useTemporary=*/false)) {
llvm::timeTraceProfilerWrite(*profilerOutput);
- // FIXME(ibiryukov): make profilerOutput flush in destructor instead.
- profilerOutput->flush();
+ profilerOutput = nullptr;
llvm::timeTraceProfilerCleanup();
Clang->clearOutputFiles(false);
}
Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===================================================================
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -248,7 +248,7 @@
if (FileOS) {
*FileOS << Buffer->Data;
// Make sure it hits disk now.
- FileOS->flush();
+ FileOS = nullptr;
}
this->HasEmittedPreamblePCH = true;
Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===================================================================
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -793,7 +793,7 @@
// FIXME: Make the kind of APISerializer configurable.
SymbolGraphSerializer SGSerializer(*API, ProductName);
SGSerializer.serialize(*OS);
- OS->flush();
+ OS = nullptr;
}
std::unique_ptr<raw_pwrite_stream>
Index: clang/tools/driver/cc1_main.cpp
===================================================================
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -260,8 +260,7 @@
Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
/*useTemporary=*/false)) {
llvm::timeTraceProfilerWrite(*profilerOutput);
- // FIXME(ibiryukov): make profilerOutput flush in destructor instead.
- profilerOutput->flush();
+ profilerOutput = nullptr;
llvm::timeTraceProfilerCleanup();
Clang->clearOutputFiles(false);
}
Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===================================================================
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -248,7 +248,7 @@
if (FileOS) {
*FileOS << Buffer->Data;
// Make sure it hits disk now.
- FileOS->flush();
+ FileOS = nullptr;
}
this->HasEmittedPreamblePCH = true;
Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===================================================================
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -793,7 +793,7 @@
// FIXME: Make the kind of APISerializer configurable.
SymbolGraphSerializer SGSerializer(*API, ProductName);
SGSerializer.serialize(*OS);
- OS->flush();
+ OS = nullptr;
}
std::unique_ptr<raw_pwrite_stream>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
