Re: Fix mismatched deallocation functions

On Thu May 7, 2026 at 10:01 PM UTC, Zsolt Parragi wrote:
> Hello!
>
> There are many cases missed by the script, for example:
>
> tab-complete.in.c:7089:
>
> `previous_words = pg_malloc_array(char *, point);`
>
> tab-complete.in.c:6364:
>
> `completion_ref_object = pg_strdup(word);`
>
> tab-complete.in.c:7090:
>
> `*buffer = (char *) pg_malloc(point * 2);`
>
> There's also completion_ref_schema, which is an out parameter of
> parse_identifier, still freed in the patch.

Thanks. I have adjusted the script to cover these instances, and fixed
the patch in v2.

> The strtokx change in stringutils.c is also strange - the patch
> converts one free at line 96, and leaves the same free a few lines
> above at line 73 as is.

Thanks. Fixed this as well in the script. Fixed in v2.

>> I generated the patch with the help of Coccinelle[0]. I'm no expert with
>> Coccinelle, but it seemed like a good candidate to get this refactor
>> done. You can run the attached script in your tree with the following
>> command:
>
> If I had to do it, I would try to approach this with static analysis
> tools instead: a custom rule that enforces attribute declarations for
> return values / output parameters allocated by pg_malloc and similar
> functions. Without attributes everywhere, these checks will never be
> complete because tools won't be able to fully reason about cross
> source file call paths.
> For example clang-tidy even has an auto fix mode that could apply
> these attributes automatically.
>
> With the attributes in place, we would automatically receive warnings
> for every incorrect free attribute, which a tool could then
> automatically fix.

This would be nice. GCC already has some of this functionality:
__attribute__((malloc(pfree))). When you add the deallocator, GCC will
warn: -Wmismatched-dealloc. Perhaps there was some confusion in my
initial proposal. I cannot confirm if this patch fixes every instance,
but what it does do is get all my __attribute__((malloc)) additions
added to the codebase without causing additional compiler warning noise,
which would be a prerequisite to committing the __attribute__((malloc))
additions. I would think that as GCC continues to evolve, we could get
even more warnings generated to help us find further issues with
mismatched deallocation functions.

> If we want to avoid generating noise by placing attributes everywhere
> in the source (I'm not sure how noisy that would be), that part could
> be a specialized CI run instead, since the transformation itself can
> be automated.

The current size of my attribute patch is not very noisy at all:

src/include/c.h | 12 ++++++++++++
src/include/common/fe_memutils.h | 36 ++++++++++++++++++------------------
src/include/utils/palloc.h | 30 +++++++++++++++---------------
3 files changed, 45 insertions(+), 33 deletions(-)

I would advocate for adding the attributes, which is why I have
submitted this preliminary patch. In addition to -Wmismatched-dealloc,
GCC also can use -Wfree-nonheap-object, when the attributes are present.

Attached you will find v2 of the patch and v2 of the Coccinelle script.
For more reading on __attribute__((malloc)), see the GCC documentation:
https://gcc.gnu.org/onlinedocs/gcc/Common-Attributes.html#index-malloc.

--
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)