From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x735.google.com (mail-qk1-x735.google.com [IPv6:2607:f8b0:4864:20::735]) by sourceware.org (Postfix) with ESMTPS id E6366385800A for ; Thu, 13 May 2021 21:49:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E6366385800A Received: by mail-qk1-x735.google.com with SMTP id f18so5157284qko.7 for ; Thu, 13 May 2021 14:49:43 -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=PVrF+3NApnquj8aAmZ1Ie77+8E6vKVb71KIZCiAEsKQ=; b=N92F8/yF9pbU5UZu7oRo91G6io1btIsr+YsNelJbvehzK2t50X53IExrr1g57jIk5b 1JhawZJcyWh0o3Laf5UFmXOIR1gLpscTRL5xY8Z1jqg7KNO+MbYBHJsxAR672gplbCfO 9s6IJpdcY1MWcwLMg6rDYO1gv0ppk9+sE5FrTaSQVZW1/wiUZXZ3RQNPYn8UG7yvgezz DI/mPPZwJnVJwqBOikpBJ6KxcXw3euM5oDXCaftx7N5DSsbvBi/ODesIRnXSGjYeI5nb FmANO+OfuUI4bZkYJA7Y9iLQylV2Vh2Yy2O5fwc9O7bAOo7zLgYUAJ52mMWbllmBVWAI QEwQ== X-Gm-Message-State: AOAM531LYyn40XlqcHGrhQlWojIbMbAqkO4aPAw/wAGB9plr2Zh3e/Rg SUscDBtNdbYXwFsY3bQeB4uq0SnxYtU= X-Google-Smtp-Source: ABdhPJxLBoY4+Q5FnXzEWoVUwaZMJmZZtTy1QmcBxjWyGlKjkEDlbnbzOLScqF9OrMPZoxJGtGywlg== X-Received: by 2002:a05:620a:1206:: with SMTP id u6mr26183914qkj.136.1620942583554; Thu, 13 May 2021 14:49:43 -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 f2sm3397605qkh.76.2021.05.13.14.49.42 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 13 May 2021 14:49:42 -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> <5ed3fe4d-c0e4-632f-2569-b60d29cf4995@gmail.com> Message-ID: <3f8259c3-fab9-ec20-4f73-174adf42d8ba@gmail.com> Date: Thu, 13 May 2021 15:49:41 -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="------------3E468410A50820960C4ABE45" Content-Language: en-US X-Spam-Status: No, score=-8.9 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: Thu, 13 May 2021 21:49:54 -0000 This is a multi-part message in MIME format. --------------3E468410A50820960C4ABE45 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Attached is the final patch tested with GCC 10 and 11 (the latter both 32- and 64-bit this time). If there are no objections I'll commit it tomorrow. Florian and/or anyone else who provided comments on it: please let me know if you would like me to mention you in the Reviewed-by tag. Thanks Martin On 5/6/21 5:54 PM, Martin Sebor wrote: > If there is no further discussion I'll retest this patch with the top > of the trunk and a few recent GCC releases and if no problems turn up > plan to commit it next week.  Please let me know if you have any > concerns. > > (I already know about the bug in the dummy definition of > __attr_dealloc(x) with GCC versions prior: it only takes one argument > instead of two.) > > On 4/22/21 6:00 PM, Martin Sebor wrote: >> 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 >> > --------------3E468410A50820960C4ABE45 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 76bda3728e..497da016ff 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[L_tmpnam]) __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 11f01f741b..30a621ab8f 100644 --- a/misc/sys/cdefs.h +++ b/misc/sys/cdefs.h @@ -603,6 +603,17 @@ _Static_assert (0, "IEEE 128-bits long double requires redirection on this platf # define __attr_access_none(argno) #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(dealloc, argno) +# 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..2ae5e4fb2b --- /dev/null +++ b/stdlib/tst-realpath.c @@ -0,0 +1,82 @@ +/* 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 +#include +#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 --------------3E468410A50820960C4ABE45--