On 27.10.26 16:51, Tristan Partin wrote:
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.
Something like in the attached patch 0002? (Patch 0001 is unchanged.)
From b3c29df1be209b46aed6006a5b9415d8c9af0ef7 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <[email protected]> Date: Mon, 5 Oct 2026 07:52:57 +0200 Subject: [PATCH v2 1/2] Add compiler option -fstrict-flex-arrays=1 Currently, every trailing array regardless of its declared bound is exempt from -Warray-bounds, __builtin_object_size/_FORTIFY_SOURCE, and -fsanitize=bounds. -fstrict-flex-arrays=1 says that only arrays declared with bounds [], [0], or [1] can be flexible array members (which are then mostly exempt from array bounds checking); for all other arrays, the declared size is taken as is for bounds checking, which is what we want. This applies to a large number of structs in the tree, for example - struct pg_hmac_ctx - fe_scram_state - struct XidCache - struct RelPathStr ... which can now be checked properly. Ideally, we'd want to use the maximum level -fstrict-flex-arrays=3, which would also disqualify flexible array members declared with bounds [0] or [1]. But we do need the [1] variant 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 Also: - struct sqlda_struct (bound by external API specification) Turning on -fstrict-flex-arrays=3 for these would conjure undefined behavior. One piece of code needs to be adjusted: JsonValueList has an array declared as [2] (via BASE_JVL_ITEMS) but addresses it as a flexible-length array in some circumstances. 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. We can't make it a fully flexible-array member [], because JsonValueList is also used for a field in the middle of JsonTablePlanState, which is not allowed in standard C. There is one structure with a genuine trailing one-element array: - WordEntryPosVector1 By staying with -fstrict-flex-arrays=1 we miss out on bounds checking on that one. --- configure | 92 +++++++++++++++++++++++++++ configure.ac | 3 + meson.build | 2 + src/backend/utils/adt/jsonpath_exec.c | 16 ++++- 4 files changed, 112 insertions(+), 1 deletion(-) diff --git a/configure b/configure index 6b05d4c84ab..8d29a0bf6a0 100755 --- a/configure +++ b/configure @@ -6421,6 +6421,98 @@ if test x"$pgac_cv_prog_CXX_cxxflags__fexcess_precision_standard" = x"yes"; then fi + # Treat only arrays with [], [0], or [1] as flexible array members. + +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -fstrict-flex-arrays=1, for CFLAGS" >&5 +$as_echo_n "checking whether ${CC} supports -fstrict-flex-arrays=1, for CFLAGS... " >&6; } +if ${pgac_cv_prog_CC_cflags__fstrict_flex_arrays_1+:} false; then : + $as_echo_n "(cached) " >&6 +else + pgac_save_CFLAGS=$CFLAGS +pgac_save_CC=$CC +CC=${CC} +CFLAGS="${CFLAGS} -fstrict-flex-arrays=1" +ac_save_c_werror_flag=$ac_c_werror_flag +ac_c_werror_flag=yes +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +int +main () +{ + + ; + return 0; +} +_ACEOF +if ac_fn_c_try_compile "$LINENO"; then : + pgac_cv_prog_CC_cflags__fstrict_flex_arrays_1=yes +else + pgac_cv_prog_CC_cflags__fstrict_flex_arrays_1=no +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext +ac_c_werror_flag=$ac_save_c_werror_flag +CFLAGS="$pgac_save_CFLAGS" +CC="$pgac_save_CC" +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CC_cflags__fstrict_flex_arrays_1" >&5 +$as_echo "$pgac_cv_prog_CC_cflags__fstrict_flex_arrays_1" >&6; } +if test x"$pgac_cv_prog_CC_cflags__fstrict_flex_arrays_1" = x"yes"; then + CFLAGS="${CFLAGS} -fstrict-flex-arrays=1" +fi + + + { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CXX} supports -fstrict-flex-arrays=1, for CXXFLAGS" >&5 +$as_echo_n "checking whether ${CXX} supports -fstrict-flex-arrays=1, for CXXFLAGS... " >&6; } +if ${pgac_cv_prog_CXX_cxxflags__fstrict_flex_arrays_1+:} false; then : + $as_echo_n "(cached) " >&6 +else + pgac_save_CXXFLAGS=$CXXFLAGS +pgac_save_CXX=$CXX +CXX=${CXX} +CXXFLAGS="${CXXFLAGS} -fstrict-flex-arrays=1" +ac_save_cxx_werror_flag=$ac_cxx_werror_flag +ac_cxx_werror_flag=yes +ac_ext=cpp +ac_cpp='$CXXCPP $CPPFLAGS' +ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5' +ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' +ac_compiler_gnu=$ac_cv_cxx_compiler_gnu + +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +int +main () +{ + + ; + return 0; +} +_ACEOF +if ac_fn_cxx_try_compile "$LINENO"; then : + pgac_cv_prog_CXX_cxxflags__fstrict_flex_arrays_1=yes +else + pgac_cv_prog_CXX_cxxflags__fstrict_flex_arrays_1=no +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext +ac_ext=c +ac_cpp='$CPP $CPPFLAGS' +ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5' +ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' +ac_compiler_gnu=$ac_cv_c_compiler_gnu + +ac_cxx_werror_flag=$ac_save_cxx_werror_flag +CXXFLAGS="$pgac_save_CXXFLAGS" +CXX="$pgac_save_CXX" +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CXX_cxxflags__fstrict_flex_arrays_1" >&5 +$as_echo "$pgac_cv_prog_CXX_cxxflags__fstrict_flex_arrays_1" >&6; } +if test x"$pgac_cv_prog_CXX_cxxflags__fstrict_flex_arrays_1" = x"yes"; then + CXXFLAGS="${CXXFLAGS} -fstrict-flex-arrays=1" +fi + + # Optimization flags for specific files that benefit from loop unrolling { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -funroll-loops, for CFLAGS_UNROLL_LOOPS" >&5 $as_echo_n "checking whether ${CC} supports -funroll-loops, for CFLAGS_UNROLL_LOOPS... " >&6; } diff --git a/configure.ac b/configure.ac index 6864268eaa0..36e4f9ca0cf 100644 --- a/configure.ac +++ b/configure.ac @@ -610,6 +610,9 @@ if test "$GCC" = yes -a "$ICC" = no; then # Disable FP optimizations that cause various errors on gcc 4.5+ or maybe 4.6+ PGAC_PROG_CC_CFLAGS_OPT([-fexcess-precision=standard]) PGAC_PROG_CXX_CFLAGS_OPT([-fexcess-precision=standard]) + # Treat only arrays with [], [0], or [1] as flexible array members. + PGAC_PROG_CC_CFLAGS_OPT([-fstrict-flex-arrays=1]) + PGAC_PROG_CXX_CFLAGS_OPT([-fstrict-flex-arrays=1]) # Optimization flags for specific files that benefit from loop unrolling PGAC_PROG_CC_VAR_OPT(CFLAGS_UNROLL_LOOPS, [-funroll-loops]) # Optimization flags for specific files that benefit from vectorization diff --git a/meson.build b/meson.build index 7e95fd5941d..89ab2b82807 100644 --- a/meson.build +++ b/meson.build @@ -2185,6 +2185,8 @@ common_functional_flags = [ # Disable optimizations that assume no overflow; needed for gcc 4.3+ '-fwrapv', '-fexcess-precision=standard', + # Treat only arrays with [], [0], or [1] as flexible array members. + '-fstrict-flex-arrays=1', # Without -fpch-deps gcc emits dependencies that are insufficient for ccache # to trigger a rebuild when the precompiled header changes. We could make # this depend on using gcc and precompiled headers being enabled, but that's diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c index 2191d67bfc0..a3d0a541f28 100644 --- a/src/backend/utils/adt/jsonpath_exec.c +++ b/src/backend/utils/adt/jsonpath_exec.c @@ -174,7 +174,21 @@ typedef struct JsonValueList int maxitems; /* allocated length of items[] */ struct JsonValueList *next; /* => next chunk, if any */ struct JsonValueList *last; /* => last chunk (only valid in base chunk) */ - JsonbValue items[BASE_JVL_ITEMS]; + union + { + /* + * To satisfy -fstrict-flex-arrays=1, we need one union member that + * makes space for the base chunk and one member that is effectively + * declared as a flexible-array member, otherwise you'd get + * -fsanitize=undefined errors. Note that we can't declare items as + * [FLEXIBLE_ARRAY_MEMBER], because JsonValueList is used in a + * non-last field of JsonTablePlanState, and that's not allowed with + * flexible-array members. But using [1] works as long as we don't go + * higher than -fstrict-flex-arrays=1. + */ + JsonbValue base_items[BASE_JVL_ITEMS]; + JsonbValue items[1]; + }; } JsonValueList; /* State data for iterating through a JsonValueList */ base-commit: 700b422af7e26cf310caf3fb76d78f4c8ca39c1f -- 2.56.0
From 667a9dbcb448738795bc9f51429160d8aed94b5c Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <[email protected]> Date: Wed, 28 Oct 2026 08:55:50 +0100 Subject: [PATCH v2 2/2] squash! Add compiler option -fstrict-flex-arrays=1 Also provide an attribute macro pg_attribute_strict_flex_array that allows overriding the strict flex array level for a particular struct. Use that to annotate WordEntryPosVector1. Note that clang supports the command-line option but not the attribute, so we can only use this to optionally raise the level for some structs, but we can't do the opposite of having a higher global level and lower it for some exceptions. Discussion: https://www.postgresql.org/message-id/flat/c79da9e1-965d-4530-9e3e-84b8bde134ec%40eisentraut.org --- src/include/c.h | 19 +++++++++++++++++++ src/include/tsearch/ts_type.h | 2 +- src/tools/pgindent/pgindent | 1 + 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/include/c.h b/src/include/c.h index b285755e8c4..944c851b3c3 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -319,6 +319,25 @@ extern "C++" #define pg_attribute_returns_nonnull #endif +/* + * pg_attribute_strict_flex_array(N) overrides -fstrict-flex-arrays=N for a + * particular struct field. It can be used for example to annotate genuine + * trailing one-element array fields, which would under -fstrict-flex-arrays=1 + * otherwise be considered flexible array members and be excluded from bounds + * checking. Use pg_attribute_strict_flex_array(3) for such a case. + * + * Note that it is possible that the command-line option is supported but not + * the attribute. To stay on the safe side, the attribute should not set a + * lower level than the global setting given by the command-line option. + * + * https://gcc.gnu.org/onlinedocs/gcc/Common-Attributes.html#index-strict_005fflex_005farray + */ +#if __has_attribute (strict_flex_array) +#define pg_attribute_strict_flex_array(N) __attribute__((strict_flex_array(N))) +#else +#define pg_attribute_strict_flex_array(N) +#endif + /* * pg_attribute_target allows specifying different target options that the * function should be compiled with (e.g., for using special CPU instructions). diff --git a/src/include/tsearch/ts_type.h b/src/include/tsearch/ts_type.h index 345a2dab930..fdec91fdf13 100644 --- a/src/include/tsearch/ts_type.h +++ b/src/include/tsearch/ts_type.h @@ -76,7 +76,7 @@ typedef struct typedef struct { uint16 npos; - WordEntryPos pos[1]; + pg_attribute_strict_flex_array(3) WordEntryPos pos[1]; } WordEntryPosVector1; #define MAXNUMPOS (256) /* semi-arbitrary limit on npos */ diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent index fa2041dc34b..f75c006bbff 100755 --- a/src/tools/pgindent/pgindent +++ b/src/tools/pgindent/pgindent @@ -253,6 +253,7 @@ sub write_source my @declaration_attributes = qw( alignas pg_attribute_counted_by + pg_attribute_strict_flex_array ); # Text of each declaration attribute, stashed by pre_indent() and restored -- 2.56.0
