[PATCH] add support for -Wmismatched-dealloc

[PATCH] add support for -Wmismatched-dealloc

[Martin Sebor]

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

To help detect common kinds of memory (and other resource) management
bugs, GCC 11 adds support for the detection of mismatched calls to
allocation and deallocation functions.  At each call site to a known
deallocation function GCC checks the set of allocation functions
the former can be paired with and, if the two don't match, issues
a -Wmismatched-dealloc warning (something similar happens in C++
for mismatched calls to new and delete).  GCC also uses the same
mechanism to detect attempts to deallocate objects not allocated
by any allocation function (or pointers past the first byte into
allocated objects) by -Wfree-nonheap-object.

This support is enabled for built-in functions like malloc and free.
To extend it beyond those, GCC extends attribute malloc to designate
a deallocation function to which pointers returned from the allocation
function may be passed to deallocate the allocated objects.  Another,
optional argument designates the positional argument to which
the pointer must be passed.

The attached change is the first step in enabling this extended support
for Glibc.  It enables GCC to diagnose mismatched calls such as in
tst-popen.c1:

   FILE *f = popen ("foo", "r");
   ...
   fclose (f);

   warning: ‘fclose’ called on pointer returned from a mismatched 
allocation function [-Wmismatched-dealloc]
      83 |     fclose (f);
         |     ^~~~~~~~~~
   note: returned from ‘popen’
      81 |     FILE *f = popen ("foo", "r");
         |               ^~~~~~~~~~~~~~~~~~

Since <stdio.h> doesn't declare free() and realloc() the patch uses
__builtin_free and __builtin_realloc as the deallocators.  This isn't
pretty but I couldn't think of a more elegant way to do it.  (I also
missed this bit in the initial implementation of the attribute so
current trunk doesn't handle the __builtin_xxx forms.  I submitted
a patch to handle it just today, but to avoid delays, I post this
solution for comments ahead of its acceptance).

Since the deallocation function referenced by the malloc attribute
must have been declared, the changes require rearranging the order
of the declarations a bit.

I tested the changes by rebuilding Glibc on x86_64 and by verifying
that warnings are issued where appropriate for my little test program
but not much else.  Neither set of changes (either in GCC or in Glibc)
impacts code generation and should make no difference in testing
(I didn't see any.)

If there is support for this approach I'll work on extending it to
other headers, hopefully before the upcoming Glibc release.

Thanks
Martin

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

diff --git a/libio/stdio.h b/libio/stdio.h
index 59f2ee01ab..f7ee14a9fd 100644
--- a/libio/stdio.h
+++ b/libio/stdio.h
@@ -165,22 +165,62 @@ 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);
+
+/* This function is a possible cancellation point and therefore not
+   marked with __THROW.  It's paired with fclose as a deallocator,
+   and (in a subsequent redeclarations) also with itself.  */
+extern FILE *freopen (const char *__restrict __filename,
+		      const char *__restrict __modes,
+		      FILE *__restrict __stream) __wur
+  __attribute_malloc__ __attr_dealloc (fclose, 1);
+
+#ifdef __USE_LARGEFILE64
+extern FILE *freopen64 (const char *__restrict __filename,
+			const char *__restrict __modes,
+			FILE *__restrict __stream) __wur
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __attr_dealloc (freopen, 3);
+extern FILE *freopen64 (const char *__restrict __filename,
+			const char *__restrict __modes,
+			FILE *__restrict __stream) __wur
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __attr_dealloc (freopen, 3)
+  __attr_dealloc (freopen64, 3);
+extern FILE *fopen64 (const char *__restrict __filename,
+		      const char *__restrict __modes) __wur
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __attr_dealloc (freopen, 3)
+  __attr_dealloc (freopen64, 3);
+
+#  define __attr_dealloc_freopen64 __attr_dealloc (freopen64, 3)
+#else
+#  define __attr_dealloc_freopen64 /* empty */
+#endif
+
 /* 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) __wur
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __attr_dealloc (freopen, 3)
+  __attr_dealloc_freopen64;
 #else
 # ifdef __REDIRECT
-extern FILE *__REDIRECT (tmpfile, (void), tmpfile64) __wur;
+extern FILE *__REDIRECT (tmpfile, (void), tmpfile64) __wur
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __attr_dealloc (freopen, 3)
+  __attr_dealloc_freopen64;
 # else
 #  define tmpfile tmpfile64
 # endif
 #endif
 
 #ifdef __USE_LARGEFILE64
-extern FILE *tmpfile64 (void) __wur;
+extern FILE *tmpfile64 (void) __wur
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __attr_dealloc (freopen, 3)
+  __attr_dealloc_freopen64;
 #endif
 
 /* Generate a temporary filename.  */
@@ -201,16 +241,21 @@ extern char *tmpnam_r (char __s[L_tmpnam]) __THROW __wur;
    If not and if DIR is not NULL, that value is checked.  If that fails,
    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;
-#endif
-
-
-/* Close STREAM.
+extern char *tempnam (const char *__dir, const char *__pfx) __wur
+  __attribute_malloc__
+# ifdef __has_builtin
+/* Reference the builtins since the declaration of neither function need be
+   in scope at this point.  */
+#   if __has_builtin (__builtin_free)
+      __attr_dealloc (__builtin_free, 1)
+#   endif
+#   if __has_builtin (__builtin_realloc)
+      __attr_dealloc (__builtin_realloc, 1)
+#   endif
+#  endif
+# endif
+  __THROW;
 
-   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,39 +289,40 @@ 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) __wur
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __attr_dealloc (freopen, 3)
+  __attr_dealloc_freopen64;
 /* Open a file, replacing an existing stream with it.
 
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
 extern FILE *freopen (const char *__restrict __filename,
 		      const char *__restrict __modes,
-		      FILE *__restrict __stream) __wur;
+		      FILE *__restrict __stream) __wur
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __attr_dealloc (freopen, 3)
+  __attr_dealloc_freopen64;
 #else
 # ifdef __REDIRECT
 extern FILE *__REDIRECT (fopen, (const char *__restrict __filename,
 				 const char *__restrict __modes), fopen64)
-  __wur;
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __attr_dealloc (freopen, 3)
+  __attr_dealloc_freopen64 __wur;
 extern FILE *__REDIRECT (freopen, (const char *__restrict __filename,
 				   const char *__restrict __modes,
 				   FILE *__restrict __stream), freopen64)
-  __wur;
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __attr_dealloc (freopen, 3)
+   __attr_dealloc_freopen64 __wur;
 # else
 #  define fopen fopen64
 #  define freopen freopen64
 # endif
 #endif
-#ifdef __USE_LARGEFILE64
-extern FILE *fopen64 (const char *__restrict __filename,
-		      const char *__restrict __modes) __wur;
-extern FILE *freopen64 (const char *__restrict __filename,
-			const char *__restrict __modes,
-			FILE *__restrict __stream) __wur;
-#endif
 
 #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 __wur
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __attr_dealloc (freopen, 3)
+  __attr_dealloc_freopen64;
 #endif
 
 #ifdef	__USE_GNU
@@ -284,18 +330,24 @@ 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 __wur
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __attr_dealloc (freopen, 3)
+  __attr_dealloc_freopen64;
 #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 __wur
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __attr_dealloc (freopen, 3)
+  __attr_dealloc_freopen64;
 
 /* 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 __wur
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __attr_dealloc (freopen, 3)
+  __attr_dealloc_freopen64;
 #endif
 
 
@@ -792,17 +844,17 @@ 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;
-
-/* Close a stream opened by popen and return the status of its child.
+extern int pclose (FILE *__stream);
+/* 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) __wur
+  __attribute_malloc__ __attr_dealloc (pclose, 1);
 #endif
 
 
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;

Re: [PATCH] add support for -Wmismatched-dealloc

[Joseph Myers]

I don't see any definition of __attr_dealloc (presumably should be a macro 
in misc/sys/cdefs.h) in this patch (or in the glibc source tree).

Given all the functions in stdio.h with the same list of deallocation 
functions, there should probably be another macro there to expand to 
__attribute_malloc__ __attr_dealloc (fclose, 1) __attr_dealloc (freopen, 
3) __attr_dealloc_freopen64.  (That would also apply to open_wmemstream in 
wchar.h, but I suppose you have the issue there with functions not being 
declared in wchar.h, only in stdio.h?)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] add support for -Wmismatched-dealloc

[Martin Sebor]

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

On 12/8/20 5:07 PM, Joseph Myers wrote:
> I don't see any definition of __attr_dealloc (presumably should be a macro
> in misc/sys/cdefs.h) in this patch (or in the glibc source tree).

Whoops!

> 
> Given all the functions in stdio.h with the same list of deallocation
> functions, there should probably be another macro there to expand to
> __attribute_malloc__ __attr_dealloc (fclose, 1) __attr_dealloc (freopen,
> 3) __attr_dealloc_freopen64.  (That would also apply to open_wmemstream in
> wchar.h, but I suppose you have the issue there with functions not being
> declared in wchar.h, only in stdio.h?)

You're right that adding the attribute to open_wmemstream runs into
the same problem as declaring the ctermid argument with L_ctermid
elements in <unistd.h>: fclose isn't declared in <wchar.h>.  I did
some more work on adding the attribute to other functions and found
out that the same problem also happens between tempnam() in <stdio.h>
and reallocarray() in <stdlib.h>.

I spent some time working around this but in the end it turned out
to be too convoluted so I decided to make the attribute a little
smarter.  Instead of associating all allocation functions with all
deallocation functions (such as fdopen, fopen, fopen64, etc. with
fclose, freopen, and freopen64) I changed it so that an allocator
only needs to be associated with a single deallocator (a reallocator
also needs to be associated with itself).  That makes things quite
a bit simpler.

The attached patch implements this for <stdio.h>, <stdlib.h>, and
<wchar.h>.  To get around the <wchar.h> dependency on <stdio.h> it
uses __REDIRECT to introduce a reserved alias for fclose.

Besides running the test suite I tested it with my own test and also
by adding the same declarations to the GCC test suite and verifying
it triggers warnings as expected.

The GCC patches needed to make this simpler scheme work haven't been
reviewed yet so this work has a dependency on them getting approved.

I grepped for __attribute_malloc__ in Glibc headers to see if there
are other APIs that would benefit from the same annotation but found
none.  At the same time, I don't have the impression that malloc is
used on all the APIs it could be.  Are there any that you or anyone
else can think of that might be worth looking at?

Martin

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

diff --git a/libio/stdio.h b/libio/stdio.h
index 998470943e..7b89090ea7 100644
--- a/libio/stdio.h
+++ b/libio/stdio.h
@@ -165,22 +165,31 @@ 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);
+
 /* 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, 1) __wur;
 #else
 # ifdef __REDIRECT
-extern FILE *__REDIRECT (tmpfile, (void), tmpfile64) __wur;
+extern FILE *__REDIRECT (tmpfile, (void), tmpfile64)
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __wur;
 # else
 #  define tmpfile tmpfile64
 # endif
 #endif
 
 #ifdef __USE_LARGEFILE64
-extern FILE *tmpfile64 (void) __wur;
+extern FILE *tmpfile64 (void)
+   __attribute_malloc__ __attr_dealloc (fclose, 1) __wur;
 #endif
 
 /* Generate a temporary filename.  */
@@ -202,15 +211,17 @@ 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;
+  __attribute_malloc__ __wur
+# ifdef __has_builtin
+/* Reference the builtin since the ordinary declaration need not be
+   in scope at this point.  */
+#   if __has_builtin (__builtin_free)
+      __attr_dealloc (__builtin_free, 1)
+#   endif
+# endif
+  __THROW;
 #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,23 +255,35 @@ 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, 1) __wur;
 /* Open a file, replacing an existing stream with it.
 
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
 extern FILE *freopen (const char *__restrict __filename,
 		      const char *__restrict __modes,
-		      FILE *__restrict __stream) __wur;
+		      FILE *__restrict __stream)
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __wur;
+/* Make freopen its own deallocator.  */
+extern FILE *freopen (const char *__restrict __filename,
+		      const char *__restrict __modes,
+		      FILE *__restrict __stream)
+  __attr_dealloc (freopen, 3);
 #else
 # ifdef __REDIRECT
 extern FILE *__REDIRECT (fopen, (const char *__restrict __filename,
 				 const char *__restrict __modes), fopen64)
-  __wur;
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __wur;
+extern FILE *__REDIRECT (freopen, (const char *__restrict __filename,
+				   const char *__restrict __modes,
+				   FILE *__restrict __stream), freopen64)
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __wur;
+/* Make freopen its own deallocator.  */
 extern FILE *__REDIRECT (freopen, (const char *__restrict __filename,
 				   const char *__restrict __modes,
 				   FILE *__restrict __stream), freopen64)
-  __wur;
+  __attr_dealloc (freopen, 3);
 # else
 #  define fopen fopen64
 #  define freopen freopen64
@@ -268,15 +291,23 @@ 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, 1) __wur;
 extern FILE *freopen64 (const char *__restrict __filename,
 			const char *__restrict __modes,
-			FILE *__restrict __stream) __wur;
+			FILE *__restrict __stream)
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __wur;
+/* Make freopen64 its own deallocator.  */
+extern FILE *freopen64 (const char *__restrict __filename,
+			const char *__restrict __modes,
+			FILE *__restrict __stream)
+  __attr_dealloc (freopen64, 3);
 #endif
 
 #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, 1) __wur;
 #endif
 
 #ifdef	__USE_GNU
@@ -284,18 +315,20 @@ 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, 1) __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, 1) __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, 1) __wur;
 #endif
 
 
@@ -792,17 +825,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-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/misc/sys/cdefs.h b/misc/sys/cdefs.h
index e94d09d7dd..7bc99ec2f5 100644
--- a/misc/sys/cdefs.h
+++ b/misc/sys/cdefs.h
@@ -560,6 +560,15 @@ _Static_assert (0, "IEEE 128-bits long double requires redirection on this platf
 #  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)))
+#else
+# define __attr_dealloc(x)
+#endif
+
 /* Specify that a function such as setjmp or vfork may return
    twice.  */
 #if __GNUC_PREREQ (4, 1)
diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
index 3aa27a9d25..a039f17d97 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, 1);
 
-/* 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)
+     __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, 1) __wur;
 #endif
 
 #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, 1) __wur;
 #endif
 
 
diff --git a/wcsmbs/wchar.h b/wcsmbs/wchar.h
index 9cf8b05a87..7442c80753 100644
--- a/wcsmbs/wchar.h
+++ b/wcsmbs/wchar.h
@@ -562,9 +562,18 @@ extern wchar_t *wcpncpy (wchar_t *__restrict __dest,
 /* Wide character I/O functions.  */
 
 #if defined __USE_XOPEN2K8 || __GLIBC_USE (LIB_EXT2)
+#  ifdef __REDIRECT
+/* Declare the __fclose alias and associate it as a deallocator
+   with open_memstream below.  */
+extern char *__REDIRECT (__fclose, (FILE *), fclose);
+#    define __attr_dealloc_fclose __attr_dealloc (__fclose, 1)
+#  else
+#    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

Re: [PATCH] add support for -Wmismatched-dealloc

[Martin Sebor]

On 12/11/20 7:25 PM, Martin Sebor wrote:
> On 12/8/20 5:07 PM, Joseph Myers wrote:
>> I don't see any definition of __attr_dealloc (presumably should be a 
>> macro
>> in misc/sys/cdefs.h) in this patch (or in the glibc source tree).
> 
> Whoops!
> 
>>
>> Given all the functions in stdio.h with the same list of deallocation
>> functions, there should probably be another macro there to expand to
>> __attribute_malloc__ __attr_dealloc (fclose, 1) __attr_dealloc (freopen,
>> 3) __attr_dealloc_freopen64.  (That would also apply to 
>> open_wmemstream in
>> wchar.h, but I suppose you have the issue there with functions not being
>> declared in wchar.h, only in stdio.h?)
> 
> You're right that adding the attribute to open_wmemstream runs into
> the same problem as declaring the ctermid argument with L_ctermid
> elements in <unistd.h>: fclose isn't declared in <wchar.h>.  I did
> some more work on adding the attribute to other functions and found
> out that the same problem also happens between tempnam() in <stdio.h>
> and reallocarray() in <stdlib.h>.
> 
> I spent some time working around this but in the end it turned out
> to be too convoluted so I decided to make the attribute a little
> smarter.  Instead of associating all allocation functions with all
> deallocation functions (such as fdopen, fopen, fopen64, etc. with
> fclose, freopen, and freopen64) I changed it so that an allocator
> only needs to be associated with a single deallocator (a reallocator
> also needs to be associated with itself).  That makes things quite
> a bit simpler.
> 
> The attached patch implements this for <stdio.h>, <stdlib.h>, and
> <wchar.h>.  To get around the <wchar.h> dependency on <stdio.h> it
> uses __REDIRECT to introduce a reserved alias for fclose.
> 
> Besides running the test suite I tested it with my own test and also
> by adding the same declarations to the GCC test suite and verifying
> it triggers warnings as expected.
> 
> The GCC patches needed to make this simpler scheme work haven't been
> reviewed yet so this work has a dependency on them getting approved.

The GCC patches have now been committed and the dependency resolved.

Florian asked this morning if getaddrinfo() and freeaddrinfo() are
covered by this change.  They're not because getaddrinfo() returns
the allocated memory indirectly, via an argument.  To handle those
kinds of APIs that return a pointer to the allocated object
indirectly, through an argument, the attribute will need to be
enhanced somehow.

> 
> I grepped for __attribute_malloc__ in Glibc headers to see if there
> are other APIs that would benefit from the same annotation but found
> none.  At the same time, I don't have the impression that malloc is
> used on all the APIs it could be.  Are there any that you or anyone
> else can think of that might be worth looking at?
> 
> Martin

Re: [PATCH] add support for -Wmismatched-dealloc

[Florian Weimer]

* Martin Sebor:

> Florian asked this morning if getaddrinfo() and freeaddrinfo() are
> covered by this change.  They're not because getaddrinfo() returns
> the allocated memory indirectly, via an argument.  To handle those
> kinds of APIs that return a pointer to the allocated object
> indirectly, through an argument, the attribute will need to be
> enhanced somehow.

asprintf is another such function, perhaps slightly more commonly used.
It would be nice if this interface pattern could be covered as well.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH] add support for -Wmismatched-dealloc

[Joseph Myers]

On Mon, 14 Dec 2020, Martin Sebor via Libc-alpha wrote:

> > I spent some time working around this but in the end it turned out
> > to be too convoluted so I decided to make the attribute a little
> > smarter.  Instead of associating all allocation functions with all
> > deallocation functions (such as fdopen, fopen, fopen64, etc. with
> > fclose, freopen, and freopen64) I changed it so that an allocator
> > only needs to be associated with a single deallocator (a reallocator
> > also needs to be associated with itself).  That makes things quite
> > a bit simpler.
[...]
> The GCC patches have now been committed and the dependency resolved.

I've looked at the attribute documentation now in GCC, but I'm afraid I'm 
unable to understand from that documentation why the proposed glibc patch 
constitutes a valid way of specifying that, for example, it's valid to use 
freopen as a deallocator for FILE pointers opened by functions whose 
attribute only mentions fclose.  Unless there's something I'm missing in 
the documentation or a separate documentation patch that's not yet 
committed, I think more work is needed on the GCC documentation to make 
clear the semantics the glibc patch is asserting for valid combinations of 
allocators and deallocators, so that those semantics can be reviewed for 
correctness.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] add support for -Wmismatched-dealloc

[Martin Sebor]

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

On 12/14/20 6:01 PM, Joseph Myers wrote:
> On Mon, 14 Dec 2020, Martin Sebor via Libc-alpha wrote:
> 
>>> I spent some time working around this but in the end it turned out
>>> to be too convoluted so I decided to make the attribute a little
>>> smarter.  Instead of associating all allocation functions with all
>>> deallocation functions (such as fdopen, fopen, fopen64, etc. with
>>> fclose, freopen, and freopen64) I changed it so that an allocator
>>> only needs to be associated with a single deallocator (a reallocator
>>> also needs to be associated with itself).  That makes things quite
>>> a bit simpler.
> [...]
>> The GCC patches have now been committed and the dependency resolved.
> 
> I've looked at the attribute documentation now in GCC, but I'm afraid I'm
> unable to understand from that documentation why the proposed glibc patch
> constitutes a valid way of specifying that, for example, it's valid to use
> freopen as a deallocator for FILE pointers opened by functions whose
> attribute only mentions fclose.  Unless there's something I'm missing in
> the documentation or a separate documentation patch that's not yet
> committed, I think more work is needed on the GCC documentation to make
> clear the semantics the glibc patch is asserting for valid combinations of
> allocators and deallocators, so that those semantics can be reviewed for
> correctness.

I flip-flopped with freopen.  Initially I wanted to mark it up as
both an allocator and a deallocator, analogously to realloc (which
is implicitly both) or reallocarray (which is annotated as both in
the latest Glibc patch).  Both the initial Glibc and GCC patches
(the manual for the latter) reflected this and had freopen annotated
that way.

But because freopen doesn't actually deallocate or allocate a stream
the markup wouldn't be correct.  It would cause false positives with
-Wmismatched-dealloc as well with other warnings like the future
-Wuse-after-free (or with -Wanalyzer-use-after-free when the GCC
analyzer adds support for the attribute that David Malcolm is
working on for GCC 11).  I've added a test case to the test suite:

   void f (FILE *f1)
   {
     FILE *f2 = freopen ("", "", f1);
     fclose (f1);   // must not warn
   }

To answer your question, without the attribute freopen is seen by
GCC as an ordinary function that happens to take a FILE* and return
another FILE*.  It neither allocates it nor deallocates it.  For
GCC 12, I'd like us to consider adding attribute returns_arg(position)
to improve the analysis here.  The GCC manual also doesn't mention
freopen anymore but I'd be happy to change the example there to
show an API that does include a reallocator (e.g., reallocarray).

Having said all this, after double-checking the latest Glibc patch
I see it still has the attribute on freopen by mistake (as well as
the ordinary attribute malloc, which would make it even worse).
I've removed both in the attached revision.  Sorry if this confused
you -- freopen obviously confused me.

Martin

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

diff --git a/libio/stdio.h b/libio/stdio.h
index 998470943e..a52324cc23 100644
--- a/libio/stdio.h
+++ b/libio/stdio.h
@@ -165,22 +165,31 @@ 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);
+
 /* 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, 1) __wur;
 #else
 # ifdef __REDIRECT
-extern FILE *__REDIRECT (tmpfile, (void), tmpfile64) __wur;
+extern FILE *__REDIRECT (tmpfile, (void), tmpfile64)
+  __attribute_malloc__ __attr_dealloc (fclose, 1) __wur;
 # else
 #  define tmpfile tmpfile64
 # endif
 #endif
 
 #ifdef __USE_LARGEFILE64
-extern FILE *tmpfile64 (void) __wur;
+extern FILE *tmpfile64 (void)
+   __attribute_malloc__ __attr_dealloc (fclose, 1) __wur;
 #endif
 
 /* Generate a temporary filename.  */
@@ -202,15 +211,17 @@ 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;
+  __attribute_malloc__ __wur
+# ifdef __has_builtin
+/* Reference the builtin since the ordinary declaration need not be
+   in scope at this point.  */
+#   if __has_builtin (__builtin_free)
+      __attr_dealloc (__builtin_free, 1)
+#   endif
+# endif
+  __THROW;
 #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 +255,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, 1) __wur;
 /* Open a file, replacing an existing stream with it.
 
    This function is a possible cancellation point and therefore not
@@ -256,7 +268,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, 1) __wur;
 extern FILE *__REDIRECT (freopen, (const char *__restrict __filename,
 				   const char *__restrict __modes,
 				   FILE *__restrict __stream), freopen64)
@@ -268,7 +280,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, 1) __wur;
 extern FILE *freopen64 (const char *__restrict __filename,
 			const char *__restrict __modes,
 			FILE *__restrict __stream) __wur;
@@ -276,7 +289,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, 1) __wur;
 #endif
 
 #ifdef	__USE_GNU
@@ -284,18 +298,20 @@ 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, 1) __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, 1) __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, 1) __wur;
 #endif
 
 
@@ -792,17 +808,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 baa13166fe..a69e4430a7 100644
--- a/libio/tst-freopen.c
+++ b/libio/tst-freopen.c
@@ -81,6 +81,28 @@ do_test_basic (void)
   fclose (f);
 }
 
+/* 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 attribute malloc
+     (which would 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);
+}
+
+
 /* Test for BZ#21398, where it tries to freopen stdio after the close
    of its file descriptor.  */
 static void
@@ -105,6 +127,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/misc/sys/cdefs.h b/misc/sys/cdefs.h
index a06f1cfd91..092e078ef8 100644
--- a/misc/sys/cdefs.h
+++ b/misc/sys/cdefs.h
@@ -560,6 +560,15 @@ _Static_assert (0, "IEEE 128-bits long double requires redirection on this platf
 #  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)))
+#else
+# define __attr_dealloc(x)
+#endif
+
 /* Specify that a function such as setjmp or vfork may return
    twice.  */
 #if __GNUC_PREREQ (4, 1)
diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
index 3aa27a9d25..a039f17d97 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, 1);
 
-/* 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)
+     __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, 1) __wur;
 #endif
 
 #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, 1) __wur;
 #endif
 
 
diff --git a/wcsmbs/wchar.h b/wcsmbs/wchar.h
index 9cf8b05a87..7442c80753 100644
--- a/wcsmbs/wchar.h
+++ b/wcsmbs/wchar.h
@@ -562,9 +562,18 @@ extern wchar_t *wcpncpy (wchar_t *__restrict __dest,
 /* Wide character I/O functions.  */
 
 #if defined __USE_XOPEN2K8 || __GLIBC_USE (LIB_EXT2)
+#  ifdef __REDIRECT
+/* Declare the __fclose alias and associate it as a deallocator
+   with open_memstream below.  */
+extern char *__REDIRECT (__fclose, (FILE *), fclose);
+#    define __attr_dealloc_fclose __attr_dealloc (__fclose, 1)
+#  else
+#    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

Re: [PATCH] add support for -Wmismatched-dealloc

[Martin Sebor]

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

More testing made me realize that further changes are needed:
1) correct the return value of the __fclose() alias to int,
2) declare and use the same alias for fclose in both <stdio.h>
    and <wchar.h>.

In addition, I noticed a few more opportunities to use the new
attribute:
  *  in include/programs/xmalloc.h,
  *  in malloc/malloc.h,
  *  and in wcsdup in <wchar.h>.

I also simplified the new macro definitions a bit, and added
a new test to verify that the warning doesn't cause false
positives for open_wmemstream.

Attached is a patch with these updates.

On 12/15/20 9:52 AM, Martin Sebor wrote:
> On 12/14/20 6:01 PM, Joseph Myers wrote:
>> On Mon, 14 Dec 2020, Martin Sebor via Libc-alpha wrote:
>>
>>>> I spent some time working around this but in the end it turned out
>>>> to be too convoluted so I decided to make the attribute a little
>>>> smarter.  Instead of associating all allocation functions with all
>>>> deallocation functions (such as fdopen, fopen, fopen64, etc. with
>>>> fclose, freopen, and freopen64) I changed it so that an allocator
>>>> only needs to be associated with a single deallocator (a reallocator
>>>> also needs to be associated with itself).  That makes things quite
>>>> a bit simpler.
>> [...]
>>> The GCC patches have now been committed and the dependency resolved.
>>
>> I've looked at the attribute documentation now in GCC, but I'm afraid I'm
>> unable to understand from that documentation why the proposed glibc patch
>> constitutes a valid way of specifying that, for example, it's valid to 
>> use
>> freopen as a deallocator for FILE pointers opened by functions whose
>> attribute only mentions fclose.  Unless there's something I'm missing in
>> the documentation or a separate documentation patch that's not yet
>> committed, I think more work is needed on the GCC documentation to make
>> clear the semantics the glibc patch is asserting for valid 
>> combinations of
>> allocators and deallocators, so that those semantics can be reviewed for
>> correctness.
> 
> I flip-flopped with freopen.  Initially I wanted to mark it up as
> both an allocator and a deallocator, analogously to realloc (which
> is implicitly both) or reallocarray (which is annotated as both in
> the latest Glibc patch).  Both the initial Glibc and GCC patches
> (the manual for the latter) reflected this and had freopen annotated
> that way.
> 
> But because freopen doesn't actually deallocate or allocate a stream
> the markup wouldn't be correct.  It would cause false positives with
> -Wmismatched-dealloc as well with other warnings like the future
> -Wuse-after-free (or with -Wanalyzer-use-after-free when the GCC
> analyzer adds support for the attribute that David Malcolm is
> working on for GCC 11).  I've added a test case to the test suite:
> 
>    void f (FILE *f1)
>    {
>      FILE *f2 = freopen ("", "", f1);
>      fclose (f1);   // must not warn
>    }
> 
> To answer your question, without the attribute freopen is seen by
> GCC as an ordinary function that happens to take a FILE* and return
> another FILE*.  It neither allocates it nor deallocates it.  For
> GCC 12, I'd like us to consider adding attribute returns_arg(position)
> to improve the analysis here.  The GCC manual also doesn't mention
> freopen anymore but I'd be happy to change the example there to
> show an API that does include a reallocator (e.g., reallocarray).
> 
> Having said all this, after double-checking the latest Glibc patch
> I see it still has the attribute on freopen by mistake (as well as
> the ordinary attribute malloc, which would make it even worse).
> I've removed both in the attached revision.  Sorry if this confused
> you -- freopen obviously confused me.
> 
> Martin


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

diff --git a/include/programs/xmalloc.h b/include/programs/xmalloc.h
index fd6cf61730..2a8d6ff23c 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 276baf58ac..0f1f536f7a 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 998470943e..49d8607bf3 100644
--- a/libio/stdio.h
+++ b/libio/stdio.h
@@ -165,22 +165,41 @@ 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);
+
+#ifdef __REDIRECT
+/* Declare the __fclose alias and associate it as a deallocator with
+   fopen below.  Use an alias for the sake of open_wmemstream in
+   <wchar.h>.  */
+extern int __REDIRECT (__fclose, (FILE *), fclose);
+# define __attr_dealloc_fclose __attr_dealloc (__fclose, 1)
+#else
+# define __attr_dealloc_fclose /* empty */
+#endif
+
 /* 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 +221,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;
+  __attribute_malloc__ __wur __attr_dealloc_free __THROW;
 #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 +257,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 +270,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 +282,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 +291,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,18 +300,20 @@ 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;
 #endif
 
 
@@ -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 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
+
+/* 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 attribute malloc
+     (which would 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
+# 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 +135,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..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
+
+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
+# 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..64461dbe48
--- /dev/null
+++ b/libio/tst-wmemstream5.c
@@ -0,0 +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 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
+
+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
+# 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 b2371f7704..d9dc359559 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 a06f1cfd91..fe5cb56511 100644
--- a/misc/sys/cdefs.h
+++ b/misc/sys/cdefs.h
@@ -560,6 +560,17 @@ _Static_assert (0, "IEEE 128-bits long double requires redirection on this platf
 #  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/stdlib.h b/stdlib/stdlib.h
index 3aa27a9d25..f88f8e55b3 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)
+     __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
@@ -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
 
 
diff --git a/wcsmbs/wchar.h b/wcsmbs/wchar.h
index 9cf8b05a87..4c1c7f1119 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,18 @@ extern wchar_t *wcpncpy (wchar_t *__restrict __dest,
 /* Wide character I/O functions.  */
 
 #if defined __USE_XOPEN2K8 || __GLIBC_USE (LIB_EXT2)
+#  ifdef __REDIRECT
+/* Declare the __fclose alias and associate it as a deallocator
+   with open_wmemstream below.  */
+extern int __REDIRECT (__fclose, (FILE *), fclose);
+#    define __attr_dealloc_fclose __attr_dealloc (__fclose, 1)
+#  else
+#    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

Ping: [PATCH] add support for -Wmismatched-dealloc

[Martin Sebor]

Florian/Joseph and/or others: is the latest patch okay to commit?

https://sourceware.org/pipermail/libc-alpha/2020-December/121121.html

On 12/27/20 4:13 PM, Martin Sebor wrote:
> More testing made me realize that further changes are needed:
> 1) correct the return value of the __fclose() alias to int,
> 2) declare and use the same alias for fclose in both <stdio.h>
>     and <wchar.h>.
> 
> In addition, I noticed a few more opportunities to use the new
> attribute:
>   *  in include/programs/xmalloc.h,
>   *  in malloc/malloc.h,
>   *  and in wcsdup in <wchar.h>.
> 
> I also simplified the new macro definitions a bit, and added
> a new test to verify that the warning doesn't cause false
> positives for open_wmemstream.
> 
> Attached is a patch with these updates.
> 
> On 12/15/20 9:52 AM, Martin Sebor wrote:
>> On 12/14/20 6:01 PM, Joseph Myers wrote:
>>> On Mon, 14 Dec 2020, Martin Sebor via Libc-alpha wrote:
>>>
>>>>> I spent some time working around this but in the end it turned out
>>>>> to be too convoluted so I decided to make the attribute a little
>>>>> smarter.  Instead of associating all allocation functions with all
>>>>> deallocation functions (such as fdopen, fopen, fopen64, etc. with
>>>>> fclose, freopen, and freopen64) I changed it so that an allocator
>>>>> only needs to be associated with a single deallocator (a reallocator
>>>>> also needs to be associated with itself).  That makes things quite
>>>>> a bit simpler.
>>> [...]
>>>> The GCC patches have now been committed and the dependency resolved.
>>>
>>> I've looked at the attribute documentation now in GCC, but I'm afraid 
>>> I'm
>>> unable to understand from that documentation why the proposed glibc 
>>> patch
>>> constitutes a valid way of specifying that, for example, it's valid 
>>> to use
>>> freopen as a deallocator for FILE pointers opened by functions whose
>>> attribute only mentions fclose.  Unless there's something I'm missing in
>>> the documentation or a separate documentation patch that's not yet
>>> committed, I think more work is needed on the GCC documentation to make
>>> clear the semantics the glibc patch is asserting for valid 
>>> combinations of
>>> allocators and deallocators, so that those semantics can be reviewed for
>>> correctness.
>>
>> I flip-flopped with freopen.  Initially I wanted to mark it up as
>> both an allocator and a deallocator, analogously to realloc (which
>> is implicitly both) or reallocarray (which is annotated as both in
>> the latest Glibc patch).  Both the initial Glibc and GCC patches
>> (the manual for the latter) reflected this and had freopen annotated
>> that way.
>>
>> But because freopen doesn't actually deallocate or allocate a stream
>> the markup wouldn't be correct.  It would cause false positives with
>> -Wmismatched-dealloc as well with other warnings like the future
>> -Wuse-after-free (or with -Wanalyzer-use-after-free when the GCC
>> analyzer adds support for the attribute that David Malcolm is
>> working on for GCC 11).  I've added a test case to the test suite:
>>
>>    void f (FILE *f1)
>>    {
>>      FILE *f2 = freopen ("", "", f1);
>>      fclose (f1);   // must not warn
>>    }
>>
>> To answer your question, without the attribute freopen is seen by
>> GCC as an ordinary function that happens to take a FILE* and return
>> another FILE*.  It neither allocates it nor deallocates it.  For
>> GCC 12, I'd like us to consider adding attribute returns_arg(position)
>> to improve the analysis here.  The GCC manual also doesn't mention
>> freopen anymore but I'd be happy to change the example there to
>> show an API that does include a reallocator (e.g., reallocarray).
>>
>> Having said all this, after double-checking the latest Glibc patch
>> I see it still has the attribute on freopen by mistake (as well as
>> the ordinary attribute malloc, which would make it even worse).
>> I've removed both in the attached revision.  Sorry if this confused
>> you -- freopen obviously confused me.
>>
>> Martin
> 

Re: [PATCH] add support for -Wmismatched-dealloc

[Florian Weimer]

* Martin Sebor via Libc-alpha:

> diff --git a/wcsmbs/wchar.h b/wcsmbs/wchar.h
> index 9cf8b05a87..4c1c7f1119 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,18 @@ extern wchar_t *wcpncpy (wchar_t *__restrict __dest,
>  /* Wide character I/O functions.  */
>  
>  #if defined __USE_XOPEN2K8 || __GLIBC_USE (LIB_EXT2)
> +#  ifdef __REDIRECT
> +/* Declare the __fclose alias and associate it as a deallocator
> +   with open_wmemstream below.  */
> +extern int __REDIRECT (__fclose, (FILE *), fclose);
> +#    define __attr_dealloc_fclose __attr_dealloc (__fclose, 1)
> +#  else
> +#    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

Why is an alias for fclose needed here, but not for free?

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH] add support for -Wmismatched-dealloc

[Martin Sebor]

On 1/4/21 9:07 AM, Florian Weimer wrote:
> * Martin Sebor via Libc-alpha:
> 
>> diff --git a/wcsmbs/wchar.h b/wcsmbs/wchar.h
>> index 9cf8b05a87..4c1c7f1119 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,18 @@ extern wchar_t *wcpncpy (wchar_t *__restrict __dest,
>>   /* Wide character I/O functions.  */
>>   
>>   #if defined __USE_XOPEN2K8 || __GLIBC_USE (LIB_EXT2)
>> +#  ifdef __REDIRECT
>> +/* Declare the __fclose alias and associate it as a deallocator
>> +   with open_wmemstream below.  */
>> +extern int __REDIRECT (__fclose, (FILE *), fclose);
>> +#    define __attr_dealloc_fclose __attr_dealloc (__fclose, 1)
>> +#  else
>> +#    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
> 
> Why is an alias for fclose needed here, but not for free?

Because fclose is not a built-in so there's no __builtin_fclose
to associate open_wmemstream with.  free is a built-in and so
__attr_dealloc_free just references __builtin_free and doesn't
need an explicit declaration.

Martin

> 
> Thanks,
> Florian
> 

Re: [PATCH] add support for -Wmismatched-dealloc

[Florian Weimer]

* Martin Sebor:

> On 1/4/21 9:07 AM, Florian Weimer wrote:
>> * Martin Sebor via Libc-alpha:
>> 
>>> diff --git a/wcsmbs/wchar.h b/wcsmbs/wchar.h
>>> index 9cf8b05a87..4c1c7f1119 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,18 @@ extern wchar_t *wcpncpy (wchar_t *__restrict __dest,
>>>   /* Wide character I/O functions.  */
>>>     #if defined __USE_XOPEN2K8 || __GLIBC_USE (LIB_EXT2)
>>> +#  ifdef __REDIRECT
>>> +/* Declare the __fclose alias and associate it as a deallocator
>>> +   with open_wmemstream below.  */
>>> +extern int __REDIRECT (__fclose, (FILE *), fclose);
>>> +#    define __attr_dealloc_fclose __attr_dealloc (__fclose, 1)
>>> +#  else
>>> +#    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
>> Why is an alias for fclose needed here, but not for free?
>
> Because fclose is not a built-in so there's no __builtin_fclose
> to associate open_wmemstream with.  free is a built-in and so
> __attr_dealloc_free just references __builtin_free and doesn't
> need an explicit declaration.

Ahh, that explains the discrepancy.

I'm a bit worried that the __fclose alias causes problems.  Would it be
possible to add __builtin_fclose to GCC instead?

Based on how this patch appears to make both __fclose and fclose
acceptable as a deallocator, GCC resolves redirects as part of the
matching check.  I wonder if this constrains the usefulness of the
attribute in some way.  I can imagine situations where at the source
level, different deallocators should be used (say to support debugging
builds), but release builds redirect different deallocators to the same
implementation.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH] add support for -Wmismatched-dealloc

[Martin Sebor]

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

On 1/4/21 9:57 AM, Florian Weimer wrote:
> * Martin Sebor:
> 
>> On 1/4/21 9:07 AM, Florian Weimer wrote:
>>> * Martin Sebor via Libc-alpha:
>>>
>>>> diff --git a/wcsmbs/wchar.h b/wcsmbs/wchar.h
>>>> index 9cf8b05a87..4c1c7f1119 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,18 @@ extern wchar_t *wcpncpy (wchar_t *__restrict __dest,
>>>>    /* Wide character I/O functions.  */
>>>>      #if defined __USE_XOPEN2K8 || __GLIBC_USE (LIB_EXT2)
>>>> +#  ifdef __REDIRECT
>>>> +/* Declare the __fclose alias and associate it as a deallocator
>>>> +   with open_wmemstream below.  */
>>>> +extern int __REDIRECT (__fclose, (FILE *), fclose);
>>>> +#    define __attr_dealloc_fclose __attr_dealloc (__fclose, 1)
>>>> +#  else
>>>> +#    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
>>> Why is an alias for fclose needed here, but not for free?
>>
>> Because fclose is not a built-in so there's no __builtin_fclose
>> to associate open_wmemstream with.  free is a built-in and so
>> __attr_dealloc_free just references __builtin_free and doesn't
>> need an explicit declaration.
> 
> Ahh, that explains the discrepancy.
> 
> I'm a bit worried that the __fclose alias causes problems.  Would it be
> possible to add __builtin_fclose to GCC instead?

I don't like the alias hack either.  Adding a built-in is possible
but it's late in GCC stage 3 and I'm doubtful the change would be
accepted before the Glibc deadline (that's this week, right?)

The alias isn't necessary in <stdio.h> where fclose is declared
so I've removed it from there.  It would have been only marginally
useful in <wchar.h>, and only when fclose isn't declared, but it's
probably best avoided.  So one possibility is to prepare the header
for __builtin_fclose if/when it's added, fall back on fclose when
it's declared (i.e., when <stdio.h> is included), and do nothing
otherwise (and accept that calling, say free, on a pointer returned
from open_wmemstteam, will not be diagnosed in those translation
units).

Attached is a patch that does that.  If you want to change it
to something else (e.g, forget about open_wmemstream altogether
for now or conditionally declare it in <stdio.h> when <wchar.h>
is included, or any other viable alternative) please let me know.

> Based on how this patch appears to make both __fclose and fclose
> acceptable as a deallocator, GCC resolves redirects as part of the
> matching check.  I wonder if this constrains the usefulness of the
> attribute in some way.  I can imagine situations where at the source
> level, different deallocators should be used (say to support debugging
> builds), but release builds redirect different deallocators to the same
> implementation.

The attribute doesn't do anything special with aliases (it was
just a way to get around the problem with functions not being
declared in some headers).

As for different configurations, the attribute is designed for
standard C/POSIX APIs and others like those.  A user-defined API
with different deallocators in one configuration than in another
has to create the associations conditionally, based on those
configurations.  But there's no way to change these associations
for GCC built-ins, just like there's no way to change their
semantics.  They're hardwired into GCC and the only way to
affect them is to disable the built-ins.

Martin

> 
> Thanks,
> Florian
> 


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

diff --git a/include/programs/xmalloc.h b/include/programs/xmalloc.h
index fd6cf61730..2a8d6ff23c 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 276baf58ac..0f1f536f7a 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 998470943e..f27d526f44 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;
+  __attribute_malloc__ __wur __attr_dealloc_free __THROW;
 #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,18 +293,20 @@ 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;
 #endif
 
 
@@ -792,17 +803,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 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
+
+/* 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 attribute malloc
+     (which would 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
+# 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 +135,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..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
+
+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
+# 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..64461dbe48
--- /dev/null
+++ b/libio/tst-wmemstream5.c
@@ -0,0 +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 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
+
+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
+# 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 b2371f7704..d9dc359559 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 5fb6e309be..21e6177fac 100644
--- a/misc/sys/cdefs.h
+++ b/misc/sys/cdefs.h
@@ -569,6 +569,17 @@ _Static_assert (0, "IEEE 128-bits long double requires redirection on this platf
 #  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/stdlib.h b/stdlib/stdlib.h
index 3aa27a9d25..f88f8e55b3 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)
+     __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
@@ -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
 
 
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

Ping: [PATCH] add support for -Wmismatched-dealloc

[Martin Sebor]

Florian, do you have any outstanding concerns or is the most recent
patch good to go?

Thanks
Martin

On 1/4/21 4:18 PM, Martin Sebor wrote:
> On 1/4/21 9:57 AM, Florian Weimer wrote:
>> * Martin Sebor:
>>
>>> On 1/4/21 9:07 AM, Florian Weimer wrote:
>>>> * Martin Sebor via Libc-alpha:
>>>>
>>>>> diff --git a/wcsmbs/wchar.h b/wcsmbs/wchar.h
>>>>> index 9cf8b05a87..4c1c7f1119 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,18 @@ extern wchar_t *wcpncpy (wchar_t *__restrict 
>>>>> __dest,
>>>>>    /* Wide character I/O functions.  */
>>>>>      #if defined __USE_XOPEN2K8 || __GLIBC_USE (LIB_EXT2)
>>>>> +#  ifdef __REDIRECT
>>>>> +/* Declare the __fclose alias and associate it as a deallocator
>>>>> +   with open_wmemstream below.  */
>>>>> +extern int __REDIRECT (__fclose, (FILE *), fclose);
>>>>> +#    define __attr_dealloc_fclose __attr_dealloc (__fclose, 1)
>>>>> +#  else
>>>>> +#    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
>>>> Why is an alias for fclose needed here, but not for free?
>>>
>>> Because fclose is not a built-in so there's no __builtin_fclose
>>> to associate open_wmemstream with.  free is a built-in and so
>>> __attr_dealloc_free just references __builtin_free and doesn't
>>> need an explicit declaration.
>>
>> Ahh, that explains the discrepancy.
>>
>> I'm a bit worried that the __fclose alias causes problems.  Would it be
>> possible to add __builtin_fclose to GCC instead?
> 
> I don't like the alias hack either.  Adding a built-in is possible
> but it's late in GCC stage 3 and I'm doubtful the change would be
> accepted before the Glibc deadline (that's this week, right?)
> 
> The alias isn't necessary in <stdio.h> where fclose is declared
> so I've removed it from there.  It would have been only marginally
> useful in <wchar.h>, and only when fclose isn't declared, but it's
> probably best avoided.  So one possibility is to prepare the header
> for __builtin_fclose if/when it's added, fall back on fclose when
> it's declared (i.e., when <stdio.h> is included), and do nothing
> otherwise (and accept that calling, say free, on a pointer returned
> from open_wmemstteam, will not be diagnosed in those translation
> units).
> 
> Attached is a patch that does that.  If you want to change it
> to something else (e.g, forget about open_wmemstream altogether
> for now or conditionally declare it in <stdio.h> when <wchar.h>
> is included, or any other viable alternative) please let me know.
> 
>> Based on how this patch appears to make both __fclose and fclose
>> acceptable as a deallocator, GCC resolves redirects as part of the
>> matching check.  I wonder if this constrains the usefulness of the
>> attribute in some way.  I can imagine situations where at the source
>> level, different deallocators should be used (say to support debugging
>> builds), but release builds redirect different deallocators to the same
>> implementation.
> 
> The attribute doesn't do anything special with aliases (it was
> just a way to get around the problem with functions not being
> declared in some headers).
> 
> As for different configurations, the attribute is designed for
> standard C/POSIX APIs and others like those.  A user-defined API
> with different deallocators in one configuration than in another
> has to create the associations conditionally, based on those
> configurations.  But there's no way to change these associations
> for GCC built-ins, just like there's no way to change their
> semantics.  They're hardwired into GCC and the only way to
> affect them is to disable the built-ins.
> 
> Martin
> 
>>
>> Thanks,
>> Florian
>>
> 

Re: [PATCH] add support for -Wmismatched-dealloc

[Florian Weimer]

* Martin Sebor:

> I don't like the alias hack either.  Adding a built-in is possible
> but it's late in GCC stage 3 and I'm doubtful the change would be
> accepted before the Glibc deadline (that's this week, right?)

I think we still have some time.

One problem is that the annotations do not permit diagnosing memory
leaks: realloc is a deallocator for malloc, xfclose is one for fopen,
and so on.  But the annotations do not capture this.  So if they are
used for diagnosing leaks, false positives will be a result.  Listing
all potential deallocators in the allocator does not scale.  It also
leads to the problem shown by open_wmemstream.

I think allocators need to assign some sort of pool identifier to
allocated pointers, and deallocators should declare on which pools they
operate.  This would allow adding additional deallocators such as
xfclose.

>> Based on how this patch appears to make both __fclose and fclose
>> acceptable as a deallocator, GCC resolves redirects as part of the
>> matching check.  I wonder if this constrains the usefulness of the
>> attribute in some way.  I can imagine situations where at the source
>> level, different deallocators should be used (say to support debugging
>> builds), but release builds redirect different deallocators to the same
>> implementation.
>
> The attribute doesn't do anything special with aliases (it was
> just a way to get around the problem with functions not being
> declared in some headers).

But it has to do something with them, otherwise __fclose and fclose are
not the same deallocator and thus different functions.

> diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h
> index 5fb6e309be..21e6177fac 100644
> --- a/misc/sys/cdefs.h
> +++ b/misc/sys/cdefs.h
> @@ -569,6 +569,17 @@ _Static_assert (0, "IEEE 128-bits long double requires redirection on this platf
>  #  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

If the GCC attribute is called “malloc”, the macro name should reflect
that.  I think it would make sense to make __attribute_malloc__
redundant, so that the headers contain only one of the two attributes
per declaration.  The new macro could expand to the argument-less
version for older GCC versions.

> 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.

> +  /* This shouldn't trigger -Wmismatched-dealloc.  */
> +  fclose (f1);

> +#if defined __GNUC__ && __GNUC__ >= 11
> +# pragma GCC diagnostic pop
> +#endif

Likewise.

> 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.

> +#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.

> +#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.

> 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.

> 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?

Maybe it is possible to redeclare open_wmemstream in <stdio.h> if
<wchar.h> has already been included?

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH] add support for -Wmismatched-dealloc

[Martin Sebor]

On 1/11/21 2:13 AM, Florian Weimer wrote:
> * Martin Sebor:
> 
>> I don't like the alias hack either.  Adding a built-in is possible
>> but it's late in GCC stage 3 and I'm doubtful the change would be
>> accepted before the Glibc deadline (that's this week, right?)
> 
> I think we still have some time.
> 
> One problem is that the annotations do not permit diagnosing memory
> leaks: realloc is a deallocator for malloc, xfclose is one for fopen,
> and so on.  But the annotations do not capture this.  So if they are
> used for diagnosing leaks, false positives will be a result.  Listing
> all potential deallocators in the allocator does not scale.  It also
> leads to the problem shown by open_wmemstream.

Florian and I discussed this in IRC and cleared most things up
so this is largely just for the benefit of others.  I'll post
a new patch with the suggested changes separately from this.

The attribute is used to detect other problems besides allocator
and deallocator mismatches (e.g., -Wfree-nonheap-object), and
should also already be in principle used by analyzer warnings
like -Wanalyzer-double-fclose and -Wanalyzer-double-free.  David
Malcolm prototyped a similar attribute in the analyzer first and
now is in the process of adjusting the analyzer to work with this
one.

The open_wmemstream problem is due to the <wchar.h> header not
having access to the fclose declaration (because it's only in
<stdio.h>).  Declaring open_wmemstream with the attribute also
in <stdio.h> (when <wchar.h> is included) solves it.

Finally [and this qualifies what I said to Florian this morning],
GCC makes it possible to avoid having to associate every allocator
with every matching deallocator for reallocators only (like realloc).
This was done to simplify its use in Glibc headers.  But no such
assumption is made about straight (non-deallocating) allocators
and they do have to be explicitly associated with all their
deallocators.

> I think allocators need to assign some sort of pool identifier to
> allocated pointers, and deallocators should declare on which pools they
> operate.  This would allow adding additional deallocators such as
> xfclose.

Other than the simplification for realloc there's no notion of
a "pool."  Supporting it wasn't a goal (the subject of the request
in GCC PR 94527).  The design could be relatively easily changed
to avoid having to associate every allocator with every deallocator
but almost certainly not for GCC 11.

>>> Based on how this patch appears to make both __fclose and fclose
>>> acceptable as a deallocator, GCC resolves redirects as part of the
>>> matching check.  I wonder if this constrains the usefulness of the
>>> attribute in some way.  I can imagine situations where at the source
>>> level, different deallocators should be used (say to support debugging
>>> builds), but release builds redirect different deallocators to the same
>>> implementation.
>>
>> The attribute doesn't do anything special with aliases (it was
>> just a way to get around the problem with functions not being
>> declared in some headers).
> 
> But it has to do something with them, otherwise __fclose and fclose are
> not the same deallocator and thus different functions.

Yes, they are different.  That's also why I agree it's not a good
solution.

I've snipped the rest of your comments and replied to them separately
to make it easier to focus on just the code changes.

Martin

Re: [PATCH] add support for -Wmismatched-dealloc

[Martin Sebor]

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

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: 18695 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 12ce41038f..99ec4bee20 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..62814c93db 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;
+  __attribute_malloc__ __wur __attr_dealloc_free __THROW;
 #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 57ca262bdf..14e25e2ac1 100644
--- a/misc/sys/cdefs.h
+++ b/misc/sys/cdefs.h
@@ -569,6 +569,17 @@ _Static_assert (0, "IEEE 128-bits long double requires redirection on this platf
 #  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/stdlib.h b/stdlib/stdlib.h
index 6360845d98..a555fb9c0f 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)
+     __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
@@ -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
 
 
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

Re: [PATCH] add support for -Wmismatched-dealloc

[Florian Weimer]

* Martin Sebor via Libc-alpha:

>> 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).

Maybe David can comment on how this interacts with his static analyzer
work.  In all other cases, the attribute means that the pointer needs to
be freed to avoid a resource leak.  If we suddenly apply it pointers
which can only conditionally be freed, that reduces the value of those
annotations, I think.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH] add support for -Wmismatched-dealloc

[Martin Sebor]

David, now that you've committed the analyzer support for
the extended attribute malloc, can you please comment on Florian's
concern about using it to annotate realpath() as in the patch below?

https://sourceware.org/pipermail/libc-alpha/2021-January/121527.html

(I tested how it interacts with -Wmismatched-dealloc but I haven't
yet had a chance to see how the annotations might interact with
the analyzer warnings.)

Thanks
Martin

On 1/12/21 1:59 AM, Florian Weimer wrote:
> * Martin Sebor via Libc-alpha:
> 
>>> 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).
> 
> Maybe David can comment on how this interacts with his static analyzer
> work.  In all other cases, the attribute means that the pointer needs to
> be freed to avoid a resource leak.  If we suddenly apply it pointers
> which can only conditionally be freed, that reduces the value of those
> annotations, I think.
> 
> Thanks,
> Florian
> 

Re: [PATCH] add support for -Wmismatched-dealloc

[David Malcolm]

On Tue, 2021-01-12 at 09:59 +0100, Florian Weimer wrote:
> * Martin Sebor via Libc-alpha:
> 
> > > 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).
> 
> Maybe David can comment on how this interacts with his static
> analyzer
> work.  

BTW, the -fanalyzer part of support for
__attribute__((malloc(deallocator))) is in gcc 11 as of yesterday:
https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=c7e276b869bdeb4a95735c1f037ee1a5f629de3d

> In all other cases, the attribute means that the pointer needs to
> be freed to avoid a resource leak.

Indeed, the analyzer doesn't have any special knowledge of realpath and
the conditional behavior.  Given that annotation it will assume that
the returned value is either a pointer that needs to be freed, or NULL
on a failure, with no knowledge that the 2nd argument could be
returned.

I haven't tested Martin's patch, but I tried this example:

$ cat t.c
#include <stdio.h>

#define PATH_MAX 4096
void free(void *ptr);
char *realpath(const char *path, char *resolved_path)
  __attribute__ ((malloc (free)))
  __attribute__ ((__warn_unused_result__));

void test_1 (const char *path)
{
  char buf[PATH_MAX];
  char *result = realpath (path, buf);
  printf ("result: %s\n", result);
}

void test_2 (const char *path)
{
  char buf[PATH_MAX];
  char *result = realpath (path, buf);
  printf ("result: %s\n", result);
  free (result);
}

I believe test_1 is correct (although redundant in its use of "result"
rather than buf, and can output a truncated path).

I believe test_2 is a crasher bug: a "free" of on-stack "buf".

Compiling with GCC 11 (with the __attribute__((malloc (DEALLOCATOR)))
support:

$ ./xgcc -B. -c -fanalyzer t.c
t.c: In function ‘test_1’:
t.c:14:1: warning: leak of ‘result’ [CWE-401] [-Wanalyzer-malloc-leak]
   14 | }
      | ^
  ‘test_1’: events 1-2
    |
    |   12 |   char *result = realpath (path, buf);
    |      |                  ^~~~~~~~~~~~~~~~~~~~
    |      |                  |
    |      |                  (1) allocated here
    |   13 |   printf ("result: %s\n", result);
    |   14 | }
    |      | ~                 
    |      | |
    |      | (2) ‘result’ leaks here; was allocated at (1)
    |

Here it falsely complains about test_1; it doesn't "know" that buf is
returned by realpath and assumes that result needs to be freed.

In test_2, there's a free of result == buf i.e. a free of an on-stack
buffer, which it doesn't complain about, treating "result" as a malloc-
ed pointer (as specified by the attribute).

So I don't think this attribute should be applied to realpath.

  If we suddenly apply it pointers
> which can only conditionally be freed, that reduces the value of
> those
> annotations, I think.

FWIW, the analyzer already special-cases some functions; see the
various region_model::impl_call_* functions in:
 https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/analyzer/region-model-impl-calls.cc

There's a case for doing this for stuff in POSIX, which would apply
here, I think.

As noted above, I haven't tested Martin's glibc patch (I don't think
I'm subscribed to this list).

> Thanks,
> Florian

Hope this is constructive
Dave

Re: [PATCH] add support for -Wmismatched-dealloc

[DJ Delorie]

Martin Sebor via Libc-alpha <libc-alpha@sourceware.org> writes:
>> 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).

Do we have a resolution on this?  I think deferring the realpath part of
this patch until it can be handled correctly is a reasonable solution.
I think an unwarned leak is better than false warnings.

The rest of the patch looks fine and has been reviewed...

Re: [PATCH] add support for -Wmismatched-dealloc

[Florian Weimer]

* Martin Sebor via Libc-alpha:


> diff --git a/libio/stdio.h b/libio/stdio.h
> index 144137cf67..62814c93db 100644
> --- a/libio/stdio.h
> +++ b/libio/stdio.h

> @@ -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;
> +  __attribute_malloc__ __wur __attr_dealloc_free __THROW;
>  #endif

This breaks boostrap because in some C++ modes, __THROW (aka noexcept)
needs to come before all attributes.

Further tests are ongoing; this is difficult to do due to general
toolchain breakage (although things have improved lately).

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH] add support for -Wmismatched-dealloc

[Florian Weimer]

* Martin Sebor via Libc-alpha:

> diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
> index 6360845d98..a555fb9c0f 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)
> +     __attr_dealloc (reallocarray, 1);
> +#endif

This causes:

In file included from ../include/stdlib.h:15,
                 from ../test-skeleton.c:36,
                 from test-math-iszero.cc:164:
../stdlib/stdlib.h:568:14: error: declaration of ‘void* reallocarray(void*, size_t, size_t)’ has a different exception specifier
  568 | extern void *reallocarray (void *__ptr, size_t __nmemb, size_t __size)
      |              ^~~~~~~~~~~~
../stdlib/stdlib.h:562:14: note: from previous declaration ‘void* reallocarray(void*, size_t, size_t) noexcept’
  562 | extern void *reallocarray (void *__ptr, size_t __nmemb, size_t __size)
      |              ^~~~~~~~~~~~

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCH] add support for -Wmismatched-dealloc

[Martin Sebor]

[-- 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

Re: [PATCH] add support for -Wmismatched-dealloc

[Martin Sebor]

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 <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
> 

Re: [PATCH] add support for -Wmismatched-dealloc

[Martin Sebor]

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

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 <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: 21658 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 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 <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 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 <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..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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <malloc.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

Re: [PATCH] add support for -Wmismatched-dealloc

[Martin Sebor]

I have committed the patch in g:c1760eaf3b.

On 5/13/21 3:49 PM, Martin Sebor wrote:
> 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 <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
>>>
>>
>