[llvm-branch-commits] [clang] release/18.x: Reland "[clang-repl] Keep the first llvm::Module empty to avoid invalid memory access. (#89031)" (PR #90544)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: None (llvmbot)

<details>
<summary>Changes</summary>

Backport a3f07d36cbc9e3a0d004609d140474c1d8a25bb6

Requested by: @<!-- -->nikic

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


2 Files Affected:

- (modified) clang/lib/Interpreter/IncrementalParser.cpp (+19-5) 
- (modified) clang/lib/Interpreter/IncrementalParser.h (+5) 


``````````diff
diff --git a/clang/lib/Interpreter/IncrementalParser.cpp b/clang/lib/Interpreter/IncrementalParser.cpp
index 370bcbfee8b014..f5f32b9f3924ae 100644
--- a/clang/lib/Interpreter/IncrementalParser.cpp
+++ b/clang/lib/Interpreter/IncrementalParser.cpp
@@ -209,6 +209,10 @@ IncrementalParser::IncrementalParser(Interpreter &Interp,
   if (Err)
     return;
   CI->ExecuteAction(*Act);
+
+  if (getCodeGen())
+    CachedInCodeGenModule = GenModule();
+
   std::unique_ptr<ASTConsumer> IncrConsumer =
       std::make_unique<IncrementalASTConsumer>(Interp, CI->takeASTConsumer());
   CI->setASTConsumer(std::move(IncrConsumer));
@@ -224,11 +228,8 @@ IncrementalParser::IncrementalParser(Interpreter &Interp,
     return;                     // PTU.takeError();
   }
 
-  if (CodeGenerator *CG = getCodeGen()) {
-    std::unique_ptr<llvm::Module> M(CG->ReleaseModule());
-    CG->StartModule("incr_module_" + std::to_string(PTUs.size()),
-                    M->getContext());
-    PTU->TheModule = std::move(M);
+  if (getCodeGen()) {
+    PTU->TheModule = GenModule();
     assert(PTU->TheModule && "Failed to create initial PTU");
   }
 }
@@ -364,6 +365,19 @@ IncrementalParser::Parse(llvm::StringRef input) {
 std::unique_ptr<llvm::Module> IncrementalParser::GenModule() {
   static unsigned ID = 0;
   if (CodeGenerator *CG = getCodeGen()) {
+    // Clang's CodeGen is designed to work with a single llvm::Module. In many
+    // cases for convenience various CodeGen parts have a reference to the
+    // llvm::Module (TheModule or Module) which does not change when a new
+    // module is pushed. However, the execution engine wants to take ownership
+    // of the module which does not map well to CodeGen's design. To work this
+    // around we created an empty module to make CodeGen happy. We should make
+    // sure it always stays empty.
+    assert((!CachedInCodeGenModule ||
+            (CachedInCodeGenModule->empty() &&
+             CachedInCodeGenModule->global_empty() &&
+             CachedInCodeGenModule->alias_empty() &&
+             CachedInCodeGenModule->ifunc_empty())) &&
+           "CodeGen wrote to a readonly module");
     std::unique_ptr<llvm::Module> M(CG->ReleaseModule());
     CG->StartModule("incr_module_" + std::to_string(ID++), M->getContext());
     return M;
diff --git a/clang/lib/Interpreter/IncrementalParser.h b/clang/lib/Interpreter/IncrementalParser.h
index e13b74c7f65948..f63bce50acd3b9 100644
--- a/clang/lib/Interpreter/IncrementalParser.h
+++ b/clang/lib/Interpreter/IncrementalParser.h
@@ -24,6 +24,7 @@
 #include <memory>
 namespace llvm {
 class LLVMContext;
+class Module;
 } // namespace llvm
 
 namespace clang {
@@ -57,6 +58,10 @@ class IncrementalParser {
   /// of code.
   std::list<PartialTranslationUnit> PTUs;
 
+  /// When CodeGen is created the first llvm::Module gets cached in many places
+  /// and we must keep it alive.
+  std::unique_ptr<llvm::Module> CachedInCodeGenModule;
+
   IncrementalParser();
 
 public:

``````````

</details>


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