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

via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 28 13:35:04 PDT 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

<details>
<summary>Changes</summary>

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
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.

``````````

</details>


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


More information about the llvm-commits mailing list