Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix mismatched deallocation functions

  1. Fix mismatched deallocation functions

    Tristan Partin <tristan@partin.io> — 2026-05-06T23:26:42Z

    In fe_memutils.h, we have two sets of allocation/deallocation functions: 
those that start pg_ and those that start with p. pg_malloc vs palloc, 
pg_free vs pfree, etc. My understanding is that we probably want to 
match the allocator with the deallocator whenever possible, but that is 
not the case in HEAD. We have quite a few mismatches.

I discovered these issues when adding some __attribute__((malloc)) 
annotations to our allocation functions. gcc presents warnings in the 
form of:

	../src/bin/psql/tab-complete.in.c: In function ‘psql_completion’:
	../src/bin/psql/tab-complete.in.c:2143:9: warning: ‘free’ called on pointer returned from a mismatched allocation function [-Wmismatched-dealloc]
	 2143 |         free(text_copy);
	      |         ^~~~~~~~~~~~~~~
	../src/bin/psql/tab-complete.in.c:1971:33: note: returned from ‘pnstrdup’
	 1971 |         char       *text_copy = pnstrdup(rl_line_buffer + start, end - start);
	      |                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Memory that we allocated pnstrdup() is being deallocated with free(). 
In actuality, this is not an issue because pnstrdup() uses malloc() 
internally. However, it does create a little bit of mental overhead when 
analyzing memory management in code paths. Warnings like this are also 
a barrier to annotating our memory allocation functions with 
__attribute__((malloc)).

The supplied patch allows for a clean compliation on my Linux x86_64 
machine with malloc attribute applied. I will send that patch later 
after a little bit more development.

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:

	spatch --sp-file allocators.cocci --allow-inconsistent-paths \
		--in-place .

[0]: https://coccinelle.gitlabpages.inria.fr/website/

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

  2. Re: Fix mismatched deallocation functions

    Zsolt Parragi <zsolt.parragi@percona.com> — 2026-05-07T22:00:54Z

    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.

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.

> 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.

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.

  3. Re: Fix mismatched deallocation functions

    Tristan Partin <tristan@partin.io> — 2026-05-28T20:21:40Z

    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)