[PATCH] D129220: [clang] Cleanup ASTContext before output files in crash recovery for modules

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir created this revision.
benlangmuir added reviewers: steven_wu, akyrtzi.
Herald added a project: All.
benlangmuir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When we recover from a crash in a module compilation thread, we need to ensure any output streams owned by the `ASTConsumer` (e.g. in `RawPCHContainerGenerator`) are deleted before we call `clearOutputFiles()`.  This has the same theoretical issues with proxy streams that Duncan discusses in the commit 2d133867833fe8eb <https://reviews.llvm.org/rG2d133867833fe8eb20c11377ff1221f71afc1db3>.  In practice, this was observed as a use-after-free crash on a downstream branch that uses such a proxy stream in this code path.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129220

Files:
  clang/lib/Frontend/CompilerInstance.cpp


Index: clang/lib/Frontend/CompilerInstance.cpp
===================================================================
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -1235,8 +1235,7 @@
 
   // Execute the action to actually build the module in-place. Use a separate
   // thread so that we get a stack large enough.
-  llvm::CrashRecoveryContext CRC;
-  CRC.RunSafelyOnThread(
+  bool Crashed = !llvm::CrashRecoveryContext().RunSafelyOnThread(
       [&]() {
         GenerateModuleFromModuleMapAction Action;
         Instance.ExecuteAction(Action);
@@ -1249,6 +1248,13 @@
                                             diag::remark_module_build_done)
     << ModuleName;
 
+  // Clear the ASTConsumer if it hasn't been already, in case it owns streams
+  // that must be closed before clearing output files.
+  if (Crashed) {
+    Instance.setSema(nullptr);
+    Instance.setASTConsumer(nullptr);
+  }
+
   // Delete any remaining temporary files related to Instance, in case the
   // module generation thread crashed.
   Instance.clearOutputFiles(/*EraseFiles=*/true);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D129220.442653.patch
Type: text/x-patch
Size: 1111 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220706/fd12c981/attachment.bin>