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

[via llvm-commits]

Author: Alexey Karyakin
Date: 2025-05-14T10:33:57-05:00
New Revision: eac7466448f920e733f12beca28ff848cfa4810d

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

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

The `CacheStream::commit()` function (defined in Caching.cpp) deletes
the underlying raw stream. Some output streamers may hold a pointer
to it, which then will outlive the stream object.
In particular, MCAsmStreamer keeps the pointer to the raw stream
though a separate `formatted_raw_stream` object, which buffers data and
there is no path to explicitly flush this data. Before this change,
the buffered data was flushed during the MCAsmStreamer destructor.
After #136121, this happened after the `commit()` function is called.
Therefore, it caused a crash because the `formatted_raw_stream` object
tries to write the buffered data into a deleted raw stream. Even if
we don't delete the stream to avoid the crash, it would be too late
as the output stream cannot accept data after commit().

Fixes: #138194.

Added: 
    llvm/test/ThinLTO/X86/cache-emit-asm.ll

Modified: 
    llvm/lib/LTO/LTOBackend.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index 8a85ac835000a..b7db70b99bcbc 100644
--- a/llvm/lib/LTO/LTOBackend.cpp
+++ b/llvm/lib/LTO/LTOBackend.cpp
@@ -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 codegen pipeline in its own scope so it gets deleted before
+  // Stream->commit() is called. The commit function of CacheStream deletes
+  // the raw stream, which is too early as streamers (e.g. MCAsmStreamer)
+  // keep the pointer and may use it until their destruction. See #138194.
+  {
+    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();
+  }
 
   if (Error Err = Stream->commit())
     report_fatal_error(std::move(Err));

diff  --git a/llvm/test/ThinLTO/X86/cache-emit-asm.ll b/llvm/test/ThinLTO/X86/cache-emit-asm.ll
new file mode 100644
index 0000000000000..ee7484053ca2e
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/cache-emit-asm.ll
@@ -0,0 +1,15 @@
+;; This test runs thin LTO with cache only to look for memory errors, either
+;; as crashes or sanitizer errors. MCAsmStreamer has specific assumptions about
+;; the lifetime of the output stream that are easy to overlook (see #138194).
+
+; RUN: rm -rf %t.cache
+; RUN: opt -module-hash -module-summary -thinlto-bc %s -o %t1.bc
+; RUN: ld.lld --thinlto-cache-dir=%t.cache --lto-emit-asm %t1.bc
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @globalfunc() {
+entry:
+  ret void
+}