On 11/30/20 3:53 PM, Jeff Law wrote:
On 11/13/20 2:45 PM, Martin Sebor via Gcc-patches wrote:
Bug 94527 is request from the kernel developers for an attribute
to indicate that a user-defined function deallocates an object
allocated by an earlier call to an allocation function. Their
goal is to detect misuses of such functions and the pointers or
objects returned from them.
The recently submitted patches(*) enable the detection of a subset
of such misuses for standard allocation functions like malloc and
free, but those are just a small fraction of allocation/deallocation
functions used in practice, and only rarely used in the kernel
(mostly in utility programs). The attached patch extends attribute
malloc to enable this detection also for user-defined functions.
The design extends attribute malloc to accept one or two optional
arguments: one naming a deallocation function that deallocates
pointers returned from the malloc-like function, and another to
denote the position of the pointer argument in the deallocation
functions parameter list. Any number of deallocators can be
associated with any number of allocators. This makes it possible
to annotate, for example, all the POSIX <stdio.h> functions that
open and close FILE streams and detect mismatches between any
pairs that aren't suitable (in addition to calling free on
a FILE* returned from fopen, for instance).
An association with an allocator results in adding an internal
"*dealloc" attribute to the deallocator so that the former can
be quickly looked up based on a call to the latter.
Tested on x86_64-linux + Glibc & Binutils/GDB (no instances
of the new warnings).
Martin
[*] Prerequisite patch
add -Wmismatched-new-delete to middle end (PR 90629)
https://gcc.gnu.org/pipermail/gcc-patches/2020-November/557987.html
PS In pr94527 Jonathan notes that failing to properly match pairs
of calls isn't limited to APIs that return pointers and applies
to other kinds of "handles" including integers (e.g., the POSIX
open/close APIs), and a detection of such mismatches would be
helpful as well. David submitted a prototype of this for
the analyzer here:
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555544.html
I chose not to implement nonpointer detection for some of the same
reasons as mentioned in comment #8 on the bug (and also because
there's no support for it in the machinery I use). I also didn't
use the same attribute as David, in part because I think it's better
to provide separate attributes for pointer APIs and for others
(integers), and in part also because the deallocated_by attribute
design as is cannot accommodate my goal of supporting app standard
functions (including the <stdio.h> freopen which "deallocates"
the third argument).
gcc-94527.diff
PR middle-end/94527 - Add an __attribute__ that marks a function as freeing an
object
gcc/ChangeLog:
PR middle-end/94527
* builtins.c (gimple_call_alloc_p): Handle user-defined functions.
(fndecl_alloc_p): New helper.
(call_dealloc_argno): New helper.
(gimple_call_dealloc_p): Call it.
(call_dealloc_p): Same.
(matching_alloc_calls_p): Handle user-defined functions.
(maybe_emit_free_warning): Same.
* doc/extend.texi (attribute malloc): Update.
* doc/invoke.texi (-Wmismatched-dealloc): Document new option.
gcc/c-family/ChangeLog:
PR middle-end/94527
* c-attribs.c (handle_dealloc_attribute): New function.
(handle_malloc_attribute): Handle argument forms of attribute.
* c.opt (-Wmismatched-dealloc): New option.
(-Wmismatched-new-delete): Update description.
gcc/testsuite/ChangeLog:
PR middle-end/94527
* g++.dg/warn/Wmismatched-dealloc-2.C: New test.
* g++.dg/warn/Wmismatched-dealloc.C: New test.
* gcc.dg/Wmismatched-dealloc.c: New test.
* gcc.dg/attr-malloc.c: New test.
OK once prereq is wrapped up.
Presumably you're referring to this patch:
https://gcc.gnu.org/pipermail/gcc-patches/2020-November/557987.html
Is there something I need to do to move it forward? (I'm
assuming you're still reviewing the rest of the patch so
please let me know if you're waiting for me to remove
the objectionable function first.)
I realize the dealloc attribute is currently internal only to make
lookups quick. Any thoughts on whether or not we might want to make it
a user visible attribute at some point?
I see no problem with exposing both attributes.
Martin
