agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedmeson: Specify -Wformat as a common warning flag for extensions
17+ messages / 5 participants
[nested] [flat]
* meson: Specify -Wformat as a common warning flag for extensions
@ 2024-01-22 05:11 Sutou Kouhei <kou@clear-code.com>
2024-02-22 07:41 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-03-08 05:39 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
0 siblings, 2 replies; 17+ messages in thread
From: Sutou Kouhei @ 2024-01-22 05:11 UTC (permalink / raw)
To: pgsql-hackers
Hi,
I'm an extension developer. If I use PostgreSQL built with
Meson, I get the following warning:
cc1: warning: '-Wformat-security' ignored without '-Wformat' [-Wformat-security]
Because "pg_config --cflags" includes -Wformat-security but
doesn't include -Wformat.
Can we specify -Wformat as a common warning flag too? If we
do it, "pg_config --cflags" includes both of
-Wformat-security and -Wformat. So I don't get the warning.
Thanks,
--
kou
Attachments:
[text/x-patch] v1-0001-meson-Specify-Wformat-explicitly-for-extensions.patch (1.6K, ../../20240122.141139.931086145628347157.kou@clear-code.com/2-v1-0001-meson-Specify-Wformat-explicitly-for-extensions.patch)
download | inline diff:
From 0913033512c9b75ee3d2941c89ff8696f3c5f53b Mon Sep 17 00:00:00 2001
From: Sutou Kouhei <kou@clear-code.com>
Date: Mon, 22 Jan 2024 13:51:58 +0900
Subject: [PATCH v1] meson: Specify -Wformat explicitly for extensions
We specify -Wformat-security as a common warning flag explicitly. Our
common warning flags are used by extensions via
pgxs/src/Makefile.global or "pg_config --cflags".
If -Wformat-security is used without -Wall/-Wformat, GCC shows the
following warning:
cc1: warning: '-Wformat-security' ignored without '-Wformat' [-Wformat-security]
We can't assume that all extensions use -Wall/-Wformat. So specifying
only -Wformat-security may cause the warning.
If we specify -Wformat explicitly, the warning isn't shown.
---
meson.build | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/meson.build b/meson.build
index 55184db248..de6ce778fc 100644
--- a/meson.build
+++ b/meson.build
@@ -1822,6 +1822,14 @@ common_warning_flags = [
'-Wimplicit-fallthrough=3',
'-Wcast-function-type',
'-Wshadow=compatible-local',
+ # This is for preventing the "cc1: warning: '-Wformat-security'
+ # ignored without '-Wformat' [-Wformat-security]" warning. We don't
+ # need this for PostgreSQL itself. This is just for
+ # extensions. Extensions use "pg_config --cflags" to build
+ # themselves. If extensions use only -Wformat-security, the warning
+ # is shown. If we have this here, extensions use both of -Wformat
+ # and -Wformat-security. So the warning isn't shown.
+ '-Wformat',
# This was included in -Wall/-Wformat in older GCC versions
'-Wformat-security',
]
--
2.43.0
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: meson: Specify -Wformat as a common warning flag for extensions
2024-01-22 05:11 meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
@ 2024-02-22 07:41 ` Sutou Kouhei <kou@clear-code.com>
1 sibling, 0 replies; 17+ messages in thread
From: Sutou Kouhei @ 2024-02-22 07:41 UTC (permalink / raw)
To: pgsql-hackers
Hi,
Could someone take a look at this?
Patch is attached in the original e-mail:
https://www.postgresql.org/message-id/20240122.141139.931086145628347157.kou%40clear-code.com
Thanks,
--
kou
In <20240122.141139.931086145628347157.kou@clear-code.com>
"meson: Specify -Wformat as a common warning flag for extensions" on Mon, 22 Jan 2024 14:11:39 +0900 (JST),
Sutou Kouhei <kou@clear-code.com> wrote:
> Hi,
>
> I'm an extension developer. If I use PostgreSQL built with
> Meson, I get the following warning:
>
> cc1: warning: '-Wformat-security' ignored without '-Wformat' [-Wformat-security]
>
> Because "pg_config --cflags" includes -Wformat-security but
> doesn't include -Wformat.
>
> Can we specify -Wformat as a common warning flag too? If we
> do it, "pg_config --cflags" includes both of
> -Wformat-security and -Wformat. So I don't get the warning.
>
>
> Thanks,
> --
> kou
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: meson: Specify -Wformat as a common warning flag for extensions
2024-01-22 05:11 meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
@ 2024-03-08 05:39 ` Tristan Partin <tristan@neon.tech>
2024-03-08 06:32 ` Re: meson: Specify -Wformat as a common warning flag for extensions Michael Paquier <michael@paquier.xyz>
1 sibling, 1 reply; 17+ messages in thread
From: Tristan Partin @ 2024-03-08 05:39 UTC (permalink / raw)
To: Sutou Kouhei <kou@clear-code.com>; +Cc: pgsql-hackers
On Sun Jan 21, 2024 at 11:11 PM CST, Sutou Kouhei wrote:
> Hi,
>
> I'm an extension developer. If I use PostgreSQL built with
> Meson, I get the following warning:
>
> cc1: warning: '-Wformat-security' ignored without '-Wformat' [-Wformat-security]
>
> Because "pg_config --cflags" includes -Wformat-security but
> doesn't include -Wformat.
>
> Can we specify -Wformat as a common warning flag too? If we
> do it, "pg_config --cflags" includes both of
> -Wformat-security and -Wformat. So I don't get the warning.
The GCC documentation[0] says the following:
> If -Wformat is specified, also warn about uses of format functions
> that represent possible security problems. At present, this warns
> about calls to printf and scanf functions where the format string is
> not a string literal and there are no format arguments, as in printf
> (foo);. This may be a security hole if the format string came from
> untrusted input and contains ‘%n’. (This is currently a subset of what
> -Wformat-nonliteral warns about, but in future warnings may be added
> to -Wformat-security that are not included in -Wformat-nonliteral.)
It sounds like a legitimate issue. I have confirmed the issue exists
with a pg_config compiled with Meson. I can also confirm that this issue
exists in the autotools build.
Here is a v2 of your patch which includes the fix for autotools. I will
mark this "Ready for Committer" in the commitfest. Thanks!
[0]: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
--
Tristan Partin
Neon (https://neon.tech)
Attachments:
[text/x-patch] v2-0001-Add-Wformat-to-common-warning-flags.patch (4.8K, ../../CZO4L8L0KP7K.1RWJWEAGL8UMN@neon.tech/2-v2-0001-Add-Wformat-to-common-warning-flags.patch)
download | inline diff:
From bce0f70f866bb0e3f8fb8eb14c05bbbdd27c51f2 Mon Sep 17 00:00:00 2001
From: Sutou Kouhei <kou@clear-code.com>
Date: Mon, 22 Jan 2024 13:51:58 +0900
Subject: [PATCH v2] 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]
Co-authored-by: Sutou Kouhei <kou@clear-code.com>
---
configure | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++
configure.ac | 3 ++
meson.build | 2 ++
3 files changed, 97 insertions(+)
diff --git a/configure b/configure
index 36feeafbb23..eafab247e1d 100755
--- a/configure
+++ b/configure
@@ -5985,6 +5985,98 @@ if test x"$pgac_cv_prog_CXX_cxxflags__Wshadow_compatible_local" = x"yes"; then
fi
+ # -Wformat-security requires -Wformat, 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 57f734879e1..87a8860ecca 100644
--- a/configure.ac
+++ b/configure.ac
@@ -527,6 +527,9 @@ 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, 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 85788f9dd8f..717fdc4746b 100644
--- a/meson.build
+++ b/meson.build
@@ -1826,6 +1826,8 @@ common_warning_flags = [
'-Wimplicit-fallthrough=3',
'-Wcast-function-type',
'-Wshadow=compatible-local',
+ # -Wformat-security requires -Wformat, so check for it
+ '-Wformat',
# This was included in -Wall/-Wformat in older GCC versions
'-Wformat-security',
]
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: meson: Specify -Wformat as a common warning flag for extensions
2024-01-22 05:11 meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-03-08 05:39 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
@ 2024-03-08 06:32 ` Michael Paquier <michael@paquier.xyz>
2024-03-08 09:17 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-03-08 16:05 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
0 siblings, 2 replies; 17+ messages in thread
From: Michael Paquier @ 2024-03-08 06:32 UTC (permalink / raw)
To: Tristan Partin <tristan@neon.tech>; +Cc: Sutou Kouhei <kou@clear-code.com>; pgsql-hackers
On Thu, Mar 07, 2024 at 11:39:39PM -0600, Tristan Partin wrote:
> It sounds like a legitimate issue. I have confirmed the issue exists with a
> pg_config compiled with Meson. I can also confirm that this issue exists in
> the autotools build.
First time I'm hearing about that, but I'll admit that I am cheating
because -Wformat is forced in my local builds for some time now. I'm
failing to see the issue with meson and ./configure even if I remove
the switch, though, using a recent version of gcc at 13.2.0, but
perhaps Debian does something underground. Are there version and/or
environment requirements to be aware of?
Forcing -Wformat implies more stuff that can be disabled with
-Wno-format-contains-nul, -Wno-format-extra-args, and
-Wno-format-zero-length, but the thing is that we're usually very
conservative with such additions in the scripts. See also
8b6f5f25102f, done, I guess, as an answer to this thread:
https://www.postgresql.org/message-id/4D431505.9010002%40dunslane.net
A quick look at the past history of pgsql-hackers does not mention
that as a problem, either, but I may have missed something.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../Zeqw9vGrYlb250aO@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: meson: Specify -Wformat as a common warning flag for extensions
2024-01-22 05:11 meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-03-08 05:39 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-08 06:32 ` Re: meson: Specify -Wformat as a common warning flag for extensions Michael Paquier <michael@paquier.xyz>
@ 2024-03-08 09:17 ` Sutou Kouhei <kou@clear-code.com>
1 sibling, 0 replies; 17+ messages in thread
From: Sutou Kouhei @ 2024-03-08 09:17 UTC (permalink / raw)
To: michael@paquier.xyz; +Cc: tristan@neon.tech; pgsql-hackers
Hi,
In <Zeqw9vGrYlb250aO@paquier.xyz>
"Re: meson: Specify -Wformat as a common warning flag for extensions" on Fri, 8 Mar 2024 15:32:22 +0900,
Michael Paquier <michael@paquier.xyz> wrote:
> Are there version and/or
> environment requirements to be aware of?
I'm using Debian GNU/Linux sid and I can reproduce with gcc
8-13:
$ for x in {8..13}; do; echo gcc-${x}; gcc-${x} -Wformat-security -E - < /dev/null > /dev/null; done
gcc-8
cc1: warning: -Wformat-security ignored without -Wformat [-Wformat-security]
gcc-9
cc1: warning: '-Wformat-security' ignored without '-Wformat' [-Wformat-security]
gcc-10
cc1: warning: '-Wformat-security' ignored without '-Wformat' [-Wformat-security]
gcc-11
cc1: warning: '-Wformat-security' ignored without '-Wformat' [-Wformat-security]
gcc-12
cc1: warning: '-Wformat-security' ignored without '-Wformat' [-Wformat-security]
gcc-13
cc1: warning: '-Wformat-security' ignored without '-Wformat' [-Wformat-security]
$
I tried this on Ubuntu 22.04 too but this isn't reproduced:
$ gcc-11 -Wformat-security -E - < /dev/null > /dev/null
$
It seems that Ubuntu enables -Wformat by default:
$ gcc-11 -Wno-format -Wformat-security -E - < /dev/null > /dev/null
cc1: warning: '-Wformat-security' ignored without '-Wformat' [-Wformat-security]
I tried this on AlmaLinux 9 too and this is reproduced:
$ gcc --version
gcc (GCC) 11.4.1 20230605 (Red Hat 11.4.1-2)
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ gcc -Wformat-security -E - < /dev/null > /dev/null
cc1: warning: '-Wformat-security' ignored without '-Wformat' [-Wformat-security]
> Forcing -Wformat implies more stuff that can be disabled with
> -Wno-format-contains-nul, -Wno-format-extra-args, and
> -Wno-format-zero-length, but the thing is that we're usually very
> conservative with such additions in the scripts. See also
> 8b6f5f25102f, done, I guess, as an answer to this thread:
> https://www.postgresql.org/message-id/4D431505.9010002%40dunslane.net
I think that this is not a problem. Because the comment
added by 8b6f5f25102f ("This was included in -Wall/-Wformat
in older GCC versions") implies that we want to always use
-Wformat-security. -Wformat-security isn't worked without
-Wformat:
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat-security
> If -Wformat is specified, also warn about uses of format
> functions that represent possible security problems.
Thanks,
--
kou
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: meson: Specify -Wformat as a common warning flag for extensions
2024-01-22 05:11 meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-03-08 05:39 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-08 06:32 ` Re: meson: Specify -Wformat as a common warning flag for extensions Michael Paquier <michael@paquier.xyz>
@ 2024-03-08 16:05 ` Tristan Partin <tristan@neon.tech>
2024-03-12 23:56 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-03-13 07:38 ` Re: meson: Specify -Wformat as a common warning flag for extensions Peter Eisentraut <peter@eisentraut.org>
1 sibling, 2 replies; 17+ messages in thread
From: Tristan Partin @ 2024-03-08 16:05 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Sutou Kouhei <kou@clear-code.com>; pgsql-hackers
On Fri Mar 8, 2024 at 12:32 AM CST, Michael Paquier wrote:
> On Thu, Mar 07, 2024 at 11:39:39PM -0600, Tristan Partin wrote:
> > It sounds like a legitimate issue. I have confirmed the issue exists with a
> > pg_config compiled with Meson. I can also confirm that this issue exists in
> > the autotools build.
>
> First time I'm hearing about that, but I'll admit that I am cheating
> because -Wformat is forced in my local builds for some time now. I'm
> failing to see the issue with meson and ./configure even if I remove
> the switch, though, using a recent version of gcc at 13.2.0, but
> perhaps Debian does something underground. Are there version and/or
> environment requirements to be aware of?
>
> Forcing -Wformat implies more stuff that can be disabled with
> -Wno-format-contains-nul, -Wno-format-extra-args, and
> -Wno-format-zero-length, but the thing is that we're usually very
> conservative with such additions in the scripts. See also
> 8b6f5f25102f, done, I guess, as an answer to this thread:
> https://www.postgresql.org/message-id/4D431505.9010002%40dunslane.net
>
> A quick look at the past history of pgsql-hackers does not mention
> that as a problem, either, but I may have missed something.
Ok, I figured this out. -Wall implies -Wformat=1. We set warning_level
to 1 in the Meson project() call, which implies -Wall, and set -Wall in
CFLAGS for autoconf. That's the reason we don't get issues building
Postgres. A user making use of the pg_config --cflags option, as Sutou
is, *will* run into the aforementioned issues, since we don't propogate
-Wall into pg_config.
$ gcc $(pg_config --cflags) -E - < /dev/null > /dev/null
cc1: warning: ‘-Wformat-security’ ignored without ‘-Wformat’ [-Wformat-security]
$ gcc -Wall $(pg_config --cflags) -E - < /dev/null > /dev/null
(nothing printed)
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: meson: Specify -Wformat as a common warning flag for extensions
2024-01-22 05:11 meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-03-08 05:39 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-08 06:32 ` Re: meson: Specify -Wformat as a common warning flag for extensions Michael Paquier <michael@paquier.xyz>
2024-03-08 16:05 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
@ 2024-03-12 23:56 ` Sutou Kouhei <kou@clear-code.com>
2024-03-13 05:43 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
1 sibling, 1 reply; 17+ messages in thread
From: Sutou Kouhei @ 2024-03-12 23:56 UTC (permalink / raw)
To: tristan@neon.tech; +Cc: michael@paquier.xyz; pgsql-hackers
Hi,
In <CZOHWDYQJQCQ.23A5RRV1E05N2@neon.tech>
"Re: meson: Specify -Wformat as a common warning flag for extensions" on Fri, 08 Mar 2024 10:05:27 -0600,
"Tristan Partin" <tristan@neon.tech> wrote:
> Ok, I figured this out. -Wall implies -Wformat=1. We set warning_level
> to 1 in the Meson project() call, which implies -Wall, and set -Wall
> in CFLAGS for autoconf. That's the reason we don't get issues building
> Postgres. A user making use of the pg_config --cflags option, as Sutou
> is, *will* run into the aforementioned issues, since we don't
> propogate -Wall into pg_config.
>
> $ gcc $(pg_config --cflags) -E - < /dev/null > /dev/null
> cc1: warning: ‘-Wformat-security’ ignored without ‘-Wformat’
> [-Wformat-security]
> $ gcc -Wall $(pg_config --cflags) -E - < /dev/null > /dev/null
> (nothing printed)
Thanks for explaining this. You're right. This is the reason
why we don't need this for PostgreSQL itself but we need
this for PostgreSQL extensions. Sorry. I should have
explained this in the first e-mail...
What should we do to proceed this patch?
Thanks,
--
kou
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: meson: Specify -Wformat as a common warning flag for extensions
2024-01-22 05:11 meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-03-08 05:39 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-08 06:32 ` Re: meson: Specify -Wformat as a common warning flag for extensions Michael Paquier <michael@paquier.xyz>
2024-03-08 16:05 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-12 23:56 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
@ 2024-03-13 05:43 ` Tristan Partin <tristan@neon.tech>
2024-03-13 07:12 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
0 siblings, 1 reply; 17+ messages in thread
From: Tristan Partin @ 2024-03-13 05:43 UTC (permalink / raw)
To: Sutou Kouhei <kou@clear-code.com>; +Cc: michael@paquier.xyz; pgsql-hackers
On Tue Mar 12, 2024 at 6:56 PM CDT, Sutou Kouhei wrote:
> Hi,
>
> In <CZOHWDYQJQCQ.23A5RRV1E05N2@neon.tech>
> "Re: meson: Specify -Wformat as a common warning flag for extensions" on Fri, 08 Mar 2024 10:05:27 -0600,
> "Tristan Partin" <tristan@neon.tech> wrote:
>
> > Ok, I figured this out. -Wall implies -Wformat=1. We set warning_level
> > to 1 in the Meson project() call, which implies -Wall, and set -Wall
> > in CFLAGS for autoconf. That's the reason we don't get issues building
> > Postgres. A user making use of the pg_config --cflags option, as Sutou
> > is, *will* run into the aforementioned issues, since we don't
> > propogate -Wall into pg_config.
> >
> > $ gcc $(pg_config --cflags) -E - < /dev/null > /dev/null
> > cc1: warning: ‘-Wformat-security’ ignored without ‘-Wformat’
> > [-Wformat-security]
> > $ gcc -Wall $(pg_config --cflags) -E - < /dev/null > /dev/null
> > (nothing printed)
>
> Thanks for explaining this. You're right. This is the reason
> why we don't need this for PostgreSQL itself but we need
> this for PostgreSQL extensions. Sorry. I should have
> explained this in the first e-mail...
>
>
> What should we do to proceed this patch?
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 :).
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: meson: Specify -Wformat as a common warning flag for extensions
2024-01-22 05:11 meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-03-08 05:39 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-08 06:32 ` Re: meson: Specify -Wformat as a common warning flag for extensions Michael Paquier <michael@paquier.xyz>
2024-03-08 16:05 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-12 23:56 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-03-13 05:43 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
@ 2024-03-13 07:12 ` Sutou Kouhei <kou@clear-code.com>
0 siblings, 0 replies; 17+ messages in thread
From: Sutou Kouhei @ 2024-03-13 07:12 UTC (permalink / raw)
To: tristan@neon.tech; +Cc: michael@paquier.xyz; pgsql-hackers
Hi,
In <CZSDSNYEUHUL.399XLPGCJSJ5H@neon.tech>
"Re: meson: Specify -Wformat as a common warning flag for extensions" on Wed, 13 Mar 2024 00:43:11 -0500,
"Tristan Partin" <tristan@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
Attachments:
[text/x-patch] v3-0001-Add-Wformat-to-common-warning-flags.patch (6.4K, ../../20240313.161200.2215289102254898854.kou@clear-code.com/2-v3-0001-Add-Wformat-to-common-warning-flags.patch)
download | inline diff:
From 0ba2e6dd55b00ee8a57313a629a1e5fa8c9e40cc Mon Sep 17 00:00:00 2001
From: Sutou Kouhei <kou@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 <tristan@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
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: meson: Specify -Wformat as a common warning flag for extensions
2024-01-22 05:11 meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-03-08 05:39 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-08 06:32 ` Re: meson: Specify -Wformat as a common warning flag for extensions Michael Paquier <michael@paquier.xyz>
2024-03-08 16:05 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
@ 2024-03-13 07:38 ` Peter Eisentraut <peter@eisentraut.org>
2024-03-15 09:36 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
1 sibling, 1 reply; 17+ messages in thread
From: Peter Eisentraut @ 2024-03-13 07:38 UTC (permalink / raw)
To: Tristan Partin <tristan@neon.tech>; Michael Paquier <michael@paquier.xyz>; +Cc: Sutou Kouhei <kou@clear-code.com>; pgsql-hackers
On 08.03.24 17:05, Tristan Partin wrote:
> Ok, I figured this out. -Wall implies -Wformat=1. We set warning_level
> to 1 in the Meson project() call, which implies -Wall, and set -Wall in
> CFLAGS for autoconf. That's the reason we don't get issues building
> Postgres. A user making use of the pg_config --cflags option, as Sutou
> is, *will* run into the aforementioned issues, since we don't propogate
> -Wall into pg_config.
(The actual mechanism for extensions is that they get CFLAGS from
Makefile.global, but pg_config has the same underlying issue.)
I think the fix then is to put -Wall into CFLAGS in Makefile.global.
Looking at a diff of Makefile.global between an autoconf and a meson
build, I also see that under meson, CFLAGS doesn't get -O2 -g (or
similar, depending on settings). This presumably has the same
underlying issue that meson handles those flags internally.
For someone who wants to write a fix for this, the relevant variable is
var_cflags in our meson scripts. And var_cxxflags as well.
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: meson: Specify -Wformat as a common warning flag for extensions
2024-01-22 05:11 meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-03-08 05:39 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-08 06:32 ` Re: meson: Specify -Wformat as a common warning flag for extensions Michael Paquier <michael@paquier.xyz>
2024-03-08 16:05 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-13 07:38 ` Re: meson: Specify -Wformat as a common warning flag for extensions Peter Eisentraut <peter@eisentraut.org>
@ 2024-03-15 09:36 ` Sutou Kouhei <kou@clear-code.com>
2024-04-07 23:26 ` Re: meson: Specify -Wformat as a common warning flag for extensions Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 17+ messages in thread
From: Sutou Kouhei @ 2024-03-15 09:36 UTC (permalink / raw)
To: peter@eisentraut.org; +Cc: tristan@neon.tech; michael@paquier.xyz; pgsql-hackers
Hi,
In <49e97fd0-c17e-4cbc-aeee-80ac51400736@eisentraut.org>
"Re: meson: Specify -Wformat as a common warning flag for extensions" on Wed, 13 Mar 2024 08:38:28 +0100,
Peter Eisentraut <peter@eisentraut.org> wrote:
> I think the fix then is to put -Wall into CFLAGS in
> Makefile.global. Looking at a diff of Makefile.global between an
> autoconf and a meson build, I also see that under meson, CFLAGS
> doesn't get -O2 -g (or similar, depending on settings). This
> presumably has the same underlying issue that meson handles those
> flags internally.
>
> For someone who wants to write a fix for this, the relevant variable
> is var_cflags in our meson scripts. And var_cxxflags as well.
How about the attached v4 patch?
Thanks,
--
kou
Attachments:
[text/x-patch] v4-0001-meson-Restore-implicit-warning-debug-optimize-fla.patch (3.3K, ../../20240315.183655.616433279210896579.kou@clear-code.com/2-v4-0001-meson-Restore-implicit-warning-debug-optimize-fla.patch)
download | inline diff:
From a515720338dc49e764f598021200316c6d01ddf9 Mon Sep 17 00:00:00 2001
From: Sutou Kouhei <kou@clear-code.com>
Date: Fri, 15 Mar 2024 18:27:30 +0900
Subject: [PATCH v4] meson: Restore implicit warning/debug/optimize flags for
extensions
Meson specifies warning/debug/optimize flags such as "-Wall", "-g" and
"-O2" automatically based on "--warnlevel" and "--buildtype"
options. And we use "--warning_level=1" and
"--buildtype=debugoptimized" by default.
We don't specify warning/debug/optimize flags explicitly to build
PostgreSQL with Meson. Because Meson does it automatically as we said.
But Meson doesn't care about flags in Makefile.global and
pg_config. So we need to care about them manually.
This changes do it. They detect warning/debug/optimize flags based on
warning_level/debug/optimization option values because Meson doesn't
tell us flags Meson guessed.
---
meson.build | 40 ++++++++++++++++++++++++++++++++++++++++
src/include/meson.build | 4 ++--
2 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/meson.build b/meson.build
index c8fdfeb0ec..3c5c449a0a 100644
--- a/meson.build
+++ b/meson.build
@@ -1824,6 +1824,46 @@ endif
vectorize_cflags = cc.get_supported_arguments(['-ftree-vectorize'])
unroll_loops_cflags = cc.get_supported_arguments(['-funroll-loops'])
+# They aren't used for building PostgreSQL itself because Meson does
+# everything internally. They are used by extensions via pg_config or
+# Makefile.global.
+common_builtin_flags = []
+
+warning_level = get_option('warning_level')
+# See https://mesonbuild.com/Builtin-options.html#details-for-warning_level for
+# warning_level values.
+if warning_level == '1'
+ common_builtin_flags += ['-Wall', '/W2']
+elif warning_level == '2'
+ common_builtin_flags += ['-Wall', '-Wextra', '/W3']
+elif warning_level == '3'
+ common_builtin_flags += ['-Wall', '-Wextra', '-Wpedantic', '/W4']
+elif warning_level == 'everything'
+ common_builtin_flags += ['-Weverything', '/Wall']
+endif
+
+if get_option('debug')
+ common_builtin_flags += ['-g']
+endif
+
+optimization = get_option('optimization')
+if optimization == '0'
+ common_builtin_flags += ['-O0']
+elif optimization == '1'
+ common_builtin_flags += ['-O1']
+elif optimization == '2'
+ common_builtin_flags += ['-O2']
+elif optimization == '3'
+ common_builtin_flags += ['-O3']
+elif optimization == 's'
+ common_builtin_flags += ['-Os']
+endif
+
+cflags_builtin = cc.get_supported_arguments(common_builtin_flags)
+if llvm.found()
+ cxxflags_builtin = cpp.get_supported_arguments(common_builtin_flags)
+endif
+
common_warning_flags = [
'-Wmissing-prototypes',
'-Wpointer-arith',
diff --git a/src/include/meson.build b/src/include/meson.build
index a28f115d86..58b7a9c1e7 100644
--- a/src/include/meson.build
+++ b/src/include/meson.build
@@ -44,9 +44,9 @@ config_paths_data.set_quoted('MANDIR', dir_prefix / dir_man)
var_cc = ' '.join(cc.cmd_array())
var_cpp = ' '.join(cc.cmd_array() + ['-E'])
-var_cflags = ' '.join(cflags + cflags_warn + get_option('c_args'))
+var_cflags = ' '.join(cflags + cflags_builtin + cflags_warn + get_option('c_args'))
if llvm.found()
- var_cxxflags = ' '.join(cxxflags + cxxflags_warn + get_option('cpp_args'))
+ var_cxxflags = ' '.join(cxxflags + cxxflags_builtin + cxxflags_warn + get_option('cpp_args'))
else
var_cxxflags = ''
endif
--
2.43.0
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: meson: Specify -Wformat as a common warning flag for extensions
2024-01-22 05:11 meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-03-08 05:39 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-08 06:32 ` Re: meson: Specify -Wformat as a common warning flag for extensions Michael Paquier <michael@paquier.xyz>
2024-03-08 16:05 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-13 07:38 ` Re: meson: Specify -Wformat as a common warning flag for extensions Peter Eisentraut <peter@eisentraut.org>
2024-03-15 09:36 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
@ 2024-04-07 23:26 ` Andres Freund <andres@anarazel.de>
2024-04-08 01:01 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
0 siblings, 1 reply; 17+ messages in thread
From: Andres Freund @ 2024-04-07 23:26 UTC (permalink / raw)
To: Sutou Kouhei <kou@clear-code.com>; +Cc: peter@eisentraut.org; tristan@neon.tech; michael@paquier.xyz; pgsql-hackers
Hi,
On 2024-03-15 18:36:55 +0900, Sutou Kouhei wrote:
> +warning_level = get_option('warning_level')
> +# See https://mesonbuild.com/Builtin-options.html#details-for-warning_level for
> +# warning_level values.
> +if warning_level == '1'
> + common_builtin_flags += ['-Wall', '/W2']
> +elif warning_level == '2'
> + common_builtin_flags += ['-Wall', '-Wextra', '/W3']
> +elif warning_level == '3'
> + common_builtin_flags += ['-Wall', '-Wextra', '-Wpedantic', '/W4']
> +elif warning_level == 'everything'
> + common_builtin_flags += ['-Weverything', '/Wall']
> +endif
> +cflags_builtin = cc.get_supported_arguments(common_builtin_flags)
> +if llvm.found()
> + cxxflags_builtin = cpp.get_supported_arguments(common_builtin_flags)
> +endif
This seems like a fair amount of extra configure tests. Particularly because
/W* isn't ever interesting for Makefile.global - they're msvc flags - because
you can't use that with msvc.
I'm also doubtful that it's worth supporting warning_level=3/everything, you
end up with a completely flood of warnings that way.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: meson: Specify -Wformat as a common warning flag for extensions
2024-01-22 05:11 meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-03-08 05:39 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-08 06:32 ` Re: meson: Specify -Wformat as a common warning flag for extensions Michael Paquier <michael@paquier.xyz>
2024-03-08 16:05 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-13 07:38 ` Re: meson: Specify -Wformat as a common warning flag for extensions Peter Eisentraut <peter@eisentraut.org>
2024-03-15 09:36 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-04-07 23:26 ` Re: meson: Specify -Wformat as a common warning flag for extensions Andres Freund <andres@anarazel.de>
@ 2024-04-08 01:01 ` Sutou Kouhei <kou@clear-code.com>
2024-05-29 06:31 ` Re: meson: Specify -Wformat as a common warning flag for extensions Peter Eisentraut <peter@eisentraut.org>
0 siblings, 1 reply; 17+ messages in thread
From: Sutou Kouhei @ 2024-04-08 01:01 UTC (permalink / raw)
To: andres@anarazel.de; +Cc: peter@eisentraut.org; tristan@neon.tech; michael@paquier.xyz; pgsql-hackers
Hi Andres,
Thanks for reviewing this!
In <20240407232635.fq4kc5556lahaoej@awork3.anarazel.de>
"Re: meson: Specify -Wformat as a common warning flag for extensions" on Sun, 7 Apr 2024 16:26:35 -0700,
Andres Freund <andres@anarazel.de> wrote:
> This seems like a fair amount of extra configure tests. Particularly because
> /W* isn't ever interesting for Makefile.global - they're msvc flags - because
> you can't use that with msvc.
>
> I'm also doubtful that it's worth supporting warning_level=3/everything, you
> end up with a completely flood of warnings that way.
OK. I've removed "/W*" flags and warning_level==3/everything
cases.
How about the attached v5 patch?
Thanks,
--
kou
Attachments:
[text/x-patch] v5-0001-meson-Restore-implicit-warning-debug-optimize-fla.patch (3.3K, ../../20240408.100117.1165016889947781960.kou@clear-code.com/2-v5-0001-meson-Restore-implicit-warning-debug-optimize-fla.patch)
download | inline diff:
From 205ef88c66cf1050eedfc1e72d951de93a02e53a Mon Sep 17 00:00:00 2001
From: Sutou Kouhei <kou@clear-code.com>
Date: Fri, 15 Mar 2024 18:27:30 +0900
Subject: [PATCH v5] meson: Restore implicit warning/debug/optimize flags for
extensions
Meson specifies warning/debug/optimize flags such as "-Wall", "-g" and
"-O2" automatically based on "--warnlevel" and "--buildtype"
options. And we use "--warning_level=1" and
"--buildtype=debugoptimized" by default.
We don't specify warning/debug/optimize flags explicitly to build
PostgreSQL with Meson. Because Meson does it automatically as we said.
But Meson doesn't care about flags in Makefile.global and
pg_config. So we need to care about them manually.
This changes do it. They detect warning/debug/optimize flags based on
warning_level/debug/optimization option values because Meson doesn't
tell us flags Meson guessed.
---
meson.build | 41 +++++++++++++++++++++++++++++++++++++++++
src/include/meson.build | 4 ++--
2 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/meson.build b/meson.build
index 5acf083ce3c..11bd56f79a7 100644
--- a/meson.build
+++ b/meson.build
@@ -1848,6 +1848,47 @@ endif
vectorize_cflags = cc.get_supported_arguments(['-ftree-vectorize'])
unroll_loops_cflags = cc.get_supported_arguments(['-funroll-loops'])
+# They aren't used for building PostgreSQL itself because Meson does
+# everything internally. They are used by extensions via pg_config or
+# Makefile.global.
+common_builtin_flags = []
+
+warning_level = get_option('warning_level')
+# See https://mesonbuild.com/Builtin-options.html#details-for-warning_level for
+# warning_level values.
+#
+# We don't use "/W*" flags here because we don't need to care about MSVC here.
+#
+# We don't have "warning_level == 3" and "warning_level ==
+# 'everything'" here because we don't use these warning levels.
+if warning_level == '1'
+ common_builtin_flags += ['-Wall']
+elif warning_level == '2'
+ common_builtin_flags += ['-Wall', '-Wextra']
+endif
+
+if get_option('debug')
+ common_builtin_flags += ['-g']
+endif
+
+optimization = get_option('optimization')
+if optimization == '0'
+ common_builtin_flags += ['-O0']
+elif optimization == '1'
+ common_builtin_flags += ['-O1']
+elif optimization == '2'
+ common_builtin_flags += ['-O2']
+elif optimization == '3'
+ common_builtin_flags += ['-O3']
+elif optimization == 's'
+ common_builtin_flags += ['-Os']
+endif
+
+cflags_builtin = cc.get_supported_arguments(common_builtin_flags)
+if llvm.found()
+ cxxflags_builtin = cpp.get_supported_arguments(common_builtin_flags)
+endif
+
common_warning_flags = [
'-Wmissing-prototypes',
'-Wpointer-arith',
diff --git a/src/include/meson.build b/src/include/meson.build
index a28f115d867..58b7a9c1e7e 100644
--- a/src/include/meson.build
+++ b/src/include/meson.build
@@ -44,9 +44,9 @@ config_paths_data.set_quoted('MANDIR', dir_prefix / dir_man)
var_cc = ' '.join(cc.cmd_array())
var_cpp = ' '.join(cc.cmd_array() + ['-E'])
-var_cflags = ' '.join(cflags + cflags_warn + get_option('c_args'))
+var_cflags = ' '.join(cflags + cflags_builtin + cflags_warn + get_option('c_args'))
if llvm.found()
- var_cxxflags = ' '.join(cxxflags + cxxflags_warn + get_option('cpp_args'))
+ var_cxxflags = ' '.join(cxxflags + cxxflags_builtin + cxxflags_warn + get_option('cpp_args'))
else
var_cxxflags = ''
endif
--
2.43.0
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: meson: Specify -Wformat as a common warning flag for extensions
2024-01-22 05:11 meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-03-08 05:39 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-08 06:32 ` Re: meson: Specify -Wformat as a common warning flag for extensions Michael Paquier <michael@paquier.xyz>
2024-03-08 16:05 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-13 07:38 ` Re: meson: Specify -Wformat as a common warning flag for extensions Peter Eisentraut <peter@eisentraut.org>
2024-03-15 09:36 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-04-07 23:26 ` Re: meson: Specify -Wformat as a common warning flag for extensions Andres Freund <andres@anarazel.de>
2024-04-08 01:01 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
@ 2024-05-29 06:31 ` Peter Eisentraut <peter@eisentraut.org>
2024-05-29 06:47 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
0 siblings, 1 reply; 17+ messages in thread
From: Peter Eisentraut @ 2024-05-29 06:31 UTC (permalink / raw)
To: Sutou Kouhei <kou@clear-code.com>; andres@anarazel.de; +Cc: tristan@neon.tech; michael@paquier.xyz; pgsql-hackers
On 07.04.24 18:01, Sutou Kouhei wrote:
> +# We don't have "warning_level == 3" and "warning_level ==
> +# 'everything'" here because we don't use these warning levels.
> +if warning_level == '1'
> + common_builtin_flags += ['-Wall']
> +elif warning_level == '2'
> + common_builtin_flags += ['-Wall', '-Wextra']
> +endif
I would trim this even further and always export just '-Wall'. The
other options aren't really something we support.
The other stanzas, on '-g' and '-O*', look good to me.
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: meson: Specify -Wformat as a common warning flag for extensions
2024-01-22 05:11 meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-03-08 05:39 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-08 06:32 ` Re: meson: Specify -Wformat as a common warning flag for extensions Michael Paquier <michael@paquier.xyz>
2024-03-08 16:05 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-13 07:38 ` Re: meson: Specify -Wformat as a common warning flag for extensions Peter Eisentraut <peter@eisentraut.org>
2024-03-15 09:36 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-04-07 23:26 ` Re: meson: Specify -Wformat as a common warning flag for extensions Andres Freund <andres@anarazel.de>
2024-04-08 01:01 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-05-29 06:31 ` Re: meson: Specify -Wformat as a common warning flag for extensions Peter Eisentraut <peter@eisentraut.org>
@ 2024-05-29 06:47 ` Sutou Kouhei <kou@clear-code.com>
2024-06-04 07:00 ` Re: meson: Specify -Wformat as a common warning flag for extensions Peter Eisentraut <peter@eisentraut.org>
2024-06-07 07:44 ` Re: meson: Specify -Wformat as a common warning flag for extensions Peter Eisentraut <peter@eisentraut.org>
0 siblings, 2 replies; 17+ messages in thread
From: Sutou Kouhei @ 2024-05-29 06:47 UTC (permalink / raw)
To: peter@eisentraut.org; +Cc: andres@anarazel.de; tristan@neon.tech; michael@paquier.xyz; pgsql-hackers
Hi,
In <4707d4ed-f268-43c0-b4dd-cdbc7520f508@eisentraut.org>
"Re: meson: Specify -Wformat as a common warning flag for extensions" on Tue, 28 May 2024 23:31:05 -0700,
Peter Eisentraut <peter@eisentraut.org> wrote:
> On 07.04.24 18:01, Sutou Kouhei wrote:
>> +# We don't have "warning_level == 3" and "warning_level ==
>> +# 'everything'" here because we don't use these warning levels.
>> +if warning_level == '1'
>> + common_builtin_flags += ['-Wall']
>> +elif warning_level == '2'
>> + common_builtin_flags += ['-Wall', '-Wextra']
>> +endif
>
> I would trim this even further and always export just '-Wall'. The
> other options aren't really something we support.
OK. How about the v6 patch? It always uses '-Wall'.
Thanks,
--
kou
Attachments:
[text/x-patch] v6-0001-meson-Restore-implicit-warning-debug-optimize-fla.patch (2.8K, ../../20240529.154708.173647409366729418.kou@clear-code.com/2-v6-0001-meson-Restore-implicit-warning-debug-optimize-fla.patch)
download | inline diff:
From 8238adba3f3fc96d4a9e50af611b1cb3566abc0e Mon Sep 17 00:00:00 2001
From: Sutou Kouhei <kou@clear-code.com>
Date: Fri, 15 Mar 2024 18:27:30 +0900
Subject: [PATCH v6] meson: Restore implicit warning/debug/optimize flags for
extensions
Meson specifies warning/debug/optimize flags such as "-Wall", "-g" and
"-O2" automatically based on "--warnlevel" and "--buildtype"
options. And we use "--warning_level=1" and
"--buildtype=debugoptimized" by default.
We don't specify warning/debug/optimize flags explicitly to build
PostgreSQL with Meson. Because Meson does it automatically as we said.
But Meson doesn't care about flags in Makefile.global and
pg_config. So we need to care about them manually.
This changes do it. They detect debug/optimize flags based on
debug/optimization option values because Meson doesn't tell us flags
Meson guessed. We always use -Wall for warning flags.
---
meson.build | 27 +++++++++++++++++++++++++++
src/include/meson.build | 4 ++--
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/meson.build b/meson.build
index d6401fb8e30..d7239dbb114 100644
--- a/meson.build
+++ b/meson.build
@@ -1851,6 +1851,33 @@ endif
vectorize_cflags = cc.get_supported_arguments(['-ftree-vectorize'])
unroll_loops_cflags = cc.get_supported_arguments(['-funroll-loops'])
+# They aren't used for building PostgreSQL itself because Meson does
+# everything internally. They are used by extensions via pg_config or
+# Makefile.global.
+common_builtin_flags = ['-Wall']
+
+if get_option('debug')
+ common_builtin_flags += ['-g']
+endif
+
+optimization = get_option('optimization')
+if optimization == '0'
+ common_builtin_flags += ['-O0']
+elif optimization == '1'
+ common_builtin_flags += ['-O1']
+elif optimization == '2'
+ common_builtin_flags += ['-O2']
+elif optimization == '3'
+ common_builtin_flags += ['-O3']
+elif optimization == 's'
+ common_builtin_flags += ['-Os']
+endif
+
+cflags_builtin = cc.get_supported_arguments(common_builtin_flags)
+if llvm.found()
+ cxxflags_builtin = cpp.get_supported_arguments(common_builtin_flags)
+endif
+
common_warning_flags = [
'-Wmissing-prototypes',
'-Wpointer-arith',
diff --git a/src/include/meson.build b/src/include/meson.build
index a28f115d867..58b7a9c1e7e 100644
--- a/src/include/meson.build
+++ b/src/include/meson.build
@@ -44,9 +44,9 @@ config_paths_data.set_quoted('MANDIR', dir_prefix / dir_man)
var_cc = ' '.join(cc.cmd_array())
var_cpp = ' '.join(cc.cmd_array() + ['-E'])
-var_cflags = ' '.join(cflags + cflags_warn + get_option('c_args'))
+var_cflags = ' '.join(cflags + cflags_builtin + cflags_warn + get_option('c_args'))
if llvm.found()
- var_cxxflags = ' '.join(cxxflags + cxxflags_warn + get_option('cpp_args'))
+ var_cxxflags = ' '.join(cxxflags + cxxflags_builtin + cxxflags_warn + get_option('cpp_args'))
else
var_cxxflags = ''
endif
--
2.43.0
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: meson: Specify -Wformat as a common warning flag for extensions
2024-01-22 05:11 meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-03-08 05:39 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-08 06:32 ` Re: meson: Specify -Wformat as a common warning flag for extensions Michael Paquier <michael@paquier.xyz>
2024-03-08 16:05 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-13 07:38 ` Re: meson: Specify -Wformat as a common warning flag for extensions Peter Eisentraut <peter@eisentraut.org>
2024-03-15 09:36 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-04-07 23:26 ` Re: meson: Specify -Wformat as a common warning flag for extensions Andres Freund <andres@anarazel.de>
2024-04-08 01:01 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-05-29 06:31 ` Re: meson: Specify -Wformat as a common warning flag for extensions Peter Eisentraut <peter@eisentraut.org>
2024-05-29 06:47 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
@ 2024-06-04 07:00 ` Peter Eisentraut <peter@eisentraut.org>
1 sibling, 0 replies; 17+ messages in thread
From: Peter Eisentraut @ 2024-06-04 07:00 UTC (permalink / raw)
To: Sutou Kouhei <kou@clear-code.com>; +Cc: andres@anarazel.de; tristan@neon.tech; michael@paquier.xyz; pgsql-hackers
On 29.05.24 08:47, Sutou Kouhei wrote:
> In <4707d4ed-f268-43c0-b4dd-cdbc7520f508@eisentraut.org>
> "Re: meson: Specify -Wformat as a common warning flag for extensions" on Tue, 28 May 2024 23:31:05 -0700,
> Peter Eisentraut <peter@eisentraut.org> wrote:
>
>> On 07.04.24 18:01, Sutou Kouhei wrote:
>>> +# We don't have "warning_level == 3" and "warning_level ==
>>> +# 'everything'" here because we don't use these warning levels.
>>> +if warning_level == '1'
>>> + common_builtin_flags += ['-Wall']
>>> +elif warning_level == '2'
>>> + common_builtin_flags += ['-Wall', '-Wextra']
>>> +endif
>>
>> I would trim this even further and always export just '-Wall'. The
>> other options aren't really something we support.
>
> OK. How about the v6 patch? It always uses '-Wall'.
Yes, this looks good to me.
All: I think we should backpatch this. Otherwise, meson-based installs
will get suboptimal behavior for extension builds via pgxs.
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: meson: Specify -Wformat as a common warning flag for extensions
2024-01-22 05:11 meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-03-08 05:39 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-08 06:32 ` Re: meson: Specify -Wformat as a common warning flag for extensions Michael Paquier <michael@paquier.xyz>
2024-03-08 16:05 ` Re: meson: Specify -Wformat as a common warning flag for extensions Tristan Partin <tristan@neon.tech>
2024-03-13 07:38 ` Re: meson: Specify -Wformat as a common warning flag for extensions Peter Eisentraut <peter@eisentraut.org>
2024-03-15 09:36 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-04-07 23:26 ` Re: meson: Specify -Wformat as a common warning flag for extensions Andres Freund <andres@anarazel.de>
2024-04-08 01:01 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-05-29 06:31 ` Re: meson: Specify -Wformat as a common warning flag for extensions Peter Eisentraut <peter@eisentraut.org>
2024-05-29 06:47 ` Re: meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
@ 2024-06-07 07:44 ` Peter Eisentraut <peter@eisentraut.org>
1 sibling, 0 replies; 17+ messages in thread
From: Peter Eisentraut @ 2024-06-07 07:44 UTC (permalink / raw)
To: Sutou Kouhei <kou@clear-code.com>; +Cc: andres@anarazel.de; tristan@neon.tech; michael@paquier.xyz; pgsql-hackers
On 29.05.24 08:47, Sutou Kouhei wrote:
> In <4707d4ed-f268-43c0-b4dd-cdbc7520f508@eisentraut.org>
> "Re: meson: Specify -Wformat as a common warning flag for extensions" on Tue, 28 May 2024 23:31:05 -0700,
> Peter Eisentraut <peter@eisentraut.org> wrote:
>
>> On 07.04.24 18:01, Sutou Kouhei wrote:
>>> +# We don't have "warning_level == 3" and "warning_level ==
>>> +# 'everything'" here because we don't use these warning levels.
>>> +if warning_level == '1'
>>> + common_builtin_flags += ['-Wall']
>>> +elif warning_level == '2'
>>> + common_builtin_flags += ['-Wall', '-Wextra']
>>> +endif
>>
>> I would trim this even further and always export just '-Wall'. The
>> other options aren't really something we support.
>
> OK. How about the v6 patch? It always uses '-Wall'.
I have committed this. Thanks.
^ permalink raw reply [nested|flat] 17+ messages in thread
end of thread, other threads:[~2024-06-07 07:44 UTC | newest]
Thread overview: 17+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-01-22 05:11 meson: Specify -Wformat as a common warning flag for extensions Sutou Kouhei <kou@clear-code.com>
2024-02-22 07:41 ` Sutou Kouhei <kou@clear-code.com>
2024-03-08 05:39 ` Tristan Partin <tristan@neon.tech>
2024-03-08 06:32 ` Michael Paquier <michael@paquier.xyz>
2024-03-08 09:17 ` Sutou Kouhei <kou@clear-code.com>
2024-03-08 16:05 ` Tristan Partin <tristan@neon.tech>
2024-03-12 23:56 ` Sutou Kouhei <kou@clear-code.com>
2024-03-13 05:43 ` Tristan Partin <tristan@neon.tech>
2024-03-13 07:12 ` Sutou Kouhei <kou@clear-code.com>
2024-03-13 07:38 ` Peter Eisentraut <peter@eisentraut.org>
2024-03-15 09:36 ` Sutou Kouhei <kou@clear-code.com>
2024-04-07 23:26 ` Andres Freund <andres@anarazel.de>
2024-04-08 01:01 ` Sutou Kouhei <kou@clear-code.com>
2024-05-29 06:31 ` Peter Eisentraut <peter@eisentraut.org>
2024-05-29 06:47 ` Sutou Kouhei <kou@clear-code.com>
2024-06-04 07:00 ` Peter Eisentraut <peter@eisentraut.org>
2024-06-07 07:44 ` Peter Eisentraut <peter@eisentraut.org>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox