Patchwork opts.c, gcc.c: Plug some memory leaks - and an out-of-bounds memory access

login
register
mail settings
Submitter Tobias Burnus
Date Oct. 3, 2012, 9:01 p.m.
Message ID <506CA7C7.2050800@net-b.de>
Download mbox | patch
Permalink /patch/188924/
State New
Headers show

Comments

Tobias Burnus - Oct. 3, 2012, 9:01 p.m.
Found using http://scan5.coverity.com/

Build on x86-64-gnu-linux with C/C++/Fortran. I will now do an 
all-language build/regtest.
OK when it passes?

(Note to the save_string call: I reduced it by 2: The "+1" in the call 
makes it long (out of bounds) and the "+1" in temp_filename_length is 
not needed (but also doesn't harm) as "tmp" is null terminated and 
save_string adds another '\0' after copying "len" bytes.)

Tobias
Richard Guenther - Oct. 4, 2012, 8:20 a.m.
On Wed, Oct 3, 2012 at 11:01 PM, Tobias Burnus <burnus@net-b.de> wrote:
> Found using http://scan5.coverity.com/
>
> Build on x86-64-gnu-linux with C/C++/Fortran. I will now do an all-language
> build/regtest.
> OK when it passes?
>
> (Note to the save_string call: I reduced it by 2: The "+1" in the call makes
> it long (out of bounds) and the "+1" in temp_filename_length is not needed
> (but also doesn't harm) as "tmp" is null terminated and save_string adds
> another '\0' after copying "len" bytes.)

-	  prefix = concat (target_sysroot_suffix, prefix, NULL);
-      prefix = concat (sysroot_no_trailing_dir_separator, prefix, NULL);
+	{
+	  char *tmp;
+	  tmp = concat (target_sysroot_suffix, prefix, NULL);
+	  prefix = concat (sysroot_no_trailing_dir_separator, tmp, NULL);
+	  free (tmp);
+	}

    prefix = concat (sysroot_no_trailing_dir_separator,
target_sysroot_suffix, prefix, NULL);

should be equivalent and easier to read, no?

+      else
+	prefix = concat (sysroot_no_trailing_dir_separator, prefix, NULL);
+

btw, we're not careing too much about memleaks in the driver ...

Otherwise the patch looks ok with the above change.

Thanks,
Richard.

> Tobias

Patch

2012-10-03  Tobias Burnus  <burnus@net-b.de>

	* gcc.c (record_temp_file, add_sysrooted_prefix, process_command,
	do_self_spec, compare_debug_dump_opt_spec_function): Plug memleaks.
	(do_spec_1): Ditto, fix out-of-bound access.
	* opts.c (common_handle_option): Plug memleak.

diff --git a/gcc/gcc.c b/gcc/gcc.c
index af3c34a..2d56d40 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -1981,21 +1981,24 @@  record_temp_file (const char *filename, int always_delete, int fail_delete)
       always_delete_queue = temp;
 
     already1:;
     }
 
   if (fail_delete)
     {
       struct temp_file *temp;
       for (temp = failure_delete_queue; temp; temp = temp->next)
 	if (! filename_cmp (name, temp->name))
-	  goto already2;
+	  {
+	    free (name);
+	    goto already2;
+	  }
 
       temp = XNEW (struct temp_file);
       temp->next = failure_delete_queue;
       temp->name = name;
       failure_delete_queue = temp;
 
     already2:;
     }
 }
 
@@ -2455,22 +2458,29 @@  add_sysrooted_prefix (struct path_prefix *pprefix, const char *prefix,
   if (target_system_root)
     {
       char *sysroot_no_trailing_dir_separator = xstrdup (target_system_root);
       size_t sysroot_len = strlen (target_system_root);
 
       if (sysroot_len > 0
 	  && target_system_root[sysroot_len - 1] == DIR_SEPARATOR)
 	sysroot_no_trailing_dir_separator[sysroot_len - 1] = '\0';
 
       if (target_sysroot_suffix)
-	  prefix = concat (target_sysroot_suffix, prefix, NULL);
-      prefix = concat (sysroot_no_trailing_dir_separator, prefix, NULL);
+	{
+	  char *tmp;
+	  tmp = concat (target_sysroot_suffix, prefix, NULL);
+	  prefix = concat (sysroot_no_trailing_dir_separator, tmp, NULL);
+	  free (tmp);
+	}
+      else
+	prefix = concat (sysroot_no_trailing_dir_separator, prefix, NULL);
+
       free (sysroot_no_trailing_dir_separator);
 
       /* We have to override this because GCC's notion of sysroot
 	 moves along with GCC.  */
       component = "GCC";
     }
 
   add_prefix (pprefix, prefix, component, priority,
 	      require_machine_suffix, os_multilib);
 }
@@ -3564,21 +3574,21 @@  set_option_handlers (struct cl_option_handlers *handlers)
 
 /* Create the vector `switches' and its contents.
    Store its length in `n_switches'.  */
 
 static void
 process_command (unsigned int decoded_options_count,
 		 struct cl_decoded_option *decoded_options)
 {
   const char *temp;
   char *temp1;
-  const char *tooldir_prefix;
+  char *tooldir_prefix, *tooldir_prefix2;
   char *(*get_relative_prefix) (const char *, const char *,
 				const char *) = NULL;
   struct cl_option_handlers handlers;
   unsigned int j;
 
   gcc_exec_prefix = getenv ("GCC_EXEC_PREFIX");
 
   n_switches = 0;
   n_infiles = 0;
   added_libraries = 0;
@@ -3913,36 +3923,38 @@  process_command (unsigned int decoded_options_count,
       add_prefix (&exec_prefixes, standard_libexec_prefix, "BINUTILS",
 		  PREFIX_PRIORITY_LAST, 2, 0);
       add_prefix (&exec_prefixes, standard_exec_prefix, "BINUTILS",
 		  PREFIX_PRIORITY_LAST, 2, 0);
 #endif
       add_prefix (&startfile_prefixes, standard_exec_prefix, "BINUTILS",
 		  PREFIX_PRIORITY_LAST, 1, 0);
     }
 
   gcc_assert (!IS_ABSOLUTE_PATH (tooldir_base_prefix));
-  tooldir_prefix = concat (tooldir_base_prefix, spec_machine,
-			   dir_separator_str, NULL);
+  tooldir_prefix2 = concat (tooldir_base_prefix, spec_machine,
+			    dir_separator_str, NULL);
 
   /* Look for tools relative to the location from which the driver is
      running, or, if that is not available, the configured prefix.  */
   tooldir_prefix
     = concat (gcc_exec_prefix ? gcc_exec_prefix : standard_exec_prefix,
 	      spec_machine, dir_separator_str,
-	      spec_version, dir_separator_str, tooldir_prefix, NULL);
+	      spec_version, dir_separator_str, tooldir_prefix2, NULL);
+  free (tooldir_prefix2);
 
   add_prefix (&exec_prefixes,
 	      concat (tooldir_prefix, "bin", dir_separator_str, NULL),
 	      "BINUTILS", PREFIX_PRIORITY_LAST, 0, 0);
   add_prefix (&startfile_prefixes,
 	      concat (tooldir_prefix, "lib", dir_separator_str, NULL),
 	      "BINUTILS", PREFIX_PRIORITY_LAST, 0, 1);
+  free (tooldir_prefix);
 
 #if defined(TARGET_SYSTEM_ROOT_RELOCATABLE) && !defined(VMS)
   /* If the normal TARGET_SYSTEM_ROOT is inside of $exec_prefix,
      then consider it to relocate with the rest of the GCC installation
      if GCC_EXEC_PREFIX is set.
      ``make_relative_prefix'' is not compiled for VMS, so don't call it.  */
   if (target_system_root && !target_system_root_changed && gcc_exec_prefix)
     {
       char *tmp_prefix = get_relative_prefix (decoded_options[0].arg,
 					      standard_bindir_prefix,
@@ -4312,20 +4324,21 @@  do_self_spec (const char *spec)
       argbuf_copy = XNEWVEC (const char *,
 			     VEC_length (const_char_p, argbuf) + 1);
       argbuf_copy[0] = "";
       memcpy (argbuf_copy + 1, VEC_address (const_char_p, argbuf),
 	      VEC_length (const_char_p, argbuf) * sizeof (const char *));
 
       decode_cmdline_options_to_array (VEC_length (const_char_p, argbuf) + 1,
 				       argbuf_copy,
 				       CL_DRIVER, &decoded_options,
 				       &decoded_options_count);
+      free (argbuf_copy);
 
       set_option_handlers (&handlers);
 
       for (j = 1; j < decoded_options_count; j++)
 	{
 	  switch (decoded_options[j].opt_index)
 	    {
 	    case OPT_SPECIAL_input_file:
 	      /* Specs should only generate options, not input
 		 files.  */
@@ -4733,22 +4746,22 @@  do_spec_1 (const char *spec, int inswitch, const char *soft_matched_part)
 		   temp file.  */
 		if (save_temps_length)
 		  {
 		    char *tmp;
 		    temp_filename_length
 		      = save_temps_length + suffix_length + 1;
 		    tmp = (char *) alloca (temp_filename_length);
 		    memcpy (tmp, save_temps_prefix, save_temps_length);
 		    memcpy (tmp + save_temps_length, suffix, suffix_length);
 		    tmp[save_temps_length + suffix_length] = '\0';
-		    temp_filename = save_string (tmp,
-						 temp_filename_length + 1);
+		    temp_filename = save_string (tmp, save_temps_length
+						      + suffix_length);
 		    obstack_grow (&obstack, temp_filename,
 				  temp_filename_length);
 		    arg_going = 1;
 		    delete_this_arg = 0;
 		    break;
 		  }
 
 		/* If the gcc_input_filename has the same suffix specified
 		   for the %g, %u, or %U, and -save-temps is specified,
 		   we could end up using that file as an intermediate
@@ -5048,20 +5061,21 @@  do_spec_1 (const char *spec, int inswitch, const char *soft_matched_part)
 	      /* See if we already recorded this option.  */
 	      FOR_EACH_VEC_ELT (char_p, linker_options, ix, opt)
 		if (! strcmp (string, opt))
 		  {
 		    free (string);
 		    return 0;
 		  }
 
 	      /* This option is new; add it.  */
 	      add_linker_option (string, strlen (string));
+	      free (string);
 	    }
 	    break;
 
 	  /* Dump out the options accumulated previously using %x.  */
 	  case 'X':
 	    do_specs_vec (linker_options);
 	    break;
 
 	  /* Dump out the options accumulated previously using -Wa,.  */
 	  case 'Y':
@@ -8179,21 +8193,21 @@  get_random_number (void)
 }
 
 /* %:compare-debug-dump-opt spec function.  Save the last argument,
    expected to be the last -fdump-final-insns option, or generate a
    temporary.  */
 
 static const char *
 compare_debug_dump_opt_spec_function (int arg,
 				      const char **argv ATTRIBUTE_UNUSED)
 {
-  const char *ret;
+  char *ret;
   char *name;
   int which;
   static char random_seed[HOST_BITS_PER_WIDE_INT / 4 + 3];
 
   if (arg != 0)
     fatal_error ("too many arguments to %%:compare-debug-dump-opt");
 
   do_spec_2 ("%{fdump-final-insns=*:%*}");
   do_spec_1 (" ", 0, NULL);
 
@@ -8233,22 +8247,26 @@  compare_debug_dump_opt_spec_function (int arg,
   debug_check_temp_file[which] = name;
 
   if (!which)
     {
       unsigned HOST_WIDE_INT value = get_random_number ();
 
       sprintf (random_seed, HOST_WIDE_INT_PRINT_HEX, value);
     }
 
   if (*random_seed)
-    ret = concat ("%{!frandom-seed=*:-frandom-seed=", random_seed, "} ",
-		  ret, NULL);
+    {
+      char *tmp = ret;
+      ret = concat ("%{!frandom-seed=*:-frandom-seed=", random_seed, "} ",
+		    ret, NULL);
+      free (tmp);
+    }
 
   if (which)
     *random_seed = 0;
 
   return ret;
 }
 
 static const char *debug_auxbase_opt;
 
 /* %:compare-debug-self-opt spec function.  Expands to the options
diff --git a/gcc/opts.c b/gcc/opts.c
index 8608a56..ccfe3c7 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1468,20 +1468,22 @@  common_handle_option (struct gcc_options *opts,
     case OPT_aux_info:
       opts->x_flag_gen_aux_info = 1;
       break;
 
     case OPT_auxbase_strip:
       {
 	char *tmp = xstrdup (arg);
 	strip_off_ending (tmp, strlen (tmp));
 	if (tmp[0])
 	  opts->x_aux_base_name = tmp;
+	else
+	  free (tmp);
       }
       break;
 
     case OPT_d:
       decode_d_option (arg, opts, loc, dc);
       break;
 
     case OPT_fcall_used_:
     case OPT_fcall_saved_:
       /* Deferred.  */