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

[Martin Sebor <msebor@gmail.com>]

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
>