[llvm] [LTO] Fix a crash with thin LTO caching and asm output (PR #138203)

[Teresa Johnson via llvm-commits]

================
@@ -439,27 +439,33 @@ static void codegen(const Config &Conf, TargetMachine *TM,
   std::unique_ptr<CachedFileStream> &Stream = *StreamOrErr;
   TM->Options.ObjectFilenameForDebug = Stream->ObjectPathName;
 
-  legacy::PassManager CodeGenPasses;
-  TargetLibraryInfoImpl TLII(Mod.getTargetTriple());
-  CodeGenPasses.add(new TargetLibraryInfoWrapperPass(TLII));
-  // No need to make index available if the module is empty.
-  // In theory these passes should not use the index for an empty
-  // module, however, this guards against doing any unnecessary summary-based
-  // analysis in the case of a ThinLTO build where this might be an empty
-  // regular LTO combined module, with a large combined index from ThinLTO.
-  if (!isEmptyModule(Mod))
-    CodeGenPasses.add(
-        createImmutableModuleSummaryIndexWrapperPass(&CombinedIndex));
-  if (Conf.PreCodeGenPassesHook)
-    Conf.PreCodeGenPassesHook(CodeGenPasses);
-  if (TM->addPassesToEmitFile(CodeGenPasses, *Stream->OS,
-                              DwoOut ? &DwoOut->os() : nullptr,
-                              Conf.CGFileType))
-    report_fatal_error("Failed to setup codegen");
-  CodeGenPasses.run(Mod);
-
-  if (DwoOut)
-    DwoOut->keep();
+  // Create the LTO pipeline in its own scope so it gets deleted before
+  // Stream->commit() is called. The commit function of CacheFile deletes
----------------
teresajohnson wrote:

I'll go ahead and approve this PR to address the crash, because I think the underlying issue lies in the asm printing passes and their setup.

Specifically, createMCStreamer creates the formatted_raw_ostream, which stores a pointer to the provided raw_ostream reference that it doesn't own, and addAsmPrinter which in turn stores that in the pass manager. All clients of addPassesToEmitFile, which takes the raw_ostream and pass manager references and invokes these APIs, currently have to ensure that the lifetime of the provided raw_ostream object outlasts the lifetime of the pass manager.

> I stand by what I said about there being latent fragility in the codegen() function in llvm/lib/LTO/LTOBackend.cpp . The MCAsmStreamer object is destroyed (and hence the formatted_raw_ostream object) when CodeGenPasses is destroyed. If the CodeGenPasses constructor were in the future to be moved above the Stream constructor then it would be destroyed after the CachedFileStream
> the position of the addPassesToEmitFile() call is not relevant

Yes, I see what you mean. It is relevant to the problem as it takes both the raw_ostream and the pass manager, and it is down in its handling where the latter takes a pointer to the former, but not to the relative ordering of their construction/destruction. As noted earlier, this issue is not specific to the LTOBackend codegen(), the potential for a problem exists in any client of this code gen pass setup.

https://github.com/llvm/llvm-project/pull/138203