[flang-commits] [flang] [flang][driver] do not crash when fc1 process multiple files (PR #138875)

[via flang-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-driver

Author: None (jeanPerier)

<details>
<summary>Changes</summary>

This is a fix for the issue https://github.com/llvm/llvm-project/issues/137126 that turned out to be a driver issue.

The FrontendActions have a loop to process multiple input file and flang -fc1 accept multiple files, but the semantic, lowering, and llvm codegen actions were not re-entrant.

- The SemanticContext cannot be reuse between two files since it is holding the scope, so symbol will clash.
- The mlir and llvm module need to be reset before overriding the previous contexts, otherwise there later deletion will use the old context and cause use after free.

**I am unsure it is the intention/a feature to process mutliple input files in a single -fc1 command**
Flang invoke one flang -fc1 process per input file, hence the issue does not show up with `flang -c foo.f90 bar.f90`). I think my fix is fragile (it is hard to keep track of where all the unique pointers are set/deleted, and how they depend on each other) and maybe it would be better to prevent multiple input to fc1. @<!-- -->banach-space, what was the intention?

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


5 Files Affected:

- (modified) flang/include/flang/Frontend/CompilerInstance.h (+6) 
- (modified) flang/lib/Frontend/CompilerInstance.cpp (-2) 
- (modified) flang/lib/Frontend/FrontendAction.cpp (+1-1) 
- (modified) flang/lib/Frontend/FrontendActions.cpp (+7) 
- (added) flang/test/Driver/multiple-fc1-input.f90 (+9) 


``````````diff
diff --git a/flang/include/flang/Frontend/CompilerInstance.h b/flang/include/flang/Frontend/CompilerInstance.h
index e37ef5e236871..4ad95c9df42d7 100644
--- a/flang/include/flang/Frontend/CompilerInstance.h
+++ b/flang/include/flang/Frontend/CompilerInstance.h
@@ -147,6 +147,12 @@ class CompilerInstance {
   /// @name Semantic analysis
   /// {
 
+  Fortran::semantics::SemanticsContext &createNewSemanticsContext() {
+    semaContext =
+        getInvocation().getSemanticsCtx(*allCookedSources, getTargetMachine());
+    return *semaContext;
+  }
+
   Fortran::semantics::SemanticsContext &getSemanticsContext() {
     return *semaContext;
   }
diff --git a/flang/lib/Frontend/CompilerInstance.cpp b/flang/lib/Frontend/CompilerInstance.cpp
index f7ed969f03bf4..cbd2c58eeeb47 100644
--- a/flang/lib/Frontend/CompilerInstance.cpp
+++ b/flang/lib/Frontend/CompilerInstance.cpp
@@ -162,8 +162,6 @@ bool CompilerInstance::executeAction(FrontendAction &act) {
   allSources->set_encoding(invoc.getFortranOpts().encoding);
   if (!setUpTargetMachine())
     return false;
-  // Create the semantics context
-  semaContext = invoc.getSemanticsCtx(*allCookedSources, getTargetMachine());
   // Set options controlling lowering to FIR.
   invoc.setLoweringOptions();
 
diff --git a/flang/lib/Frontend/FrontendAction.cpp b/flang/lib/Frontend/FrontendAction.cpp
index ab77d143fa4b6..d178fd6a9395d 100644
--- a/flang/lib/Frontend/FrontendAction.cpp
+++ b/flang/lib/Frontend/FrontendAction.cpp
@@ -183,7 +183,7 @@ bool FrontendAction::runSemanticChecks() {
 
   // Transfer any pending non-fatal messages from parsing to semantics
   // so that they are merged and all printed in order.
-  auto &semanticsCtx{ci.getSemanticsContext()};
+  auto &semanticsCtx{ci.createNewSemanticsContext()};
   semanticsCtx.messages().Annex(std::move(ci.getParsing().messages()));
   semanticsCtx.set_debugModuleWriter(ci.getInvocation().getDebugModuleDir());
 
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index c1f47b12abee2..e5a15c555fa5e 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -171,6 +171,10 @@ static void addDependentLibs(mlir::ModuleOp mlirModule, CompilerInstance &ci) {
 }
 
 bool CodeGenAction::beginSourceFileAction() {
+  // Delete previous LLVM module depending on old context before making a new
+  // one.
+  if (llvmModule)
+    llvmModule.reset(nullptr);
   llvmCtx = std::make_unique<llvm::LLVMContext>();
   CompilerInstance &ci = this->getInstance();
   mlir::DefaultTimingManager &timingMgr = ci.getTimingManager();
@@ -197,6 +201,9 @@ bool CodeGenAction::beginSourceFileAction() {
     return true;
   }
 
+  // Reset MLIR module if it was set before overriding the old context.
+  if (mlirModule)
+    mlirModule = mlir::OwningOpRef<mlir::ModuleOp>(nullptr);
   // Load the MLIR dialects required by Flang
   mlirCtx = std::make_unique<mlir::MLIRContext>();
   fir::support::loadDialects(*mlirCtx);
diff --git a/flang/test/Driver/multiple-fc1-input.f90 b/flang/test/Driver/multiple-fc1-input.f90
new file mode 100644
index 0000000000000..57f7c5e92b4c4
--- /dev/null
+++ b/flang/test/Driver/multiple-fc1-input.f90
@@ -0,0 +1,9 @@
+! Test that flang -fc1 can be called with several input files without
+! crashing.
+! Regression tests for: https://github.com/llvm/llvm-project/issues/137126
+
+! RUN: %flang_fc1 -emit-fir %s %s -o - | FileCheck %s
+subroutine foo()
+end subroutine
+! CHECK: func @_QPfoo() 
+! CHECK: func @_QPfoo() 

``````````

</details>


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