From patchwork Wed Sep 2 01:35:02 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Pluzhnikov X-Patchwork-Id: 8549 Received: (qmail 58458 invoked by alias); 2 Sep 2015 01:35:36 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 58447 invoked by uid 89); 2 Sep 2015 01:35:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mail-vk0-f47.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; bh=wUL4C9zufU7psEjT0RcZgoeWio3wRSp2X51KopNFCwE=; b=Kiiqx6nUH1bam9/MqwIP77Kzgr1w0y+7bq6er87WGvPQy/gWAlkmcOkaoBk/EVxtAH ou0zzz+1/eH1FW6IjSjxFkeXRVSCiUukU86ksGC+qo6gQb5sc0WApeFbBCsPqqwOBRfX fEyl20KPJJKpxBV0C1wg2UXKBLHznSB9+I9/xfGGVMk6Usqb+GApnPRqxH6cQCDCM8ur g5p8dDvTuGNe1LTWnxmXPACsx5LqNLAFo6U+lvQXy4HhyJfXk7/a1CoC0Zt8xKE8M0vN K2IPbWLKB57AKASt/oJ3ljCYdrsAPOEyY8q+XA6kGnndxj5CaU/Wu0Opux6V9leLuJGE yk4A== X-Gm-Message-State: ALoCoQnUMdG+N+Z+20WCjXnVlJBokysd0yqz12B/k4ASVQHy2yZddrEi1wxU/8sJaY5GZv67wBnq X-Received: by 10.53.13.33 with SMTP id ev1mr32926068vdd.69.1441157731728; Tue, 01 Sep 2015 18:35:31 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1440571295-20230-1-git-send-email-eggert@cs.ucla.edu> <55DFB7C7.50307@redhat.com> <55E06924.2000209@redhat.com> From: Paul Pluzhnikov Date: Tue, 1 Sep 2015 18:35:02 -0700 Message-ID: Subject: Re: [PATCH] Fix memory leak in printf_positional To: Joseph Myers Cc: "Carlos O'Donell" , Paul Eggert , GLIBC Devel On Mon, Aug 31, 2015 at 3:41 AM, Joseph Myers 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 Paul Pluzhnikov [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. 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 + . */ + +#include +#include + +/* + 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;