[clang] 2d13386 - Frontend: Delete output streams before closing CompilerInstance outputs

[Duncan P. N. Exon Smith via cfe-commits]

Author: Duncan P. N. Exon Smith
Date: 2022-04-28T19:07:40-07:00
New Revision: 2d133867833fe8eb20c11377ff1221f71afc1db3

URL: https://github.com/llvm/llvm-project/commit/2d133867833fe8eb20c11377ff1221f71afc1db3
DIFF: https://github.com/llvm/llvm-project/commit/2d133867833fe8eb20c11377ff1221f71afc1db3.diff

LOG: 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

Added: 
    

Modified: 
    clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
    clang/lib/Frontend/PrecompiledPreamble.cpp
    clang/tools/driver/cc1_main.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
index 0217c656cbb04..b1de2674b622b 100644
--- a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -793,7 +793,7 @@ void ExtractAPIAction::EndSourceFileAction() {
   // FIXME: Make the kind of APISerializer configurable.
   SymbolGraphSerializer SGSerializer(*API, ProductName);
   SGSerializer.serialize(*OS);
-  OS->flush();
+  OS.reset();
 }
 
 std::unique_ptr<raw_pwrite_stream>

diff  --git a/clang/lib/Frontend/PrecompiledPreamble.cpp b/clang/lib/Frontend/PrecompiledPreamble.cpp
index 341ea6121637d..d5aab4aedadd6 100644
--- a/clang/lib/Frontend/PrecompiledPreamble.cpp
+++ b/clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -248,7 +248,7 @@ class PrecompilePreambleAction : public ASTFrontendAction {
     if (FileOS) {
       *FileOS << Buffer->Data;
       // Make sure it hits disk now.
-      FileOS->flush();
+      FileOS.reset();
     }
 
     this->HasEmittedPreamblePCH = true;

diff  --git a/clang/tools/driver/cc1_main.cpp b/clang/tools/driver/cc1_main.cpp
index f648adeba4834..5adc07154f88c 100644
--- a/clang/tools/driver/cc1_main.cpp
+++ b/clang/tools/driver/cc1_main.cpp
@@ -260,8 +260,7 @@ int cc1_main(ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr) {
             Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
             /*useTemporary=*/false)) {
       llvm::timeTraceProfilerWrite(*profilerOutput);
-      // FIXME(ibiryukov): make profilerOutput flush in destructor instead.
-      profilerOutput->flush();
+      profilerOutput.reset();
       llvm::timeTraceProfilerCleanup();
       Clang->clearOutputFiles(false);
     }