[PATCH] expandargv: fix memory leak
When the responsefile's contents are interpolated into the argument vector,
the pointer to original option string ("@filename") became lost. This
caused a small leak for every responsefile on the commandline.
---
libiberty/argv.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libiberty/argv.c b/libiberty/argv.c
index c544dad73ee..bdd4ace4cb6 100644
--- a/libiberty/argv.c
+++ b/libiberty/argv.c
@@ -455,6 +455,8 @@ expandargv (int *argcp, char ***argvp)
file_argc = 0;
while (file_argv[file_argc])
++file_argc;
+ /* Free the original option's memory. */
+ free((*argvp)[i]);
/* Now, insert FILE_ARGV into ARGV. The "+1" below handles the
NULL terminator at the end of ARGV. */
*argvp = ((char **)
--
2.11.0
Re: [PATCH] expandargv: fix memory leak
On Sat, 30 Dec 2017 18:27:30 +0100
Andreas Schwab wrote:
> On Dez 30 2017, Daniel van Gerpen wrote:
>
> > When the responsefile's contents are interpolated into the argument
> > vector, the pointer to original option string ("@filename") became
> > lost. This caused a small leak for every responsefile on the
> > commandline.
>
> argv elements generally don't point to the heap.
No, but expandargv() copies argv to the heap and then inserts the contents of
the responsefile.
The libiberty testsuite uses "@test-expandargv-0.lst" as an argument:
valgrind --leak-check=full testsuite/test-expandargv
==15851== Memcheck, a memory error detector
==15851== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==15851== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info
==15851== Command: testsuite/test-expandargv
==15851==
PASS: test-expandargv-0.
PASS: test-expandargv-1.
PASS: test-expandargv-2.
PASS: test-expandargv-3.
PASS: test-expandargv-4.
PASS: test-expandargv-5.
PASS: test-expandargv-6.
==15851==
==15851== HEAP SUMMARY:
==15851== in use at exit: 602 bytes in 28 blocks
==15851== total heap usage: 145 allocs, 117 frees, 931,880 bytes allocated
==15851==
==15851== 23 bytes in 1 blocks are definitely lost in loss record 1 of 4
==15851==at 0x4C2DB2F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==15851==by 0x10A037: xmalloc (xmalloc.c:147)
==15851==by 0x10A0F9: xstrdup (xstrdup.c:34)
==15851==by 0x1097C4: dupargv (argv.c:86)
==15851==by 0x109DBA: expandargv (argv.c:453)
==15851==by 0x109630: run_tests (test-expandargv.c:275)
==15851==by 0x10916F: main (test-expandargv.c:328)
[...]
Daniel
[PATCH] expandargv: fix check for dynamic allocation of argument vector
When the code interpolates the contents of response files, the
argument vector is reallocated to the new size. This only works
if it was dynamically allocated once before -- we do not want to
mess with the argv memory given to us by the init code.
The code tried to detect this with a flag, but that was never
written to, leading to multiple dynamic allocations -- one
for each response file.
---
libiberty/argv.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libiberty/argv.c b/libiberty/argv.c
index bdd4ace4cb6..fa88e50e894 100644
--- a/libiberty/argv.c
+++ b/libiberty/argv.c
@@ -367,8 +367,8 @@ expandargv (int *argcp, char ***argvp)
{
/* The argument we are currently processing. */
int i = 0;
- /* Non-zero if ***argvp has been dynamically allocated. */
- int argv_dynamic = 0;
+ /* To check if ***argvp has been dynamically allocated. */
+ char ** const original_argv = *argvp;
/* Limit the number of response files that we parse in order
to prevent infinite recursion. */
unsigned int iteration_limit = 2000;
@@ -449,7 +449,7 @@ expandargv (int *argcp, char ***argvp)
/* Parse the string. */
file_argv = buildargv (buffer);
/* If *ARGVP is not already dynamically allocated, copy it. */
- if (!argv_dynamic)
+ if (*argvp == original_argv)
*argvp = dupargv (*argvp);
/* Count the number of arguments. */
file_argc = 0;
--
2.11.0
