Detecting use-after-free bugs using gcc's malloc() attribute

Hi,

I played around with adding
  __attribute__((malloc(free_func), malloc(another_free_func)))
annotations to a few functions in pg. See
https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html


Adding them to pg_list.h seems to have found two valid issues when compiling
without optimization:

[1001/2331 22  42%] Compiling C object src/backend/postgres_lib.a.p/commands_tablecmds.c.o
../../../../home/andres/src/postgresql/src/backend/commands/tablecmds.c: In function ‘ATExecAttachPartition’:
../../../../home/andres/src/postgresql/src/backend/commands/tablecmds.c:17966:25: warning: pointer
‘partBoundConstraint’may be used after ‘list_concat’ [-Wuse-after-free]
 
17966 |                         get_proposed_default_constraint(partBoundConstraint);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../../home/andres/src/postgresql/src/backend/commands/tablecmds.c:17919:26: note: call to ‘list_concat’ here
17919 |         partConstraint = list_concat(partBoundConstraint,
      |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
17920 |                                                                  RelationGetPartitionQual(rel));
      |                                                                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[1233/2331 22  52%] Compiling C object src/backend/postgres_lib.a.p/rewrite_rewriteHandler.c.o
../../../../home/andres/src/postgresql/src/backend/rewrite/rewriteHandler.c: In function ‘rewriteRuleAction’:
../../../../home/andres/src/postgresql/src/backend/rewrite/rewriteHandler.c:550:41: warning: pointer ‘newjointree’ may
beused after ‘list_concat’ [-Wuse-after-free]
 
  550 |                                         checkExprHasSubLink((Node *) newjointree);
      |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../../home/andres/src/postgresql/src/backend/rewrite/rewriteHandler.c:542:33: note: call to ‘list_concat’ here
  542 |                                 list_concat(newjointree, sub_action->jointree->fromlist);
      |                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Both of these bugs seem somewhat older, the latter going back to 19ff959bff0,
in 2005.  I'm a bit surprised that we haven't hit them before, via
DEBUG_LIST_MEMORY_USAGE?


When compiling with optimization, another issue is reported:

[508/1814 22  28%] Compiling C object src/backend/postgres_lib.a.p/commands_explain.c.o
../../../../home/andres/src/postgresql/src/backend/commands/explain.c: In function 'ExplainNode':
../../../../home/andres/src/postgresql/src/backend/commands/explain.c:2037:25: warning: pointer 'ancestors' may be used
after'lcons' [-Wuse-after-free]
 
 2037 |                         show_upper_qual(plan->qual, "Filter", planstate, ancestors, es);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function 'show_group_keys',
    inlined from 'ExplainNode' at ../../../../home/andres/src/postgresql/src/backend/commands/explain.c:2036:4:
../../../../home/andres/src/postgresql/src/backend/commands/explain.c:2564:21: note: call to 'lcons' here
 2564 |         ancestors = lcons(plan, ancestors);
      |                     ^~~~~~~~~~~~~~~~~~~~~~

which looks like it might be valid - the caller's "ancestors" variable could
now be freed?  There do appear to be further instances of the issue, e.g. in
show_agg_keys(), that aren't flagged for some reason.



For something like pg_list.h the malloc(free) attribute is a bit awkward to
use, because one a) needs to list ~30 functions that can free a list and b)
the referenced functions need to be declared.

In my quick hack I just duplicated the relevant part of pg_list.h and added
the appropriate attributes to the copy of the original declaration.


I also added such attributes to bitmapset.h and palloc() et al, but that
didn't find existing bugs.  It does find use-after-free instances if I add
some, similarly it does find cases of mismatching palloc with free etc.


The attached prototype is quite rough and will likely fail on anything but a
recent gcc (likely >= 12).


Do others think this would be useful enough to be worth polishing? And do you
agree the warnings above are bugs?

Greetings,

Andres Freund