Fix memory leak in printf_positional
Commit Message
On Thu, Sep 3, 2015 at 7:44 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> On Thu, Sep 3, 2015 at 7:38 AM, Andreas Schwab <schwab@suse.de> wrote:
>
>>> Sorry, I didn't understand that comment.
>>
>> The target must always be either complete or absent. A redirection is
>> not atomic.
Revised patch addressing all comments attached.
Thanks,
2015-09-03 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.sh: Generate new test.
* stdio-common/vfprintf.c: Fix memory leaks.
Comments
On Thu, Sep 3, 2015 at 8:59 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> Revised patch addressing all comments attached.
> Thanks,
>
> 2015-09-03 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.sh: Generate new test.
> * stdio-common/vfprintf.c: Fix memory leaks.
Ping?
https://sourceware.org/ml/libc-alpha/2015-09/msg00086.html
On 09/12/2015 12:37 PM, Paul Pluzhnikov wrote:
> On Thu, Sep 3, 2015 at 8:59 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
>
>> Revised patch addressing all comments attached.
>> Thanks,
>>
>> 2015-09-03 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.sh: Generate new test.
>> * stdio-common/vfprintf.c: Fix memory leaks.
>
> Ping?
> https://sourceware.org/ml/libc-alpha/2015-09/msg00086.html
I see the test disables optimization to work around the GCC issue
but I wonder if a better solution might be to create an array object
for the format string rather than passing it as a literal to printf.
Martin
On Sat, Sep 12, 2015 at 12:16 PM, Martin Sebor <msebor@gmail.com> wrote:
> I see the test disables optimization to work around the GCC issue
> but I wonder if a better solution might be to create an array object
> for the format string rather than passing it as a literal to printf.
You mean like this:
char fmt[] = "%$1s %$2s ...";
printf(fmt, "a", "a", ...);
If so, no: the format literal is not what triggers the the GCC problem
-- the number of arguments being passed to printf does.
On 09/12/2015 01:22 PM, Paul Pluzhnikov wrote:
> On Sat, Sep 12, 2015 at 12:16 PM, Martin Sebor <msebor@gmail.com> wrote:
>
>> I see the test disables optimization to work around the GCC issue
>> but I wonder if a better solution might be to create an array object
>> for the format string rather than passing it as a literal to printf.
>
> You mean like this:
>
> char fmt[] = "%$1s %$2s ...";
> printf(fmt, "a", "a", ...);
>
> If so, no: the format literal is not what triggers the the GCC problem
> -- the number of arguments being passed to printf does.
I see. Thanks for clarifying that.
FWIW, I asked because I noticed other tests that take quite long
to compile (besides stressing the compiler in other ways) and I'd
like to get a sense of what constructs in the test suite tend to
trigger these types of problems and try to come up with alternate
approaches that avoid such problems.
But in this instance, if it's the arguments, I don't see a better
way around it than to disable optimization.
Martin
Paul Pluzhnikov <ppluzhnikov@google.com> writes:
> [BZ #18872]
> * stdio-common/Makefile (tst-printf-bz18872): New test.
> (tst-printf-bz18872-mem.out): Likewise.
> * stdio-common/tst-printf-bz18872.sh: Generate new test.
> * stdio-common/vfprintf.c: Fix memory leaks.
This is ok.
Andreas.
@@ -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.c tst-printf-bz18872.mtrace \
+ tst-printf-bz18872-mem.out
endif
include ../Rules
+tst-printf-bz18872-ENV = MALLOC_TRACE=$(objpfx)tst-printf-bz18872.mtrace
+
ifeq ($(run-built-tests),yes)
$(objpfx)tst-unbputc.out: tst-unbputc.sh $(objpfx)tst-unbputc
$(SHELL) $< $(common-objpfx) '$(test-program-prefix)'; \
@@ -76,6 +82,15 @@ $(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)
+
+# We generate this source because it requires a printf invocation with
+# 10K arguments.
+$(objpfx)tst-printf-bz18872.c: tst-printf-bz18872.sh
+ rm -f $@ && $(BASH) $^ > $@.new && mv $@.new $@
+
+$(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
new file mode 100755
@@ -0,0 +1,70 @@
+#!/bin/bash
+# Copyright (C) 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/>.
+
+# To test BZ #18872, we need a printf() with 10K arguments.
+# Such a printf could be generated with non-trivial macro
+# application, but it's simpler to generate the test source
+# via this script.
+
+n_args=10000
+
+cat <<'EOF'
+#include <stdio.h>
+#include <mcheck.h>
+
+/*
+ Compile do_test 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")
+
+int do_test (void)
+{
+ mtrace ();
+ printf (
+EOF
+
+for ((j = 0; j < $n_args / 10; j++)); do
+ for ((k = 0; k < 10; k++)); do
+ printf '"%%%d$s" ' $((10 * $j + $k + 1))
+ done
+ printf "\n"
+done
+
+printf '"%%%d$s",\n' $(($n_args + 1))
+
+for ((j = 0; j < $n_args / 10; j++)); do
+ for ((k = 0; k < 10; k++)); do
+ printf '"a", '
+ done
+ printf " /* %4d */\n" $((10 * $j + $k))
+done
+
+printf '"\\n");'
+
+
+cat <<'EOF'
+
+ return 0;
+}
+#pragma GCC pop_options
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
+
+EOF
@@ -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;