[PATCH, Fortran] PR71523 - Static variables given automatic initializers with -finit-* and -fmax-stack-var-size

[PATCH, Fortran] PR71523 - Static variables given automatic initializers with -finit-* and -fmax-stack-var-size

[Fritz Reese]

[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]

RE: https://gcc.gnu.org/ml/fortran/2016-06/msg00023.html

On Thu, Jun 9, 2016 at 2:01 PM, Fritz Reese <fritzoreese@gmail.com> wrote:
> It looks like when -fautomatic and -finit-local-zero are set with
> -fmax-stack-var-size=X, an automatic initializer is generated even for
> variables larger than X which are given static storage, causing such
> static variables to have their value re-initialized upon each entry to
> their namespace.
> ...
<snip>

After doing more research I noticed PR41860
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41860) was very similar
to this issue, so I've decided this is a bug and created PR71523
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71523). Here's a patch
for it.

The bug seems to be due to an oversight - since the size of a variable
is not known at resolution time when initializer expressions are
applied, -finit-* is too greedy in the case that the variable is large
enough to be removed from the stack according to -fmax-stack-var-size.
This patch removes automatic initializers at translation time which
were inserted by -finit-* (and inserts the appropriate static
initializer) according to -fmax-stack-var-size.

The patch passes all regression tests (on x86_64-redhat-linux),
including the two additional tests of its own demonstrating the issue.

---
Fritz Reese

[-- Attachment #2: 0001-PR-Fortran-71523.patch --]
[-- Type: application/octet-stream, Size: 4763 bytes --]

From 9a806b1f8a2fd634a87a8c58eb7578c3604960b8 Mon Sep 17 00:00:00 2001
From: Fritz O. Reese <fritzoreese@gmail.com>
Date: Thu, 9 Jun 2016 13:08:53 -0400
Subject: [PATCH] PR Fortran/71523

gcc/fortran/
	* trans-decl.c (gfc_finish_var_decl, gfc_get_symbol_decl): With
	TREE_STATIC decls, use static instead of automatic initializers
	with -finit-* flags.

gcc/testsuite/gfortran.dg/
	* pr71523_1.f90: New testcase.
	* pr71523_2.f90: New testcase.
---
 gcc/fortran/trans-decl.c                |   40 +++++++++++++++++++++++++++++-
 gcc/testsuite/gfortran.dg/pr71523_1.f90 |   22 +++++++++++++++++
 gcc/testsuite/gfortran.dg/pr71523_2.f90 |   38 +++++++++++++++++++++++++++++
 3 files changed, 98 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/pr71523_1.f90
 create mode 100644 gcc/testsuite/gfortran.dg/pr71523_2.f90

diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 2f5e434..1c4912d 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -657,7 +657,43 @@ gfc_finish_var_decl (tree decl, gfc_symbol * sym)
 	  || sym->attr.pointer
 	  || sym->attr.allocatable)
       && !DECL_ARTIFICIAL (decl))
-    TREE_STATIC (decl) = 1;
+    {
+      TREE_STATIC (decl) = 1;
+
+      /* Because the size of this variable isn't known until now, we may have
+         greedily added an initializer to this variable (in build_init_assign)
+         even though the max-stack-var-size indicates the variable should be
+         static. Therefore we rip out the automatic initializer here and
+         replace it with a static one.  */
+      gfc_symtree *st = gfc_find_symtree (sym->ns->sym_root, sym->name);
+      gfc_code *prev = NULL;
+      gfc_code *code = sym->ns->code;
+      while (code && code->op == EXEC_INIT_ASSIGN)
+        {
+          /* Look for an initializer meant for this symbol.  */
+          if (code->expr1->symtree == st)
+            {
+              if (prev)
+                prev->next = code->next;
+              else
+                sym->ns->code = code->next;
+
+              break;
+            }
+
+          prev = code;
+          code = code->next;
+        }
+      if (code && code->op == EXEC_INIT_ASSIGN)
+        {
+          /* Keep the init expression for a static initializer.  */
+          sym->value = code->expr2;
+          /* Cleanup the defunct code object, without freeing the init expr.  */
+          code->expr2 = NULL;
+          gfc_free_statement (code);
+          free (code);
+        }
+    }
 
   /* Handle threadprivate variables.  */
   if (sym->attr.threadprivate
@@ -1693,7 +1729,7 @@ gfc_get_symbol_decl (gfc_symbol * sym)
   if (TREE_STATIC (decl)
       && !(sym->attr.use_assoc && !intrinsic_array_parameter)
       && (sym->attr.save || sym->ns->proc_name->attr.is_main_program
-	  || flag_max_stack_var_size == 0
+	  || !gfc_can_put_var_on_stack (DECL_SIZE_UNIT (decl))
 	  || sym->attr.data || sym->ns->proc_name->attr.flavor == FL_MODULE)
       && (flag_coarray != GFC_FCOARRAY_LIB
 	  || !sym->attr.codimension || sym->attr.allocatable))
diff --git a/gcc/testsuite/gfortran.dg/pr71523_1.f90 b/gcc/testsuite/gfortran.dg/pr71523_1.f90
new file mode 100644
index 0000000..7b5b2ab
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr71523_1.f90
@@ -0,0 +1,22 @@
+! PR Fortran/71523
+!
+! { dg-do compile }
+! { dg-options "-fdump-tree-original -finit-local-zero -fautomatic -fmax-stack-var-size=8" }
+!
+! Make sure that variables larger than max-stack-var-size which become
+! static are not given automatic initializers on function entry.
+!
+
+function set(idx, val)
+  implicit none
+  integer, intent(in) :: idx, val
+  integer set
+  integer arr(100)
+
+  set = arr(idx)
+  arr(idx) = val
+  return
+end function
+
+! There should be no automatic initializer for arr
+! { dg-final { scan-tree-dump-times "arr = " 0 "original" } }
diff --git a/gcc/testsuite/gfortran.dg/pr71523_2.f90 b/gcc/testsuite/gfortran.dg/pr71523_2.f90
new file mode 100644
index 0000000..5be0dd7
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr71523_2.f90
@@ -0,0 +1,38 @@
+! PR Fortran/71523
+!
+! { dg-do run }
+! { dg-options "-finit-integer=12345 -fautomatic -fmax-stack-var-size=8" }
+!
+! Make sure that variables larger than max-stack-var-size become
+! static and are given the correct _static_ initializer.
+!
+
+function set(idx, val)
+  implicit none
+  integer, intent(in) :: idx, val
+  integer set
+  integer arr(100)
+
+  set = arr(idx)
+  arr(idx) = val
+  return
+end function
+
+  integer set, val
+
+  val = set(1, 5)
+  if (val .ne. 12345) then
+    call abort()
+  endif
+
+  val = set(1, 10)
+  if (val .ne. 5) then
+    call abort()
+  endif
+
+  val = set(1, 100)
+  if (val .ne. 10) then
+    call abort()
+  endif
+
+end
-- 
1.7.1