compilers: c: add Modern C -Werrors for GCC, Clang (again) #13659

thesamesam · 2024-09-10T02:10:48Z

Retry of #12682.

Commits:

The first commit is just compilers: c: add Modern C -Werrors for GCC, Clang #12682 rebased.
The second is switching to a builtin option. This avoids us having to add one for each language (see next point). It also gives us more scope in future for "this is so important it should really be a default"-style flags but compilers don't (yet) set it for one reason or another.
The third is wiring this up for LTO too where the LTO-specific options are relevant to at least C++, but I think it may apply to Fortran as well as any other language GCC can do (it supports LTO across all FEs).

If we get consensus on how to move forward, I'll squash & fix up the commit messages for the other commits & docs, ofc.

--

Add the following flags to error out by default via a new default-on 'Dc_legal_code' option for GCC and Clang:

-Werror=implicit (-> -Werror=implicit-int,implicit-function-declaration)
-Werror=int-conversion
-Werror=incompatible-pointer-types

Implicit function declarations were removed in C99 with no deprecation period because of how dangerous they are.

Implicit conversions between integers and pointers were also never allowed in >= C89.

These were allowed by GCC and Clang until recently for compatibility reasons:

GCC makes these an error by default in the upcoming GCC 14 release
Clang made -Werror=int-conversion a default error in Clang 15 and made the others an error in Clang 16

The reason for Meson to do this even for older compilers is straightforward:

It'll take time for these newer versions to propagate into distributions.
The code emitting these is broken now.
Projects like PipeWire and various GNOME components are adding these flags manually to catch them already.

Add the following flags to error out by default via a new default-on 'Dc_legal_code' option for GCC and Clang: * -Werror=implicit (-> -Werror=implicit-int,implicit-function-declaration) * -Werror=int-conversion * -Werror=incompatible-pointer-types Implicit function declarations were removed in C99 with no deprecation period because of how dangerous they are. Implicit conversions between integers and pointers were also never allowed in >= C89. These were allowed by GCC and Clang until recently for compatibility reasons: * GCC makes these an error by default in the upcoming GCC 14 release * Clang made -Werror=int-conversion a default error in Clang 15 and made the others an error in Clang 16 The reason for Meson to do this even for older compilers is straightforward: * It'll take time for these newer versions to propagate into distributions. * The code emitting these is broken *now*. * Projects like PipeWire and various GNOME components are adding these flags manually to catch them already. Signed-off-by: Sam James <sam@gentoo.org>

thesamesam · 2024-09-10T02:20:14Z

mesonbuild/compilers/compilers.py

@@ -295,6 +296,10 @@ def get_base_compile_args(options: 'KeyedOptionDictType', compiler: 'Compiler',
                mode=get_option_value(options, OptionKey('b_lto_mode'), 'default')))
    except (KeyError, AttributeError):
        pass
+
+    if option_enabled(compiler.base_options, options, OptionKey('b_legal_code')):


@eli-schwartz This conditional is always false but I don't get why?

You still need to add to the compiler that it suppots b_legal_code, say in the GnuCompiler.__init__ method

Although, come to think of it, GnuDCompiler inherits that, as does the out of tree GnuRustCompiler patches I have somewhere...

Thanks, I was wondering if it was something like that but I couldn't spot it. Doh.

So, probably have to add them to `GnuCCompiler and GnuCPPCompiler

Signed-off-by: Sam James <sam@gentoo.org>

For GCC LTO, we can set the following options with b_legal_code: * -Werror=lto-type-mismatch * -Werror=odr * -Werror=strict-aliasing TODO: write more Signed-off-by: Sam James <sam@gentoo.org>

dnicolodi · 2024-09-10T06:48:53Z

This may have been discussed elsewhere already, but why does Meson need to have a built-in option to enable an (opinionated?) set of compilation flags? Is this only a way to avoid some projects to have to repeat a few lines into their meson.build?

If this is really needed, the option name does a very poor job in describing what it does. b_legal_code=true has the effect of making some code that would be allowed to compile into code that does not compile anymore, thus it turns legal code into illegal code: not what I would have expected from the option name.

dnicolodi · 2024-09-10T06:52:37Z

mesonbuild/compilers/compilers.py

@@ -234,6 +234,7 @@ def init_option(self, name: OptionKey) -> options._U:
    OptionKey('b_thinlto_cache_dir'): BaseOption(options.UserStringOption, 'Directory to store ThinLTO cache objects', ''),
    OptionKey('b_sanitize'): BaseOption(options.UserComboOption, 'Code sanitizer to use', 'none',
                                        choices=['none', 'address', 'thread', 'undefined', 'memory', 'leak', 'address,undefined']),
+    OptionKey('b_legal_code'): BaseOption(options.UserBooleanOption, 'Whether to ban dangerous constructs in a language', True),


Isn't having this enable by default going to break compilation for a lot of existing projects?

These people are going to find their code fails to compile as soon as they upgrade to 2024 compilers. It's a "don't shoot the messenger" scenario.

The constructs aren't "dangerous", they are "the C standard as released in c99, says that this is not real C code and a C compiler shouldn't successfully compile it". It's also Undefined Behavior, and a particular kind of UB that results in programs having a high tendency to crash on non-amd64 architectures. That's why GCC and clang have started actually enforcing the text of the standard, and making the compiler error out on it.

It's a user service to make that visible to people by default. There's no point in having the option if no one uses it. But an option is necessary so that people can disable it, if they have a codebase originally written in 1991 and cannot update it to conform to the 1999 standard.

dnicolodi · 2024-09-10T06:53:36Z

docs/markdown/snippets/c-legal-code.md

+
+The new `c_legal_code` option is enabled by default. It bans
+dangerous C constructs in C99 or later to emulate newer C compiler
+defaults.


What does "newer C compiler" mean here?

It means gcc 14 or clang 16.

thesamesam requested a review from eli-schwartz September 10, 2024 02:10

thesamesam commented Sep 10, 2024

View reviewed changes

squashme: make legal_code a builtin

dacb217

Signed-off-by: Sam James <sam@gentoo.org>

thesamesam force-pushed the meson-stricter branch from 3d179e4 to 5a8b32b Compare September 10, 2024 03:00

compilers: wire up b_legal_code for LTO

9841177

For GCC LTO, we can set the following options with b_legal_code: * -Werror=lto-type-mismatch * -Werror=odr * -Werror=strict-aliasing TODO: write more Signed-off-by: Sam James <sam@gentoo.org>

thesamesam force-pushed the meson-stricter branch from 5a8b32b to 9841177 Compare September 10, 2024 03:03

dnicolodi reviewed Sep 10, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

compilers: c: add Modern C -Werrors for GCC, Clang (again) #13659

compilers: c: add Modern C -Werrors for GCC, Clang (again) #13659

thesamesam commented Sep 10, 2024 •

edited

Loading

thesamesam Sep 10, 2024

dcbaker Sep 10, 2024

dcbaker Sep 10, 2024

thesamesam Sep 10, 2024

dcbaker Sep 10, 2024

dnicolodi commented Sep 10, 2024

dnicolodi Sep 10, 2024

eli-schwartz Sep 10, 2024

dnicolodi Sep 10, 2024

eli-schwartz Sep 10, 2024

compilers: c: add Modern C -Werrors for GCC, Clang (again) #13659

Are you sure you want to change the base?

compilers: c: add Modern C -Werrors for GCC, Clang (again) #13659

Conversation

thesamesam commented Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnicolodi commented Sep 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thesamesam commented Sep 10, 2024 •

edited

Loading