diff mbox

Options handling and reload memory leak fixes

Message ID 20111110192839.GN27242@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Nov. 10, 2011, 7:28 p.m. UTC 
Hi!

Running valgrind even on simple testcases shows a bunch of
memory leaks (definitely lost).  This patch cures some of them.
There are a few further leaks in the options handling.

The first hunk is when this function already called concat to set
opt_text, and then doesn't write opt_text anywhere, but concat
of that and something else (which malloces a new memory and doesn't
free the old one).

The second hunk is because register_pass_name stores a copy of the
string or does nothing (another alternative would be to store
the passed in name or free it in register_pass_name, but that
would be quite weird API for the function).

The first reload mem leak is because we allocate the array
and then immediately call init_eliminable_invariants (..., true)
which allocates it again.

The second leaks are because init_eliminable_invariants
allocates those two arrays when it is called with either do_subregs
true or false.  In the former case they are freed by free_reg_equiv
called at the end of reload (), but in the latter case nothing freed
them.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2011-11-10  Jakub Jelinek  <jakub@redhat.com>

	* opts-common.c (generate_canonical_option): Free opt_text
	it it has been allocated here and not stored anywhere.

	* passes.c (register_one_dump_file): Free full_name.

	* reload1.c (reload): Don't allocate reg_max_ref_width
	here.
	(calculate_elim_costs_all_insns): Free offsets_at and
	offsets_known_at at the end and clear the pointers.


	Jakub

Comments

Joseph Myers Nov. 11, 2011, 12:42 a.m. UTC | #1 
On Thu, 10 Nov 2011, Jakub Jelinek wrote:

> Hi!
> 
> Running valgrind even on simple testcases shows a bunch of
> memory leaks (definitely lost).  This patch cures some of them.
> There are a few further leaks in the options handling.
> 
> The first hunk is when this function already called concat to set
> opt_text, and then doesn't write opt_text anywhere, but concat
> of that and something else (which malloces a new memory and doesn't
> free the old one).

The option-handling change is OK.  But I suspect eliminating memory leaks 
completely from option handling will require defining various fields, that 
may at present sometimes hold malloced memory and sometimes hold pointers 
into constant data or the original command line, always to hold malloced 
memory (and so require various new allocations that aren't required at 
present) - as without defining that there are probably cases where you 
won't know whether to free the previous value of a field.  (I've generally 
presumed that memory usage that is O(n) in the size of the command line 
isn't significant.)
diff mbox

Patch

--- gcc/opts-common.c.jj	2011-11-04 07:49:45.000000000 +0100
+++ gcc/opts-common.c	2011-11-10 15:35:27.917116296 +0100
@@ -304,6 +304,8 @@  generate_canonical_option (size_t opt_in
 	  decoded->canonical_option[0] = concat (opt_text, arg, NULL);
 	  decoded->canonical_option[1] = NULL;
 	  decoded->canonical_option_num_elements = 1;
+	  if (opt_text != option->opt_text)
+	    free (CONST_CAST (char *, opt_text));
 	}
     }
   else
--- gcc/passes.c.jj	2011-11-08 23:35:12.000000000 +0100
+++ gcc/passes.c	2011-11-10 15:13:34.789021796 +0100
@@ -409,6 +409,7 @@  register_one_dump_file (struct opt_pass 
   set_pass_for_id (id, pass);
   full_name = concat (prefix, pass->name, num, NULL);
   register_pass_name (pass, full_name);
+  free (CONST_CAST (char *, full_name));
 }
 
 /* Recursive worker function for register_dump_files.  */
--- gcc/reload1.c.jj	2011-11-08 23:35:12.000000000 +0100
+++ gcc/reload1.c	2011-11-10 15:31:16.100601192 +0100
@@ -768,7 +768,6 @@  reload (rtx first, int global)
      be substituted eventually by altering the REG-rtx's.  */
 
   grow_reg_equivs ();
-  reg_max_ref_width = XCNEWVEC (unsigned int, max_regno);
   reg_old_renumber = XCNEWVEC (short, max_regno);
   memcpy (reg_old_renumber, reg_renumber, max_regno * sizeof (short));
   pseudo_forbidden_regs = XNEWVEC (HARD_REG_SET, max_regno);
@@ -1688,6 +1687,10 @@  calculate_elim_costs_all_insns (void)
     }
 
   free (reg_equiv_init_cost);
+  free (offsets_known_at);
+  free (offsets_at);
+  offsets_at = NULL;
+  offsets_known_at = NULL;
 }
 
 /* Comparison function for qsort to decide which of two reloads