Free memory used by optimization/target options

Hi,
this fixes memory leak for xstrduped strings in option summaries which may
get ggc_freed by tree merging.

Bootstrapped/regtested x86_64-linux, OK?

Honza

	* optc-save-gen.awk: Generate cl_target_option_free
	and cl_optimization_option_free.
	* opth-en.awk: Declare cl_target_option_free
	and cl_optimization_option_free.
	* tree.c (free_node): Use it.

On Tue, 5 Nov 2019, Jan Hubicka wrote:

> Hi,
> this fixes memory leak for xstrduped strings in option summaries which may
> get ggc_freed by tree merging.
> 
> Bootstrapped/regtested x86_64-linux, OK?

OK.  If those are ever reaped by regular GC the memory still leaks, no?
So wouldn't it be better to put the strings into GC memory?

Thanks,
Richard.

> Honza
> 
> 	* optc-save-gen.awk: Generate cl_target_option_free
> 	and cl_optimization_option_free.
> 	* opth-en.awk: Declare cl_target_option_free
> 	and cl_optimization_option_free.
> 	* tree.c (free_node): Use it.
> Index: optc-save-gen.awk
> ===================================================================
> --- optc-save-gen.awk	(revision 277796)
> +++ optc-save-gen.awk	(working copy)
> @@ -802,6 +802,17 @@ for (i = 0; i < n_target_val; i++) {
>  
>  print "}";
>  
> +print "/* free heap memory used by target options  */";
> +print "void";
> +print "cl_target_option_free (struct cl_target_option *ptr ATTRIBUTE_UNUSED)";
> +print "{";
> +for (i = 0; i < n_target_str; i++) {
> +	name = var_target_str[i]
> +	print "  if (ptr->" name")";
> +	print "    free (const_cast <char *>(ptr->" name"));";
> +}
> +print "}";
> +
>  n_opt_val = 4;
>  var_opt_val[0] = "x_optimize"
>  var_opt_val_type[0] = "char "
> @@ -921,4 +932,18 @@ for (i = 0; i < n_opt_val; i++) {
>  	      print "  ptr->" name" = (" var_opt_val_type[i] ") bp_unpack_value (bp, 64);";
>  }
>  print "}";
> +print "/* Free heap memory used by optimization options  */";
> +print "void";
> +print "cl_optimization_option_free (struct cl_optimization *ptr ATTRIBUTE_UNUSED)";
> +print "{";
> +for (i = 0; i < n_opt_val; i++) {
> +	name = var_opt_val[i]
> +	otype = var_opt_val_type[i];
> +	if (otype ~ "^const char \\**$")
> +	{
> +	      print "  if (ptr->" name")";
> +	      print "    free (const_cast <char *>(ptr->" name"));";
> +	}
> +}
> +print "}";
>  }
> Index: opth-gen.awk
> ===================================================================
> --- opth-gen.awk	(revision 277796)
> +++ opth-gen.awk	(working copy)
> @@ -303,6 +303,9 @@ print "";
>  print "/* Compare two target option variables from a structure.  */";
>  print "extern bool cl_target_option_eq (const struct cl_target_option *, const struct cl_target_option *);";
>  print "";
> +print "/* Free heap memory used by target option variables.  */";
> +print "extern void cl_target_option_free (struct cl_target_option *);";
> +print "";
>  print "/* Hash option variables from a structure.  */";
>  print "extern hashval_t cl_target_option_hash (const struct cl_target_option *);";
>  print "";
> @@ -312,6 +315,9 @@ print "";
>  print "/* Compare two optimization options.  */";
>  print "extern bool cl_optimization_option_eq (cl_optimization const *ptr1, cl_optimization const *ptr2);"
>  print "";
> +print "/* Free heap memory used by optimization options.  */";
> +print "extern void cl_optimization_option_free (cl_optimization *ptr1);"
> +print "";
>  print "/* Generator files may not have access to location_t, and don't need these.  */"
>  print "#if defined(UNKNOWN_LOCATION)"
>  print "bool                                                                  "
> Index: tree.c
> ===================================================================
> --- tree.c	(revision 277796)
> +++ tree.c	(working copy)
> @@ -1170,6 +1170,10 @@ free_node (tree node)
>      vec_free (BLOCK_NONLOCALIZED_VARS (node));
>    else if (code == TREE_BINFO)
>      vec_free (BINFO_BASE_ACCESSES (node));
> +  else if (code == OPTIMIZATION_NODE)
> +    cl_optimization_option_free (TREE_OPTIMIZATION (node));
> +  else if (code == TARGET_OPTION_NODE)
> +    cl_target_option_free (TREE_TARGET_OPTION (node));
>    ggc_free (node);
>  }
>  
>

> On Tue, 5 Nov 2019, Jan Hubicka wrote:
> 
> > Hi,
> > this fixes memory leak for xstrduped strings in option summaries which may
> > get ggc_freed by tree merging.
> > 
> > Bootstrapped/regtested x86_64-linux, OK?
> 
> OK.  If those are ever reaped by regular GC the memory still leaks, no?
> So wouldn't it be better to put the strings into GC memory?
I am not sure why those are malloced, but I think there was some fun
with gengtype/awk generator interactions. Also I think all those option
nodes come into cl_option_hash that is fully permanent.

Honza

On 11/5/19 11:40 AM, Jan Hubicka wrote:
> +	print "  if (ptr->" name")";
> +	print "    free (const_cast <char *>(ptr->" name"));";

If I'm correct, you can call free even for a NULL pointer.

Martin

On 11/6/19 2:22 AM, Martin Liška wrote:
> On 11/5/19 11:40 AM, Jan Hubicka wrote:
>> +    print "  if (ptr->" name")";
>> +    print "    free (const_cast <char *>(ptr->" name"));";
> 
> If I'm correct, you can call free even for a NULL pointer.
You can and I think we expunged all the NULL tests before calling free
years ago.

jeff