Patchwork [fortran] PR 42051 : Access to freed symbols.

login
register
mail settings
Submitter Mikael Morin
Date July 28, 2010, 10:06 p.m.
Message ID <4C50A9FC.7040002@sfr.fr>
Download mbox | patch
Permalink /patch/60179/
State New
Headers show

Comments

Mikael Morin - July 28, 2010, 10:06 p.m.
Hello,

This patch fixes the PRs 42051 & 44064, where symbols were freed to soon.
As explained in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42051#c13,
gfc_find_derived_vtab was creating new symbols but was not committing 
them, so that they were removed at the next gfc_undo_symbols call.
The fix is obvious: commit the symbols.

The testcase provided was needing valgrind to show the error (no 
segfault) with me, but I add it anyway, just in case.

Regression testing in progress. OK for trunk when done ?
Mikael
2010-07-28  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/42051
	PR fortran/44064
	* class.c (gfc_find_derived_vtab): Accept or discard newly created
	symbols before returning.

! { dg-do compile }
! { dg-options "-fno-whole-file" }
!
! PR fortran/42051
! PR fortran/44064
! Access to freed symbols
!
! Testcase provided by Damian Rouson <damian@rouson.net>,
! reduced by Janus Weil <janus@gcc.gnu.org>.

module grid_module
  implicit none 
  type grid
  end type
  type field
    type(grid) :: mesh
  end type
contains
  real function return_x(this)
    class(grid) :: this
  end function
end module 

module field_module
  use grid_module, only: field,return_x
  implicit none 
contains
  subroutine output(this)
    class(field) :: this
    print *,return_x(this%mesh)
  end subroutine
end module

end

! { dg-final { cleanup-modules "grid_module field_module" } }
Tobias Burnus - July 29, 2010, 6:29 a.m.
Mikael Morin wrote:
> This patch fixes the PRs 42051 & 44064, where symbols were freed to soon.
> As explained in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42051#c13,
> gfc_find_derived_vtab was creating new symbols but was not committing
> them, so that they were removed at the next gfc_undo_symbols call.
> The fix is obvious: commit the symbols.
>
> Regression testing in progress. OK for trunk when done ?

OK. Thanks!

Tobias
Mikael Morin - July 29, 2010, 11:23 a.m.
Le 29.07.2010 08:29, Tobias Burnus a écrit :
>
> Mikael Morin wrote:
>> This patch fixes the PRs 42051&  44064, where symbols were freed to soon.
>> As explained in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42051#c13,
>> gfc_find_derived_vtab was creating new symbols but was not committing
>> them, so that they were removed at the next gfc_undo_symbols call.
>> The fix is obvious: commit the symbols.
>>
>> Regression testing in progress. OK for trunk when done ?
>
> OK. Thanks!
>
> Tobias
>
>

Sending        gcc/fortran/ChangeLog
Sending        gcc/fortran/class.c
Sending        gcc/testsuite/ChangeLog
Adding         gcc/testsuite/gfortran.dg/pr42051.f03
Transmitting file data ....
Committed revision 162674.

Thanks for looking.

Mikael

Patch

Index: class.c
===================================================================
--- class.c	(revision 162648)
+++ class.c	(working copy)
@@ -321,7 +321,7 @@  gfc_symbol *
 gfc_find_derived_vtab (gfc_symbol *derived)
 {
   gfc_namespace *ns;
-  gfc_symbol *vtab = NULL, *vtype = NULL;
+  gfc_symbol *vtab = NULL, *vtype = NULL, *found_sym = NULL;
   char name[2 * GFC_MAX_SYMBOL_LEN + 8];
 
   ns = gfc_current_ns;
@@ -356,13 +356,13 @@  gfc_find_derived_vtab (gfc_symbol *derived)
 	      gfc_get_symbol (name, ns, &vtype);
 	      if (gfc_add_flavor (&vtype->attr, FL_DERIVED,
 				  NULL, &gfc_current_locus) == FAILURE)
-		return NULL;
+		goto cleanup;
 	      vtype->refs++;
 	      gfc_set_sym_referenced (vtype);
 
 	      /* Add component '$hash'.  */
 	      if (gfc_add_component (vtype, "$hash", &c) == FAILURE)
-		return NULL;
+		goto cleanup;
 	      c->ts.type = BT_INTEGER;
 	      c->ts.kind = 4;
 	      c->attr.access = ACCESS_PRIVATE;
@@ -371,7 +371,7 @@  gfc_find_derived_vtab (gfc_symbol *derived)
 
 	      /* Add component '$size'.  */
 	      if (gfc_add_component (vtype, "$size", &c) == FAILURE)
-		return NULL;
+		goto cleanup;
 	      c->ts.type = BT_INTEGER;
 	      c->ts.kind = 4;
 	      c->attr.access = ACCESS_PRIVATE;
@@ -384,7 +384,7 @@  gfc_find_derived_vtab (gfc_symbol *derived)
 
 	      /* Add component $extends.  */
 	      if (gfc_add_component (vtype, "$extends", &c) == FAILURE)
-		return NULL;
+		goto cleanup;
 	      c->attr.pointer = 1;
 	      c->attr.access = ACCESS_PRIVATE;
 	      parent = gfc_get_derived_super_type (derived);
@@ -414,7 +414,17 @@  gfc_find_derived_vtab (gfc_symbol *derived)
 	}
     }
 
-  return vtab;
+  found_sym = vtab;
+
+cleanup:
+  /* It is unexpected to have some symbols added at resolution or code
+     generation time. We commit the changes in order to keep a clean state.  */
+  if (found_sym)
+    gfc_commit_symbols ();
+  else
+    gfc_undo_symbols ();
+
+  return found_sym;
 }