[flang-commits] [flang] 996c0fb - [flang] Avoid cycles during instantiation of derived types

Roger Ferrer Ibanez via flang-commits flang-commits at lists.llvm.org
Thu Sep 21 05:40:59 PDT 2023 

Author: Roger Ferrer Ibanez
Date: 2023-09-21T12:39:23Z
New Revision: 996c0fb3a2c28047d19b6a151f6f357c5022c137

URL: https://github.com/llvm/llvm-project/commit/996c0fb3a2c28047d19b6a151f6f357c5022c137
DIFF: https://github.com/llvm/llvm-project/commit/996c0fb3a2c28047d19b6a151f6f357c5022c137.diff

LOG: [flang] Avoid cycles during instantiation of derived types

Derived-type-spec (such as `type(t)`) typically cause the instantiation
of a class which is also used to define the offsets of its data
components and the size of the class.

Fortran derived types are always "completely" defined (i.e., no
incomplete / opaque derived types exist on which we can build a pointer
to them like in C/C++) so they can have their offsets always computed.

However, we must be careful not to instantiate a derived type while it
is being defined. This can happen due to cycles introduced by forward
references, such as the one below.

```lang=fortran
  type t1
    type(t2), pointer :: b ! (A)
  end type t1

  type :: t2 ! (B)
    type(t1), pointer :: a ! (C)
  end type t2 ! (D)
```

At `(A)`, flang determines that this is a forward declaration so no
instantiation happens.

At `(B)`, flang determines `t2` is not a forward declaration anymore,
because we are defining it.

At `(C)`, flang chooses to instantiate `t1`. Instantiation of `t1` finds
the field `b` at `(A)`. Now `t2` is not a forward declaration anymore,
so it can be instantiated. But at this point the field `a` has not been
added to `t2`, so we compute the size of an empty class. Because this
computation is done just once, we end emitting a wrong derived type
descriptor with a `sizeinbytes` field set to 0.

Because these kind of cycles can only happen via forward referenced
derived types specifiers, the idea here is to avoid instantiating the
derived type being defined (i.e. `t2`) until `(D)`. Keeping the
attribute "is forward reference" set until `(D)` avoids that.

Fixes https://github.com/llvm/llvm-project/issues/64973

Differential Revision: https://reviews.llvm.org/D159117

Added: 
    flang/test/Semantics/typeinfo05.f90
    flang/test/Semantics/typeinfo06.f90

Modified: 
    flang/lib/Semantics/resolve-names.cpp

Removed: 
    


################################################################################
diff  --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 47ecaa1b82e53ed..60a604bd0888838 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -5189,7 +5189,6 @@ bool DeclarationVisitor::Pre(const parser::DerivedTypeDef &x) {
   CHECK(scope.symbol());
   CHECK(scope.symbol()->scope() == &scope);
   auto &details{scope.symbol()->get<DerivedTypeDetails>()};
-  details.set_isForwardReferenced(false);
   std::set<SourceName> paramNames;
   for (auto &paramName : std::get<std::list<parser::Name>>(stmt.statement.t)) {
     details.add_paramName(paramName.source);
@@ -5242,6 +5241,7 @@ bool DeclarationVisitor::Pre(const parser::DerivedTypeDef &x) {
   }
   Walk(std::get<std::optional<parser::TypeBoundProcedurePart>>(x.t));
   Walk(std::get<parser::Statement<parser::EndTypeStmt>>(x.t));
+  details.set_isForwardReferenced(false);
   derivedTypeInfo_ = {};
   PopScope();
   return false;
@@ -5257,7 +5257,13 @@ void DeclarationVisitor::Post(const parser::DerivedTypeStmt &x) {
   auto *extendsName{derivedTypeInfo_.extends};
   std::optional<DerivedTypeSpec> extendsType{
       ResolveExtendsType(name, extendsName)};
-  auto &symbol{MakeSymbol(name, GetAttrs(), DerivedTypeDetails{})};
+  DerivedTypeDetails derivedTypeDetails;
+  if (Symbol *typeSymbol{FindInScope(currScope(), name)}; typeSymbol &&
+      typeSymbol->has<DerivedTypeDetails>() &&
+      typeSymbol->get<DerivedTypeDetails>().isForwardReferenced()) {
+    derivedTypeDetails.set_isForwardReferenced(true);
+  }
+  auto &symbol{MakeSymbol(name, GetAttrs(), std::move(derivedTypeDetails))};
   symbol.ReplaceName(name.source);
   derivedTypeInfo_.type = &symbol;
   PushScope(Scope::Kind::DerivedType, &symbol);

diff  --git a/flang/test/Semantics/typeinfo05.f90 b/flang/test/Semantics/typeinfo05.f90
new file mode 100644
index 000000000000000..2a7f12a153eb839
--- /dev/null
+++ b/flang/test/Semantics/typeinfo05.f90
@@ -0,0 +1,16 @@
+!RUN: bbc --dump-symbols %s | FileCheck %s
+!RUN: %flang_fc1 -fdebug-dump-symbols %s | FileCheck %s
+! Ensure that cycles via POINTERs do not instantiate incomplete derived
+! types that would lead to types whose sizeinbytes=0
+program main
+  implicit none
+  type t1
+    type(t2), pointer :: b
+  end type t1
+!CHECK: .dt.t1, SAVE, TARGET (CompilerCreated, ReadOnly): ObjectEntity type: TYPE(derivedtype) init:derivedtype(binding=NULL(),name=.n.t1,sizeinbytes=40_8,uninstantiated=NULL(),kindparameter=NULL(),lenparameterkind=NULL(),component=.c.t1,procptr=NULL(),special=NULL(),specialbitset=0_4,hasparent=0_1,noinitializationneeded=0_1,nodestructionneeded=1_1,nofinalizationneeded=1_1)
+  type :: t2
+    type(t1) :: a
+  end type t2
+! CHECK: .dt.t2, SAVE, TARGET (CompilerCreated, ReadOnly): ObjectEntity type: TYPE(derivedtype) init:derivedtype(binding=NULL(),name=.n.t2,sizeinbytes=40_8,uninstantiated=NULL(),kindparameter=NULL(),lenparameterkind=NULL(),component=.c.t2,procptr=NULL(),special=NULL(),specialbitset=0_4,hasparent=0_1,noinitializationneeded=0_1,nodestructionneeded=1_1,nofinalizationneeded=1_1)
+end program main
+

diff  --git a/flang/test/Semantics/typeinfo06.f90 b/flang/test/Semantics/typeinfo06.f90
new file mode 100644
index 000000000000000..2385709a8eb4494
--- /dev/null
+++ b/flang/test/Semantics/typeinfo06.f90
@@ -0,0 +1,16 @@
+!RUN: bbc --dump-symbols %s | FileCheck %s
+!RUN: %flang_fc1 -fdebug-dump-symbols %s | FileCheck %s
+! Ensure that cycles via ALLOCATABLEs do not instantiate incomplete derived
+! types that would lead to types whose sizeinbytes=0
+program main
+  implicit none
+  type t1
+    type(t2), allocatable :: b
+  end type t1
+!CHECK: .dt.t1, SAVE, TARGET (CompilerCreated, ReadOnly): ObjectEntity type: TYPE(derivedtype) init:derivedtype(binding=NULL(),name=.n.t1,sizeinbytes=40_8,uninstantiated=NULL(),kindparameter=NULL(),lenparameterkind=NULL(),component=.c.t1,procptr=NULL(),special=NULL(),specialbitset=0_4,hasparent=0_1,noinitializationneeded=0_1,nodestructionneeded=0_1,nofinalizationneeded=1_1)
+  type :: t2
+    type(t1) :: a
+  end type t2
+! CHECK: .dt.t2, SAVE, TARGET (CompilerCreated, ReadOnly): ObjectEntity type: TYPE(derivedtype) init:derivedtype(binding=NULL(),name=.n.t2,sizeinbytes=40_8,uninstantiated=NULL(),kindparameter=NULL(),lenparameterkind=NULL(),component=.c.t2,procptr=NULL(),special=NULL(),specialbitset=0_4,hasparent=0_1,noinitializationneeded=0_1,nodestructionneeded=0_1,nofinalizationneeded=1_1)
+end program main
+

More information about the flang-commits mailing list