[PATCH] D124635: Frontend: Delete output streams before closing CompilerInstance outputs

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

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>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124635.425893.patch
Type: text/x-patch
Size: 1472 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220428/158c481a/attachment-0001.bin>