[PATCH] analyzer: Recognize __builtin_free as a matching deallocator

[PATCH] analyzer: Recognize __builtin_free as a matching deallocator

[Siddhesh Poyarekar]

Recognize __builtin_free as being equivalent to free when passed into
__attribute__((malloc ())), similar to how it is treated when it is
encountered as a call.  This fixes spurious warnings in glibc where
xmalloc family of allocators as well as reallocarray, memalign,
etc. are declared to have __builtin_free as the free function.

	gcc/analyzer/ChangeLog:
	* sm-malloc.cc
	(malloc_state_machine::get_or_create_deallocator): Recognize
	__builtin_free.

	gcc/testsuite/ChangeLog:
	* gcc.dg/analyzer/attr-malloc-1.c (compatible_alloc,
	compatible_alloc2): New extern allocator declarations.
	(test_9, test_10): New tests.
---
 gcc/analyzer/sm-malloc.cc                     |  3 ++-
 gcc/testsuite/gcc.dg/analyzer/attr-malloc-1.c | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc
index 9707a6863ce..1d69d57df0e 100644
--- a/gcc/analyzer/sm-malloc.cc
+++ b/gcc/analyzer/sm-malloc.cc
@@ -1511,7 +1511,8 @@ malloc_state_machine::get_or_create_deallocator (tree deallocator_fndecl)
   /* Reuse "free".  */
   deallocator *d;
   if (is_named_call_p (deallocator_fndecl, "free")
-      || is_std_named_call_p (deallocator_fndecl, "free"))
+      || is_std_named_call_p (deallocator_fndecl, "free")
+      || is_named_call_p (deallocator_fndecl, "__builtin_free"))
     d = &m_free.m_deallocator;
   else
     {
diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-1.c b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-1.c
index 3de32b1b14b..a9a2a3dee85 100644
--- a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-1.c
@@ -11,6 +11,12 @@ extern struct foo *foo_acquire (void)
 extern void use_foo (const struct foo *)
   __attribute__((nonnull));
 
+extern struct foo *compatible_alloc (void)
+  __attribute__ ((malloc (__builtin_free)));
+
+extern struct foo *compatible_alloc2 (void)
+  __attribute__ ((malloc (free)));
+
 void test_1 (void)
 {
   struct foo *p = foo_acquire ();
@@ -73,3 +79,16 @@ int test_8 (struct foo *p)
   foo_release (p);
   return p->m_int; /* { dg-warning "use after 'foo_release' of 'p'" } */
 }
+
+/* Recognize that __builtin_free and free are the same thing.  */
+void test_9 (void)
+{
+  struct foo *p = compatible_alloc ();
+  free (p);
+}
+
+void test_10 (void)
+{
+  struct foo *p = compatible_alloc2 ();
+  __builtin_free (p);
+}
-- 
2.31.1

Re: [PATCH] analyzer: Recognize __builtin_free as a matching deallocator

[David Malcolm]

On Wed, 2021-07-28 at 10:34 +0530, Siddhesh Poyarekar wrote:
> Recognize __builtin_free as being equivalent to free when passed into
> __attribute__((malloc ())), similar to how it is treated when it is
> encountered as a call.  This fixes spurious warnings in glibc where
> xmalloc family of allocators as well as reallocarray, memalign,
> etc. are declared to have __builtin_free as the free function.
> 
>         gcc/analyzer/ChangeLog:
>         * sm-malloc.cc
>         (malloc_state_machine::get_or_create_deallocator): Recognize
>         __builtin_free.
> 
>         gcc/testsuite/ChangeLog:
>         * gcc.dg/analyzer/attr-malloc-1.c (compatible_alloc,
>         compatible_alloc2): New extern allocator declarations.
>         (test_9, test_10): New tests.

Looks good to me, thanks
Dave

Re: [PATCH] analyzer: Recognize __builtin_free as a matching deallocator

[Matthias Klose]

On 7/28/21 1:44 PM, David Malcolm via Gcc-patches wrote:
> On Wed, 2021-07-28 at 10:34 +0530, Siddhesh Poyarekar wrote:
>> Recognize __builtin_free as being equivalent to free when passed into
>> __attribute__((malloc ())), similar to how it is treated when it is
>> encountered as a call.  This fixes spurious warnings in glibc where
>> xmalloc family of allocators as well as reallocarray, memalign,
>> etc. are declared to have __builtin_free as the free function.
>>
>>         gcc/analyzer/ChangeLog:
>>         * sm-malloc.cc
>>         (malloc_state_machine::get_or_create_deallocator): Recognize
>>         __builtin_free.
>>
>>         gcc/testsuite/ChangeLog:
>>         * gcc.dg/analyzer/attr-malloc-1.c (compatible_alloc,
>>         compatible_alloc2): New extern allocator declarations.
>>         (test_9, test_10): New tests.
> 
> Looks good to me, thanks
> Dave
> 
> 


Please could this be backported to all active branches?

Re: [PATCH] analyzer: Recognize __builtin_free as a matching deallocator

[Siddhesh Poyarekar]

On 8/25/21 5:44 PM, Matthias Klose wrote:
> On 7/28/21 1:44 PM, David Malcolm via Gcc-patches wrote:
>> On Wed, 2021-07-28 at 10:34 +0530, Siddhesh Poyarekar wrote:
>>> Recognize __builtin_free as being equivalent to free when passed into
>>> __attribute__((malloc ())), similar to how it is treated when it is
>>> encountered as a call.  This fixes spurious warnings in glibc where
>>> xmalloc family of allocators as well as reallocarray, memalign,
>>> etc. are declared to have __builtin_free as the free function.
>>>
>>>          gcc/analyzer/ChangeLog:
>>>          * sm-malloc.cc
>>>          (malloc_state_machine::get_or_create_deallocator): Recognize
>>>          __builtin_free.
>>>
>>>          gcc/testsuite/ChangeLog:
>>>          * gcc.dg/analyzer/attr-malloc-1.c (compatible_alloc,
>>>          compatible_alloc2): New extern allocator declarations.
>>>          (test_9, test_10): New tests.
>>
>> Looks good to me, thanks
>> Dave
>>
>>
> 
> 
> Please could this be backported to all active branches?
> 

Sure, it looks like only gcc11 needs this since malloc attribute 
matching seems recent.  David, I've never done a backport before, may I 
just cherry-pick, push and post a [committed] patch on list or does it 
need to go through review?

Thanks,
Siddhesh

Re: [PATCH] analyzer: Recognize __builtin_free as a matching deallocator

[Jeff Law]

On 8/25/2021 9:16 AM, Siddhesh Poyarekar wrote:
> On 8/25/21 5:44 PM, Matthias Klose wrote:
>> On 7/28/21 1:44 PM, David Malcolm via Gcc-patches wrote:
>>> On Wed, 2021-07-28 at 10:34 +0530, Siddhesh Poyarekar wrote:
>>>> Recognize __builtin_free as being equivalent to free when passed into
>>>> __attribute__((malloc ())), similar to how it is treated when it is
>>>> encountered as a call.  This fixes spurious warnings in glibc where
>>>> xmalloc family of allocators as well as reallocarray, memalign,
>>>> etc. are declared to have __builtin_free as the free function.
>>>>
>>>>          gcc/analyzer/ChangeLog:
>>>>          * sm-malloc.cc
>>>>          (malloc_state_machine::get_or_create_deallocator): Recognize
>>>>          __builtin_free.
>>>>
>>>>          gcc/testsuite/ChangeLog:
>>>>          * gcc.dg/analyzer/attr-malloc-1.c (compatible_alloc,
>>>>          compatible_alloc2): New extern allocator declarations.
>>>>          (test_9, test_10): New tests.
>>>
>>> Looks good to me, thanks
>>> Dave
>>>
>>>
>>
>>
>> Please could this be backported to all active branches?
>>
>
> Sure, it looks like only gcc11 needs this since malloc attribute 
> matching seems recent.  David, I've never done a backport before, may 
> I just cherry-pick, push and post a [committed] patch on list or does 
> it need to go through review?
In general it's fine to cherry-pick fixes from the mainline to a release 
branch that fix regressions, incorrect code and the like.

This doesn't fall into one of those categories -- but given this is 
limited to the analyzer, I think it's fine to cherry-pick into the 
release branch as long as David is OK with it as well.

jeff