Patchwork Fix memory leaks in options handling

login
register
mail settings
Submitter Jakub Jelinek
Date Feb. 26, 2013, 9:21 p.m.
Message ID <20130226212115.GC12913@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/223401/
State New
Headers show

Comments

Jakub Jelinek - Feb. 26, 2013, 9:21 p.m.
Hi!

The memory leaks in the options handling, while they perhaps aren't that
big, keep cluttering valgrind --leak-check=full outputs and make it harder
to find other, more severe, bugs.

The following patch attempts to fix that by allocating strings which we want
to keep allocated forever, and from time to time lose all the pointers
pointing to them, in an obstack.  That could in theory make the allocations
tiny bit faster, but more importantly all the memory is reachable at exit
and thus valgrind doesn't complain about those.

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

2013-02-26  Jakub Jelinek  <jakub@redhat.com>

	* opts.h: Include obstack.h.
	(opts_concat): New prototype.
	(opts_obstack): New declaration.
	* opts.c (opts_concat): New function.
	(opts_obstack): New variable.
	(init_options_struct): Call gcc_init_obstack on opts_obstack.
	(finish_options): Use opts_concat instead of concat
	and XOBNEWVEC instead of XNEWVEC.
	* opts-common.c (generate_canonical_option, decode_cmdline_option,
	generate_option): Likewise.
	* Makefile.in (OPTS_H): Depend on $(OBSTACK_H).
	* lto-wrapper.c (main): Call gcc_init_obstack on opts_obstack.


	Jakub
Joseph S. Myers - Feb. 26, 2013, 11:21 p.m.
On Tue, 26 Feb 2013, Jakub Jelinek wrote:

> The following patch attempts to fix that by allocating strings which we want
> to keep allocated forever, and from time to time lose all the pointers
> pointing to them, in an obstack.  That could in theory make the allocations
> tiny bit faster, but more importantly all the memory is reachable at exit
> and thus valgrind doesn't complain about those.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.  (The idea behind not freeing data here, apart from the total amount 
being at most a constant times the size of the command line, is that to 
free data you'd then need to keep track of what was allocated and what was 
pointing into argv, or strdup everywhere a pointer into argv is used, and 
probably wouldn't actually end up saving any memory.)

Patch

--- gcc/opts.h.jj	2013-01-11 09:02:48.000000000 +0100
+++ gcc/opts.h	2013-02-26 14:08:32.956457725 +0100
@@ -22,6 +22,7 @@  along with GCC; see the file COPYING3.
 
 #include "input.h"
 #include "vec.h"
+#include "obstack.h"
 
 /* Specifies how a switch's VAR_VALUE relates to its FLAG_VAR.  */
 enum cl_var_type {
@@ -304,6 +305,12 @@  extern const char **in_fnames;
 
 extern unsigned num_in_fnames;
 
+extern char *opts_concat (const char *first, ...);
+
+/* Obstack for option strings.  */
+
+extern struct obstack opts_obstack;
+
 size_t find_opt (const char *input, unsigned int lang_mask);
 extern int integral_argument (const char *arg);
 extern bool enum_value_to_arg (const struct cl_enum_arg *enum_args,
--- gcc/opts.c.jj	2013-02-15 16:30:01.000000000 +0100
+++ gcc/opts.c	2013-02-26 14:13:14.939802328 +0100
@@ -268,6 +268,40 @@  add_comma_separated_to_vector (void **pv
   *pvec = v;
 }
 
+/* Like libiberty concat, but allocate using opts_obstack.  */
+
+char *
+opts_concat (const char *first, ...)
+{
+  char *newstr, *end;
+  size_t length = 0;
+  const char *arg;
+  va_list ap;
+
+  /* First compute the size of the result and get sufficient memory.  */
+  va_start (ap, first);
+  for (arg = first; arg; arg = va_arg (ap, const char *))
+    length += strlen (arg);
+  newstr = XOBNEWVEC (&opts_obstack, char, length + 1);
+  va_end (ap);
+
+  /* Now copy the individual pieces to the result string. */
+  va_start (ap, first);
+  for (arg = first, end = newstr; arg; arg = va_arg (ap, const char *))
+    {
+      length = strlen (arg);
+      memcpy (end, arg, length);
+      end += length;
+    }
+  *end = '\0';
+  va_end (ap);
+  return newstr;
+}
+
+/* Obstack for option strings.  */
+
+struct obstack opts_obstack;
+
 /* Initialize OPTS and OPTS_SET before using them in parsing options.  */
 
 void
@@ -275,6 +309,8 @@  init_options_struct (struct gcc_options
 {
   size_t num_params = get_num_compiler_params ();
 
+  gcc_obstack_init (&opts_obstack);
+
   *opts = global_options_init;
   memset (opts_set, 0, sizeof (*opts_set));
 
@@ -638,8 +674,8 @@  finish_options (struct gcc_options *opts
 	 directory, typically the directory to contain the object
 	 file.  */
       if (opts->x_dump_dir_name)
-	opts->x_dump_base_name = concat (opts->x_dump_dir_name,
-					 opts->x_dump_base_name, NULL);
+	opts->x_dump_base_name = opts_concat (opts->x_dump_dir_name,
+					      opts->x_dump_base_name, NULL);
       else if (opts->x_aux_base_name
 	       && strcmp (opts->x_aux_base_name, HOST_BIT_BUCKET) != 0)
 	{
@@ -649,8 +685,9 @@  finish_options (struct gcc_options *opts
 	  if (opts->x_aux_base_name != aux_base)
 	    {
 	      int dir_len = aux_base - opts->x_aux_base_name;
-	      char *new_dump_base_name =
-		XNEWVEC (char, strlen (opts->x_dump_base_name) + dir_len + 1);
+	      char *new_dump_base_name
+		= XOBNEWVEC (&opts_obstack, char,
+			     strlen (opts->x_dump_base_name) + dir_len + 1);
 
 	      /* Copy directory component from OPTS->X_AUX_BASE_NAME.  */
 	      memcpy (new_dump_base_name, opts->x_aux_base_name, dir_len);
--- gcc/opts-common.c.jj	2013-01-11 09:02:28.000000000 +0100
+++ gcc/opts-common.c	2013-02-26 14:11:14.834505092 +0100
@@ -276,7 +276,7 @@  generate_canonical_option (size_t opt_in
       && !option->cl_reject_negative
       && (opt_text[1] == 'W' || opt_text[1] == 'f' || opt_text[1] == 'm'))
     {
-      char *t = XNEWVEC (char, option->opt_len + 5);
+      char *t = XOBNEWVEC (&opts_obstack, char, option->opt_len + 5);
       t[0] = '-';
       t[1] = opt_text[1];
       t[2] = 'n';
@@ -301,11 +301,9 @@  generate_canonical_option (size_t opt_in
       else
 	{
 	  gcc_assert (option->flags & CL_JOINED);
-	  decoded->canonical_option[0] = concat (opt_text, arg, NULL);
+	  decoded->canonical_option[0] = opts_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
@@ -590,7 +588,7 @@  decode_cmdline_option (const char **argv
     {
       size_t j;
       size_t len = strlen (arg);
-      char *arg_lower = XNEWVEC (char, len + 1);
+      char *arg_lower = XOBNEWVEC (&opts_obstack, char, len + 1);
 
       for (j = 0; j < len; j++)
 	arg_lower[j] = TOLOWER ((unsigned char) arg[j]);
@@ -670,7 +668,8 @@  decode_cmdline_option (const char **argv
 	  decoded->canonical_option_num_elements = result;
 	}
     }
-  decoded->orig_option_with_args_text = p = XNEWVEC (char, total_len);
+  decoded->orig_option_with_args_text
+    = p = XOBNEWVEC (&opts_obstack, char, total_len);
   for (i = 0; i < result; i++)
     {
       size_t len = strlen (argv[i]);
@@ -932,8 +931,8 @@  generate_option (size_t opt_index, const
 
     case 2:
       decoded->orig_option_with_args_text
-	= concat (decoded->canonical_option[0], " ",
-		  decoded->canonical_option[1], NULL);
+	= opts_concat (decoded->canonical_option[0], " ",
+		       decoded->canonical_option[1], NULL);
       break;
 
     default:
--- gcc/Makefile.in.jj	2013-02-20 18:40:49.000000000 +0100
+++ gcc/Makefile.in	2013-02-20 18:40:49.000000000 +0100
@@ -919,7 +919,7 @@  PREDICT_H = predict.h predict.def
 CPPLIB_H = $(srcdir)/../libcpp/include/line-map.h \
 	$(srcdir)/../libcpp/include/cpplib.h
 INPUT_H = $(srcdir)/../libcpp/include/line-map.h input.h
-OPTS_H = $(INPUT_H) $(VEC_H) opts.h
+OPTS_H = $(INPUT_H) $(VEC_H) opts.h $(OBSTACK_H)
 DECNUM_H = $(DECNUM)/decContext.h $(DECNUM)/decDPD.h $(DECNUM)/decNumber.h \
 	$(DECNUMFMT)/decimal32.h $(DECNUMFMT)/decimal64.h \
 	$(DECNUMFMT)/decimal128.h $(DECNUMFMT)/decimal128Local.h
--- gcc/lto-wrapper.c.jj	2013-01-11 16:15:52.000000000 +0100
+++ gcc/lto-wrapper.c	2013-02-26 20:12:14.339926950 +0100
@@ -915,6 +915,8 @@  main (int argc, char *argv[])
 {
   const char *p;
 
+  gcc_obstack_init (&opts_obstack);
+
   p = argv[0] + strlen (argv[0]);
   while (p != argv[0] && !IS_DIR_SEPARATOR (p[-1]))
     --p;