Hi,
In <czsdsnyeuhul.399xlpgcjs...@neon.tech>
"Re: meson: Specify -Wformat as a common warning flag for extensions" on Wed,
13 Mar 2024 00:43:11 -0500,
"Tristan Partin" <tris...@neon.tech> wrote:
> Perhaps adding some more clarification in the comments that I wrote.
>
> - # -Wformat-security requires -Wformat, so check for it
> + # -Wformat-secuirty requires -Wformat. We compile with -Wall in + #
> Postgres, which includes -Wformat=1. -Wformat is shorthand for + #
> -Wformat=1. The set of flags which includes -Wformat-security is + #
> persisted into pg_config --cflags, which is commonly used by + #
> PGXS-based extensions. The lack of -Wformat in the persisted flags
> + # will produce a warning on many GCC versions, so even though adding
> + # -Wformat here is a no-op for Postgres, it silences other use
> cases.
>
> That might be too long-winded though :).
Thanks for the wording! I used it for the v3 patch.
Thanks,
--
kou
>From 0ba2e6dd55b00ee8a57313a629a1e5fa8c9e40cc Mon Sep 17 00:00:00 2001
From: Sutou Kouhei <k...@clear-code.com>
Date: Wed, 13 Mar 2024 16:10:34 +0900
Subject: [PATCH v3] Add -Wformat to common warning flags
We specify -Wformat-security as a common warning flag explicitly. GCC
requires -Wformat to be added for -Wformat-security to take effect. If
-Wformat-security is used without -Wformat, GCC shows the following
warning:
cc1: warning: '-Wformat-security' ignored without '-Wformat' [-Wformat-security]
Note that this is not needed for PostgreSQL itself because PostgreSQL
uses -Wall, which includes -Wformat=1. -Wformat is shorthand for
-Wformat=1. These flags without -Wall are persisted into "pg_config
--cflags", which is commonly used by PGXS-based extensions. So
PGXS-based extensions will get the warning without -Wformat.
Co-authored-by: Tristan Partin <tris...@neon.tech>
---
configure | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++
configure.ac | 10 ++++++
meson.build | 9 +++++
3 files changed, 118 insertions(+)
diff --git a/configure b/configure
index 70a1968003..7b0fda3825 100755
--- a/configure
+++ b/configure
@@ -6013,6 +6013,105 @@ if test x"$pgac_cv_prog_CXX_cxxflags__Wshadow_compatible_local" = x"yes"; then
fi
+ # -Wformat-security requires -Wformat. We compile with -Wall in
+ # PostgreSQL, which includes -Wformat=1. -Wformat is shorthand for
+ # -Wformat=1. The set of flags which includes -Wformat-security is
+ # persisted into pg_config --cflags, which is commonly used by
+ # PGXS-based extensions. The lack of -Wformat in the persisted flags
+ # will produce a warning on many GCC versions, so even though adding
+ # -Wformat here is a no-op for PostgreSQL, it silences other use
+ # cases., so check for it.
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wformat, for CFLAGS" >&5
+$as_echo_n "checking whether ${CC} supports -Wformat, for CFLAGS... " >&6; }
+if ${pgac_cv_prog_CC_cflags__Wformat+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CFLAGS=$CFLAGS
+pgac_save_CC=$CC
+CC=${CC}
+CFLAGS="${CFLAGS} -Wformat"
+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__Wformat=yes
+else
+ pgac_cv_prog_CC_cflags__Wformat=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__Wformat" >&5
+$as_echo "$pgac_cv_prog_CC_cflags__Wformat" >&6; }
+if test x"$pgac_cv_prog_CC_cflags__Wformat" = x"yes"; then
+ CFLAGS="${CFLAGS} -Wformat"
+fi
+
+
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CXX} supports -Wformat, for CXXFLAGS" >&5
+$as_echo_n "checking whether ${CXX} supports -Wformat, for CXXFLAGS... " >&6; }
+if ${pgac_cv_prog_CXX_cxxflags__Wformat+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CXXFLAGS=$CXXFLAGS
+pgac_save_CXX=$CXX
+CXX=${CXX}
+CXXFLAGS="${CXXFLAGS} -Wformat"
+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__Wformat=yes
+else
+ pgac_cv_prog_CXX_cxxflags__Wformat=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__Wformat" >&5
+$as_echo "$pgac_cv_prog_CXX_cxxflags__Wformat" >&6; }
+if test x"$pgac_cv_prog_CXX_cxxflags__Wformat" = x"yes"; then
+ CXXFLAGS="${CXXFLAGS} -Wformat"
+fi
+
+
# This was included in -Wall/-Wformat in older GCC versions
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wformat-security, for CFLAGS" >&5
diff --git a/configure.ac b/configure.ac
index 52fd7af446..e4776b9963 100644
--- a/configure.ac
+++ b/configure.ac
@@ -533,6 +533,16 @@ if test "$GCC" = yes -a "$ICC" = no; then
PGAC_PROG_CXX_CFLAGS_OPT([-Wcast-function-type])
PGAC_PROG_CC_CFLAGS_OPT([-Wshadow=compatible-local])
PGAC_PROG_CXX_CFLAGS_OPT([-Wshadow=compatible-local])
+ # -Wformat-security requires -Wformat. We compile with -Wall in
+ # PostgreSQL, which includes -Wformat=1. -Wformat is shorthand for
+ # -Wformat=1. The set of flags which includes -Wformat-security is
+ # persisted into pg_config --cflags, which is commonly used by
+ # PGXS-based extensions. The lack of -Wformat in the persisted flags
+ # will produce a warning on many GCC versions, so even though adding
+ # -Wformat here is a no-op for PostgreSQL, it silences other use
+ # cases., so check for it.
+ PGAC_PROG_CC_CFLAGS_OPT([-Wformat])
+ PGAC_PROG_CXX_CFLAGS_OPT([-Wformat])
# This was included in -Wall/-Wformat in older GCC versions
PGAC_PROG_CC_CFLAGS_OPT([-Wformat-security])
PGAC_PROG_CXX_CFLAGS_OPT([-Wformat-security])
diff --git a/meson.build b/meson.build
index 55184db248..732cb59b1f 100644
--- a/meson.build
+++ b/meson.build
@@ -1822,6 +1822,15 @@ common_warning_flags = [
'-Wimplicit-fallthrough=3',
'-Wcast-function-type',
'-Wshadow=compatible-local',
+ # -Wformat-security requires -Wformat. We compile with -Wall in
+ # PostgreSQL, which includes -Wformat=1. -Wformat is shorthand for
+ # -Wformat=1. The set of flags which includes -Wformat-security is
+ # persisted into pg_config --cflags, which is commonly used by
+ # PGXS-based extensions. The lack of -Wformat in the persisted flags
+ # will produce a warning on many GCC versions, so even though adding
+ # -Wformat here is a no-op for PostgreSQL, it silences other use
+ # cases., so check for it.
+ '-Wformat',
# This was included in -Wall/-Wformat in older GCC versions
'-Wformat-security',
]
--
2.43.0
