| Message ID | CALoOobMusa6usQCEAR2bs8ES2EbHRa9YuCARd9=ewq1eROPLmQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Paul Pluzhnikov <ppluzhnikov@google.com> writes: > @@ -76,6 +82,13 @@ $(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.c: tst-printf-bz18872.sh > + rm -f $@ > + $(BASH) $^ > $@ Please make that atomic by using a temporary file. > +for j in $(seq 1 $n_args); do > + if [[ $(($j % 10)) == 1 ]]; then printf "\n"; fi > + printf '"%%%d$s" ' $j > +done > + > +printf ' "%%%d$s",' $(($n_args + 1)) > + > +for j in $(seq 1 $n_args); do > + if [[ $(($j % 10)) == 1 ]]; then printf "\n"; fi > + printf '"a", ' > +done Since you are already using bash you can also use `for ((j = 1; j <= n_args; j++))'. Andreas.
On Wed, 2 Sep 2015, Paul Pluzhnikov wrote: > On Wed, Sep 2, 2015 at 10:39 AM, Carlos O'Donell <carlos@redhat.com> wrote: > > >>> I think at that point it becomes easier to just use a generator script > >>> to write the test. Any objection to that? > >> > >> I don't object to that given an appropriate comment on why it is being > >> used. > > > > Agreed. > > What's the appropriate place to put this comment? I'd suggest in the Makefile. > +# Copyright (C) 1999-2015 Free Software Foundation, Inc. This is a new test; I'd expect just 2015 unless closely based on an existing test.
On 09/03/2015 01:48 AM, Paul Pluzhnikov wrote: > On Wed, Sep 2, 2015 at 10:39 AM, Carlos O'Donell <carlos@redhat.com> wrote: > >>>> I think at that point it becomes easier to just use a generator script >>>> to write the test. Any objection to that? >>> >>> I don't object to that given an appropriate comment on why it is being >>> used. >> >> Agreed. > > What's the appropriate place to put this comment? > > 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.sh: Generate new test. > * stdio-common/vfprintf.c: Fix memory leaks. This looks good to me. The comment in tst-printf-bz18872.sh is sufficient for me. Cheers, Carlos.
On Thu, Sep 3, 2015 at 12:23 AM, Andreas Schwab <schwab@suse.de> wrote: > Paul Pluzhnikov <ppluzhnikov@google.com> writes: >> +$(objpfx)tst-printf-bz18872.c: tst-printf-bz18872.sh >> + rm -f $@ >> + $(BASH) $^ > $@ > > Please make that atomic by using a temporary file. Sorry, I didn't understand that comment. Do you mean that I should have a rule like this: $(objpfx)tst-printf-bz18872.out: tst-printf-bz18872.sh $(BASH) $^ | $(CC) ... -o $(objpfx)tst-printf-bz18872 -xc - $(test-something-or-other) or something else? (Figuring out the commands for that rule gives me a pause.) Thanks,
Paul Pluzhnikov <ppluzhnikov@google.com> writes: > On Thu, Sep 3, 2015 at 12:23 AM, Andreas Schwab <schwab@suse.de> wrote: >> Paul Pluzhnikov <ppluzhnikov@google.com> writes: > >>> +$(objpfx)tst-printf-bz18872.c: tst-printf-bz18872.sh >>> + rm -f $@ >>> + $(BASH) $^ > $@ >> >> Please make that atomic by using a temporary file. > > Sorry, I didn't understand that comment. The target must always be either complete or absent. A redirection is not atomic. Andreas.
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. Understood. Thanks,
diff --git a/stdio-common/Makefile b/stdio-common/Makefile index d0bf0e1..d91533f 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.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,13 @@ $(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.c: tst-printf-bz18872.sh + rm -f $@ + $(BASH) $^ > $@ +$(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.sh b/stdio-common/tst-printf-bz18872.sh new file mode 100755 index 0000000..75ec92f --- /dev/null +++ b/stdio-common/tst-printf-bz18872.sh @@ -0,0 +1,66 @@ +#!/bin/bash +# Copyright (C) 1999-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 in $(seq 1 $n_args); do + if [[ $(($j % 10)) == 1 ]]; then printf "\n"; fi + printf '"%%%d$s" ' $j +done + +printf ' "%%%d$s",' $(($n_args + 1)) + +for j in $(seq 1 $n_args); do + if [[ $(($j % 10)) == 1 ]]; then printf "\n"; fi + printf '"a", ' +done + +printf '"\\n");' + + +cat <<'EOF' + + return 0; +} +#pragma GCC pop_options + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" + +EOF 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;