[libvirt PATCH 0/6] meson: sanitize our compiler flags handling
Currently our compiler flags handling has a number of flaws and misleading characteristics - It triggers warnings from Meson due to our use of -Wall/-Wextra that prevents us using --fatal-meson-warnings - We enable flags, only to turn them back off again eg "-Wcast-function-type ... -Wno-cast-function-type" - We check for certain flags twice - We don't check compiler support for some flags Daniel P. Berrangé (6): meson: get rid of list of unused warning flags meson: actually check for -Wframe-larger-than meson: merge all cc_flags arrays into one meson: avoid checking compiler flags twice meson: honour meson warning_level option meson: don't probe for -Werror if -Dwerror is enabled meson.build | 312 ++++++++++++++++++---------------------------------- 1 file changed, 104 insertions(+), 208 deletions(-) -- 2.30.2
We're not using these warning flags with libvirt, and it is not worth keeping them just to issue a warning if someone tries to enable them. If someone does try to enable them, either libvirt will build cleanly or it won't. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- meson.build | 76 ----------------------------------------------------- 1 file changed, 76 deletions(-) diff --git a/meson.build b/meson.build index b827bd9275..7565eace17 100644 --- a/meson.build +++ b/meson.build @@ -491,82 +491,6 @@ if not cc.compiles(w_unused_function_code, args: w_unused_function_args) cc_flags += ['-Wno-unused-function'] endif -cc_flags_disabled = [ - # In meson this is specified using 'c_std=gnu99' in project() function. - '-std=gnu99', - - # don't care about C++ compiler compat - '-Wc++-compat', - '-Wabi', - '-Wdeprecated', - - # Don't care about ancient C standard compat - '-Wtraditional', - '-Wtraditional-conversion', - - # Ignore warnings in /usr/include - '-Wsystem-headers', - - # Happy for compiler to add struct padding - '-Wpadded', - - # GCC very confused with -O2 - '-Wunreachable-code', - - # Too many to deal with - '-Wconversion', - '-Wsign-conversion', - - # Need to allow bad cast for execve() - '-Wcast-qual', - - # We need to use long long in many places - '-Wlong-long', - - # We allow manual list of all enum cases without default - '-Wswitch-default', - - # Not a problem since we don't use -fstrict-overflow - '-Wstrict-overflow', - - # Not a problem since we don't use -funsafe-loop-optimizations - '-Wunsafe-loop-optimizations', - - # gcc 4.4.6 complains this is C++ only; gcc 4.7.0 implies this from -Wall - '-Wenum-compare', - - # gcc 5.1 -Wformat-signedness mishandles enums, not ready for prime time - '-Wformat-signedness', - - # Several conditionals expand the same on both branches depending on the - # particular platform/architecture - '-Wduplicated-branches', - - # > This warning does not generally indicate that there is anything wrong - # > with your code; it merely indicates that GCC's optimizers are unable - # > to handle the code effectively. - # Source: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html - '-Wdisabled-optimization', - - # Various valid glib APIs/macros trigger this warning - '-Wbad-function-cast', - - # We might fundamentally need some of these disabled forever, but - # ideally we'd turn many of them on - '-Wfloat-equal', - '-Wpacked', - '-Wunused-macros', - '-Woverlength-strings', - '-Wstack-protector', - '-Wsuggest-attribute=malloc', -] - -foreach flag : cc_flags_disabled - if cc_flags.contains(flag) - error('@0@ is disabled but listed in cc_flags'.format(flag)) - endif -endforeach - supported_cc_flags = cc.get_supported_arguments(cc_flags) add_project_arguments(supported_cc_flags, language: 'c') -- 2.30.2
All other warning flags are checked for compiler support, so we shouldn't blindly assume this one always exists. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- meson.build | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/meson.build b/meson.build index 7565eace17..97d9c52165 100644 --- a/meson.build +++ b/meson.build @@ -499,10 +499,12 @@ if cc.has_argument('-Wsuggest-attribute=format') endif # used in tests -cc_flags_relaxed_frame_limit = [ +cc_flags_relaxed_frame_limit = [] +if cc.has_argument('-Wframe-larger-than=262144') + cc_flags_relaxed_frame_limit += [ '-Wframe-larger-than=262144', -] - + ] +endif # various linker checks -- 2.30.2
The split of arrays is fairly arbitrary and a hang over from the way we had to structure lists of flags when we used GNULIB's compiler flag checking m4 logic. The separate lists leads to cases where we enable a flag in one list and have contradictory setting in another list, which leads to confusion. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- meson.build | 116 +++++++++++++++++++--------------------------------- 1 file changed, 43 insertions(+), 73 deletions(-) diff --git a/meson.build b/meson.build index 97d9c52165..55dde6d963 100644 --- a/meson.build +++ b/meson.build @@ -211,7 +211,23 @@ if git_werror.enabled() or git_werror.auto() and git cc_flags += [ '-Werror' ] endif + +# gcc --help=warnings outputs +ptrdiff_max = cc.sizeof('ptrdiff_t', prefix: '#include <stddef.h>') +size_max = cc.sizeof('size_t', prefix: '#include <stdint.h>') +# Compute max safe object size by checking ptrdiff_t and size_t sizes. +# Ideally we would get PTRDIFF_MAX and SIZE_MAX values but it would +# give us (2147483647L) and we would have to remove the () and the suffix +# in order to convert it to numbers to be able to pick the smaller one. +alloc_max = run_command( + 'python3', '-c', + 'print(min(2**(@0@ * 8 - 1) - 1, 2**(@1@ * 8) - 1))'.format(ptrdiff_max, size_max), +) + cc_flags += [ + '-fasynchronous-unwind-tables', + '-fexceptions', + '-fipa-pure-const', '-fno-common', '-W', '-Wabsolute-value', @@ -219,6 +235,9 @@ cc_flags += [ '-Waddress-of-packed-member', '-Waggressive-loop-optimizations', '-Wall', + '-Walloc-size-larger-than=@0@'.format(alloc_max.stdout().strip()), + '-Warray-bounds=2', + '-Wattribute-alias=2', '-Wattribute-warning', '-Wattributes', '-Wbool-compare', @@ -228,7 +247,8 @@ cc_flags += [ '-Wcannot-profile', '-Wcast-align', '-Wcast-align=strict', - '-Wcast-function-type', + # We do "bad" function casts all the time for event callbacks + '-Wno-cast-function-type', '-Wchar-subscripts', '-Wclobbered', '-Wcomment', @@ -251,17 +271,24 @@ cc_flags += [ '-Wextra', '-Wformat-contains-nul', '-Wformat-extra-args', - '-Wformat-nonliteral', + # -Wformat=2 implies -Wformat-nonliteral so we need to manually exclude it + '-Wno-format-nonliteral', + '-Wformat-overflow=2', '-Wformat-security', + # -Wformat enables this by default, and we should keep it, + # but need to rewrite various areas of code first + '-Wno-format-truncation', '-Wformat-y2k', '-Wformat-zero-length', '-Wframe-address', + '-Wframe-larger-than=4096', '-Wfree-nonheap-object', '-Whsa', '-Wif-not-aligned', '-Wignored-attributes', '-Wignored-qualifiers', '-Wimplicit', + '-Wimplicit-fallthrough=5', '-Wimplicit-function-declaration', '-Wimplicit-int', '-Wincompatible-pointer-types', @@ -272,6 +299,7 @@ cc_flags += [ '-Wint-to-pointer-cast', '-Winvalid-memory-model', '-Winvalid-pch', + '-Wjump-misses-init', '-Wlogical-not-parentheses', '-Wlogical-op', '-Wmain', @@ -293,6 +321,7 @@ cc_flags += [ '-Wnested-externs', '-Wnonnull', '-Wnonnull-compare', + '-Wnormalized=nfc', '-Wnull-dereference', '-Wodr', '-Wold-style-declaration', @@ -318,32 +347,41 @@ cc_flags += [ '-Wshift-count-negative', '-Wshift-count-overflow', '-Wshift-negative-value', + '-Wshift-overflow=2', + # So we have -W enabled, and then have to explicitly turn off... + '-Wno-sign-compare', '-Wsizeof-array-argument', '-Wsizeof-pointer-div', '-Wsizeof-pointer-memaccess', '-Wstrict-aliasing', '-Wstrict-prototypes', + '-Wstringop-overflow=2', '-Wstringop-truncation', '-Wsuggest-attribute=cold', - '-Wsuggest-attribute=const', + '-Wno-suggest-attribute=const', '-Wsuggest-attribute=format', '-Wsuggest-attribute=noreturn', - '-Wsuggest-attribute=pure', + '-Wno-suggest-attribute=pure', '-Wsuggest-final-methods', '-Wsuggest-final-types', '-Wswitch', '-Wswitch-bool', + '-Wswitch-enum', '-Wswitch-unreachable', '-Wsync-nand', '-Wtautological-compare', '-Wtrampolines', '-Wtrigraphs', '-Wtype-limits', + # Clang incorrectly complains about dup typedefs win gnu99 mode + # so use this Clang-specific arg to keep it quiet + '-Wno-typedef-redefinition', '-Wuninitialized', '-Wunknown-pragmas', '-Wunused', '-Wunused-but-set-parameter', '-Wunused-but-set-variable', + '-Wunused-const-variable=2', '-Wunused-function', '-Wunused-label', '-Wunused-local-typedefs', @@ -355,79 +393,11 @@ cc_flags += [ '-Wvariadic-macros', '-Wvector-operation-performance', '-Wvla', + '-Wvla-larger-then=4031', '-Wvolatile-register-var', '-Wwrite-strings', ] -# gcc --help=warnings outputs -ptrdiff_max = cc.sizeof('ptrdiff_t', prefix: '#include <stddef.h>') -size_max = cc.sizeof('size_t', prefix: '#include <stdint.h>') -# Compute max safe object size by checking ptrdiff_t and size_t sizes. -# Ideally we would get PTRDIFF_MAX and SIZE_MAX values but it would -# give us (2147483647L) and we would have to remove the () and the suffix -# in order to convert it to numbers to be able to pick the smaller one. -alloc_max = run_command( - 'python3', '-c', - 'print(min(2**(@0@ * 8 - 1) - 1, 2**(@1@ * 8) - 1))'.format(ptrdiff_max, size_max), -) -cc_flags += [ - '-Walloc-size-larger-than=@0@'.format(alloc_max.stdout().strip()), - '-Warray-bounds=2', - '-Wattribute-alias=2', - '-Wformat-overflow=2', - '-Wformat-truncation=2', - '-Wimplicit-fallthrough=5', - '-Wnormalized=nfc', - '-Wshift-overflow=2', - '-Wstringop-overflow=2', - '-Wunused-const-variable=2', - '-Wvla-larger-then=4031', -] - -cc_flags += [ - # So we have -W enabled, and then have to explicitly turn off... - '-Wno-sign-compare', - - # We do "bad" function casts all the time for event callbacks - '-Wno-cast-function-type', - - # Clang incorrectly complains about dup typedefs win gnu99 mode - # so use this Clang-specific arg to keep it quiet - '-Wno-typedef-redefinition', - - # We don't use -Wc++-compat so we have to enable it explicitly - '-Wjump-misses-init', - - # -Wswitch is enabled but that doesn't report missing enums if a default: - # is present - '-Wswitch-enum', - - # -Wformat=2 implies -Wformat-nonliteral so we need to manually exclude it - '-Wno-format-nonliteral', - - # -Wformat enables this by default, and we should keep it, - # but need to rewrite various areas of code first - '-Wno-format-truncation', - - # This should be < 256 really. Currently we're down to 4096, - # but using 1024 bytes sized buffers (mostly for virStrerror) - # stops us from going down further - '-Wframe-larger-than=4096', - - # extra special flags - '-fexceptions', - '-fasynchronous-unwind-tables', - - # Need -fipa-pure-const in order to make -Wsuggest-attribute=pure - # fire even without -O. - '-fipa-pure-const', - - # We should eventually enable this, but right now there are at - # least 75 functions triggering warnings. - '-Wno-suggest-attribute=pure', - '-Wno-suggest-attribute=const', -] - # on aarch64 error: -fstack-protector not supported for this target if host_machine.cpu_family() != 'aarch64' if host_machine.system() in [ 'linux', 'freebsd', 'windows' ] -- 2.30.2
On Thu, Apr 08, 2021 at 11:58:20AM +0100, Daniel P. Berrangé wrote:
The split of arrays is fairly arbitrary and a hang over from the way we had to structure lists of flags when we used GNULIB's compiler flag checking m4 logic.
The separate lists leads to cases where we enable a flag in one list and have contradictory setting in another list, which leads to confusion.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- meson.build | 116 +++++++++++++++++++--------------------------------- 1 file changed, 43 insertions(+), 73 deletions(-)
diff --git a/meson.build b/meson.build index 97d9c52165..55dde6d963 100644 --- a/meson.build +++ b/meson.build @@ -211,7 +211,23 @@ if git_werror.enabled() or git_werror.auto() and git cc_flags += [ '-Werror' ] endif
+ +# gcc --help=warnings outputs +ptrdiff_max = cc.sizeof('ptrdiff_t', prefix: '#include <stddef.h>') +size_max = cc.sizeof('size_t', prefix: '#include <stdint.h>') +# Compute max safe object size by checking ptrdiff_t and size_t sizes. +# Ideally we would get PTRDIFF_MAX and SIZE_MAX values but it would +# give us (2147483647L) and we would have to remove the () and the suffix +# in order to convert it to numbers to be able to pick the smaller one. +alloc_max = run_command( + 'python3', '-c', + 'print(min(2**(@0@ * 8 - 1) - 1, 2**(@1@ * 8) - 1))'.format(ptrdiff_max, size_max), +) + cc_flags += [ + '-fasynchronous-unwind-tables', + '-fexceptions', + '-fipa-pure-const', '-fno-common', '-W', '-Wabsolute-value', @@ -219,6 +235,9 @@ cc_flags += [ '-Waddress-of-packed-member', '-Waggressive-loop-optimizations', '-Wall', + '-Walloc-size-larger-than=@0@'.format(alloc_max.stdout().strip()), + '-Warray-bounds=2', + '-Wattribute-alias=2', '-Wattribute-warning', '-Wattributes', '-Wbool-compare', @@ -228,7 +247,8 @@ cc_flags += [ '-Wcannot-profile', '-Wcast-align', '-Wcast-align=strict', - '-Wcast-function-type', + # We do "bad" function casts all the time for event callbacks + '-Wno-cast-function-type', '-Wchar-subscripts', '-Wclobbered', '-Wcomment', @@ -251,17 +271,24 @@ cc_flags += [ '-Wextra', '-Wformat-contains-nul', '-Wformat-extra-args', - '-Wformat-nonliteral', + # -Wformat=2 implies -Wformat-nonliteral so we need to manually exclude it + '-Wno-format-nonliteral', + '-Wformat-overflow=2', '-Wformat-security', + # -Wformat enables this by default, and we should keep it, + # but need to rewrite various areas of code first + '-Wno-format-truncation', '-Wformat-y2k', '-Wformat-zero-length', '-Wframe-address',
I would keep the comment here as well. # This should be < 256 really. Currently we're down to 4096, # but using 1024 bytes sized buffers (mostly for virStrerror) # stops us from going down further
+ '-Wframe-larger-than=4096', '-Wfree-nonheap-object', '-Whsa', '-Wif-not-aligned', '-Wignored-attributes', '-Wignored-qualifiers', '-Wimplicit', + '-Wimplicit-fallthrough=5', '-Wimplicit-function-declaration', '-Wimplicit-int', '-Wincompatible-pointer-types', @@ -272,6 +299,7 @@ cc_flags += [ '-Wint-to-pointer-cast', '-Winvalid-memory-model', '-Winvalid-pch', + '-Wjump-misses-init', '-Wlogical-not-parentheses', '-Wlogical-op', '-Wmain', @@ -293,6 +321,7 @@ cc_flags += [ '-Wnested-externs', '-Wnonnull', '-Wnonnull-compare', + '-Wnormalized=nfc', '-Wnull-dereference', '-Wodr', '-Wold-style-declaration', @@ -318,32 +347,41 @@ cc_flags += [ '-Wshift-count-negative', '-Wshift-count-overflow', '-Wshift-negative-value', + '-Wshift-overflow=2', + # So we have -W enabled, and then have to explicitly turn off... + '-Wno-sign-compare', '-Wsizeof-array-argument', '-Wsizeof-pointer-div', '-Wsizeof-pointer-memaccess', '-Wstrict-aliasing', '-Wstrict-prototypes', + '-Wstringop-overflow=2', '-Wstringop-truncation', '-Wsuggest-attribute=cold', - '-Wsuggest-attribute=const', + '-Wno-suggest-attribute=const', '-Wsuggest-attribute=format', '-Wsuggest-attribute=noreturn', - '-Wsuggest-attribute=pure', + '-Wno-suggest-attribute=pure', '-Wsuggest-final-methods', '-Wsuggest-final-types', '-Wswitch', '-Wswitch-bool', + '-Wswitch-enum', '-Wswitch-unreachable', '-Wsync-nand', '-Wtautological-compare', '-Wtrampolines', '-Wtrigraphs', '-Wtype-limits', + # Clang incorrectly complains about dup typedefs win gnu99 mode + # so use this Clang-specific arg to keep it quiet + '-Wno-typedef-redefinition', '-Wuninitialized', '-Wunknown-pragmas', '-Wunused', '-Wunused-but-set-parameter', '-Wunused-but-set-variable', + '-Wunused-const-variable=2', '-Wunused-function', '-Wunused-label', '-Wunused-local-typedefs', @@ -355,79 +393,11 @@ cc_flags += [ '-Wvariadic-macros', '-Wvector-operation-performance', '-Wvla', + '-Wvla-larger-then=4031', '-Wvolatile-register-var', '-Wwrite-strings', ]
-# gcc --help=warnings outputs -ptrdiff_max = cc.sizeof('ptrdiff_t', prefix: '#include <stddef.h>') -size_max = cc.sizeof('size_t', prefix: '#include <stdint.h>') -# Compute max safe object size by checking ptrdiff_t and size_t sizes. -# Ideally we would get PTRDIFF_MAX and SIZE_MAX values but it would -# give us (2147483647L) and we would have to remove the () and the suffix -# in order to convert it to numbers to be able to pick the smaller one. -alloc_max = run_command( - 'python3', '-c', - 'print(min(2**(@0@ * 8 - 1) - 1, 2**(@1@ * 8) - 1))'.format(ptrdiff_max, size_max), -) -cc_flags += [ - '-Walloc-size-larger-than=@0@'.format(alloc_max.stdout().strip()), - '-Warray-bounds=2', - '-Wattribute-alias=2', - '-Wformat-overflow=2', - '-Wformat-truncation=2', - '-Wimplicit-fallthrough=5', - '-Wnormalized=nfc', - '-Wshift-overflow=2', - '-Wstringop-overflow=2', - '-Wunused-const-variable=2', - '-Wvla-larger-then=4031', -] - -cc_flags += [ - # So we have -W enabled, and then have to explicitly turn off... - '-Wno-sign-compare', - - # We do "bad" function casts all the time for event callbacks - '-Wno-cast-function-type', - - # Clang incorrectly complains about dup typedefs win gnu99 mode - # so use this Clang-specific arg to keep it quiet - '-Wno-typedef-redefinition', - - # We don't use -Wc++-compat so we have to enable it explicitly - '-Wjump-misses-init', - - # -Wswitch is enabled but that doesn't report missing enums if a default: - # is present - '-Wswitch-enum', - - # -Wformat=2 implies -Wformat-nonliteral so we need to manually exclude it - '-Wno-format-nonliteral', - - # -Wformat enables this by default, and we should keep it, - # but need to rewrite various areas of code first - '-Wno-format-truncation', - - # This should be < 256 really. Currently we're down to 4096, - # but using 1024 bytes sized buffers (mostly for virStrerror) - # stops us from going down further - '-Wframe-larger-than=4096', - - # extra special flags - '-fexceptions', - '-fasynchronous-unwind-tables', - - # Need -fipa-pure-const in order to make -Wsuggest-attribute=pure - # fire even without -O. - '-fipa-pure-const', - - # We should eventually enable this, but right now there are at - # least 75 functions triggering warnings. - '-Wno-suggest-attribute=pure', - '-Wno-suggest-attribute=const', -] - # on aarch64 error: -fstack-protector not supported for this target if host_machine.cpu_family() != 'aarch64' if host_machine.system() in [ 'linux', 'freebsd', 'windows' ] -- 2.30.2
On Thu, Apr 08, 2021 at 01:30:18PM +0200, Pavel Hrdina wrote:
On Thu, Apr 08, 2021 at 11:58:20AM +0100, Daniel P. Berrangé wrote:
The split of arrays is fairly arbitrary and a hang over from the way we had to structure lists of flags when we used GNULIB's compiler flag checking m4 logic.
The separate lists leads to cases where we enable a flag in one list and have contradictory setting in another list, which leads to confusion.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- meson.build | 116 +++++++++++++++++++--------------------------------- 1 file changed, 43 insertions(+), 73 deletions(-)
diff --git a/meson.build b/meson.build index 97d9c52165..55dde6d963 100644 --- a/meson.build +++ b/meson.build @@ -211,7 +211,23 @@ if git_werror.enabled() or git_werror.auto() and git cc_flags += [ '-Werror' ] endif
I would keep the comment here as well.
# This should be < 256 really. Currently we're down to 4096, # but using 1024 bytes sized buffers (mostly for virStrerror) # stops us from going down further
It is outdated because virStrerror doesnt exist, and also at this point I don't think we need to really care about optimizing further than 4k - my original 256 byte statement is overkill. With 8 MB stacks, a 4k limit lets us recurse 2048 times in the worst case, and much much much in the common case.
+ '-Wframe-larger-than=4096', '-Wfree-nonheap-object', '-Whsa', '-Wif-not-aligned', '-Wignored-attributes',
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, Apr 08, 2021 at 12:54:13PM +0100, Daniel P. Berrangé wrote:
On Thu, Apr 08, 2021 at 01:30:18PM +0200, Pavel Hrdina wrote:
On Thu, Apr 08, 2021 at 11:58:20AM +0100, Daniel P. Berrangé wrote:
The split of arrays is fairly arbitrary and a hang over from the way we had to structure lists of flags when we used GNULIB's compiler flag checking m4 logic.
The separate lists leads to cases where we enable a flag in one list and have contradictory setting in another list, which leads to confusion.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- meson.build | 116 +++++++++++++++++++--------------------------------- 1 file changed, 43 insertions(+), 73 deletions(-)
diff --git a/meson.build b/meson.build index 97d9c52165..55dde6d963 100644 --- a/meson.build +++ b/meson.build @@ -211,7 +211,23 @@ if git_werror.enabled() or git_werror.auto() and git cc_flags += [ '-Werror' ] endif
I would keep the comment here as well.
# This should be < 256 really. Currently we're down to 4096, # but using 1024 bytes sized buffers (mostly for virStrerror) # stops us from going down further
It is outdated because virStrerror doesnt exist, and also at this point I don't think we need to really care about optimizing further than 4k - my original 256 byte statement is overkill.
With 8 MB stacks, a 4k limit lets us recurse 2048 times in the worst case, and much much much in the common case.
Makes sense, would be probably nice to mention it in the commit message or have it as separate commit because this one only moves the flags around. Pavel
+ '-Wframe-larger-than=4096', '-Wfree-nonheap-object', '-Whsa', '-Wif-not-aligned', '-Wignored-attributes',
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
In several cases we check if a compiler flag is supported, and then add it to the 'cc_flags' array. The entire 'cc_flags' array is then later tested to see if each flag is supported, which duplicates the check in some cases. Move the check of cc_flags earlier, and for the extra flags append directly to supported_cc_flags to avoid the duplicate check Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- meson.build | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/meson.build b/meson.build index 55dde6d963..cf0e4f5715 100644 --- a/meson.build +++ b/meson.build @@ -398,6 +398,8 @@ cc_flags += [ '-Wwrite-strings', ] +supported_cc_flags = cc.get_supported_arguments(cc_flags) + # on aarch64 error: -fstack-protector not supported for this target if host_machine.cpu_family() != 'aarch64' if host_machine.system() in [ 'linux', 'freebsd', 'windows' ] @@ -406,7 +408,7 @@ if host_machine.cpu_family() != 'aarch64' '-fstack-protector-strong', '-fstack-protector-all', ]) - cc_flags += fstack_cflags + supported_cc_flags += fstack_cflags # When building with mingw using -fstack-protector requires libssp library # which is included by using -fstack-protector with linker. @@ -416,7 +418,7 @@ if host_machine.cpu_family() != 'aarch64' endif endif -if cc.has_argument('-Wlogical-op') +if supported_cc_flags.contains('-Wlogical-op') # Broken in 6.0 and later # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69602 w_logical_op_args = ['-O2', '-Wlogical-op', '-Werror'] @@ -445,7 +447,7 @@ w_double_promotion_code = ''' } ''' if cc.compiles(w_double_promotion_code, args: w_double_promotion_args, name: '-Wdouble-promotion') - cc_flags += ['-Wdouble-promotion'] + supported_cc_flags += ['-Wdouble-promotion'] endif # Clang complains about unused static inline functions which are common @@ -458,10 +460,9 @@ w_unused_function_code = ''' ''' # -Wunused-function is implied by -Wall, we must turn it off explicitly. if not cc.compiles(w_unused_function_code, args: w_unused_function_args) - cc_flags += ['-Wno-unused-function'] + supported_cc_flags += ['-Wno-unused-function'] endif -supported_cc_flags = cc.get_supported_arguments(cc_flags) add_project_arguments(supported_cc_flags, language: 'c') if cc.has_argument('-Wsuggest-attribute=format') -- 2.30.2
Meson defines a warning_level option which has the following behaviour with C code 0: no warning flags 1: -Wall 2: -Wall -Wextra 3: -Wall -Wextra -Wpedantic Currently we add our extra warning flags unconditionally if the compiler supports them, regardless of the meson warning_level setting. This has effectively nullified the warning_level setting in meson, and also results in meson printing these messages: meson.build:498: WARNING: Consider using the built-in warning_level option instead of using "-Wall". meson.build:498: WARNING: Consider using the built-in warning_level option instead of using "-Wextra". Semantically we can think of our huge list of flags as being an "extra" set of warnings, and thus we ought to only add them when meson would itself use -Wextra. aka warning_level == 2 or 3. In practice libvirt code can't be built with -Wpedantic so we can ignore meson warning_level 3, and only add our flags when warning_level==2. In doing this change, we no longer have to check -Wall/-Wextra ourselves as we can assume meson already set them. -W is an alias of -Wextra so it is removed too. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- meson.build | 112 ++++++++++++++++++++++++++-------------------------- 1 file changed, 56 insertions(+), 56 deletions(-) diff --git a/meson.build b/meson.build index cf0e4f5715..9fda0c7948 100644 --- a/meson.build +++ b/meson.build @@ -229,12 +229,10 @@ cc_flags += [ '-fexceptions', '-fipa-pure-const', '-fno-common', - '-W', '-Wabsolute-value', '-Waddress', '-Waddress-of-packed-member', '-Waggressive-loop-optimizations', - '-Wall', '-Walloc-size-larger-than=@0@'.format(alloc_max.stdout().strip()), '-Warray-bounds=2', '-Wattribute-alias=2', @@ -268,7 +266,6 @@ cc_flags += [ '-Wempty-body', '-Wendif-labels', '-Wexpansion-to-defined', - '-Wextra', '-Wformat-contains-nul', '-Wformat-extra-args', # -Wformat=2 implies -Wformat-nonliteral so we need to manually exclude it @@ -398,71 +395,74 @@ cc_flags += [ '-Wwrite-strings', ] -supported_cc_flags = cc.get_supported_arguments(cc_flags) - -# on aarch64 error: -fstack-protector not supported for this target -if host_machine.cpu_family() != 'aarch64' - if host_machine.system() in [ 'linux', 'freebsd', 'windows' ] - # we prefer -fstack-protector-strong but fallback to -fstack-protector-all - fstack_cflags = cc.first_supported_argument([ - '-fstack-protector-strong', - '-fstack-protector-all', - ]) - supported_cc_flags += fstack_cflags - - # When building with mingw using -fstack-protector requires libssp library - # which is included by using -fstack-protector with linker. - if fstack_cflags.length() == 1 and host_machine.system() == 'windows' - add_project_link_arguments(fstack_cflags, language: 'c') +supported_cc_flags = [] +if get_option('warning_level') == '2' + supported_cc_flags = cc.get_supported_arguments(cc_flags) + + # on aarch64 error: -fstack-protector not supported for this target + if host_machine.cpu_family() != 'aarch64' + if host_machine.system() in [ 'linux', 'freebsd', 'windows' ] + # we prefer -fstack-protector-strong but fallback to -fstack-protector-all + fstack_cflags = cc.first_supported_argument([ + '-fstack-protector-strong', + '-fstack-protector-all', + ]) + supported_cc_flags += fstack_cflags + + # When building with mingw using -fstack-protector requires libssp library + # which is included by using -fstack-protector with linker. + if fstack_cflags.length() == 1 and host_machine.system() == 'windows' + add_project_link_arguments(fstack_cflags, language: 'c') + endif + endif + endif + + if supported_cc_flags.contains('-Wlogical-op') + # Broken in 6.0 and later + # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69602 + w_logical_op_args = ['-O2', '-Wlogical-op', '-Werror'] + w_logical_op_code = ''' + #define TEST1 1 + #define TEST2 TEST1 + + int main(void) { + int test = 0; + return test == TEST1 || test == TEST2; + } + ''' + if not cc.compiles(w_logical_op_code, args: w_logical_op_args) + conf.set('BROKEN_GCC_WLOGICALOP_EQUAL_EXPR', 1) endif endif -endif -if supported_cc_flags.contains('-Wlogical-op') - # Broken in 6.0 and later - # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69602 - w_logical_op_args = ['-O2', '-Wlogical-op', '-Werror'] - w_logical_op_code = ''' - #define TEST1 1 - #define TEST2 TEST1 + # Check whether clang gives bogus warning for -Wdouble-promotion. + w_double_promotion_args = ['-O2', '-Wdouble-promotion', '-Werror'] + w_double_promotion_code = ''' + #include <math.h> int main(void) { - int test = 0; - return test == TEST1 || test == TEST2; + float f = 0.0; + return isnan(f); } ''' - if not cc.compiles(w_logical_op_code, args: w_logical_op_args) - conf.set('BROKEN_GCC_WLOGICALOP_EQUAL_EXPR', 1) + if cc.compiles(w_double_promotion_code, args: w_double_promotion_args, name: '-Wdouble-promotion') + supported_cc_flags += ['-Wdouble-promotion'] endif -endif -# Check whether clang gives bogus warning for -Wdouble-promotion. -w_double_promotion_args = ['-O2', '-Wdouble-promotion', '-Werror'] -w_double_promotion_code = ''' - #include <math.h> + # Clang complains about unused static inline functions which are common + # with G_DEFINE_AUTOPTR_CLEANUP_FUNC. + w_unused_function_args = ['-Wunused-function', '-Werror'] + w_unused_function_code = ''' + static inline void foo(void) {} - int main(void) { - float f = 0.0; - return isnan(f); - } -''' -if cc.compiles(w_double_promotion_code, args: w_double_promotion_args, name: '-Wdouble-promotion') - supported_cc_flags += ['-Wdouble-promotion'] -endif - -# Clang complains about unused static inline functions which are common -# with G_DEFINE_AUTOPTR_CLEANUP_FUNC. -w_unused_function_args = ['-Wunused-function', '-Werror'] -w_unused_function_code = ''' - static inline void foo(void) {} + int main(void) { return 0; } + ''' + # -Wunused-function is implied by -Wall, we must turn it off explicitly. + if not cc.compiles(w_unused_function_code, args: w_unused_function_args) + supported_cc_flags += ['-Wno-unused-function'] + endif - int main(void) { return 0; } -''' -# -Wunused-function is implied by -Wall, we must turn it off explicitly. -if not cc.compiles(w_unused_function_code, args: w_unused_function_args) - supported_cc_flags += ['-Wno-unused-function'] endif - add_project_arguments(supported_cc_flags, language: 'c') if cc.has_argument('-Wsuggest-attribute=format') -- 2.30.2
Meson has its own mechanism to turn on -Werror with the -Dwerror option. If this is set, then there is no reason for libvirt to check for -Werror itself. We remove the summary line output because it is potentially misleading when libvirt hasn't enabled -Werror, but meson has. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- meson.build | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/meson.build b/meson.build index 9fda0c7948..725d0e1cf0 100644 --- a/meson.build +++ b/meson.build @@ -207,7 +207,7 @@ cc = meson.get_compiler('c') cc_flags = [] git_werror = get_option('git_werror') -if git_werror.enabled() or git_werror.auto() and git +if (git_werror.enabled() or git_werror.auto()) and git and not get_option('werror') cc_flags += [ '-Werror' ] endif @@ -2300,7 +2300,6 @@ else loader_res = '' endif misc_summary = { - 'Use -Werror': cc_flags.contains('-Werror'), 'Warning Flags': supported_cc_flags, 'docs': gen_docs, 'tests': build_tests, -- 2.30.2
On Thu, Apr 08, 2021 at 11:58:23AM +0100, Daniel P. Berrangé wrote:
Meson has its own mechanism to turn on -Werror with the -Dwerror option. If this is set, then there is no reason for libvirt to check for -Werror itself.
s/-Dwerror/--werror/ here and in $SUBJECT In meson --werror is build-in option so it is not used together with the -D{name} option.
We remove the summary line output because it is potentially misleading when libvirt hasn't enabled -Werror, but meson has.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- meson.build | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/meson.build b/meson.build index 9fda0c7948..725d0e1cf0 100644 --- a/meson.build +++ b/meson.build @@ -207,7 +207,7 @@ cc = meson.get_compiler('c') cc_flags = []
git_werror = get_option('git_werror') -if git_werror.enabled() or git_werror.auto() and git +if (git_werror.enabled() or git_werror.auto()) and git and not get_option('werror') cc_flags += [ '-Werror' ] endif
@@ -2300,7 +2300,6 @@ else loader_res = '' endif misc_summary = { - 'Use -Werror': cc_flags.contains('-Werror'), 'Warning Flags': supported_cc_flags, 'docs': gen_docs, 'tests': build_tests, -- 2.30.2
On Thu, Apr 08, 2021 at 11:58:17AM +0100, Daniel P. Berrangé wrote:
Currently our compiler flags handling has a number of flaws and misleading characteristics
- It triggers warnings from Meson due to our use of -Wall/-Wextra that prevents us using --fatal-meson-warnings
- We enable flags, only to turn them back off again eg "-Wcast-function-type ... -Wno-cast-function-type"
- We check for certain flags twice
- We don't check compiler support for some flags
Daniel P. Berrangé (6): meson: get rid of list of unused warning flags meson: actually check for -Wframe-larger-than meson: merge all cc_flags arrays into one meson: avoid checking compiler flags twice meson: honour meson warning_level option meson: don't probe for -Werror if -Dwerror is enabled
With the comments addressed: Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (2)
-
Daniel P. Berrangé -
Pavel Hrdina