Add compiler and linker flag options and warning-as-errors switch - #517
Add compiler and linker flag options and warning-as-errors switch#517Flowdalic wants to merge 2 commits into
Conversation
This adds --cflags, --cxxflags, and --ldflags configuration options to allow appending custom compiler and linker flags to the build. Additionally, it introduces a --warnings-as-errors switch (enabled by default). When disabled, this strips -Werror from the default warning flags and avoids injecting -Werror=... into QEMU's CFLAGS. The --warnings-as-errors switch was introduced to improve compatibility with newer host environments. For example, recent versions of glibc implement strchr(), and friends, using a _Generic macro, causing many existing call sites to emit "incompatible pointer type" warnings. Providing an opt-out mechanism for the "warnings as errors" behavior helps running (and adopting) cheribuild with modern compilers and/or C standard libs.
The previous commit introduced a warnings-as-errors flag and cflags/cxxflags/ldflags options, but they were not wired up for the FreeBSD/CheriBSD build framework, which bypasses the generic compiler_warning_flags mechanism. Furthermore, certain FreeBSD bootstrap makefiles (like tools/build/mk/Makefile.boot) explicitly append their own -Werror=... flags using CFLAGS+=. These explicit flags supersedes any generic -Wno-error passed via the compiler command due to clang evaluating specific -Werror=X options as having precedence over -Wno-error. Fix the issue by intercepting the compiler commands, for both host and cross compilers, and wrapping them in a dynamically generated Python script when warnings_as_errors is disabled or custom flags are present. The wrapper strips out any argument starting with -Werror and appends any user-provided cflags or cxxflags, ensuring the build respects the user configuration.
|
I don't think we want to be going down this road, it's already awkward enough to ensure cross-builds get the right set of flags. And I don't see how the wrappers you generate can possibly work reliably with BUILD_WITH_STRICT_TMPPATH. |
| help="Treat compiler warnings as errors (disabling this strips -Werror from default flags)", | ||
| default=True, | ||
| ) | ||
| cls.cflags = cls.add_list_option( |
There was a problem hiding this comment.
I like the idea of adding a way to add extra CFLAGS/CXXFLAGS/LDFLAGS flag, but we are starting to hit the limits of argparse scalability so adding new options for every projects is actually hurting startup (and testing) speed quite noticeably.
Are you setting this in a config file and building multiple targets at once? If not we could probably work around this by reading somethig like a CHERIBUILD_CFLAGS environment variable?
There was a problem hiding this comment.
Environment variable would work for me as well. I wonder if it should even prefixed with CHERIBUILD_. Or maybe simply use EXTRA_CFLAGS.
| if ccinfo.compiler != "gcc": | ||
| # QEMU's configure script adds --extra-cflags to CXXFLAGS and these flags are rejected by g++ | ||
| cflags += ["-Werror=implicit-function-declaration", "-Werror=incompatible-pointer-types"] | ||
| if self.warnings_as_errors: |
There was a problem hiding this comment.
Are you actually hitting those errors? I added those flags after spending too much time debugging issues that would have been caught. The first one should definitely not be happening, the second one has found some real problems.
There was a problem hiding this comment.
Sorry for the late reply.
Are you actually hitting those errors?
Yes, as stated in one of the commit message I think two things happend
- glibc started to use the
_Genericmacro on things likestrchr(), causingincompatible pointer type - clang 22 (?) upgraded
incompatible pointer typeto an error
I understand and totally agree, from personal experience, with using strict compiler flags. However, as newer compilers get stricter and new code is using new language features, it is nice to be able to temporary disable certain features/warnings/errors, just to see if things still build with newer compilers/libc. Therefore, it is nice to have an opt-out mechanism.
There was a problem hiding this comment.
Hmm that's annoying. I agree that we will need to drop the werror once more distributions update glibc. Unless it's an easy fix?
There was a problem hiding this comment.
Unless it's an easy fix?
More like tons of easy fixes. strchr is used a lot. :)
There was a problem hiding this comment.
Those should just be fixed, though I'm a bit surprised QEMU doesn't pass -std= upstream.
| ldflags.append("-lpcre") | ||
| if ldflags: | ||
| self.configure_args.append("--extra-ldflags=" + self.commandline_to_str(ldflags)) | ||
| cflags += ["-Werror=implicit-function-declaration", "-Werror=incompatible-pointer-types"] |
There was a problem hiding this comment.
Maybe we just delete this and only do it for clang?
Not sure why it's here twice...
3 participants
I experimented a bit with running cheribuild with different compilers and C standard libcs. This is hindered by warnings-as-errors, so this proposes an opt-out mechanism (and a mechanism to inject custom flags).