[flang-commits] [flang] [flang][semantics] fix crash involving equivalences (PR #168909)

[via flang-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-semantics

Author: Andre Kuhlenschmidt (akuhlens)

<details>
<summary>Changes</summary>

This PR does three things.

1. Adds a test case that before this PR caused the compiler to crash.
2. It makes `ProcessScopes` more resilient to data that hasn't had it's size set, instead it now just returns false to indicate that it encountered an error during processing instead of signalling an internal compiler error. 
   - If we decide to omit the reduction in the number of times we run `ProcessScopes` we will need to keep this change.
   - Otherwise, I think this code should be unreachable, if it is reachable we should have some semantic error that covers the
      reason we were unable to give the variable a size.
   - This might be unacceptable hardening because there might be the possibility of having zero sized arrays that flow to this code. If so, I would appreciate suggestion about how to correct this hardening.
3. It only runs ProcessScopes at the end of the semantic analysis, therefore
   - If we decide to keep this, we should be able to get rid of the first change.

Mostly presenting both fixes, because I am not sure if there are missing test cases that don't cover why the code wasn't changed to be there second option from the beginning.

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


4 Files Affected:

- (modified) flang/lib/Semantics/check-data.cpp (+1-1) 
- (modified) flang/lib/Semantics/data-to-inits.cpp (+22-5) 
- (modified) flang/lib/Semantics/data-to-inits.h (+2-2) 
- (added) flang/test/Semantics/equivalence02.f90 (+51) 


``````````diff
diff --git a/flang/lib/Semantics/check-data.cpp b/flang/lib/Semantics/check-data.cpp
index 3bcf711735158..af89fd7a29e37 100644
--- a/flang/lib/Semantics/check-data.cpp
+++ b/flang/lib/Semantics/check-data.cpp
@@ -273,7 +273,7 @@ void DataChecker::Leave(const parser::EntityDecl &decl) {
 }
 
 void DataChecker::CompileDataInitializationsIntoInitializers() {
-  ConvertToInitializers(inits_, exprAnalyzer_);
+  ConvertToInitializers(inits_, exprAnalyzer_, /*processScopes=*/true);
 }
 
 } // namespace Fortran::semantics
diff --git a/flang/lib/Semantics/data-to-inits.cpp b/flang/lib/Semantics/data-to-inits.cpp
index bbf3b28fe03e6..af4fffaf9f23b 100644
--- a/flang/lib/Semantics/data-to-inits.cpp
+++ b/flang/lib/Semantics/data-to-inits.cpp
@@ -863,8 +863,18 @@ static bool ProcessScopes(const Scope &scope,
       if (std::find_if(associated.begin(), associated.end(), [](SymbolRef ref) {
             return IsInitialized(*ref);
           }) != associated.end()) {
-        result &=
-            CombineEquivalencedInitialization(associated, exprAnalyzer, inits);
+        if (std::find_if(associated.begin(), associated.end(),
+                [](SymbolRef ref) { return !ref->size(); }) !=
+            associated.end()) {
+          // If a symbol has a non-legacy initialization, it won't have a size,
+          // so we can't combine its initializations the DataChecker Runs after
+          // this size is computed and runs this code again so it will have a
+          // size when encountered later.
+          result = false;
+        } else {
+          result &= CombineEquivalencedInitialization(
+              associated, exprAnalyzer, inits);
+        }
       }
     }
     if constexpr (makeDefaultInitializationExplicit) {
@@ -944,8 +954,8 @@ void ConstructInitializer(const Symbol &symbol,
   }
 }
 
-void ConvertToInitializers(
-    DataInitializations &inits, evaluate::ExpressionAnalyzer &exprAnalyzer) {
+void ConvertToInitializers(DataInitializations &inits,
+    evaluate::ExpressionAnalyzer &exprAnalyzer, bool processScopes) {
   // Process DATA-style component /initializers/ now, so that they appear as
   // default values in time for EQUIVALENCE processing in ProcessScopes.
   for (auto &[symbolPtr, initialization] : inits) {
@@ -953,7 +963,14 @@ void ConvertToInitializers(
       ConstructInitializer(*symbolPtr, initialization, exprAnalyzer);
     }
   }
-  if (ProcessScopes(
+  // FIXME: It is kinda weird that we need to repeatedly process the entire
+  // symbol table each time this is called by LegacyDataInitialization in
+  // ResolveNames. Could we do this once before the DataChecker and once after
+  // to combine initializations from Non-Legacy Initialization? Note, it passes
+  // all tests with just Running this code in
+  // CompileDataInitializationsIntoInitializers.
+  if (processScopes &&
+      ProcessScopes(
           exprAnalyzer.context().globalScope(), exprAnalyzer, inits)) {
     for (auto &[symbolPtr, initialization] : inits) {
       if (!symbolPtr->owner().IsDerivedType()) {
diff --git a/flang/lib/Semantics/data-to-inits.h b/flang/lib/Semantics/data-to-inits.h
index 7486ac8113e90..57fce123ee325 100644
--- a/flang/lib/Semantics/data-to-inits.h
+++ b/flang/lib/Semantics/data-to-inits.h
@@ -62,8 +62,8 @@ void AccumulateDataInitializations(DataInitializations &,
     evaluate::ExpressionAnalyzer &, const Symbol &,
     const std::list<common::Indirection<parser::DataStmtValue>> &);
 
-void ConvertToInitializers(
-    DataInitializations &, evaluate::ExpressionAnalyzer &);
+void ConvertToInitializers(DataInitializations &,
+    evaluate::ExpressionAnalyzer &, bool ProcessScopes = false);
 
 } // namespace Fortran::semantics
 #endif // FORTRAN_SEMANTICS_DATA_TO_INITS_H_
diff --git a/flang/test/Semantics/equivalence02.f90 b/flang/test/Semantics/equivalence02.f90
new file mode 100644
index 0000000000000..6087f43d8c2c3
--- /dev/null
+++ b/flang/test/Semantics/equivalence02.f90
@@ -0,0 +1,51 @@
+! RUN: %python %S/test_errors.py %s %flang_fc1
+
+program main
+type t1
+  sequence
+  integer, dimension(2):: i/42, 1/      
+end type
+type t2
+  sequence
+  integer :: j(2) = [41, 1]
+end type
+
+! ERROR: Distinct default component initializations of equivalenced objects affect 'a%i(1_8)' more than once
+type (t1) :: A
+! ERROR: Distinct default component initializations of equivalenced objects affect 'b%j(1_8)' more than once
+type (t2) :: B
+! ERROR: Distinct default component initializations of equivalenced objects affect 'x' more than once
+integer :: x
+data x/42/
+equivalence (A, B, x)
+call p(x)
+call s()
+end
+
+subroutine s()
+  type g1
+    sequence
+    integer(kind=8)::d/1_8/
+  end type
+  type g2
+    sequence
+    integer(kind=8)::d = 2_8
+  end type
+  ! ERROR: Distinct default component initializations of equivalenced objects affect 'c%d' more than once
+  type (g1) :: C
+  ! ERROR: Distinct default component initializations of equivalenced objects affect 'd%d' more than once
+  type (g2) :: D
+  ! ERROR: Distinct default component initializations of equivalenced objects affect 'x' more than once
+  ! ERROR: Distinct default component initializations of equivalenced objects affect 'y' more than once
+  integer :: x, y
+  data x/1/, y/2/
+  equivalence (C, x)
+  equivalence (D, y)
+  equivalence (x, y)
+  print *, x, y
+end subroutine
+
+subroutine p(x)
+  integer :: x
+  print *, x
+end subroutine

``````````

</details>


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