Thread

  1. Add malloc attribute to memory allocation functions

    Tristan Partin <tristan@partin.io> — 2026-07-01T17:51:25Z

    Given that we now have a tree that compiles fine against 
    -Werror=mismatched-dealloc, we need to make sure that we don't regress. 
    By adding the malloc attribute[0], we can protect against regressions, 
    enable more accurate code coverage with -fanalyzer, and allow the 
    compiler to do some optimizations.
    
    Let me know if I missed any allocators. Additionally, does it make sense 
    to add the attribute to the various memory context allocator functions 
    like AllocSetAlloc(), or only on the higher level functions.
    
    [0]: https://gcc.gnu.org/onlinedocs/gcc/Common-Attributes.html#index-malloc
    
    -- 
    Tristan Partin
    PostgreSQL Contributors Team
    AWS (https://aws.amazon.com)
    
  2. Re: Add malloc attribute to memory allocation functions

    solai v <solai.cdac@gmail.com> — 2026-07-06T03:54:25Z

    Hi Tristan,
    I tested this patch on the current master branch.
    Before applying the patch, I checked that functions like palloc(),
    MemoryContextAlloc(), and pstrdup() were not annotated with a malloc
    attribute.
    After applying the patch, I verified that pg_attribute_malloc is added
    in src/include/c.h and that the relevant backend and frontend memory
    allocation functions are annotated appropriately. I also confirmed
    that functions intended to be freed with pfree() use
    pg_attribute_malloc(pfree), while the frontend allocation functions
    use pg_attribute_malloc(pg_free).
    The patch applied cleanly, PostgreSQL built successfully, and make
    check completed successfully with all 245 tests passing. I didn't
    notice any regressions during testing.
    
    Thanks for working on this patch.
    
    Regards,
    Solai
    
    
    
    
  3. Re: Add malloc attribute to memory allocation functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-07-06T04:26:54Z

    "Tristan Partin" <tristan@partin.io> writes:
    > Given that we now have a tree that compiles fine against 
    > -Werror=mismatched-dealloc, we need to make sure that we don't regress. 
    > By adding the malloc attribute[0], we can protect against regressions, 
    > enable more accurate code coverage with -fanalyzer, and allow the 
    > compiler to do some optimizations.
    
    I'm skeptical that this is going to lead to anything but grief.
    In particular, since gcc has never heard of memory contexts,
    I don't see how we are not going to get buried in bogus
    -Wanalyzer-malloc-leak warnings.  It doesn't really help
    to add compiler annotations that only sort-of match our semantics.
    
    (This opinion is based on years of dismissing useless Coverity
    warnings of this kind.)
    
    			regards, tom lane
    
    
    
    
  4. Re: Add malloc attribute to memory allocation functions

    Tristan Partin <tristan@partin.io> — 2026-07-06T16:34:58Z

    On Mon Jul 6, 2026 at 4:26 AM UTC, Tom Lane wrote:
    > "Tristan Partin" <tristan@partin.io> writes:
    >> Given that we now have a tree that compiles fine against 
    >> -Werror=mismatched-dealloc, we need to make sure that we don't regress. 
    >> By adding the malloc attribute[0], we can protect against regressions, 
    >> enable more accurate code coverage with -fanalyzer, and allow the 
    >> compiler to do some optimizations.
    >
    > I'm skeptical that this is going to lead to anything but grief.
    > In particular, since gcc has never heard of memory contexts,
    > I don't see how we are not going to get buried in bogus
    > -Wanalyzer-malloc-leak warnings.  It doesn't really help
    > to add compiler annotations that only sort-of match our semantics.
    >
    > (This opinion is based on years of dismissing useless Coverity
    > warnings of this kind.)
    
    This is a fair criticism. On master, the number of 
    -Wanalyzer-malloc-leak warnings is 62. With this patch applied, it 
    balloons to 598.
    
    -- 
    Tristan Partin
    PostgreSQL Contributors Team
    AWS (https://aws.amazon.com)
    
    
    
    
  5. Re: Add malloc attribute to memory allocation functions

    Peter Eisentraut <peter@eisentraut.org> — 2026-07-07T15:34:43Z

    On 06.07.26 18:34, Tristan Partin wrote:
    > On Mon Jul 6, 2026 at 4:26 AM UTC, Tom Lane wrote:
    >> "Tristan Partin" <tristan@partin.io> writes:
    >>> Given that we now have a tree that compiles fine against
    >>> -Werror=mismatched-dealloc, we need to make sure that we don't regress.
    >>> By adding the malloc attribute[0], we can protect against regressions,
    >>> enable more accurate code coverage with -fanalyzer, and allow the
    >>> compiler to do some optimizations.
    >>
    >> I'm skeptical that this is going to lead to anything but grief.
    >> In particular, since gcc has never heard of memory contexts,
    >> I don't see how we are not going to get buried in bogus
    >> -Wanalyzer-malloc-leak warnings.  It doesn't really help
    >> to add compiler annotations that only sort-of match our semantics.
    >>
    >> (This opinion is based on years of dismissing useless Coverity
    >> warnings of this kind.)
    > 
    > This is a fair criticism. On master, the number of
    > -Wanalyzer-malloc-leak warnings is 62. With this patch applied, it
    > balloons to 598.
    
    But this can also check for a lot more, such as
    
    - mismatching deallocator
    - double free
    - use after free
    - free of things that are not an allocation
    
    If we could tell it, check for all these things but don't worry about 
    the leaks, that could be useful.
    
    Also, for frontend tools, libpq, etc. that don't use memory contexts.
    
    
    
    
    
  6. Re: Add malloc attribute to memory allocation functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-07-07T15:53:36Z

    Peter Eisentraut <peter@eisentraut.org> writes:
    > On 06.07.26 18:34, Tristan Partin wrote:
    >> On Mon Jul 6, 2026 at 4:26 AM UTC, Tom Lane wrote:
    >>> I'm skeptical that this is going to lead to anything but grief.
    >>> In particular, since gcc has never heard of memory contexts,
    >>> I don't see how we are not going to get buried in bogus
    >>> -Wanalyzer-malloc-leak warnings.  It doesn't really help
    >>> to add compiler annotations that only sort-of match our semantics.
    
    > But this can also check for a lot more, such as
    > - mismatching deallocator
    > - double free
    > - use after free
    > - free of things that are not an allocation
    > If we could tell it, check for all these things but don't worry about 
    > the leaks, that could be useful.
    
    > Also, for frontend tools, libpq, etc. that don't use memory contexts.
    
    Yeah, I was thinking about that last point.  The frontend environment
    is a lot closer to the semantics these markers expect, so we could
    try doing this in frontend only and see how well that works.
    
    There are still places that I'd expect to be trouble.  For example,
    Coverity has never understood the pattern we use in pg_dump's data
    collection subroutines, ie, malloc a big array of structs, fill
    the individual structs and insert pointers to them into the hash
    tables, done.  It always thinks we leaked the array, and I suspect
    tools like this will too.  In Coverity's case there's enough
    infrastructure to dismiss individual false-positive complaints,
    and then it won't bug you about them (until somebody changes the
    relevant code enough that the dismissal doesn't match :-().  Unless
    there's some similar way to silence individual reports, I don't
    foresee tools like this to be usable.  We're not going to change
    coding patterns like that one just because some static analyzer
    doesn't understand them.
    
    			regards, tom lane
    
    
    
    
  7. Re: Add malloc attribute to memory allocation functions

    Tristan Partin <tristan@partin.io> — 2026-07-07T15:54:45Z

    On Tue Jul 7, 2026 at 10:34 AM CDT, Peter Eisentraut wrote:
    > On 06.07.26 18:34, Tristan Partin wrote:
    >> On Mon Jul 6, 2026 at 4:26 AM UTC, Tom Lane wrote:
    >>> "Tristan Partin" <tristan@partin.io> writes:
    >>>> Given that we now have a tree that compiles fine against
    >>>> -Werror=mismatched-dealloc, we need to make sure that we don't regress.
    >>>> By adding the malloc attribute[0], we can protect against regressions,
    >>>> enable more accurate code coverage with -fanalyzer, and allow the
    >>>> compiler to do some optimizations.
    >>>
    >>> I'm skeptical that this is going to lead to anything but grief.
    >>> In particular, since gcc has never heard of memory contexts,
    >>> I don't see how we are not going to get buried in bogus
    >>> -Wanalyzer-malloc-leak warnings.  It doesn't really help
    >>> to add compiler annotations that only sort-of match our semantics.
    >>>
    >>> (This opinion is based on years of dismissing useless Coverity
    >>> warnings of this kind.)
    >> 
    >> This is a fair criticism. On master, the number of
    >> -Wanalyzer-malloc-leak warnings is 62. With this patch applied, it
    >> balloons to 598.
    >
    > But this can also check for a lot more, such as
    >
    > - mismatching deallocator
    > - double free
    > - use after free
    > - free of things that are not an allocation
    >
    > If we could tell it, check for all these things but don't worry about 
    > the leaks, that could be useful.
    >
    > Also, for frontend tools, libpq, etc. that don't use memory contexts.
    
    We could add -Wno-analyzer-malloc-leak to backend code.
    
    -- 
    Tristan Partin
    PostgreSQL Contributors Team
    AWS (https://aws.amazon.com)
    
    
    
    
  8. Re: Add malloc attribute to memory allocation functions

    Michael Paquier <michael@paquier.xyz> — 2026-07-07T22:58:14Z

    On Tue, Jul 07, 2026 at 11:53:36AM -0400, Tom Lane wrote:
    > There are still places that I'd expect to be trouble.  For example,
    > Coverity has never understood the pattern we use in pg_dump's data
    > collection subroutines, ie, malloc a big array of structs, fill
    > the individual structs and insert pointers to them into the hash
    > tables, done.  It always thinks we leaked the array, and I suspect
    > tools like this will too.  In Coverity's case there's enough
    > infrastructure to dismiss individual false-positive complaints,
    > and then it won't bug you about them (until somebody changes the
    > relevant code enough that the dismissal doesn't match :-().  Unless
    > there's some similar way to silence individual reports, I don't
    > foresee tools like this to be usable.  We're not going to change
    > coding patterns like that one just because some static analyzer
    > doesn't understand them.
    
    Additional question.  Does this help with requirements like the one
    listed in fe-exec.c for PQfreemem() under WIN32?  If the answer to
    this question is yes, then it would sound like a win for me, we'd had
    our share of issues in the past where we would use a free() call that
    interacts with an allocation done in a completely different context
    library-wise.  That's something WIN32 cares a lot about, to mention
    one place.
    --
    Michael
    
  9. Re: Add malloc attribute to memory allocation functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-07-07T23:54:06Z

    Michael Paquier <michael@paquier.xyz> writes:
    > Additional question.  Does this help with requirements like the one
    > listed in fe-exec.c for PQfreemem() under WIN32?
    
    Yeah, I was wondering about that point too.  It would be really nice
    to have automated checks for that.  Maybe the right thing to do here
    (to start anyway) is to put in targeted annotations that address
    specific pain points like that one.
    
    One issue that'd have to be dealt with is that that would involve
    putting annotations into the public header file libpq-fe.h.  We'd need
    to be sure we do not break things for applications using compilers
    other than what we built libpq with.  That seems reasonably easy to do
    with some macro trickery, but it's a point to keep in mind.
    
    			regards, tom lane
    
    
    
    
  10. Re: Add malloc attribute to memory allocation functions

    Tristan Partin <tristan@partin.io> — 2026-07-07T23:57:23Z

    On Tue Jul 7, 2026 at 6:54 PM CDT, Tom Lane wrote:
    > Michael Paquier <michael@paquier.xyz> writes:
    >> Additional question.  Does this help with requirements like the one
    >> listed in fe-exec.c for PQfreemem() under WIN32?
    >
    > Yeah, I was wondering about that point too.  It would be really nice
    > to have automated checks for that.  Maybe the right thing to do here
    > (to start anyway) is to put in targeted annotations that address
    > specific pain points like that one.
    >
    > One issue that'd have to be dealt with is that that would involve
    > putting annotations into the public header file libpq-fe.h.  We'd need
    > to be sure we do not break things for applications using compilers
    > other than what we built libpq with.  That seems reasonably easy to do
    > with some macro trickery, but it's a point to keep in mind.
    
    Ok, I think the path forward is two-fold:
    
    1. Add the annotations to specific pain points as Tom suggested 
       (PGfreemem(), frontend memory allocators)
    2. Add the annotations to the backend allocators knowing that the patch 
       may not be committed
    
    -- 
    Tristan Partin
    PostgreSQL Contributors Team
    AWS (https://aws.amazon.com)