[flang-commits] [flang] [flang][OpenMP] Fix segfault on module USE cycle with -fopenmp (PR #228037)

[via flang-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-semantics

Author: Mihai Zegheru (mihaizegheru)

<details>
<summary>Changes</summary>

Add guards against `nullptr` dereferencing in `OmpStructureChecker::Enter()` for `parser::Module` and `parser::Submodule`; change overloads' return type from `void` to `bool`, returning `false` when the symbol is missing. Update generic `Pre()` caller code in `SemanticsVisitor` to honor the Boolean return type of `Enter()`. Returning `false` skips the node’s children and its `Leave()` callback, preventing an unmatched pop from `scopeStack_`. Undo any `context_` stack push performed by `Pre()`, since `Post()` will also be skipped.

#### Context

With `-fopenmp`, Flang crashes during semantic analysis when modules form a USE cycle. Name resolution diagnoses the cycle but leaves the module names without symbols. `OmpStructureChecker` subsequently dereferences a null symbol while obtaining the module’s scope.

#### Changes summary

- Add `nullptr` guards in `check-omp-structure.cpp` and change module and submodule `Enter()` overloads to return Boolean.
- Add check in `semantics.cpp` to keep both scope stack and construct stack operations balanced.
- Extend `modfile54.f90` with an additional `RUN`, covering for the `-fopenmp` flag.

Fixes #<!-- -->227265.

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


4 Files Affected:

- (modified) flang/lib/Semantics/check-omp-structure.cpp (+10-2) 
- (modified) flang/lib/Semantics/check-omp-structure.h (+2-2) 
- (modified) flang/lib/Semantics/semantics.cpp (+9) 
- (modified) flang/test/Semantics/modfile54.f90 (+1) 


``````````diff
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 09b6c6bc3449f..df973f26f1d3a 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -224,20 +224,28 @@ void OmpStructureChecker::Leave(const parser::BlockData &x) {
   scopeStack_.pop_back();
 }
 
-void OmpStructureChecker::Enter(const parser::Module &x) {
+bool OmpStructureChecker::Enter(const parser::Module &x) {
   auto &stmt{std::get<parser::Statement<parser::ModuleStmt>>(x.t)};
   const Symbol *sym{stmt.statement.v.symbol};
+  if (!sym) {
+    return false;
+  }
   scopeStack_.push_back(sym->scope());
+  return true;
 }
 
 void OmpStructureChecker::Leave(const parser::Module &x) {
   scopeStack_.pop_back();
 }
 
-void OmpStructureChecker::Enter(const parser::Submodule &x) {
+bool OmpStructureChecker::Enter(const parser::Submodule &x) {
   auto &stmt{std::get<parser::Statement<parser::SubmoduleStmt>>(x.t)};
   const Symbol *sym{std::get<parser::Name>(stmt.statement.t).symbol};
+  if (!sym) {
+    return false;
+  }
   scopeStack_.push_back(sym->scope());
+  return true;
 }
 
 void OmpStructureChecker::Leave(const parser::Submodule &x) {
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index a77506e329934..1a2fb58381d4b 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -95,9 +95,9 @@ class OmpStructureChecker : public OmpStructureCheckerBase {
   void Leave(const parser::MainProgram &);
   void Enter(const parser::BlockData &);
   void Leave(const parser::BlockData &);
-  void Enter(const parser::Module &);
+  bool Enter(const parser::Module &);
   void Leave(const parser::Module &);
-  void Enter(const parser::Submodule &);
+  bool Enter(const parser::Submodule &);
   void Leave(const parser::Submodule &);
   void Enter(const parser::SubroutineStmt &);
   void Enter(const parser::EndSubroutineStmt &);
diff --git a/flang/lib/Semantics/semantics.cpp b/flang/lib/Semantics/semantics.cpp
index 761ade6c1e97d..b826f0495fe26 100644
--- a/flang/lib/Semantics/semantics.cpp
+++ b/flang/lib/Semantics/semantics.cpp
@@ -85,6 +85,15 @@ class SemanticsVisitor : public virtual BaseChecker, public virtual C... {
     if constexpr (common::HasMember<const N *, ConstructNode>) {
       context_.PushConstruct(node);
     }
+    if constexpr (std::is_same_v<decltype(Enter(node)), bool>) {
+      if (Enter(node)) {
+        return true;
+      }
+      if constexpr (common::HasMember<const N *, ConstructNode>) {
+        context_.PopConstruct();
+      }
+      return false;
+    }
     Enter(node);
     return true;
   }
diff --git a/flang/test/Semantics/modfile54.f90 b/flang/test/Semantics/modfile54.f90
index 7605e2ac49b07..e318e1dc4ad72 100644
--- a/flang/test/Semantics/modfile54.f90
+++ b/flang/test/Semantics/modfile54.f90
@@ -1,4 +1,5 @@
 ! RUN: %python %S/test_errors.py %s %flang_fc1 -pedantic
+! RUN: %python %S/test_errors.py %s %flang_fc1 -pedantic -fopenmp
 !ERROR: Some modules in this compilation unit form one or more cycles of dependence
 module m1
   use m2

``````````

</details>


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