[Bug fortran/40877] memory leaks with gfc_charlen?
--- Comment #9 from janus at gcc dot gnu dot org 2009-08-17 09:11 --- Subject: Bug 40877 Author: janus Date: Mon Aug 17 09:11:00 2009 New Revision: 150823 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=150823 Log: 2009-08-17 Janus Weil ja...@gcc.gnu.org PR fortran/40877 * array.c (gfc_resolve_character_array_constructor): Add NULL argument to gfc_new_charlen. * decl.c (add_init_expr_to_sym,variable_decl,match_char_spec, gfc_match_implicit): Ditto. * expr.c (simplify_const_ref): Fix memory leak. (gfc_simplify_expr): Add NULL argument to gfc_new_charlen. * gfortran.h (gfc_new_charlen): Modified prototype. * iresolve.c (check_charlen_present,gfc_resolve_char_achar): Add NULL argument to gfc_new_charlen. * module.c (mio_charlen): Ditto. * resolve.c (gfc_resolve_substring_charlen, gfc_resolve_character_operator,fixup_charlen): Ditto. (resolve_fl_derived,resolve_symbol): Add argument to gfc_charlen. * symbol.c (gfc_new_charlen): Add argument 'old_cl' (to make a copy of an existing charlen). (gfc_set_default_type,generate_isocbinding_symbol): Fix memory leak. (gfc_copy_formal_args_intr): Add NULL argument to gfc_new_charlen. * trans-decl.c (create_function_arglist): Fix memory leak. Modified: trunk/gcc/fortran/ChangeLog trunk/gcc/fortran/array.c trunk/gcc/fortran/decl.c trunk/gcc/fortran/expr.c trunk/gcc/fortran/gfortran.h trunk/gcc/fortran/iresolve.c trunk/gcc/fortran/module.c trunk/gcc/fortran/resolve.c trunk/gcc/fortran/symbol.c trunk/gcc/fortran/trans-decl.c -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40877
[Bug fortran/40877] memory leaks with gfc_charlen?
--- Comment #10 from janus at gcc dot gnu dot org 2009-08-17 09:14 --- Fixed with r150823. Closing. -- janus at gcc dot gnu dot org changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution||FIXED http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40877
[Bug fortran/40877] memory leaks with gfc_charlen?
--- Comment #1 from pault at gcc dot gnu dot org 2009-08-05 08:57 --- * expr.c (simplify_const_ref) * symbol.c (gfc_set_default_type, generate_isocbinding_symbol) These two produce leaks. * trans-decl.c (create_function_arglist) This is OK - the new cl is threaded into the list. Thanks for spotting that. Cheers Paul -- pault at gcc dot gnu dot org changed: What|Removed |Added AssignedTo|unassigned at gcc dot gnu |pault at gcc dot gnu dot org |dot org | Status|UNCONFIRMED |ASSIGNED Ever Confirmed|0 |1 Last reconfirmed|-00-00 00:00:00 |2009-08-05 08:57:08 date|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40877
[Bug fortran/40877] memory leaks with gfc_charlen?
--- Comment #2 from janus at gcc dot gnu dot org 2009-08-05 11:27 --- (In reply to comment #1) * trans-decl.c (create_function_arglist) This is OK - the new cl is threaded into the list. Actually I think that this is not done properly. The code in question is: gfc_charlen *cl, *cl2; cl = f-sym-ts.cl; f-sym-ts.cl = gfc_get_charlen(); f-sym-ts.cl-length = cl-length; f-sym-ts.cl-backend_decl = cl-backend_decl; f-sym-ts.cl-length_from_typespec = cl-length_from_typespec; f-sym-ts.cl-resolved = cl-resolved; cl2 = f-sym-ts.cl-next; f-sym-ts.cl-next = cl; cl-next = cl2; First problem: cl2 is always NULL, since f-sym-ts.cl gets assigned a fresh gfc_charlen. Second problem: The new charlen is inserted in front of the old one, and not behind it. Let's say before this code is executed we have a linked list of charlen structures like this: something - cl - ... After the code is executed this will look like: something--- | new_cl - cl - NULL where 'something' still points to 'cl', so 'new_cl' is not reachable from within the linked list (and also cl-next is lost). -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40877
[Bug fortran/40877] memory leaks with gfc_charlen?
--- Comment #3 from janus at gcc dot gnu dot org 2009-08-05 11:44 --- The code from trans-decl.c (create_function_arglist) cited in comment #2 was committed by Tobias as r148517 just a few weeks ago, as a fix for PR 40383. Tobias, do you remember any of your thoughts when writing this? -- janus at gcc dot gnu dot org changed: What|Removed |Added CC||burnus at gcc dot gnu dot ||org http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40877
[Bug fortran/40877] memory leaks with gfc_charlen?
--- Comment #4 from burnus at gcc dot gnu dot org 2009-08-05 12:50 --- (In reply to comment #3) Tobias, do you remember any of your thoughts when writing this? Well, they are essentially written in the patch email (linked from the PR 40383): http://gcc.gnu.org/ml/gcc-patches/2009-06/msg01147.html The test case was: subroutine sub(a,b) character(4), optional :: a, b ! Known compile-time length which translates into: sub (a, b, a_, b_) where a_ and b_ are not used in the function (as the character length is known) except for the bounds checking. In that case one needs to access the TREE (or backend declaration) for a_ and for b_. However, as a and b are the same, they share the same cl - and the same backend type declaration (which is fine). Before Daniel's bounds check, the a_ and b_ TREEs were generated but not anywhere saved, as they are not needed. Daniel's patch simply added a pointer to a TREE to cl which points at that hidden argument. As there is only one cl shared by both a and b, one could only access b_ - that way one would effectively only check one of the arguments (incomplete check). However, if optional arguments are involved, that argument might be missing and b_ is 0, which turns it into a wrong diagnostic for a. One solution would be to create two cl - one for a and one for b. However, that would also create two type declaration TREEs even though a and b have exactly the same type. Already at present the middle end has problems with different TREE types for variables which have the same Fortran type. (Maybe I am wrong and we fixed this meanwhile and gfortran re-uses the TREE, but I do not think so.) -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40877
[Bug fortran/40877] memory leaks with gfc_charlen?
--- Comment #5 from janus at gcc dot gnu dot org 2009-08-05 13:21 --- (In reply to comment #4) (In reply to comment #3) Tobias, do you remember any of your thoughts when writing this? Well, they are essentially written in the patch email (linked from the PR Yeah, ok, the general idea sounds fine. What I meant was that the way in which you copy the cl seems to create a memory leak (cf. comment #2), since the newly created cl is not reachable from any ns-cl_list. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40877
[Bug fortran/40877] memory leaks with gfc_charlen?
--- Comment #8 from janus at gcc dot gnu dot org 2009-08-05 20:25 --- I guess I'll take over then. Got a patch. -- janus at gcc dot gnu dot org changed: What|Removed |Added AssignedTo|unassigned at gcc dot gnu |janus at gcc dot gnu dot org |dot org | Status|NEW |ASSIGNED Last reconfirmed|2009-08-05 08:57:08 |2009-08-05 20:25:46 date|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40877
