This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFAv2] Fix leaks in macro definitions.
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
Simon> On 2019-01-19 12:28 p.m., Philippe Waroquiers wrote:
>> @@ -774,8 +780,28 @@ macro_define_object_internal (struct macro_source_file *source, int line,
>> return;
>>
>> k = new_macro_key (t, name, source, line);
>> - d = new_macro_definition (t, macro_object_like, kind, 0, replacement);
>> - splay_tree_insert (t->definitions, (splay_tree_key) k, (splay_tree_value) d);
>> + d = new_macro_definition (t, kind, argc, argv, replacement);
>> + s = splay_tree_insert (t->definitions, (splay_tree_key) k, (splay_tree_value) d);
>> + if ((struct macro_key *) s->key != k)
>> + {
>> + /* If the node inserted is not using k, it means we have a redefinition.
>> + We must free the newly allocated k, as it will not be stored in
>> + the splay tree. */
>> + macro_tree_delete_key (k);
Simon> If this is really the expected behavior, I really think this should be documented in the
Simon> splay tree API. It's really a trap everybody will fall into.
I guess I wasn't thinking, or maybe reading or both, clearly in the
previous round of splay-tree fixing.
It seems that the contract of splay_tree_insert must be that it takes
ownership of the key and the value. So, it should indeed delete a
duplicate key here.
In gcc, this only affects lto.c, so I suppose that has a latent leak:
unsigned HOST_WIDE_INT *idp = XCNEW (unsigned HOST_WIDE_INT);
*idp = id;
splay_tree_insert (t, (splay_tree_key) idp, (splay_tree_value) file_data);
... assuming duplicate keys can ever be seen, which I don't know.
Tom