[PATCH] Fix memory leak in printf_positional

[Paul Pluzhnikov]

On Mon, Aug 31, 2015 at 3:41 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Sat, 29 Aug 2015, Paul Pluzhnikov wrote:
>
>> It doesn't take very long: all I needed is a printf invocation with >=
>> 65536 / 3 / sizeof(void*) arguments.
>>
>> Writing such invocation by hand is of course toublesome, plumbing
>> Makefile to generate it for me, and figuring out why it doesn't work
>> is what takes time :-(
>
> Normally such invocations would be generated with macros, e.g.
>
> #define A a, a, a, a, a, a, a, a, a, a
> #define B A, A, A, A, A, A, A, A, A, A
>
> etc., unless the sort of expansion you require is unsuited to that for
> some reason.

Well, to show the leak requires positional format, like this:

  "%1$s %2$s ..."

However, I was able to use

  "%1$s %s %s ..."

if I disable -Wformat for the test.


>> In addition, there is a GCC regression: compiling a printf call with
>> 2800 arguments takes 4.8.4-2ubuntu1~14.04 0.06s without optimization,
>> 0.86s with -O2. Same numbers for current GCC trunk (@r227321): 0.06s
>> and  4m46s. This is on a very recent and fast PC. I expect there could
>> be PCs in current use where the time will be 3x longer.
>
> To me this suggests building the test with -O0

That doesn't work: glibc requires to be built with optimization :-)

But #pragma GCC optimize does work.

The test case revealed an additional leak, so at least that effort was
not in vain.

Combined patch attached. Tested on Linux/x86_64.

Thanks,

2015-09-01  Paul Eggert  <eggert@cs.ucla.edu>
   Paul Pluzhnikov  <ppluzhnikov@google.com>

        [BZ #18872]
        * stdio-common/Makefile (tst-printf-bz18872): New test.
        (tst-printf-bz18872-mem.out): Likewise.
        * stdio-common/tst-printf-bz18872.c: New test.
        * stdio-common/vfprintf.c: Fix memory leaks.



-- 
Paul Pluzhnikov
-------------- next part --------------
diff --git a/stdio-common/Makefile b/stdio-common/Makefile
index d0bf0e1..ad2c8a3 100644
--- a/stdio-common/Makefile
+++ b/stdio-common/Makefile
@@ -57,17 +57,23 @@ tests := tstscanf test_rdwr test-popen tstgetln test-fseek \
 	 bug19 bug19a tst-popen2 scanf13 scanf14 scanf15 bug20 bug21 bug22 \
 	 scanf16 scanf17 tst-setvbuf1 tst-grouping bug23 bug24 \
 	 bug-vfprintf-nargs tst-long-dbl-fphex tst-fphex-wide tst-sprintf3 \
-	 bug25 tst-printf-round bug23-2 bug23-3 bug23-4 bug26 tst-fmemopen3
+	 bug25 tst-printf-round bug23-2 bug23-3 bug23-4 bug26 tst-fmemopen3 \
+	 tst-printf-bz18872
 
 test-srcs = tst-unbputc tst-printf
 
 ifeq ($(run-built-tests),yes)
 tests-special += $(objpfx)tst-unbputc.out $(objpfx)tst-printf.out \
+		 $(objpfx)tst-printf-bz18872-mem.out \
 		 $(objpfx)tst-setvbuf1-cmp.out
+generated += tst-printf-bz18872.mtrace tst-printf-bz18872-mem.out
 endif
 
 include ../Rules
 
+tst-printf-bz18872-ENV = MALLOC_TRACE=$(objpfx)tst-printf-bz18872.mtrace
+CFLAGS-tst-printf-bz18872.c += -Wno-format
+
 ifeq ($(run-built-tests),yes)
 $(objpfx)tst-unbputc.out: tst-unbputc.sh $(objpfx)tst-unbputc
 	$(SHELL) $< $(common-objpfx) '$(test-program-prefix)'; \
@@ -76,6 +82,10 @@ $(objpfx)tst-unbputc.out: tst-unbputc.sh $(objpfx)tst-unbputc
 $(objpfx)tst-printf.out: tst-printf.sh $(objpfx)tst-printf
 	$(SHELL) $< $(common-objpfx) '$(test-program-prefix)'; \
 	$(evaluate-test)
+
+$(objpfx)tst-printf-bz18872-mem.out: $(objpfx)tst-printf-bz18872.out
+	$(common-objpfx)malloc/mtrace $(objpfx)tst-printf-bz18872.mtrace > $@; \
+	$(evaluate-test)
 endif
 
 CFLAGS-vfprintf.c = -Wno-uninitialized
diff --git a/stdio-common/tst-printf-bz18872.c b/stdio-common/tst-printf-bz18872.c
new file mode 100644
index 0000000..c5f3513
--- /dev/null
+++ b/stdio-common/tst-printf-bz18872.c
@@ -0,0 +1,47 @@
+/* Copyright (C) 1991-2015 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <mcheck.h>
+
+/*
+  Compile do_bz18872 without optimization: GCC 4.9/5.0/6.0 takes a long time
+  to build this source. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67396  */
+#pragma GCC push_options
+#pragma GCC optimize ("-O0")
+
+static int
+do_bz18872 (void)
+{
+  mtrace ();
+
+#define A10 "a", "a", "a", "a", "a", "a", "a", "a", "a", "a",
+#define S10 "%s" "%s" "%s" "%s" "%s" "%s" "%s" "%s" "%s" "%s"
+#define X10(a) a a a a a a a a a a
+
+  printf ("%1$s" X10(X10(X10(S10))) "%s", X10(X10(X10(A10))) "\n");
+
+#undef A10
+#undef S10
+#undef X10
+    return 0;
+}
+
+#pragma GCC pop_options
+
+#define TEST_FUNCTION do_bz18872 ()
+#include "../test-skeleton.c"
diff --git a/stdio-common/vfprintf.c b/stdio-common/vfprintf.c
index 0592e70..45c4779 100644
--- a/stdio-common/vfprintf.c
+++ b/stdio-common/vfprintf.c
@@ -2091,6 +2091,10 @@ printf_positional (_IO_FILE *s, const CHAR_T *format, int readonly_format,
 		 - specs[nspecs_done].end_of_fmt);
     }
  all_done:
+  if (__glibc_unlikely (specs_malloced))
+    free (specs);
+  if (__glibc_unlikely (args_malloced != NULL))
+    free (args_malloced);
   if (__glibc_unlikely (workstart != NULL))
     free (workstart);
   return done;