Skip to content

Commit 2d13386

Browse files
committed
Frontend: Delete output streams before closing CompilerInstance outputs
Delete the output streams coming from CompilerInstance::createOutputFile() and friends once writes are finished. Concretely, replacing `OS->flush()` with `OS.reset()` in: - `ExtractAPIAction::EndSourceFileAction()` - `PrecompiledPreambleAction::setEmittedPreamblePCH()` - `cc1_main()'s support for `-ftime-trace` This fixes theoretical bugs related to proxy streams, which may have cleanups to run in their destructor. For example, a proxy that CompilerInstance sometimes uses is `buffer_ostream`, which wraps a `raw_ostream` lacking pwrite support and adds it. `flush()` does not promise that output is complete; `buffer_ostream` needs to wait until the destructor to forward anything so that it can service later calls to `pwrite()`. If the destructor isn't called then the proxied stream hasn't received any content. This also protects against some logic bugs, triggering a null dereference on a later attempt to write to the stream. No tests, since in practice these particular code paths never use use `buffer_ostream`; you need to be writing a binary file to a pipe (such as stdout) to hit it, but `-extract-api` writes a text file and the other two use computed filenames that will never (in practice) be a pipe. This is effectively NFC, for now. But I have some other patches in the works that add guard rails, crashing if the stream hasn't been destructed by the time the CompilerInstance is told to keep the output file, since in most cases this is a problem. Differential Revision: https://reviews.llvm.org/D124635
1 parent f68c0a2 commit 2d13386

File tree

3 files changed

+3
-4
lines changed

3 files changed

+3
-4
lines changed

clang/lib/ExtractAPI/ExtractAPIConsumer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,7 @@ void ExtractAPIAction::EndSourceFileAction() {
793793
// FIXME: Make the kind of APISerializer configurable.
794794
SymbolGraphSerializer SGSerializer(*API, ProductName);
795795
SGSerializer.serialize(*OS);
796-
OS->flush();
796+
OS.reset();
797797
}
798798

799799
std::unique_ptr<raw_pwrite_stream>

clang/lib/Frontend/PrecompiledPreamble.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ class PrecompilePreambleAction : public ASTFrontendAction {
248248
if (FileOS) {
249249
*FileOS << Buffer->Data;
250250
// Make sure it hits disk now.
251-
FileOS->flush();
251+
FileOS.reset();
252252
}
253253

254254
this->HasEmittedPreamblePCH = true;

clang/tools/driver/cc1_main.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,8 +260,7 @@ int cc1_main(ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr) {
260260
Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
261261
/*useTemporary=*/false)) {
262262
llvm::timeTraceProfilerWrite(*profilerOutput);
263-
// FIXME(ibiryukov): make profilerOutput flush in destructor instead.
264-
profilerOutput->flush();
263+
profilerOutput.reset();
265264
llvm::timeTraceProfilerCleanup();
266265
Clang->clearOutputFiles(false);
267266
}

0 commit comments

Comments
 (0)