[Patch, fortran] PR112407 - [13/14 Regression] Fix for PR37336 triggers an ICE in gfc_format_decoder while constructing a vtab

2024-03-30 Thread Paul Richard Thomas 
Hi All,

This bug emerged in a large code and involves possible recursion with a
"hidden" module procedure; ie. where the symtree name starts with '@'. This
throws the format decoder. As the last message in the PR shows, I have
vacillated between silently passing on the possible recursion or adding an
alternative warning message. In the end, as a conservative choice I went
for emitting the message.

In the course of trying to develop a compact test case, I found that type
bound procedures were not being tested for recursion and that class
dummies, with intent out, were being incorrectly initialized with an empty
default initializer. Both of these have been fixed.

Unfortunately, the most compact reproducer that Tomas was able to come up
with required more than 100kbytes of module files. I tried from the bottom
up but failed. Both the tests check the fixes for the other bugs.

Regtests on x86_64 - OK for mainline and, in a couple of weeks, 13-branch?

Paul

Fortran: Fix wrong recursive errors and class initialization [PR112407]

2024-03-30  Paul Thomas  

gcc/fortran
PR fortran/112407
*resolve.cc (resolve_procedure_expression): Change the test for
for recursion in the case of hidden procedures from modules.
(resolve_typebound_static): Add warning for possible recursive
calls to typebound procedures.
* trans-expr.cc (gfc_trans_class_init_assign): Do not apply
default initializer to class dummy where component initializers
are all null.

gcc/testsuite/
PR fortran/112407
* gfortran.dg/pr112407a.f90: New test.
* gfortran.dg/pr112407b.f90: New test.
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 50d51b06c92..43315a6a550 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -1963,12 +1963,20 @@ resolve_procedure_expression (gfc_expr* expr)
   || (sym->attr.function && sym->result == sym))
 return true;
 
-  /* A non-RECURSIVE procedure that is used as procedure expression within its
+   /* A non-RECURSIVE procedure that is used as procedure expression within its
  own body is in danger of being called recursively.  */
   if (is_illegal_recursion (sym, gfc_current_ns))
-gfc_warning (0, "Non-RECURSIVE procedure %qs at %L is possibly calling"
-		 " itself recursively.  Declare it RECURSIVE or use"
-		 " %<-frecursive%>", sym->name, &expr->where);
+{
+  if (sym->attr.use_assoc && expr->symtree->name[0] == '@')
+	gfc_warning (0, "Non-RECURSIVE procedure %qs from module %qs is "
+		 " possibly calling itself recursively in procedure %qs. "
+		 " Declare it RECURSIVE or use %<-frecursive%>",
+		 sym->name, sym->module, gfc_current_ns->proc_name->name);
+  else
+	gfc_warning (0, "Non-RECURSIVE procedure %qs at %L is possibly calling"
+		 " itself recursively.  Declare it RECURSIVE or use"
+		 " %<-frecursive%>", sym->name, &expr->where);
+}
 
   return true;
 }
@@ -6820,6 +6828,13 @@ resolve_typebound_static (gfc_expr* e, gfc_symtree** target,
   if (st)
 	*target = st;
 }
+
+  if (is_illegal_recursion ((*target)->n.sym, gfc_current_ns)
+  && !e->value.compcall.tbp->deferred)
+gfc_warning (0, "Non-RECURSIVE procedure %qs at %L is possibly calling"
+		 " itself recursively.  Declare it RECURSIVE or use"
+		 " %<-frecursive%>", (*target)->n.sym->name, &e->where);
+
   return true;
 }
 
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 76bed9830c4..3b54874cf1f 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -1719,6 +1719,7 @@ gfc_trans_class_init_assign (gfc_code *code)
   tree tmp;
   gfc_se dst,src,memsz;
   gfc_expr *lhs, *rhs, *sz;
+  gfc_component *cmp;
 
   gfc_start_block (&block);
 
@@ -1735,6 +1736,21 @@ gfc_trans_class_init_assign (gfc_code *code)
   /* The _def_init is always scalar.  */
   rhs->rank = 0;
 
+  /* Check def_init for initializers.  If this is a dummy with all default
+ initializer components NULL, return NULL_TREE and use the passed value as
+ required by F2018(8.5.10).  */
+  if (!lhs->ref && lhs->symtree->n.sym->attr.dummy)
+{
+  cmp = rhs->ref->next->u.c.component->ts.u.derived->components;
+  for (; cmp; cmp = cmp->next)
+	{
+	  if (cmp->initializer)
+	break;
+	  else if (!cmp->next)
+	return build_empty_stmt (input_location);
+	}
+}
+
   if (code->expr1->ts.type == BT_CLASS
   && CLASS_DATA (code->expr1)->attr.dimension)
 {
@@ -12511,11 +12527,14 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr * expr2, bool init_flag,
   gfc_add_block_to_block (&body, &lse.pre);
   gfc_add_expr_to_block (&body, tmp);
 
-  /* Add the post blocks to the body.  */
-  if (!l_is_temp)
+  /* Add the post blocks to the body.  Scalar finalization must appear before
+ the post block in case any dellocations are done.  */
+  if (rse.finalblock.head
+  && (!l_is_temp || (expr2->expr_type == EXPR_FUNCTION
+			 && gfc_expr_attr (expr2).elemental)))
 {
-  gfc_add_block_to_block (&rse.finalblock, &rse.post);

Re: [Patch, fortran] PR112407 - [13/14 Regression] Fix for PR37336 triggers an ICE in gfc_format_decoder while constructing a vtab

2024-03-30 Thread Harald Anlauf 

Hi Paul,

I had only a quick glance at your patch.  I guess you unintentionally
forgot to remove those parts that you already committed for PR110987,
along with the finalize-testcases.

I am still trying to find the precise paragraph in the standard
you refer to regarding INTENT(OUT) and default initialization.

While at it, I think I found a minor nit in testcase pr112407a.f90:
component x%i appears undefined the first time it is printed.
This can be verified by either adding an explicit

  x% i = -42

in the main after the allocate(x).  Alternatively, running the
code with Intel and using MALLOC_PERTURB_ shows a random arg1%i,
but is otherwise fine.  However, if by chance (random memory)

  x% i = +42

then the test would likely fail everywhere.

Cheers,
Harald


Am 30.03.24 um 10:06 schrieb Paul Richard Thomas:

Hi All,

This bug emerged in a large code and involves possible recursion with a
"hidden" module procedure; ie. where the symtree name starts with '@'. This
throws the format decoder. As the last message in the PR shows, I have
vacillated between silently passing on the possible recursion or adding an
alternative warning message. In the end, as a conservative choice I went
for emitting the message.

In the course of trying to develop a compact test case, I found that type
bound procedures were not being tested for recursion and that class
dummies, with intent out, were being incorrectly initialized with an empty
default initializer. Both of these have been fixed.

Unfortunately, the most compact reproducer that Tomas was able to come up
with required more than 100kbytes of module files. I tried from the bottom
up but failed. Both the tests check the fixes for the other bugs.

Regtests on x86_64 - OK for mainline and, in a couple of weeks, 13-branch?

Paul

Fortran: Fix wrong recursive errors and class initialization [PR112407]

2024-03-30  Paul Thomas  

gcc/fortran
PR fortran/112407
*resolve.cc (resolve_procedure_expression): Change the test for
for recursion in the case of hidden procedures from modules.
(resolve_typebound_static): Add warning for possible recursive
calls to typebound procedures.
* trans-expr.cc (gfc_trans_class_init_assign): Do not apply
default initializer to class dummy where component initializers
are all null.

gcc/testsuite/
PR fortran/112407
* gfortran.dg/pr112407a.f90: New test.
* gfortran.dg/pr112407b.f90: New test.

Re: [Patch, fortran] PR112407 - [13/14 Regression] Fix for PR37336 triggers an ICE in gfc_format_decoder while constructing a vtab

2024-03-31 Thread Paul Richard Thomas 
Hi Harald,

>
> I had only a quick glance at your patch.  I guess you unintentionally
> forgot to remove those parts that you already committed for PR110987,
> along with the finalize-testcases.
>

Guilty as charged. I guess I got out of the wrong side of the bed :-)

>
> I am still trying to find the precise paragraph in the standard
> you refer to regarding INTENT(OUT) and default initialization.
>

Page 114 of the draft F2023 standard:
"The INTENT (OUT) attribute for a nonpointer dummy argument specifies that
the dummy argument becomes undefined on invocation of the procedure, except
for any subcomponents that are default-initialized (7.5.4.6)."
With the fix, gfortran behaves in the same way as ifort and nagfor.

On rereading the patch, I think that s/"and use the passed value"/"and
leave undefined"/ or some such is in order.


> While at it, I think I found a minor nit in testcase pr112407a.f90:
> component x%i appears undefined the first time it is printed.
>

Fixed - thanks for pointing it out.

A correct patch is attached.

Thanks for looking at the previous, overloaded version.

Paul



>
> > 2024-03-30  Paul Thomas  
> >
> > gcc/fortran
> > PR fortran/112407
> > *resolve.cc (resolve_procedure_expression): Change the test for
> > for recursion in the case of hidden procedures from modules.
> > (resolve_typebound_static): Add warning for possible recursive
> > calls to typebound procedures.
> > * trans-expr.cc (gfc_trans_class_init_assign): Do not apply
> > default initializer to class dummy where component initializers
> > are all null.
> >
> > gcc/testsuite/
> > PR fortran/112407
> > * gfortran.dg/pr112407a.f90: New test.
> > * gfortran.dg/pr112407b.f90: New test.
> >
>
>
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 50d51b06c92..43315a6a550 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -1963,12 +1963,20 @@ resolve_procedure_expression (gfc_expr* expr)
   || (sym->attr.function && sym->result == sym))
 return true;

-  /* A non-RECURSIVE procedure that is used as procedure expression within its
+   /* A non-RECURSIVE procedure that is used as procedure expression within its
  own body is in danger of being called recursively.  */
   if (is_illegal_recursion (sym, gfc_current_ns))
-gfc_warning (0, "Non-RECURSIVE procedure %qs at %L is possibly calling"
-		 " itself recursively.  Declare it RECURSIVE or use"
-		 " %<-frecursive%>", sym->name, &expr->where);
+{
+  if (sym->attr.use_assoc && expr->symtree->name[0] == '@')
+	gfc_warning (0, "Non-RECURSIVE procedure %qs from module %qs is "
+		 " possibly calling itself recursively in procedure %qs. "
+		 " Declare it RECURSIVE or use %<-frecursive%>",
+		 sym->name, sym->module, gfc_current_ns->proc_name->name);
+  else
+	gfc_warning (0, "Non-RECURSIVE procedure %qs at %L is possibly calling"
+		 " itself recursively.  Declare it RECURSIVE or use"
+		 " %<-frecursive%>", sym->name, &expr->where);
+}

   return true;
 }
@@ -6820,6 +6828,13 @@ resolve_typebound_static (gfc_expr* e, gfc_symtree** target,
   if (st)
 	*target = st;
 }
+
+  if (is_illegal_recursion ((*target)->n.sym, gfc_current_ns)
+  && !e->value.compcall.tbp->deferred)
+gfc_warning (0, "Non-RECURSIVE procedure %qs at %L is possibly calling"
+		 " itself recursively.  Declare it RECURSIVE or use"
+		 " %<-frecursive%>", (*target)->n.sym->name, &e->where);
+
   return true;
 }

diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 76bed9830c4..f3fcba2bd59 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -1719,6 +1719,7 @@ gfc_trans_class_init_assign (gfc_code *code)
   tree tmp;
   gfc_se dst,src,memsz;
   gfc_expr *lhs, *rhs, *sz;
+  gfc_component *cmp;

   gfc_start_block (&block);

@@ -1735,6 +1736,21 @@ gfc_trans_class_init_assign (gfc_code *code)
   /* The _def_init is always scalar.  */
   rhs->rank = 0;

+  /* Check def_init for initializers.  If this is a dummy with all default
+ initializer components NULL, return NULL_TREE and use the passed value as
+ required by F2018(8.5.10).  */
+  if (!lhs->ref && lhs->symtree->n.sym->attr.dummy)
+{
+  cmp = rhs->ref->next->u.c.component->ts.u.derived->components;
+  for (; cmp; cmp = cmp->next)
+	{
+	  if (cmp->initializer)
+	break;
+	  else if (!cmp->next)
+	return build_empty_stmt (input_location);
+	}
+}
+
   if (code->expr1->ts.type == BT_CLASS
   && CLASS_DATA (code->expr1)->attr.dimension)
 {
diff --git a/gcc/testsuite/gfortran.dg/pr112407a.f90 b/gcc/testsuite/gfortran.dg/pr112407a.f90
new file mode 100644
index 000..470f4191611
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr112407a.f90
@@ -0,0 +1,71 @@
+! { dg-do run }
+! Test of an issue found in the investigation of PR112407
+! Contributed by Tomas Trnka  
+!
+module m
+  private new_t
+
+  type s
+procedure(),pointer,nopass :: op
+  end type
+
+  type :: t
+integer :: i
+

Re: [Patch, fortran] PR112407 - [13/14 Regression] Fix for PR37336 triggers an ICE in gfc_format_decoder while constructing a vtab

2024-04-01 Thread Harald Anlauf 

Hi Paul!

Am 31.03.24 um 14:08 schrieb Paul Richard Thomas:

Hi Harald,



I had only a quick glance at your patch.  I guess you unintentionally
forgot to remove those parts that you already committed for PR110987,
along with the finalize-testcases.



Guilty as charged. I guess I got out of the wrong side of the bed :-)



I am still trying to find the precise paragraph in the standard
you refer to regarding INTENT(OUT) and default initialization.



Page 114 of the draft F2023 standard:
"The INTENT (OUT) attribute for a nonpointer dummy argument specifies that
the dummy argument becomes undefined on invocation of the procedure, except
for any subcomponents that are default-initialized (7.5.4.6)."
With the fix, gfortran behaves in the same way as ifort and nagfor.

On rereading the patch, I think that s/"and use the passed value"/"and
leave undefined"/ or some such is in order.


Yes, something along this line is better.

I also did test with NAG and Intel, and was surprised (confused?) at how
the count of finalizer calls changes if component "i" gets a default
value or not.  Something one wouldn't do right after getting out of bed!

So the patch looks good to me - except for one philosophical question:

Fortran 2018 makes procedures recursive by default, but this is not
yet implemented as such, and NON_RECURSIVE is not yet implemented.

The new testcase pr112407b.f90 compiles with nagfor -f2018 without
any warnings, and gives an error with nagfor -f2008.  It appears
that it works in the testsuite after the patch and when adding
"-std=f2008" instead of using the default "-std=gnu".

Would you mind adding "-std=f2008" as dg-option to that testcase?
This would avoid one bogus regression when gfortran moves forward.

Thanks for the patch!

Harald




While at it, I think I found a minor nit in testcase pr112407a.f90:
component x%i appears undefined the first time it is printed.



Fixed - thanks for pointing it out.

A correct patch is attached.

Thanks for looking at the previous, overloaded version.

Paul






2024-03-30  Paul Thomas  

gcc/fortran
PR fortran/112407
*resolve.cc (resolve_procedure_expression): Change the test for
for recursion in the case of hidden procedures from modules.
(resolve_typebound_static): Add warning for possible recursive
calls to typebound procedures.
* trans-expr.cc (gfc_trans_class_init_assign): Do not apply
default initializer to class dummy where component initializers
are all null.

gcc/testsuite/
PR fortran/112407
* gfortran.dg/pr112407a.f90: New test.
* gfortran.dg/pr112407b.f90: New test.