[Bug ipa/97346] IPA reference reference_vars_to_consider leaks

2021-02-14 Thread rguenth at gcc dot gnu.org via Gcc-bugs 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97346

Richard Biener  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #10 from Richard Biener  ---
Fixed.

[Bug ipa/97346] IPA reference reference_vars_to_consider leaks

2021-02-14 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97346

--- Comment #9 from CVS Commits  ---
The master branch has been updated by Jan Hubicka :

https://gcc.gnu.org/g:9966699d7a9d8e35c0c4cf9a945bcf90ef874f2d

commit r11-7240-g9966699d7a9d8e35c0c4cf9a945bcf90ef874f2d
Author: Jan Hubicka 
Date:   Sun Feb 14 23:24:44 2021 +0100

Fix memory leak in ipa-refernece

2021-02-14  Jan Hubicka  
Richard Biener  

PR ipa/97346
* ipa-reference.c (ipa_init): Only conditinally initialize
reference_vars_to_consider.
(propagate): Conditionally deninitialize
reference_vars_to_consider.
(ipa_reference_write_optimization_summary): Sanity check that
reference_vars_to_consider is not allocated.

[Bug ipa/97346] IPA reference reference_vars_to_consider leaks

2021-02-10 Thread rguenth at gcc dot gnu.org via Gcc-bugs 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97346

--- Comment #8 from Richard Biener  ---
(In reply to Jan Hubicka from comment #7)
> I tested yesterday this one (which makes the lifetime bit more explicit
> - during propagation it is for dumps only). Sorry for not posting it
> earlier. I just wanted to double check tha tleak is gone.
> 
> diff --git a/gcc/ipa-reference.c b/gcc/ipa-reference.c
> index 2ea2a6d5327..84c018ff57c 100644
> --- a/gcc/ipa-reference.c
> +++ b/gcc/ipa-reference.c
> @@ -458,8 +458,8 @@ ipa_init (void)
>  
>ipa_init_p = true;
>  
> -  vec_alloc (reference_vars_to_consider, 10);
> -
> +  if (dump_file)
> +vec_alloc (reference_vars_to_consider, 10);
>  
>if (ipa_ref_opt_sum_summaries != NULL)
>  {
> @@ -967,8 +967,12 @@ propagate (void)
>  }
>  
>if (dump_file)
> -vec_free (reference_vars_to_consider);
> -  reference_vars_to_consider = NULL;
> +{
> +  vec_free (reference_vars_to_consider);
> +  reference_vars_to_consider = NULL;
> +}
> +  else
> +gcc_checking_assert (!reference_vars_to_consider);
>return remove_p ? TODO_remove_functions : 0;
>  }
>  
> @@ -1059,6 +1063,7 @@ ipa_reference_write_optimization_summary (void)
>auto_bitmap ltrans_statics;
>int i;
>  
> +  gcc_checking_assert (!reference_vars_to_consider);
>vec_alloc (reference_vars_to_consider, ipa_reference_vars_uids);
>reference_vars_to_consider->safe_grow (ipa_reference_vars_uids, true);
>  
> @@ -1117,7 +1122,7 @@ ipa_reference_write_optimization_summary (void)
> }
>}
>lto_destroy_simple_output_block (ob);
> -  delete reference_vars_to_consider;
> +  vec_free (reference_vars_to_consider);

maybe set reference_vars_to_consider to NULL here for consistency,
otherwise also looks good to me!

>  }
>  
>  /* Deserialize the ipa info for lto.  */

[Bug ipa/97346] IPA reference reference_vars_to_consider leaks

2021-02-10 Thread hubicka at ucw dot cz via Gcc-bugs 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97346

--- Comment #7 from Jan Hubicka  ---
I tested yesterday this one (which makes the lifetime bit more explicit
- during propagation it is for dumps only). Sorry for not posting it
earlier. I just wanted to double check tha tleak is gone.

diff --git a/gcc/ipa-reference.c b/gcc/ipa-reference.c
index 2ea2a6d5327..84c018ff57c 100644
--- a/gcc/ipa-reference.c
+++ b/gcc/ipa-reference.c
@@ -458,8 +458,8 @@ ipa_init (void)

   ipa_init_p = true;

-  vec_alloc (reference_vars_to_consider, 10);
-
+  if (dump_file)
+vec_alloc (reference_vars_to_consider, 10);

   if (ipa_ref_opt_sum_summaries != NULL)
 {
@@ -967,8 +967,12 @@ propagate (void)
 }

   if (dump_file)
-vec_free (reference_vars_to_consider);
-  reference_vars_to_consider = NULL;
+{
+  vec_free (reference_vars_to_consider);
+  reference_vars_to_consider = NULL;
+}
+  else
+gcc_checking_assert (!reference_vars_to_consider);
   return remove_p ? TODO_remove_functions : 0;
 }

@@ -1059,6 +1063,7 @@ ipa_reference_write_optimization_summary (void)
   auto_bitmap ltrans_statics;
   int i;

+  gcc_checking_assert (!reference_vars_to_consider);
   vec_alloc (reference_vars_to_consider, ipa_reference_vars_uids);
   reference_vars_to_consider->safe_grow (ipa_reference_vars_uids, true);

@@ -1117,7 +1122,7 @@ ipa_reference_write_optimization_summary (void)
  }
   }
   lto_destroy_simple_output_block (ob);
-  delete reference_vars_to_consider;
+  vec_free (reference_vars_to_consider);
 }

 /* Deserialize the ipa info for lto.  */

[Bug ipa/97346] IPA reference reference_vars_to_consider leaks

2021-02-10 Thread rguenth at gcc dot gnu.org via Gcc-bugs 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97346

--- Comment #6 from Richard Biener  ---
I'm testing the following which fixes the leak(s)

diff --git a/gcc/ipa-reference.c b/gcc/ipa-reference.c
index 2ea2a6d5327..a6b79fb9381 100644
--- a/gcc/ipa-reference.c
+++ b/gcc/ipa-reference.c
@@ -966,8 +966,7 @@ propagate (void)
   ipa_ref_var_info_summaries = NULL;
 }

-  if (dump_file)
-vec_free (reference_vars_to_consider);
+  vec_free (reference_vars_to_consider);
   reference_vars_to_consider = NULL;
   return remove_p ? TODO_remove_functions : 0;
 }
@@ -1059,6 +1058,7 @@ ipa_reference_write_optimization_summary (void)
   auto_bitmap ltrans_statics;
   int i;

+  vec_free (reference_vars_to_consider);
   vec_alloc (reference_vars_to_consider, ipa_reference_vars_uids);
   reference_vars_to_consider->safe_grow (ipa_reference_vars_uids, true);

@@ -1117,7 +1117,8 @@ ipa_reference_write_optimization_summary (void)
  }
   }
   lto_destroy_simple_output_block (ob);
-  delete reference_vars_to_consider;
+  vec_free (reference_vars_to_consider);
+  reference_vars_to_consider = NULL;
 }

 /* Deserialize the ipa info for lto.  */

[Bug ipa/97346] IPA reference reference_vars_to_consider leaks

2021-02-10 Thread rguenth at gcc dot gnu.org via Gcc-bugs 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97346

--- Comment #5 from Richard Biener  ---
(In reply to Richard Biener from comment #4)
> So like the following?  Note the leak is the allcoation from ipa_init
> being not released when we do the vec_alloc in
> ipa_reference_write_optimization_summary (maybe this function wants to
> use its own private vector?!)
> 
> diff --git a/gcc/ipa-reference.c b/gcc/ipa-reference.c
> index 2ea2a6d5327..328fa8f732c 100644
> --- a/gcc/ipa-reference.c
> +++ b/gcc/ipa-reference.c
> @@ -966,8 +966,7 @@ propagate (void)
>ipa_ref_var_info_summaries = NULL;
>  }
>  
> -  if (dump_file)
> -vec_free (reference_vars_to_consider);
> +  vec_free (reference_vars_to_consider);
>reference_vars_to_consider = NULL;
>return remove_p ? TODO_remove_functions : 0;
>  }
> @@ -1059,7 +1058,7 @@ ipa_reference_write_optimization_summary (void)
>auto_bitmap ltrans_statics;
>int i;
>  
> -  vec_alloc (reference_vars_to_consider, ipa_reference_vars_uids);
> +  vec_truncate (reference_vars_to_consider, 0);

vec_safe_truncate

>reference_vars_to_consider->safe_grow (ipa_reference_vars_uids, true);
>  
>/* See what variables we are interested in.  */
> @@ -1117,7 +1116,8 @@ ipa_reference_write_optimization_summary (void)
>   }
>}
>lto_destroy_simple_output_block (ob);
> -  delete reference_vars_to_consider;
> +  vec_free (reference_vars_to_consider);
> +  reference_vars_to_consider = NULL;
>  }
>  
>  /* Deserialize the ipa info for lto.  */

[Bug ipa/97346] IPA reference reference_vars_to_consider leaks

2021-02-10 Thread rguenth at gcc dot gnu.org via Gcc-bugs 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97346

--- Comment #4 from Richard Biener  ---
So like the following?  Note the leak is the allcoation from ipa_init
being not released when we do the vec_alloc in
ipa_reference_write_optimization_summary (maybe this function wants to
use its own private vector?!)

diff --git a/gcc/ipa-reference.c b/gcc/ipa-reference.c
index 2ea2a6d5327..328fa8f732c 100644
--- a/gcc/ipa-reference.c
+++ b/gcc/ipa-reference.c
@@ -966,8 +966,7 @@ propagate (void)
   ipa_ref_var_info_summaries = NULL;
 }

-  if (dump_file)
-vec_free (reference_vars_to_consider);
+  vec_free (reference_vars_to_consider);
   reference_vars_to_consider = NULL;
   return remove_p ? TODO_remove_functions : 0;
 }
@@ -1059,7 +1058,7 @@ ipa_reference_write_optimization_summary (void)
   auto_bitmap ltrans_statics;
   int i;

-  vec_alloc (reference_vars_to_consider, ipa_reference_vars_uids);
+  vec_truncate (reference_vars_to_consider, 0);
   reference_vars_to_consider->safe_grow (ipa_reference_vars_uids, true);

   /* See what variables we are interested in.  */
@@ -1117,7 +1116,8 @@ ipa_reference_write_optimization_summary (void)
  }
   }
   lto_destroy_simple_output_block (ob);
-  delete reference_vars_to_consider;
+  vec_free (reference_vars_to_consider);
+  reference_vars_to_consider = NULL;
 }

 /* Deserialize the ipa info for lto.  */

[Bug ipa/97346] IPA reference reference_vars_to_consider leaks

2021-02-08 Thread hubicka at gcc dot gnu.org via Gcc-bugs 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97346

Jan Hubicka  changed:

   What|Removed |Added

 Ever confirmed|0   |1
   Last reconfirmed||2021-02-08
 Status|UNCONFIRMED |ASSIGNED
   Assignee|unassigned at gcc dot gnu.org  |hubicka at gcc dot 
gnu.org

--- Comment #3 from Jan Hubicka  ---
I will check

If I recall correctly, during analysis the vector is only collected for dumps,
so that is why vec_free is conditional. Since vec_free is noop on NULL, we
could just free it anyway.  I think it is the delete call that causes the leak.

[Bug ipa/97346] IPA reference reference_vars_to_consider leaks

2021-02-08 Thread rguenth at gcc dot gnu.org via Gcc-bugs 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97346

--- Comment #2 from Richard Biener  ---
Ping.  This is annoying and pops up with each and every valgrind
--leak-check=full ...

[Bug ipa/97346] IPA reference reference_vars_to_consider leaks

2020-10-09 Thread rguenth at gcc dot gnu.org via Gcc-bugs 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97346

--- Comment #1 from Richard Biener  ---
At least

diff --git a/gcc/ipa-reference.c b/gcc/ipa-reference.c
index 4a6c011c525..6df4be09471 100644
--- a/gcc/ipa-reference.c
+++ b/gcc/ipa-reference.c
@@ -1114,7 +1113,8 @@ ipa_reference_write_optimization_summary (void)
  }
   }
   lto_destroy_simple_output_block (ob);
-  delete reference_vars_to_consider;
+  vec_free (reference_vars_to_consider);
+  reference_vars_to_consider = NULL;
 }

 /* Deserialize the ipa info for lto.  */

is maybe obvious.  Then there's

diff --git a/gcc/ipa-reference.c b/gcc/ipa-reference.c
index 4a6c011c525..f0555929340 100644
--- a/gcc/ipa-reference.c
+++ b/gcc/ipa-reference.c
@@ -1056,6 +1056,7 @@ ipa_reference_write_optimization_summary (void)
   auto_bitmap ltrans_statics;
   int i;

+  gcc_assert (!reference_vars_to_consider);
   vec_alloc (reference_vars_to_consider, ipa_reference_vars_uids);
   reference_vars_to_consider->safe_grow (ipa_reference_vars_uids, true);


that will likely trip because of the ipa_init allocation.  And

diff --git a/gcc/ipa-reference.c b/gcc/ipa-reference.c
index 4a6c011c525..93b2b677a76 100644
--- a/gcc/ipa-reference.c
+++ b/gcc/ipa-reference.c
@@ -964,8 +964,10 @@ propagate (void)
 }

   if (dump_file)
-vec_free (reference_vars_to_consider);
-  reference_vars_to_consider = NULL;
+{
+  vec_free (reference_vars_to_consider);
+  reference_vars_to_consider = NULL;
+}
   return remove_p ? TODO_remove_functions : 0;
 }


anyway, it all looks like a mess ;)