On Mon Oct 19, 2026 at 10:27 AM UTC, Peter Eisentraut wrote: > (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.
We could define a pg_attribute_strict_flex_array() macro and add it to the entry in WordEntryPosVector1. We can add a comment to the macro indicating how and why it should be used since it'll likely be a little non-obvious, but we could continue to keep bounds checking on WordEntryPosVector1. > 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. Assuming you think my previous suggestion is reasonable, I think the patch with my proposed change is good to go. Otherwise, the patch as is will is also good. -- Tristan Partin PostgreSQL Contributors Team AWS (https://aws.amazon.com)
