| Message ID | 20200121010654.3414484-1-ppalka@gcc.gnu.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix a couple of memory leaks in the C++ frontend | expand |
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
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 > >
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
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; }