From 430eb5733ffea322307864169f4d51507bc61ec6 Mon Sep 17 00:00:00 2001
From: Alexey Karyakin <akaryaki@quicinc.com>
Date: Thu, 1 May 2025 14:28:20 -0700
Subject: [PATCH 1/2] [LTO] Fix a crash with thin LTO caching and asm output

The commit function of CacheStream deletes the underlying raw stream.
Some output streamers may hold a pointer to it, which in this case will
outlive the stream object. In particular, MCAsmStreamer keeps the
pointer to the raw stream though formatted_raw_stream, which calls
flush() in its destructor. If commit is called before MCAsmStreamer
destruction, it will access freed memory.

S
---
 llvm/lib/LTO/LTOBackend.cpp | 48 +++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index dd9197efa7718..537e0ff89ce6a 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));

From c5402c18d40ed8a5d8d25ba64607d4bff78d903d Mon Sep 17 00:00:00 2001
From: Alexey Karyakin <akaryaki@quicinc.com>
Date: Fri, 2 May 2025 10:06:21 -0700
Subject: [PATCH 2/2] Add a test

---
 llvm/test/ThinLTO/X86/cache-emit-asm.ll | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 llvm/test/ThinLTO/X86/cache-emit-asm.ll

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
+}