[llvm] Add comment and assert for CloneModule edge case

[lamb-j]

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.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/67734.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/Utils/CloneFunction.cpp (+2)
• (modified) llvm/lib/Transforms/Utils/CloneModule.cpp (+2)

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.

[arsenm]

Don't know about this specific restriction but seems plausible