public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc(refs/vendors/redhat/heads/gcc-8-branch)] When avoiding double deallocation, look at namespace, expression and component.
@ 2020-09-17 17:07 Jakub Jelinek
  0 siblings, 0 replies; only message in thread
From: Jakub Jelinek @ 2020-09-17 17:07 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:4469ee5f22ef656bf99f2e2d8a0e50e5ecff9318

commit 4469ee5f22ef656bf99f2e2d8a0e50e5ecff9318
Author: Thomas Koenig <tkoenig@gcc.gnu.org>
Date:   Sun Jun 14 13:54:19 2020 +0200

    When avoiding double deallocation, look at namespace, expression and component.
    
    Our finalization handling is a mess.  Really, we should get to try and get
    this fixed for gcc 11.
    
    In the meantime, here is a patch which fixes a regression I introduced
    when fixing a regression with a memory leak.  The important thing
    here is to realize that we do not need to finalize (and deallocate)
    multiple times for the same expression and the same component
    in the same namespace.  It might cause code size regressions, but
    better big code than wrong code...
    
    Backported from r11-1296-g1af22e455584ef5fcad2b4474c1efc3fd26f6cb3 .
    
    gcc/fortran/ChangeLog:
    
            PR fortran/94109
            * class.c (finalize_component): Return early if finalization has
            already happened for expression and component within namespace.
            * gfortran.h (gfc_was_finalized): New type.
            (gfc_namespace): Add member was_finalzed.
            (gfc_expr): Remove finalized.
            * symbol.c (gfc_free_namespace): Free was_finalized.
    
    gcc/testsuite/ChangeLog:
    
            PR fortran/94109
            * gfortran.dg/finalize_34.f90: Adjust free counts.
            * gfortran.dg/finalize_36.f90: New test.

Diff:
---
 gcc/fortran/class.c                       | 18 +++++++++++---
 gcc/fortran/gfortran.h                    | 18 +++++++++++---
 gcc/fortran/symbol.c                      | 12 ++++++++++
 gcc/testsuite/gfortran.dg/finalize_34.f90 |  2 +-
 gcc/testsuite/gfortran.dg/finalize_36.f90 | 39 +++++++++++++++++++++++++++++++
 5 files changed, 82 insertions(+), 7 deletions(-)

diff --git a/gcc/fortran/class.c b/gcc/fortran/class.c
index 90563cac7b0..8d7ea836800 100644
--- a/gcc/fortran/class.c
+++ b/gcc/fortran/class.c
@@ -903,12 +903,18 @@ finalize_component (gfc_expr *expr, gfc_symbol *derived, gfc_component *comp,
 {
   gfc_expr *e;
   gfc_ref *ref;
+  gfc_was_finalized *f;
 
   if (!comp_is_finalizable (comp))
     return;
 
-  if (expr->finalized)
-    return;
+  /* If this expression with this component has been finalized
+     already in this namespace, there is nothing to do.  */
+  for (f = sub_ns->was_finalized; f; f = f->next)
+    {
+      if (f->e == expr && f->c == comp)
+	return;
+    }
 
   e = gfc_copy_expr (expr);
   if (!e->ref)
@@ -1038,7 +1044,13 @@ finalize_component (gfc_expr *expr, gfc_symbol *derived, gfc_component *comp,
 			    sub_ns);
       gfc_free_expr (e);
     }
-  expr->finalized = 1;
+
+  /* Record that this was finalized already in this namespace.  */
+  f = sub_ns->was_finalized;
+  sub_ns->was_finalized = XCNEW (gfc_was_finalized);
+  sub_ns->was_finalized->e = expr;
+  sub_ns->was_finalized->c = comp;
+  sub_ns->was_finalized->next = f;
 }
 
 
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index d9ba843b096..8256bdc61a3 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -1739,6 +1739,16 @@ gfc_oacc_routine_name;
 
 #define gfc_get_oacc_routine_name() XCNEW (gfc_oacc_routine_name)
 
+/* Node in linked list to see what has already been finalized
+   earlier.  */
+
+typedef struct gfc_was_finalized {
+  gfc_expr *e;
+  gfc_component *c;
+  struct gfc_was_finalized *next;
+}
+gfc_was_finalized;
+
 /* A namespace describes the contents of procedure, module, interface block
    or BLOCK construct.  */
 /* ??? Anything else use these?  */
@@ -1831,6 +1841,11 @@ typedef struct gfc_namespace
   /* Linked list of !$omp declare simd constructs.  */
   struct gfc_omp_declare_simd *omp_declare_simd;
 
+  /* A hash set for the the gfc expressions that have already
+     been finalized in this namespace.  */
+
+  gfc_was_finalized *was_finalized;
+
   /* Set to 1 if namespace is a BLOCK DATA program unit.  */
   unsigned is_block_data:1;
 
@@ -2155,9 +2170,6 @@ typedef struct gfc_expr
 
   unsigned int do_not_warn : 1;
 
-  /* Set this if the expression has already been finalized.  */
-  unsigned int finalized : 1;
-
   /* If an expression comes from a Hollerith constant or compile-time
      evaluation of a transfer statement, it may have a prescribed target-
      memory representation, and these cannot always be backformed from
diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index e07b48ab082..b63b625eb70 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -4059,6 +4059,7 @@ gfc_free_namespace (gfc_namespace *ns)
 {
   gfc_namespace *p, *q;
   int i;
+  gfc_was_finalized *f;
 
   if (ns == NULL)
     return;
@@ -4091,6 +4092,17 @@ gfc_free_namespace (gfc_namespace *ns)
     gfc_free_interface (ns->op[i]);
 
   gfc_free_data (ns->data);
+
+  /* Free all the expr + component combinations that have been
+     finalized.  */
+  f = ns->was_finalized;
+  while (f)
+    {
+      gfc_was_finalized* current = f;
+      f = f->next;
+      free (current);
+    }
+
   p = ns->contained;
   free (ns);
 
diff --git a/gcc/testsuite/gfortran.dg/finalize_34.f90 b/gcc/testsuite/gfortran.dg/finalize_34.f90
index fef7dac6d89..8fb801d4235 100644
--- a/gcc/testsuite/gfortran.dg/finalize_34.f90
+++ b/gcc/testsuite/gfortran.dg/finalize_34.f90
@@ -22,4 +22,4 @@ program main
   use testmodule
   type(evtlist_type), dimension(10) :: a
 end program main
-! { dg-final  { scan-tree-dump-times "__builtin_free" 12 "original" } }
+! { dg-final  { scan-tree-dump-times "__builtin_free" 24 "original" } }
diff --git a/gcc/testsuite/gfortran.dg/finalize_36.f90 b/gcc/testsuite/gfortran.dg/finalize_36.f90
new file mode 100644
index 00000000000..432f5472aeb
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/finalize_36.f90
@@ -0,0 +1,39 @@
+! { dg-do run }
+! { dg-additional-options "-fdump-tree-original" }
+! PR 94109
+! This used to leak memory.  Test case by Antony Lewis.
+    module debug
+    implicit none
+
+    Type Tester
+        real, dimension(:), allocatable :: Dat, Dat2
+    end Type
+
+    Type TestType2
+        Type(Tester) :: T
+    end type TestType2
+
+    contains
+
+    subroutine Leaker
+    class(TestType2), pointer :: ActiveState
+    Type(Tester) :: Temp
+
+    allocate(Temp%Dat2(10000))
+
+    allocate(TestType2::ActiveState)
+    ActiveState%T = Temp
+    deallocate(ActiveState)
+
+    end subroutine
+
+    end module
+
+
+    program run
+    use debug
+
+    call Leaker()
+
+    end program
+! { dg-final { scan-tree-dump-times "__builtin_free\\ \\(ptr2" 4 "original" } }


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2020-09-17 17:07 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-17 17:07 [gcc(refs/vendors/redhat/heads/gcc-8-branch)] When avoiding double deallocation, look at namespace, expression and component Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).