Add compiler option -fstrict-flex-arrays=1

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: Tristan Partin <tristan(at)partin(dot)io>, Greg Burd <greg(at)burd(dot)me>
Subject: Add compiler option -fstrict-flex-arrays=1
Date: 2026-10-19 10:27:50
Message-ID: c79da9e1-965d-4530-9e3e-84b8bde134ec@eisentraut.org
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

(I already posted this patch in another thread [0], but since this was
under another topic, I'm posting it again here with a clearer subject.)

[0]:
https://www.postgresql.org/message-id/flat/DLRI3Q0C0MR2.1V3TOIS3N16FJ%40partin.io

Compilers can check object sizes in various ways. For example, there is
-Warray-bounds, __builtin_object_size, which drives _FORTIFY_SOURCE, and
also -fsanitize=bounds, which is part of ubsan.

One perhaps surprising aspect of that is that both gcc and clang by
default assume that any trailing array in a struct could be used as a
flexible array member, independent of its declared size, and therefore
don't believe the size declaration and thus exempt that array from all
those checks.

This patch fixes that. Below is a relatively elaborate explanation, but
the patch is actually small.

Here is an example of such a struct:

typedef struct
{
...

char *server_final_message;
uint8 ServerSignature[SCRAM_MAX_KEY_LEN];
} fe_scram_state;

(SCRAM_MAX_KEY_LEN is 32.)

Because of the default assumption mentioned above, the compilers don't
check that code doesn't overrun the ServerSignature field.

There are many other structs in our code that are similarly affected.

I suppose this behavior is to accommodate legacy code, but our code uses
C99 flexible array members correctly (mostly, see below), so we can make
these checks more strict and get more benefit from the various bounds
checking mechanisms.

gcc and clang have the option -fstrict-flex-arrays [1] for this.

[1]:
https://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html#index-fstrict-flex-arrays

Ideally, we'd want to use the maximum level -fstrict-flex-arrays=3,
which would only allow flexible array members declared with the C
standard "[]". But we do need to permit flexible array members declared
with "[1]" in a few cases because standard C does not allow flexible
array members as the only element in a struct. Affected are:

- struct RevmapContents
- struct ReplicationSlotCtlData
- union pgresult_data

(Some of these have comments "It should be declared
[FLEXIBLE_ARRAY_MEMBER], but for some reason you can't do that in an
otherwise-empty struct.")

There is also:

- struct sqlda_struct

which, as I understand it, is bound by external API specification, so
not easy to change.

If we turned on -fstrict-flex-arrays=3, these would become undefined
behavior and trigger the ubsan sanitizer.

If we use the lower level -fstrict-flex-arrays=1, then flexible-array
members declared as "[]", "[0]", and "[1]" are permitted, but all the
obviously-not-flexible-array-members like the fe_scram_state example
above can now be bounds checked properly. The only struct in the code
that genuinely has a trailing one-element array is WordEntryPosVector1,
and so we would miss bounds checking on that one, but I think we can
live with that.

There is one piece of code that still needs fixing to permit
-fstrict-flex-arrays=1, which was the topic of the [0] thread:
JsonValueList has an array declared as "[2]" (via BASE_JVL_ITEMS) but
addresses it as a flexible-length array in some circumstances. My
proposal included here is to change this to an anonymous union with one
branch having the original size "[2]", to keep local variables sized the
same as before, and one branch of size "[1]", which effectively makes
this a flexible-array member under -fstrict-flex-arrays=1.

Attachment Content-Type Size
v1.5-0001-Add-compiler-option-fstrict-flex-arrays-1.patch text/plain 8.4 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Matthias van de Meent 2026-10-19 10:47:03 Re: Reducing relcache memory usage 2: shrink sizeof(RelationData)
Previous Message Matthias van de Meent 2026-10-19 10:02:55 Re: regress: query discrepancy between explain vs execution