From patchwork Thu Sep 3 15:59:25 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Pluzhnikov X-Patchwork-Id: 8565 Received: (qmail 95419 invoked by alias); 3 Sep 2015 16:00:01 -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 95384 invoked by uid 89); 3 Sep 2015 16:00:00 -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-f50.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=A75dbHvxcz/MBym3sMAM01FoJdzVEykV375fDb8Upas=; b=ZtozOGkwfp9Vc9Pk4XCU+9/j68E9lpmt9muo0s/pkmmCD9QW/C4vqIcUAoddYdMCVS kJWzDuqXGaNBNnTSj5nBf+7IqBG7gkItJxi7Tk4EW9NkLWgTr/2hQCrjaN1Qh9BWabQ3 3QUFJQK72h1Q9XS96UbK9XHR3cT54+MuQ5Ev4JbnqAMgjpy9LwHAsN2D9Je/LGw3ajsZ GvRR9cO0WA69l+cQNz60IetXjZdIU7MWFakT0sQ2DF/9omwAJyX97Idqo8yHJ7U6xzZ3 ILYU+DlDCBLrm5HG3epwh3cqBIFW+h3vIWt4xWdwIpwllfGhKDDjZ8cUPPGwXTdcWiqN r7Sw== X-Gm-Message-State: ALoCoQnN/ts0rwdtlQHR20QCIlKrbpMzKuRYv/FZsnet0FIyHugkt+KmTGM1UuFZnLcJZm/sSDDB X-Received: by 10.52.243.232 with SMTP id xb8mr46065235vdc.40.1441295995517; Thu, 03 Sep 2015 08:59:55 -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> <55E73436.1050305@redhat.com> From: Paul Pluzhnikov Date: Thu, 3 Sep 2015 08:59:25 -0700 Message-ID: Subject: Re: [PATCH] Fix memory leak in printf_positional To: Andreas Schwab Cc: "Carlos O'Donell" , Joseph Myers , Paul Eggert , GLIBC Devel On Thu, Sep 3, 2015 at 7:44 AM, Paul Pluzhnikov wrote: > On Thu, Sep 3, 2015 at 7:38 AM, Andreas Schwab 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 Paul Pluzhnikov [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. diff --git a/stdio-common/Makefile b/stdio-common/Makefile index d0bf0e1..40f5cdf 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,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 diff --git a/stdio-common/tst-printf-bz18872.sh b/stdio-common/tst-printf-bz18872.sh new file mode 100755 index 0000000..0127e73 --- /dev/null +++ b/stdio-common/tst-printf-bz18872.sh @@ -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 +# . + +# 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 +#include + +/* + 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 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;