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 <mli...@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
>From 8c3c6812cb094888696302001c114dc11cfa2694 Mon Sep 17 00:00:00 2001
From: Martin Liska <mli...@suse.cz>
Date: Mon, 15 Feb 2021 11:28:19 +0100
Subject: [PATCH] Fix 2 more leaks related to gen_command_line_string.
gcc/ChangeLog:
* toplev.c (init_asm_output): Free output of
gen_command_line_string function.
(process_options): Likewise.
---
gcc/toplev.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 05bd449eafc..d8cc254adef 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -748,9 +748,10 @@ init_asm_output (const char *name)
print_version (asm_out_file, ASM_COMMENT_START, true);
fputs (ASM_COMMENT_START, asm_out_file);
fputs (" options passed: ", asm_out_file);
- fputs (gen_command_line_string (save_decoded_options,
- save_decoded_options_count),
- asm_out_file);
+ char *cmdline = gen_command_line_string (save_decoded_options,
+ save_decoded_options_count);
+ fputs (cmdline, asm_out_file);
+ free (cmdline);
fputc ('\n', asm_out_file);
}
}
@@ -1384,8 +1385,11 @@ process_options (void)
if (!quiet_flag)
{
fputs ("options passed: ", stderr);
- fputs (gen_command_line_string (save_decoded_options,
- save_decoded_options_count), stderr);
+ char *cmdline = gen_command_line_string (save_decoded_options,
+ save_decoded_options_count);
+
+ fputs (cmdline, stderr);
+ free (cmdline);
fputc ('\n', stderr);
}
}
--
2.30.0
