public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Martin Sebor <msebor@gmail.com>
To: Florian Weimer <fweimer@redhat.com>
Cc: libc-alpha@sourceware.org, Joseph Myers <joseph@codesourcery.com>
Subject: Re: [PATCH] add support for -Wmismatched-dealloc
Date: Thu, 22 Apr 2021 18:00:52 -0600	[thread overview]
Message-ID: <5ed3fe4d-c0e4-632f-2569-b60d29cf4995@gmail.com> (raw)
In-Reply-To: <e78b5e6d-391b-5312-cfde-5944bc51f7d3@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 7143 bytes --]

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 <wchar.h>
>>> +/* Verify that calling fclose on the result of open_wmemstream doesn't
>>> +   trigger GCC -Wmismatched-dealloc with fclose forward-declared and
>>> +   without <stdio.h> 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 <wchar.h>
>>> +
>>> +/* Verify that calling fclose on the result of open_wmemstream doesn't
>>> +   trigger GCC -Wmismatched-dealloc with fclose forward-declared and
>>> +   without <stdio.h> 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 <stdio.h>) 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 <wchar.h> first and then <stdio.h>
>> 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 <stdio.h> if
>> <wchar.h> 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


[-- Attachment #2: glibc-attr-malloc.diff --]
[-- Type: text/x-patch, Size: 21868 bytes --]

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 <wchar.h> 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 <wchar.h>
 
+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 <stdio.h> 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <wchar.h>
+
+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 <stdio.h> 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, <ref-index> [, <size-index>])  */
-#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 <alloca.h>
@@ -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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+
+#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 <stdio.h>) 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

  parent reply	other threads:[~2021-04-23  0:00 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-08 22:52 Martin Sebor
2020-12-09  0:07 ` Joseph Myers
2020-12-12  2:25   ` Martin Sebor
2020-12-14 21:39     ` Martin Sebor
2020-12-14 22:16       ` Florian Weimer
2020-12-15  1:01       ` Joseph Myers
2020-12-15 16:52         ` Martin Sebor
2020-12-27 23:13           ` Martin Sebor
2021-01-04 15:56             ` Ping: " Martin Sebor
2021-01-04 16:07             ` Florian Weimer
2021-01-04 16:18               ` Martin Sebor
2021-01-04 16:57                 ` Florian Weimer
2021-01-04 23:18                   ` Martin Sebor
2021-01-10 20:42                     ` Ping: " Martin Sebor
2021-01-11  9:13                     ` Florian Weimer
2021-01-12  0:00                       ` Martin Sebor
2021-01-12  0:01                       ` Martin Sebor
2021-01-12  8:59                         ` Florian Weimer
2021-01-19  1:08                           ` Martin Sebor
2021-01-19 16:54                           ` David Malcolm
2021-01-22 21:26                         ` DJ Delorie
2021-01-25 10:56                         ` Florian Weimer
2021-01-25 11:31                         ` Florian Weimer
2021-04-23  0:00                         ` Martin Sebor [this message]
2021-05-06 23:54                           ` Martin Sebor
2021-05-13 21:49                             ` Martin Sebor
2021-05-16 21:25                               ` Martin Sebor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5ed3fe4d-c0e4-632f-2569-b60d29cf4995@gmail.com \
    --to=msebor@gmail.com \
    --cc=fweimer@redhat.com \
    --cc=joseph@codesourcery.com \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).