Attached is the final patch tested with GCC 10 and 11 (the latter both 32- and 64-bit this time). If there are no objections I'll commit it tomorrow. Florian and/or anyone else who provided comments on it: please let me know if you would like me to mention you in the Reviewed-by tag. Thanks Martin On 5/6/21 5:54 PM, Martin Sebor wrote: > If there is no further discussion I'll retest this patch with the top > of the trunk and a few recent GCC releases and if no problems turn up > plan to commit it next week.  Please let me know if you have any > concerns. > > (I already know about the bug in the dummy definition of > __attr_dealloc(x) with GCC versions prior: it only takes one argument > instead of two.) > > On 4/22/21 6:00 PM, Martin Sebor wrote: >> Attached is a revised patch that does not apply the new attribute >> malloc to realpath.  It also adds a new test to verify that calling >> a deallocator other than free on the result of realpath() with >> the resolved_path obtained from the corresponding allocator doesn't >> trigger a warning.  This is a trade-off between false positives and >> negatives: there new attribute isn't expressive enough to specify >> that a function like realpath returns a pointer allocated by malloc >> (when the second argument is null) or its argument otherwise. >> >> I also made sure that in the modified declarations __THROW comes >> before attributes and not after (as Florian pointed out is required >> in C++) and verified by compiling the modified headers with G++ 11. >> >> Otherwise this patch is unchanged from the last version. >> >> Martin >> >> On 1/11/21 5:01 PM, Martin Sebor wrote: >>> On 1/11/21 2:13 AM, Florian Weimer wrote: >>> ... >>> >>> The attached revision has the changes below. >>> >>>>> diff --git a/libio/tst-freopen.c b/libio/tst-freopen.c >>>>> index baa13166fe..c4df29d171 100644 >>>>> --- a/libio/tst-freopen.c >>>>> +++ b/libio/tst-freopen.c >>>>> @@ -81,6 +81,36 @@ do_test_basic (void) >>>>>     fclose (f); >>>>>   } >>>>> +#if defined __GNUC__ && __GNUC__ >= 11 >>>>> +#pragma GCC diagnostic push >>>>> +#pragma GCC diagnostic error "-Wmismatched-dealloc" >>>>> +#endif >>>> >>>> Please add a comment referencing the fclose in the test below. >>> >>> Done. >>> >>>> >>>>> +  /* This shouldn't trigger -Wmismatched-dealloc.  */ >>>>> +  fclose (f1); >>>> >>>>> +#if defined __GNUC__ && __GNUC__ >= 11 >>>>> +# pragma GCC diagnostic pop >>>>> +#endif >>>> >>>> Likewise. >>> >>> Ditto. >>> >>>> >>>>> diff --git a/libio/tst-wmemstream1.c b/libio/tst-wmemstream1.c >>>>> index f8b308bc6c..8a0e253862 100644 >>>>> --- a/libio/tst-wmemstream1.c >>>>> +++ b/libio/tst-wmemstream1.c >>>>> @@ -1,5 +1,40 @@ >>>>>   #include >>>>> +/* Verify that calling fclose on the result of open_wmemstream >>>>> doesn't >>>>> +   trigger GCC -Wmismatched-dealloc with fclose forward-declared and >>>>> +   without included first (it is included later, in. >>>>> +   "tst-memstream1.c").  */ >>>>> + >>>>> +extern int fclose (FILE*); >>>>> + >>>>> +#if defined __GNUC__ && __GNUC__ >= 11 >>>>> +#pragma GCC diagnostic push >>>>> +#pragma GCC diagnostic error "-Wmismatched-dealloc" >>>>> +#endif >>>> >>>> Likewise. >>> >>> The comment is just above so I moved it down to the #if block. >>> >>>> >>>>> +#if defined __GNUC__ && __GNUC__ >= 11 >>>>> +# pragma GCC diagnostic pop >>>>> +#endif >>>> >>>> Likewise. >>>> >>>>> diff --git a/libio/tst-wmemstream5.c b/libio/tst-wmemstream5.c >>>>> new file mode 100644 >>>>> index 0000000000..64461dbe48 >>>>> --- /dev/null >>>>> +++ b/libio/tst-wmemstream5.c >>>>> @@ -0,0 +1,40 @@ >>>> >>>> Missing file header. >>> >>> I copied tst-wmemstream1.c which doesn't have a header either, but >>> I found one elsewhere so I added it to the new test. >>> >>>> >>>>> +#include >>>>> + >>>>> +/* Verify that calling fclose on the result of open_wmemstream >>>>> doesn't >>>>> +   trigger GCC -Wmismatched-dealloc with fclose forward-declared and >>>>> +   without included in the same translation unit.  */ >>>>> + >>>>> +extern int fclose (FILE*); >>>>> + >>>>> +#if defined __GNUC__ && __GNUC__ >= 11 >>>>> +#pragma GCC diagnostic push >>>>> +#pragma GCC diagnostic error "-Wmismatched-dealloc" >>>>> +#endif >>>> >>>> Likewise. >>> >>> Okay. >>> >>>> >>>>> diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h >>>>> index 3aa27a9d25..f88f8e55b3 100644 >>>>> --- a/stdlib/stdlib.h >>>>> +++ b/stdlib/stdlib.h >>>> >>>>>   #if defined __USE_MISC || defined __USE_XOPEN_EXTENDED >>>>> @@ -798,7 +804,8 @@ extern char *canonicalize_file_name (const char >>>>> *__name) >>>>>      ENAMETOOLONG; if the name fits in fewer than PATH_MAX chars, >>>>>      returns the name in RESOLVED.  */ >>>>>   extern char *realpath (const char *__restrict __name, >>>>> -               char *__restrict __resolved) __THROW __wur; >>>>> +               char *__restrict __resolved) __THROW >>>>> +     __attr_dealloc_free __wur; >>>>>   #endif >>>> >>>> realpath only returns a pointer to the heap if RESOLVED is null, so the >>>> annotation is wrong here. >>>> >>> >>> This is intentional.  When realpath() returns the last argument >>> (when it's nonnull) passing the returned pointer to free will not >>> be diagnosed but passing it to some other deallocator not associated >>> with the function will be.  That means for example that passing >>> a pointer allocated by C++ operator new() to realpath() and then >>> deleting the pointer returned from the function as opposed to >>> the argument will trigger a false positive.  I decided this was >>> an okay trade-off because unless the function allocates memory >>> I expect the returned pointer to be ignored (similarly to how >>> the pointer returned from memcpy is ignored).  If you don't like >>> the odds I can remove the attribute from the function until we >>> have one that captures this conditional return value (I'd like >>> to add one in GCC 12). >>> >>>>> diff --git a/wcsmbs/wchar.h b/wcsmbs/wchar.h >>>>> index 9cf8b05a87..e31734158c 100644 >>>>> --- a/wcsmbs/wchar.h >>>>> +++ b/wcsmbs/wchar.h >>>>> @@ -151,7 +151,8 @@ extern size_t wcsxfrm_l (wchar_t *__s1, const >>>>> wchar_t *__s2, >>>>>                size_t __n, locale_t __loc) __THROW; >>>>>   /* Duplicate S, returning an identical malloc'd string.  */ >>>>> -extern wchar_t *wcsdup (const wchar_t *__s) __THROW >>>>> __attribute_malloc__; >>>>> +extern wchar_t *wcsdup (const wchar_t *__s) __THROW >>>>> +  __attribute_malloc__ __attr_dealloc_free; >>>>>   #endif >>>>>   /* Find the first occurrence of WC in WCS.  */ >>>>> @@ -562,9 +563,23 @@ extern wchar_t *wcpncpy (wchar_t *__restrict >>>>> __dest, >>>>>   /* Wide character I/O functions.  */ >>>>>   #if defined __USE_XOPEN2K8 || __GLIBC_USE (LIB_EXT2) >>>>> +# ifndef __attr_dealloc_fclose >>>>> +#   if defined __has_builtin >>>>> +#     if __has_builtin (__builtin_fclose) >>>>> +/* If the attribute macro hasn't been defined yet (by ) and >>>>> +   fclose is a built-in, use it.  */ >>>>> +#      define __attr_dealloc_fclose __attr_dealloc >>>>> (__builtin_fclose, 1) >>>>> +#     endif >>>>> +#   endif >>>>> +# endif >>>>> +# ifndef __attr_dealloc_fclose >>>>> +#  define __attr_dealloc_fclose /* empty */ >>>>> +# endif >>>>> + >>>>>   /* Like OPEN_MEMSTREAM, but the stream is wide oriented and produces >>>>>      a wide character string.  */ >>>>> -extern __FILE *open_wmemstream (wchar_t **__bufloc, size_t >>>>> *__sizeloc) __THROW; >>>>> +extern __FILE *open_wmemstream (wchar_t **__bufloc, size_t >>>>> *__sizeloc) __THROW >>>>> +  __attribute_malloc__ __attr_dealloc_fclose; >>>>>   #endif >>>>>   #if defined __USE_ISOC95 || defined __USE_UNIX98 >>>> >>>> Does this mean that if one includes first and then >>>> to get the declaration of fclose, fclose is not registered as a >>>> deallocator for open_wmemstream? >>> >>> Yes. >>> >>>> >>>> Maybe it is possible to redeclare open_wmemstream in if >>>> has already been included? >>> >>> It is, and I suggested that in my last reply but didn't implement >>> it because I didn't expect it to be a palatable.  I've added it >>> in the updated revision. >>> >>> Martin >> >