| From: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
|---|---|
| To: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Cc: | Andres Freund <andres(at)anarazel(dot)de> |
| Subject: | compiler attribute alloc_size |
| Date: | 2026-11-13 11:50:38 |
| Message-ID: | cadec43a-d031-41cb-b887-3b4a29720017@eisentraut.org |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
In my ventures to apply more compiler attributes, I came to alloc_size.
This is to annotate memory allocation functions so that the compiler can
know the size of the allocated memory and so some cross-checking based
on that. This is all pretty straightforward and I think we should do it.
Andres had previously reported some issues he encountered in [0]. (That
message didn't go into details on what attributes or compiler options
were used, but it certainly looked very closely related.)
The issue is that under-allocating a struct is bad. With this
attribute, the compiler flags this, because it looks like a mistake.
For uses where this is legitimate, you'd normally use a struct with a
trailing flexible-array member, and we mostly make use of that. But
there are a couple of cases where we assign to a struct pointer less
memory than what the struct expects. In some cases, this is done with a
union where we only allocate enough memory for one branch of the union,
which seems like an intuitive thing to do but is actually also bad.
In the attached patches I have some fixes and/or workarounds for the
issues that I found.
Note that as documented this attribute and the associated warning
options are for diagnostics only, they don't change compilation. So in
that sense, once all warnings are fixed, this shouldn't create any more
hidden issues.
- varattrib_4b is a union of a normal varlena with a 4-byte header and 0
or more payload bytes, and a compressed format with a 4-byte header, a
4-byte "tcinfo", followed by payload data. The typical coding pattern
val = palloc(VARHDRSZ + len);
SET_VARSIZE(val, VARHDRSZ + len);
could get flagged by -Warray-bounds because the allocation could be too
short to cover both branches of the union. In this case, I think we
don't actually need the union, so I'm replacing this with two separate
structs.
- TRGM in contrib/pg_trgm/ is an instance of what [0] talks about: It
has a trailing flexible array member, but it also has trailing padding,
so allocating palloc(TRGMHDRSIZE) causes a warning, because it's less
than sizeof(TRGM). In my patch I work around this by allocating like
palloc(Max(TRGMHDRSIZE, sizeof(TRGM))).
- There is a bit of a maze of different structs and unions that underlie
the numeric type. The allocation (Numeric) palloc(NUMERIC_HDRSZ_SHORT)
gets flagged by the compiler. In my patch I change this to (Numeric)
palloc(sizeof(struct NumericData)), which allocates a bit more but makes
the sizes and types match. Maybe some more judicious reshuffling of the
underlying data structures could avoid this, or maybe it's okay like this.
- As of commit 6a3da13d376 from this week, FormData_pg_attribute has 4
bytes of trailing padding, and so sizeof(FormData_pg_attribute) is now
different from ATTRIBUTE_FIXED_PART_SIZE. This causes a warning in one
place. We had a discussion about this some time ago [1], where the
opinions diverged between like, it's important to keep this distinction
in case we ever have padding bytes, and, it's already incorrect and
would be even more incorrect if we had padding bytes. ;-) Well, we're
here now, so perhaps this should be revisited.
This is all that I found. [0] lists a lot more sources of potential
problems, but apparently they are not flagged as they are currently used.
[1]:
https://www.postgresql.org/message-id/flat/7c7d41c1-fd5d-f9a5-afd5-f00168d91fe2%402ndquadrant.com
| Attachment | Content-Type | Size |
|---|---|---|
| 0001-Apply-alloc_size-attribute-to-memory-allocation-func.patch | text/plain | 11.6 KB |
| 0002-Fixes-for-alloc_size-attribute-varatt.patch | text/plain | 7.5 KB |
| 0003-Fixes-for-alloc_size-attribute-pg_trgm.patch | text/plain | 2.1 KB |
| 0004-Fixes-for-alloc_size-attribute-numeric.patch | text/plain | 1.1 KB |
| 0005-XXX-ATTRIBUTE_FIXED_PART_SIZE-fix.patch | text/plain | 1.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Alexandre Felipe | 2026-11-13 11:57:24 | Re: pg_regress: schedule multi-line test groups |
| Previous Message | Hayato Kuroda (Fujitsu) | 2026-11-13 11:49:11 | RE: Parallel Apply |