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