From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x729.google.com (mail-qk1-x729.google.com [IPv6:2607:f8b0:4864:20::729]) by sourceware.org (Postfix) with ESMTPS id 8952B3854813 for ; Fri, 23 Apr 2021 00:00:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8952B3854813 Received: by mail-qk1-x729.google.com with SMTP id u20so15999972qku.10 for ; Thu, 22 Apr 2021 17:00:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:references:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=8T49+yPoE+NVaU9Wv6jQKtC1lxL2nlQBZroZN0taVuc=; b=RNRkIFs8XcoQjgMmm6T0RcEECripbjSF7EwHkUgcrAAEudglQdFHYwZLoH2y3aV4hA QVL7G1bcbduOZ7z+YTdyxOUEqpBVGIP5bxTPcoCEzFEz/5kMvZWGZxmGjZdCP3B6FHXB 2bBa1P8zHMdG9LQvpIZ4PvmFl4t//NG+6829zlVwY6ziY7OxEH1cXEZexqv71MkHf6O+ 3qRQ7AxwiKVJ+4f7j2XxucSMcjaWnIqKiSz/4ltxnnXf9y7dGikd1m8y/8Ebvi+UFQgX RlV7ArjRqtM63sO1d0MWbw3ZRy9U0Id+Heq028prPCZUv9EwpAme7s/4rU308cET0KKc HPnQ== X-Gm-Message-State: AOAM533CSIrOloOxJWNtPSkrUDV475astl6daaZV/bgSZpLVcyhI2dd1 425lHmWft9ElZk8EyUZwIck= X-Google-Smtp-Source: ABdhPJytLbXPBtxy8o/ws/x4gajjfDESnLKrLOtuJxwZtbGApbiF3shRp8GWtpH5qYfWs4H31yEogQ== X-Received: by 2002:a05:620a:e16:: with SMTP id y22mr1371401qkm.310.1619136054151; Thu, 22 Apr 2021 17:00:54 -0700 (PDT) Received: from [192.168.0.41] (71-218-14-121.hlrn.qwest.net. [71.218.14.121]) by smtp.gmail.com with ESMTPSA id m4sm3378239qkh.131.2021.04.22.17.00.52 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 22 Apr 2021 17:00:53 -0700 (PDT) Subject: Re: [PATCH] add support for -Wmismatched-dealloc From: Martin Sebor To: Florian Weimer Cc: libc-alpha@sourceware.org, Joseph Myers References: <74efece7-9a4b-83ee-7fdd-475c0d514378@gmail.com> <758e723b-67cf-a211-7bc2-2ccd3fc744e5@gmail.com> <2555516b-4583-21fc-e844-fd44619488cd@gmail.com> <655918b2-16c6-74b1-6a49-505a7607007f@gmail.com> <87mtxok7ob.fsf@oldenburg2.str.redhat.com> <0aae9006-6001-8fc8-ad6d-c8e3ee60f82c@gmail.com> <87turwiqqw.fsf@oldenburg2.str.redhat.com> <87czybsuoe.fsf@oldenburg2.str.redhat.com> Message-ID: <5ed3fe4d-c0e4-632f-2569-b60d29cf4995@gmail.com> Date: Thu, 22 Apr 2021 18:00:52 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------E63FE44990BE039CE174F10E" Content-Language: en-US X-Spam-Status: No, score=-9.1 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 23 Apr 2021 00:01:00 -0000 This is a multi-part message in MIME format. --------------E63FE44990BE039CE174F10E Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Attached is a revised patch that does not apply the new attribute malloc to realpath. It also adds a new test to verify that calling a deallocator other than free on the result of realpath() with the resolved_path obtained from the corresponding allocator doesn't trigger a warning. This is a trade-off between false positives and negatives: there new attribute isn't expressive enough to specify that a function like realpath returns a pointer allocated by malloc (when the second argument is null) or its argument otherwise. I also made sure that in the modified declarations __THROW comes before attributes and not after (as Florian pointed out is required in C++) and verified by compiling the modified headers with G++ 11. Otherwise this patch is unchanged from the last version. Martin On 1/11/21 5:01 PM, Martin Sebor wrote: > On 1/11/21 2:13 AM, Florian Weimer wrote: > ... > > The attached revision has the changes below. > >>> diff --git a/libio/tst-freopen.c b/libio/tst-freopen.c >>> index baa13166fe..c4df29d171 100644 >>> --- a/libio/tst-freopen.c >>> +++ b/libio/tst-freopen.c >>> @@ -81,6 +81,36 @@ do_test_basic (void) >>>     fclose (f); >>>   } >>> +#if defined __GNUC__ && __GNUC__ >= 11 >>> +#pragma GCC diagnostic push >>> +#pragma GCC diagnostic error "-Wmismatched-dealloc" >>> +#endif >> >> Please add a comment referencing the fclose in the test below. > > Done. > >> >>> +  /* This shouldn't trigger -Wmismatched-dealloc.  */ >>> +  fclose (f1); >> >>> +#if defined __GNUC__ && __GNUC__ >= 11 >>> +# pragma GCC diagnostic pop >>> +#endif >> >> Likewise. > > Ditto. > >> >>> diff --git a/libio/tst-wmemstream1.c b/libio/tst-wmemstream1.c >>> index f8b308bc6c..8a0e253862 100644 >>> --- a/libio/tst-wmemstream1.c >>> +++ b/libio/tst-wmemstream1.c >>> @@ -1,5 +1,40 @@ >>>   #include >>> +/* Verify that calling fclose on the result of open_wmemstream doesn't >>> +   trigger GCC -Wmismatched-dealloc with fclose forward-declared and >>> +   without included first (it is included later, in. >>> +   "tst-memstream1.c").  */ >>> + >>> +extern int fclose (FILE*); >>> + >>> +#if defined __GNUC__ && __GNUC__ >= 11 >>> +#pragma GCC diagnostic push >>> +#pragma GCC diagnostic error "-Wmismatched-dealloc" >>> +#endif >> >> Likewise. > > The comment is just above so I moved it down to the #if block. > >> >>> +#if defined __GNUC__ && __GNUC__ >= 11 >>> +# pragma GCC diagnostic pop >>> +#endif >> >> Likewise. >> >>> diff --git a/libio/tst-wmemstream5.c b/libio/tst-wmemstream5.c >>> new file mode 100644 >>> index 0000000000..64461dbe48 >>> --- /dev/null >>> +++ b/libio/tst-wmemstream5.c >>> @@ -0,0 +1,40 @@ >> >> Missing file header. > > I copied tst-wmemstream1.c which doesn't have a header either, but > I found one elsewhere so I added it to the new test. > >> >>> +#include >>> + >>> +/* Verify that calling fclose on the result of open_wmemstream doesn't >>> +   trigger GCC -Wmismatched-dealloc with fclose forward-declared and >>> +   without included in the same translation unit.  */ >>> + >>> +extern int fclose (FILE*); >>> + >>> +#if defined __GNUC__ && __GNUC__ >= 11 >>> +#pragma GCC diagnostic push >>> +#pragma GCC diagnostic error "-Wmismatched-dealloc" >>> +#endif >> >> Likewise. > > Okay. > >> >>> diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h >>> index 3aa27a9d25..f88f8e55b3 100644 >>> --- a/stdlib/stdlib.h >>> +++ b/stdlib/stdlib.h >> >>>   #if defined __USE_MISC || defined __USE_XOPEN_EXTENDED >>> @@ -798,7 +804,8 @@ extern char *canonicalize_file_name (const char >>> *__name) >>>      ENAMETOOLONG; if the name fits in fewer than PATH_MAX chars, >>>      returns the name in RESOLVED.  */ >>>   extern char *realpath (const char *__restrict __name, >>> -               char *__restrict __resolved) __THROW __wur; >>> +               char *__restrict __resolved) __THROW >>> +     __attr_dealloc_free __wur; >>>   #endif >> >> realpath only returns a pointer to the heap if RESOLVED is null, so the >> annotation is wrong here. >> > > This is intentional.  When realpath() returns the last argument > (when it's nonnull) passing the returned pointer to free will not > be diagnosed but passing it to some other deallocator not associated > with the function will be.  That means for example that passing > a pointer allocated by C++ operator new() to realpath() and then > deleting the pointer returned from the function as opposed to > the argument will trigger a false positive.  I decided this was > an okay trade-off because unless the function allocates memory > I expect the returned pointer to be ignored (similarly to how > the pointer returned from memcpy is ignored).  If you don't like > the odds I can remove the attribute from the function until we > have one that captures this conditional return value (I'd like > to add one in GCC 12). > >>> diff --git a/wcsmbs/wchar.h b/wcsmbs/wchar.h >>> index 9cf8b05a87..e31734158c 100644 >>> --- a/wcsmbs/wchar.h >>> +++ b/wcsmbs/wchar.h >>> @@ -151,7 +151,8 @@ extern size_t wcsxfrm_l (wchar_t *__s1, const >>> wchar_t *__s2, >>>                size_t __n, locale_t __loc) __THROW; >>>   /* Duplicate S, returning an identical malloc'd string.  */ >>> -extern wchar_t *wcsdup (const wchar_t *__s) __THROW >>> __attribute_malloc__; >>> +extern wchar_t *wcsdup (const wchar_t *__s) __THROW >>> +  __attribute_malloc__ __attr_dealloc_free; >>>   #endif >>>   /* Find the first occurrence of WC in WCS.  */ >>> @@ -562,9 +563,23 @@ extern wchar_t *wcpncpy (wchar_t *__restrict >>> __dest, >>>   /* Wide character I/O functions.  */ >>>   #if defined __USE_XOPEN2K8 || __GLIBC_USE (LIB_EXT2) >>> +# ifndef __attr_dealloc_fclose >>> +#   if defined __has_builtin >>> +#     if __has_builtin (__builtin_fclose) >>> +/* If the attribute macro hasn't been defined yet (by ) and >>> +   fclose is a built-in, use it.  */ >>> +#      define __attr_dealloc_fclose __attr_dealloc >>> (__builtin_fclose, 1) >>> +#     endif >>> +#   endif >>> +# endif >>> +# ifndef __attr_dealloc_fclose >>> +#  define __attr_dealloc_fclose /* empty */ >>> +# endif >>> + >>>   /* Like OPEN_MEMSTREAM, but the stream is wide oriented and produces >>>      a wide character string.  */ >>> -extern __FILE *open_wmemstream (wchar_t **__bufloc, size_t >>> *__sizeloc) __THROW; >>> +extern __FILE *open_wmemstream (wchar_t **__bufloc, size_t >>> *__sizeloc) __THROW >>> +  __attribute_malloc__ __attr_dealloc_fclose; >>>   #endif >>>   #if defined __USE_ISOC95 || defined __USE_UNIX98 >> >> Does this mean that if one includes first and then >> to get the declaration of fclose, fclose is not registered as a >> deallocator for open_wmemstream? > > Yes. > >> >> Maybe it is possible to redeclare open_wmemstream in if >> has already been included? > > It is, and I suggested that in my last reply but didn't implement > it because I didn't expect it to be a palatable.  I've added it > in the updated revision. > > Martin --------------E63FE44990BE039CE174F10E Content-Type: text/x-patch; charset=UTF-8; name="glibc-attr-malloc.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="glibc-attr-malloc.diff" diff --git a/include/programs/xmalloc.h b/include/programs/xmalloc.h index 76e345f4ee..33871e22ef 100644 --- a/include/programs/xmalloc.h +++ b/include/programs/xmalloc.h @@ -23,11 +23,11 @@ /* Prototypes for a few program-wide used functions. */ extern void *xmalloc (size_t n) - __attribute_malloc__ __attribute_alloc_size__ ((1)); + __attribute_malloc__ __attribute_alloc_size__ ((1)) __attr_dealloc_free; extern void *xcalloc (size_t n, size_t s) - __attribute_malloc__ __attribute_alloc_size__ ((1, 2)); + __attribute_malloc__ __attribute_alloc_size__ ((1, 2)) __attr_dealloc_free; extern void *xrealloc (void *o, size_t n) - __attribute_malloc__ __attribute_alloc_size__ ((2)); -extern char *xstrdup (const char *) __attribute_malloc__; + __attribute_malloc__ __attribute_alloc_size__ ((2)) __attr_dealloc_free; +extern char *xstrdup (const char *) __attribute_malloc__ __attr_dealloc_free; #endif /* xmalloc.h */ diff --git a/libio/Makefile b/libio/Makefile index ed0ce4ee81..73f731e064 100644 --- a/libio/Makefile +++ b/libio/Makefile @@ -61,7 +61,7 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc \ bug-ungetc2 bug-ftell bug-ungetc3 bug-ungetc4 tst-fopenloc2 \ tst-memstream1 tst-memstream2 tst-memstream3 tst-memstream4 \ tst-wmemstream1 tst-wmemstream2 tst-wmemstream3 tst-wmemstream4 \ - bug-memstream1 bug-wmemstream1 \ + tst-wmemstream5 bug-memstream1 bug-wmemstream1 \ tst-setvbuf1 tst-popen1 tst-fgetwc bug-wsetpos tst-fseek \ tst-fwrite-error tst-ftell-partial-wide tst-ftell-active-handler \ tst-ftell-append tst-fputws tst-bz22415 tst-fgetc-after-eof \ diff --git a/libio/stdio.h b/libio/stdio.h index 144137cf67..4ac82ae5ff 100644 --- a/libio/stdio.h +++ b/libio/stdio.h @@ -165,22 +165,34 @@ extern int renameat2 (int __oldfd, const char *__old, int __newfd, const char *__new, unsigned int __flags) __THROW; #endif +/* Close STREAM. + + This function is a possible cancellation point and therefore not + marked with __THROW. */ +extern int fclose (FILE *__stream); + +#undef __attr_dealloc_fclose +#define __attr_dealloc_fclose __attr_dealloc (fclose, 1) + /* Create a temporary file and open it read/write. This function is a possible cancellation point and therefore not marked with __THROW. */ #ifndef __USE_FILE_OFFSET64 -extern FILE *tmpfile (void) __wur; +extern FILE *tmpfile (void) + __attribute_malloc__ __attr_dealloc_fclose __wur; #else # ifdef __REDIRECT -extern FILE *__REDIRECT (tmpfile, (void), tmpfile64) __wur; +extern FILE *__REDIRECT (tmpfile, (void), tmpfile64) + __attribute_malloc__ __attr_dealloc_fclose __wur; # else # define tmpfile tmpfile64 # endif #endif #ifdef __USE_LARGEFILE64 -extern FILE *tmpfile64 (void) __wur; +extern FILE *tmpfile64 (void) + __attribute_malloc__ __attr_dealloc_fclose __wur; #endif /* Generate a temporary filename. */ @@ -202,15 +214,9 @@ extern char *tmpnam_r (char *__s) __THROW __wur; P_tmpdir is tried and finally "/tmp". The storage for the filename is allocated by `malloc'. */ extern char *tempnam (const char *__dir, const char *__pfx) - __THROW __attribute_malloc__ __wur; + __THROW __attribute_malloc__ __wur __attr_dealloc_free; #endif - -/* Close STREAM. - - This function is a possible cancellation point and therefore not - marked with __THROW. */ -extern int fclose (FILE *__stream); /* Flush STREAM, or all streams if STREAM is NULL. This function is a possible cancellation point and therefore not @@ -244,7 +250,8 @@ extern int fcloseall (void); This function is a possible cancellation point and therefore not marked with __THROW. */ extern FILE *fopen (const char *__restrict __filename, - const char *__restrict __modes) __wur; + const char *__restrict __modes) + __attribute_malloc__ __attr_dealloc_fclose __wur; /* Open a file, replacing an existing stream with it. This function is a possible cancellation point and therefore not @@ -256,7 +263,7 @@ extern FILE *freopen (const char *__restrict __filename, # ifdef __REDIRECT extern FILE *__REDIRECT (fopen, (const char *__restrict __filename, const char *__restrict __modes), fopen64) - __wur; + __attribute_malloc__ __attr_dealloc_fclose __wur; extern FILE *__REDIRECT (freopen, (const char *__restrict __filename, const char *__restrict __modes, FILE *__restrict __stream), freopen64) @@ -268,7 +275,8 @@ extern FILE *__REDIRECT (freopen, (const char *__restrict __filename, #endif #ifdef __USE_LARGEFILE64 extern FILE *fopen64 (const char *__restrict __filename, - const char *__restrict __modes) __wur; + const char *__restrict __modes) + __attribute_malloc__ __attr_dealloc_fclose __wur; extern FILE *freopen64 (const char *__restrict __filename, const char *__restrict __modes, FILE *__restrict __stream) __wur; @@ -276,7 +284,8 @@ extern FILE *freopen64 (const char *__restrict __filename, #ifdef __USE_POSIX /* Create a new stream that refers to an existing system file descriptor. */ -extern FILE *fdopen (int __fd, const char *__modes) __THROW __wur; +extern FILE *fdopen (int __fd, const char *__modes) __THROW + __attribute_malloc__ __attr_dealloc_fclose __wur; #endif #ifdef __USE_GNU @@ -284,21 +293,30 @@ extern FILE *fdopen (int __fd, const char *__modes) __THROW __wur; and uses the given functions for input and output. */ extern FILE *fopencookie (void *__restrict __magic_cookie, const char *__restrict __modes, - cookie_io_functions_t __io_funcs) __THROW __wur; + cookie_io_functions_t __io_funcs) __THROW + __attribute_malloc__ __attr_dealloc_fclose __wur; #endif #if defined __USE_XOPEN2K8 || __GLIBC_USE (LIB_EXT2) /* Create a new stream that refers to a memory buffer. */ extern FILE *fmemopen (void *__s, size_t __len, const char *__modes) - __THROW __wur; + __THROW __attribute_malloc__ __attr_dealloc_fclose __wur; /* Open a stream that writes into a malloc'd buffer that is expanded as necessary. *BUFLOC and *SIZELOC are updated with the buffer's location and the number of characters written on fflush or fclose. */ -extern FILE *open_memstream (char **__bufloc, size_t *__sizeloc) __THROW __wur; +extern FILE *open_memstream (char **__bufloc, size_t *__sizeloc) __THROW + __attribute_malloc__ __attr_dealloc_fclose __wur; + +#ifdef _WCHAR_H +/* Like OPEN_MEMSTREAM, but the stream is wide oriented and produces + a wide character string. Declared here only to add attribute malloc + and only if has been previously #included. */ +extern __FILE *open_wmemstream (wchar_t **__bufloc, size_t *__sizeloc) __THROW + __attribute_malloc__ __attr_dealloc_fclose; +# endif #endif - /* If BUF is NULL, make STREAM unbuffered. Else make it use buffer BUF, of size BUFSIZ. */ extern void setbuf (FILE *__restrict __stream, char *__restrict __buf) __THROW; @@ -792,17 +810,19 @@ extern int fileno_unlocked (FILE *__stream) __THROW __wur; #ifdef __USE_POSIX2 -/* Create a new stream connected to a pipe running the given command. +/* Close a stream opened by popen and return the status of its child. This function is a possible cancellation point and therefore not marked with __THROW. */ -extern FILE *popen (const char *__command, const char *__modes) __wur; +extern int pclose (FILE *__stream); -/* Close a stream opened by popen and return the status of its child. +/* Create a new stream connected to a pipe running the given command. This function is a possible cancellation point and therefore not marked with __THROW. */ -extern int pclose (FILE *__stream); +extern FILE *popen (const char *__command, const char *__modes) + __attribute_malloc__ __attr_dealloc (pclose, 1) __wur; + #endif diff --git a/libio/tst-freopen.c b/libio/tst-freopen.c index 660882a28a..c8bc0a3d07 100644 --- a/libio/tst-freopen.c +++ b/libio/tst-freopen.c @@ -81,6 +81,42 @@ do_test_basic (void) fclose (f); } +#if defined __GNUC__ && __GNUC__ >= 11 +/* Force an error to detect incorrectly making freopen a deallocator + for its last argument via attribute malloc. The function closes + the stream without deallocating it so either the argument or + the pointer returned from the function (but not both) can be passed + to fclose. */ +#pragma GCC diagnostic push +#pragma GCC diagnostic error "-Wmismatched-dealloc" +#endif + +/* Verify that freopen returns stream. */ +static void +do_test_return_stream (void) +{ + FILE *f1 = fopen (name, "r"); + if (f1 == NULL) + FAIL_EXIT1 ("fopen: %m"); + + FILE *f2 = freopen (name, "r+", f1); + if (f2 == NULL) + FAIL_EXIT1 ("freopen: %m"); + + /* Verify that freopen isn't declared with the no-argument attribute + malloc (which could let GCC fold the inequality to false). */ + if (f1 != f2) + FAIL_EXIT1 ("freopen returned a different stream"); + + /* This shouldn't trigger -Wmismatched-dealloc. */ + fclose (f1); +} + +#if defined __GNUC__ && __GNUC__ >= 11 +/* Pop -Wmismatched-dealloc set to error above. */ +# pragma GCC diagnostic pop +#endif + /* Test for BZ#21398, where it tries to freopen stdio after the close of its file descriptor. */ static void @@ -105,6 +141,7 @@ do_test (void) { do_test_basic (); do_test_bz21398 (); + do_test_return_stream (); return 0; } diff --git a/libio/tst-popen1.c b/libio/tst-popen1.c index bae6615b9b..9f36b20090 100644 --- a/libio/tst-popen1.c +++ b/libio/tst-popen1.c @@ -21,7 +21,7 @@ do_test (void) res = 1; } - fclose (fp); + pclose (fp); } fp = popen ("echo hello", "re"); @@ -39,7 +39,7 @@ do_test (void) res = 1; } - fclose (fp); + pclose (fp); } return res; diff --git a/libio/tst-wmemstream1.c b/libio/tst-wmemstream1.c index f8b308bc6c..f80655b2a4 100644 --- a/libio/tst-wmemstream1.c +++ b/libio/tst-wmemstream1.c @@ -1,5 +1,40 @@ #include +extern int fclose (FILE*); + +#if defined __GNUC__ && __GNUC__ >= 11 +/* Verify that calling fclose on the result of open_wmemstream doesn't + trigger GCC -Wmismatched-dealloc with fclose forward-declared and + without included first (it is included later, in. + "tst-memstream1.c"). */ +#pragma GCC diagnostic push +#pragma GCC diagnostic error "-Wmismatched-dealloc" +#endif + +int test_open_wmemstream_no_stdio (void) +{ + { + wchar_t *buf; + size_t size; + FILE *f = open_wmemstream (&buf, &size); + fclose (f); + } + + { + FILE* (*pf)(wchar_t**, size_t*) = open_wmemstream; + wchar_t *buf; + size_t size; + FILE *f = pf (&buf, &size); + fclose (f); + } + return 0; +} + +#if defined __GNUC__ && __GNUC__ >= 11 +/* Restore -Wmismatched-dealloc setting. */ +# pragma GCC diagnostic pop +#endif + #define CHAR_T wchar_t #define W(o) L##o #define OPEN_MEMSTREAM open_wmemstream diff --git a/libio/tst-wmemstream5.c b/libio/tst-wmemstream5.c new file mode 100644 index 0000000000..47f5e63603 --- /dev/null +++ b/libio/tst-wmemstream5.c @@ -0,0 +1,57 @@ +/* Copyright (C) 2021 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 + +extern int fclose (FILE*); + +#if defined __GNUC__ && __GNUC__ >= 11 +/* Verify that calling fclose on the result of open_wmemstream doesn't + trigger GCC -Wmismatched-dealloc with fclose forward-declared and + without included in the same translation unit. */ +#pragma GCC diagnostic push +#pragma GCC diagnostic error "-Wmismatched-dealloc" +#endif + +static int +do_test (void) +{ + { + wchar_t *buf; + size_t size; + FILE *f = open_wmemstream (&buf, &size); + fclose (f); + } + + { + FILE* (*pf)(wchar_t**, size_t*) = open_wmemstream; + wchar_t *buf; + size_t size; + FILE *f = pf (&buf, &size); + fclose (f); + } + + return 0; +} + +#if defined __GNUC__ && __GNUC__ >= 11 +/* Restore -Wmismatched-dealloc setting. */ +# pragma GCC diagnostic pop +#endif + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" diff --git a/malloc/malloc.h b/malloc/malloc.h index e9157ddfc3..c1c0896d29 100644 --- a/malloc/malloc.h +++ b/malloc/malloc.h @@ -56,22 +56,25 @@ __THROW __attribute_warn_unused_result__ __attribute_alloc_size__ ((2)); the same pointer that was passed to it, aliasing needs to be allowed between objects pointed by the old and new pointers. */ extern void *reallocarray (void *__ptr, size_t __nmemb, size_t __size) -__THROW __attribute_warn_unused_result__ __attribute_alloc_size__ ((2, 3)); + __THROW __attribute_warn_unused_result__ __attribute_alloc_size__ ((2, 3)) + __attr_dealloc_free; /* Free a block allocated by `malloc', `realloc' or `calloc'. */ extern void free (void *__ptr) __THROW; /* Allocate SIZE bytes allocated to ALIGNMENT bytes. */ extern void *memalign (size_t __alignment, size_t __size) -__THROW __attribute_malloc__ __attribute_alloc_size__ ((2)) __wur; + __THROW __attribute_malloc__ __attribute_alloc_size__ ((2)) __wur + __attr_dealloc_free; /* Allocate SIZE bytes on a page boundary. */ extern void *valloc (size_t __size) __THROW __attribute_malloc__ - __attribute_alloc_size__ ((1)) __wur; + __attribute_alloc_size__ ((1)) __wur __attr_dealloc_free; /* Equivalent to valloc(minimum-page-that-holds(n)), that is, round up __size to nearest pagesize. */ -extern void *pvalloc (size_t __size) __THROW __attribute_malloc__ __wur; +extern void *pvalloc (size_t __size) __THROW __attribute_malloc__ + __wur __attr_dealloc_free; /* Underlying allocation function; successive calls should return contiguous pieces of memory. */ diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h index 8e244a77cf..3c4e6baf81 100644 --- a/misc/sys/cdefs.h +++ b/misc/sys/cdefs.h @@ -592,11 +592,22 @@ _Static_assert (0, "IEEE 128-bits long double requires redirection on this platf array according to access mode, or at least one element when size-index is not provided: access (access-mode, [, ]) */ -#define __attr_access(x) __attribute__ ((__access__ x)) +# define __attr_access(x) __attribute__ ((__access__ x)) #else # define __attr_access(x) #endif +#if __GNUC_PREREQ (11, 0) +/* Designates dealloc as a function to call to deallocate objects + allocated by the declared function. */ +# define __attr_dealloc(dealloc, argno) \ + __attribute__ ((__malloc__ (dealloc, argno))) +# define __attr_dealloc_free __attr_dealloc (__builtin_free, 1) +#else +# define __attr_dealloc(x) +# define __attr_dealloc_free +#endif + /* Specify that a function such as setjmp or vfork may return twice. */ #if __GNUC_PREREQ (4, 1) diff --git a/stdlib/Makefile b/stdlib/Makefile index b3b30ab73e..ec18673e5a 100644 --- a/stdlib/Makefile +++ b/stdlib/Makefile @@ -86,7 +86,8 @@ tests := tst-strtol tst-strtod testmb testrand testsort testdiv \ tst-makecontext-align test-bz22786 tst-strtod-nan-sign \ tst-swapcontext1 tst-setcontext4 tst-setcontext5 \ tst-setcontext6 tst-setcontext7 tst-setcontext8 \ - tst-setcontext9 tst-bz20544 tst-canon-bz26341 + tst-setcontext9 tst-bz20544 tst-canon-bz26341 \ + tst-realpath tests-internal := tst-strtod1i tst-strtod3 tst-strtod4 tst-strtod5i \ tst-tls-atexit tst-tls-atexit-nodelete diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h index 6360845d98..0481c12355 100644 --- a/stdlib/stdlib.h +++ b/stdlib/stdlib.h @@ -550,6 +550,9 @@ extern void *calloc (size_t __nmemb, size_t __size) extern void *realloc (void *__ptr, size_t __size) __THROW __attribute_warn_unused_result__ __attribute_alloc_size__ ((2)); +/* Free a block allocated by `malloc', `realloc' or `calloc'. */ +extern void free (void *__ptr) __THROW; + #ifdef __USE_MISC /* Re-allocate the previously allocated block in PTR, making the new block large enough for NMEMB elements of SIZE bytes each. */ @@ -558,11 +561,13 @@ extern void *realloc (void *__ptr, size_t __size) between objects pointed by the old and new pointers. */ extern void *reallocarray (void *__ptr, size_t __nmemb, size_t __size) __THROW __attribute_warn_unused_result__ - __attribute_alloc_size__ ((2, 3)); -#endif + __attribute_alloc_size__ ((2, 3)) + __attr_dealloc_free; -/* Free a block allocated by `malloc', `realloc' or `calloc'. */ -extern void free (void *__ptr) __THROW; +/* Add reallocarray as its own deallocator. */ +extern void *reallocarray (void *__ptr, size_t __nmemb, size_t __size) + __THROW __attr_dealloc (reallocarray, 1); +#endif #ifdef __USE_MISC # include @@ -788,7 +793,8 @@ extern int system (const char *__command) __wur; /* Return a malloc'd string containing the canonical absolute name of the existing named file. */ extern char *canonicalize_file_name (const char *__name) - __THROW __nonnull ((1)) __wur; + __THROW __nonnull ((1)) __attribute_malloc__ + __attr_dealloc_free __wur; #endif #if defined __USE_MISC || defined __USE_XOPEN_EXTENDED diff --git a/stdlib/tst-realpath.c b/stdlib/tst-realpath.c new file mode 100644 index 0000000000..48a42110be --- /dev/null +++ b/stdlib/tst-realpath.c @@ -0,0 +1,80 @@ +/* Test to verify that realpath() doesn't cause false positives due + to GCC attribute malloc. + + Test failure exposes the presence of the attribute in the following + declaration: + + __attribute__ ((__malloc__ (free, 1))) char* + realpath (const char *, char *); + + Copyright (C) 2021 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 + +#if defined __GNUC__ && __GNUC__ >= 11 +/* Turn GCC -Wmismatched-dealloc warnings into errors to expose false + positives. */ +# pragma GCC diagnostic push +# pragma GCC diagnostic error "-Wmismatched-dealloc" + +/* Associate dealloc as the only deallocator suitable for pointers + returned from alloc. + GCC automatically disables inlining of allocator and deallocator + functions marked with the argument form of attribute malloc but + it doesn't hurt to disable it explicitly. */ +__attribute ((noipa)) void dealloc (void *); +__attribute ((malloc (dealloc, 1))) char* alloc (void); +#endif + +void dealloc (void *p) +{ + free (p); +} + +char* alloc (void) +{ + return (char *)malloc (8); +} + +static int +do_test (void) +{ + char *resolved_path = alloc (); + char *ret = realpath ("/", resolved_path); + dealloc (ret); + + resolved_path = alloc (); + ret = realpath ("/", resolved_path); + dealloc (resolved_path); + + /* The following should emit a warning (but doesn't with GCC 11): + resolved_path = alloc (); + ret = realpath ("/", resolved_path); + free (ret); // expect -Wmismatched-dealloc + */ + + return 0; +} + +#if defined __GNUC__ && __GNUC__ >= 11 +/* Restore -Wmismatched-dealloc setting. */ +# pragma GCC diagnostic pop +#endif + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" diff --git a/wcsmbs/wchar.h b/wcsmbs/wchar.h index ce0acb1c28..075776890f 100644 --- a/wcsmbs/wchar.h +++ b/wcsmbs/wchar.h @@ -151,7 +151,8 @@ extern size_t wcsxfrm_l (wchar_t *__s1, const wchar_t *__s2, size_t __n, locale_t __loc) __THROW; /* Duplicate S, returning an identical malloc'd string. */ -extern wchar_t *wcsdup (const wchar_t *__s) __THROW __attribute_malloc__; +extern wchar_t *wcsdup (const wchar_t *__s) __THROW + __attribute_malloc__ __attr_dealloc_free; #endif /* Find the first occurrence of WC in WCS. */ @@ -562,9 +563,23 @@ extern wchar_t *wcpncpy (wchar_t *__restrict __dest, /* Wide character I/O functions. */ #if defined __USE_XOPEN2K8 || __GLIBC_USE (LIB_EXT2) +# ifndef __attr_dealloc_fclose +# if defined __has_builtin +# if __has_builtin (__builtin_fclose) +/* If the attribute macro hasn't been defined yet (by ) and + fclose is a built-in, use it. */ +# define __attr_dealloc_fclose __attr_dealloc (__builtin_fclose, 1) +# endif +# endif +# endif +# ifndef __attr_dealloc_fclose +# define __attr_dealloc_fclose /* empty */ +# endif + /* Like OPEN_MEMSTREAM, but the stream is wide oriented and produces a wide character string. */ -extern __FILE *open_wmemstream (wchar_t **__bufloc, size_t *__sizeloc) __THROW; +extern __FILE *open_wmemstream (wchar_t **__bufloc, size_t *__sizeloc) __THROW + __attribute_malloc__ __attr_dealloc_fclose; #endif #if defined __USE_ISOC95 || defined __USE_UNIX98 --------------E63FE44990BE039CE174F10E--