[llvm] [llvm] Add comment and assert for CloneModule edge case (PR #67734)

[Jacob Lambert via llvm-commits]

https://github.com/lamb-j created https://github.com/llvm/llvm-project/pull/67734

CloneModule is not currently designed to handle un-materialized Modules, for example one created via a lazy initializer like getLazyBitcodeModule(). In this case we get a somewhat cryptic segmentation fault without a clear path forward.

In this patch, we add a comment to inform CloneModule users of this shortcoming, and an assert to test for empty function bodies before the segmentation fault is triggered.

>From d147a78f93b9ea0ec0f2aae449cf6a37db266cbb Mon Sep 17 00:00:00 2001
From: Jacob Lambert <jacob.lambert at amd.com>
Date: Thu, 28 Sep 2023 13:26:11 -0700
Subject: [PATCH] [llvm] Add comment and assert for CloneModule edge case

CloneModule is not currently designed to handle un-materialized
Modules, for example one created via a lazy initializer like
getLazyBitcodeModule(). In this case we get a somewhat cryptic
segmentation fault without a clear path forward.

In this patch, we add a comment to inform CloneModule users of
this shortcoming, and an assert to test for empty function bodies
before the segmentation fault is triggered.
---
 llvm/lib/Transforms/Utils/CloneFunction.cpp | 2 ++
 llvm/lib/Transforms/Utils/CloneModule.cpp   | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index 39ac48c99ca398d..a18f82f81bf6cda 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -268,6 +268,8 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
 
   // Loop over all of the instructions in the new function, fixing up operand
   // references as we go. This uses VMap to do all the hard work.
+  assert(!OldFunc->empty() && "Cannot clone module with empty function body.\
+         Module must be materialized before cloning!");
   for (Function::iterator
            BB = cast<BasicBlock>(VMap[&OldFunc->front()])->getIterator(),
            BE = NewFunc->end();
diff --git a/llvm/lib/Transforms/Utils/CloneModule.cpp b/llvm/lib/Transforms/Utils/CloneModule.cpp
index 55e051298a9a352..d5163efa8ca8184 100644
--- a/llvm/lib/Transforms/Utils/CloneModule.cpp
+++ b/llvm/lib/Transforms/Utils/CloneModule.cpp
@@ -34,6 +34,8 @@ static void copyComdat(GlobalObject *Dst, const GlobalObject *Src) {
 /// copies of global variables and functions, and making their (initializers and
 /// references, respectively) refer to the right globals.
 ///
+/// Cloning un-materialized modules is not currently supported, so any
+/// modules initialized via lazy loading should be materialized before cloning
 std::unique_ptr<Module> llvm::CloneModule(const Module &M) {
   // Create the value map that maps things from the old module over to the new
   // module.