[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