Fix a couple of memory leaks in the C++ frontend
diff mbox series

Message ID 20200121010654.3414484-1-ppalka@gcc.gnu.org
State New
Headers show
Series
  • Fix a couple of memory leaks in the C++ frontend
Related show

Commit Message

Patrick Palka Jan. 21, 2020, 1:06 a.m. UTC
The leak in get_mapped_args is due to auto_vec not properly supporting
destructible elements, in that auto_vec's destructor doesn't call the
destructors of its elements.

Successfully bootstrapped and regtested on x86_64-pc-linux-gnu, OK to commit?

gcc/cp/ChangeLog:

	* constraint.cc (get_mapped_args): Avoid using auto_vec
	as a vector element.  Release the vectors inside the lists
	vector.
	* parser.c (cp_literal_operator_id): Free the buffer.
---
 gcc/cp/constraint.cc | 7 ++++---
 gcc/cp/parser.c      | 1 +
 2 files changed, 5 insertions(+), 3 deletions(-)

Comments

Jason Merrill Jan. 22, 2020, 1:58 a.m. UTC | #1
On 1/20/20 8:06 PM, Patrick Palka wrote:
> The leak in get_mapped_args is due to auto_vec not properly supporting
> destructible elements, in that auto_vec's destructor doesn't call the
> destructors of its elements.

Hmm, perhaps vec should static_assert __is_trivial(T) when supported.

> @@ -15344,6 +15344,7 @@ cp_literal_operator_id (const char* name)
>   			      + strlen (name) + 10);
>     sprintf (buffer, UDLIT_OP_ANSI_FORMAT, name);
>     identifier = get_identifier (buffer);
> +  free (buffer);

I guess we should use XDELETEVEC to match XNEWVEC.  OK with that change.

Jason
Patrick Palka Jan. 22, 2020, 4:19 p.m. UTC | #2
On Tue, 21 Jan 2020, Jason Merrill wrote:

> On 1/20/20 8:06 PM, Patrick Palka wrote:
>> The leak in get_mapped_args is due to auto_vec not properly supporting
>> destructible elements, in that auto_vec's destructor doesn't call the
>> destructors of its elements.
>
> Hmm, perhaps vec should static_assert __is_trivial(T) when supported.

Unfortunately this would break other seemingly well-behaved users of vec
in which T doesn't have a trivial default constructor.  Relaxing the
condition to __has_trivial_copy(T) && __has_trivial_destructor(T) seems
to be fine though.

>
>> @@ -15344,6 +15344,7 @@ cp_literal_operator_id (const char* name)
>>   			      + strlen (name) + 10);
>>     sprintf (buffer, UDLIT_OP_ANSI_FORMAT, name);
>>     identifier = get_identifier (buffer);
>> +  free (buffer);
>
> I guess we should use XDELETEVEC to match XNEWVEC.  OK with that change.

Patch committed with that change.

>
> Jason
>
>
Jason Merrill Jan. 22, 2020, 5:18 p.m. UTC | #3
On 1/22/20 11:19 AM, Patrick Palka wrote:
> On Tue, 21 Jan 2020, Jason Merrill wrote:
> 
>> On 1/20/20 8:06 PM, Patrick Palka wrote:
>>> The leak in get_mapped_args is due to auto_vec not properly supporting
>>> destructible elements, in that auto_vec's destructor doesn't call the
>>> destructors of its elements.
>>
>> Hmm, perhaps vec should static_assert __is_trivial(T) when supported.
> 
> Unfortunately this would break other seemingly well-behaved users of vec
> in which T doesn't have a trivial default constructor.  Relaxing the
> condition to __has_trivial_copy(T) && __has_trivial_destructor(T) seems
> to be fine though.

Sure, that makes sense; grow does try to default-construct elements.

Jason

Patch
diff mbox series

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 128ab8ae0b2..823604afb89 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -2431,7 +2431,7 @@  get_mapped_args (tree map)
      list. Note that the list will be sparse (not all arguments supplied),
      but instantiation is guaranteed to only use the parameters in the
      mapping, so null arguments would never be used.  */
-  auto_vec< auto_vec<tree> > lists (count);
+  auto_vec< vec<tree> > lists (count);
   lists.quick_grow_cleared (count);
   for (tree p = map; p; p = TREE_CHAIN (p))
     {
@@ -2440,7 +2440,7 @@  get_mapped_args (tree map)
       template_parm_level_and_index (TREE_VALUE (p), &level, &index);
 
       /* Insert the argument into its corresponding position.  */
-      auto_vec<tree> &list = lists[level - 1];
+      vec<tree> &list = lists[level - 1];
       if (index >= (int)list.length ())
 	list.safe_grow_cleared (index + 1);
       list[index] = TREE_PURPOSE (p);
@@ -2450,11 +2450,12 @@  get_mapped_args (tree map)
   tree args = make_tree_vec (lists.length ());
   for (unsigned i = 0; i != lists.length (); ++i)
     {
-      auto_vec<tree> &list = lists[i];
+      vec<tree> &list = lists[i];
       tree level = make_tree_vec (list.length ());
       for (unsigned j = 0; j < list.length(); ++j)
 	TREE_VEC_ELT (level, j) = list[j];
       SET_TMPL_ARGS_LEVEL (args, i + 1, level);
+      list.release ();
     }
   SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (args, 0);
 
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index c5f9798a5ed..e1e27a574f1 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -15344,6 +15344,7 @@  cp_literal_operator_id (const char* name)
 			      + strlen (name) + 10);
   sprintf (buffer, UDLIT_OP_ANSI_FORMAT, name);
   identifier = get_identifier (buffer);
+  free (buffer);
 
   return identifier;
 }