[PATCH] Fix producer string memory leaks

Martin Liška mliska@suse.cz
Mon Feb 15 13:46:55 GMT 2021


On 2/12/21 5:56 PM, Martin Sebor wrote:
> On 2/12/21 2:09 AM, Richard Biener via Gcc-patches wrote:
>> On Thu, Feb 11, 2021 at 6:41 PM Martin Liška <mliska@suse.cz> wrote:
>>>
>>> Hello.
>>>
>>> This fixes 2 memory leaks I noticed.
>>>
>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>
>>> Ready to be installed?
>>
>> OK.
>>
>>> Thanks,
>>> Martin
>>>
>>> gcc/ChangeLog:
>>>
>>>          * opts-common.c (decode_cmdline_option): Release werror_arg.
>>>          * opts.c (gen_producer_string): Release output of
>>>          gen_command_line_string.
>>> ---
>>>    gcc/opts-common.c | 1 +
>>>    gcc/opts.c        | 7 +++++--
>>>    2 files changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/gcc/opts-common.c b/gcc/opts-common.c
>>> index 6cb5602896d..5e10edeb4cf 100644
>>> --- a/gcc/opts-common.c
>>> +++ b/gcc/opts-common.c
>>> @@ -766,6 +766,7 @@ decode_cmdline_option (const char *const *argv, unsigned int lang_mask,
>>>          werror_arg[0] = 'W';
>>>
>>>          size_t warning_index = find_opt (werror_arg, lang_mask);
>>> +      free (werror_arg);
> 
> Sorry to butt in here, but since we're having a discussion on this
> same subject in another review of a fix for a memory leak, I thought
> I'd pipe up: I would suggest a better (more in line with C++ and more
> future-proof) solution is to replace the call to xstrdup (and the need
> to subsequently call free) with std::string.

Hello.

To be honest, I like the suggested approach using std::string. On the other hand,
I don't want to mix existing C API (char *) with std::string.

I'm sending a patch that fixed the remaining leaks.

> 
>>>          if (warning_index != OPT_SPECIAL_unknown)
>>>          {
>>>            const struct cl_option *warning_option
>>> diff --git a/gcc/opts.c b/gcc/opts.c
>>> index fc5f61e13cc..24bb64198c8 100644
>>> --- a/gcc/opts.c
>>> +++ b/gcc/opts.c
>>> @@ -3401,8 +3401,11 @@ char *
>>>    gen_producer_string (const char *language_string, cl_decoded_option *options,
>>>                       unsigned int options_count)
>>>    {
>>> -  return concat (language_string, " ", version_string, " ",
>>> -                gen_command_line_string (options, options_count), NULL);
>>> +  char *cmdline = gen_command_line_string (options, options_count);
>>> +  char *combined = concat (language_string, " ", version_string, " ",
>>> +                          cmdline, NULL);
>>> +  free (cmdline);
>>> +  return combined;
>>>    }
> 
> Here, changing gen_command_line_string() to return std::string instead
> of a raw pointer would similarly avoid having to remember to free
> the pointer (and forgetting).  The function has two other callers,
> both in gcc/toplev.c, and both also leak the memory for the same
> reason as here.

Btw. have we made a general consensus that using std::string is fine in the
GCC internals?

Martin

> 
> Martin
> 
>>>
>>>    #if CHECKING_P
>>> -- 
>>> 2.30.0
>>>
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-2-more-leaks-related-to-gen_command_line_string.patch
Type: text/x-patch
Size: 1498 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20210215/7e746d41/attachment.bin>


More information about the Gcc-patches mailing list