Skip to content

Add compiler and linker flag options and warning-as-errors switch #517

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Flowdalic wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add compiler and linker flag options and warning-as-errors switch #517

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9bcdd06
Add compiler and linker flag options and warning-as-errors switch
Flowdalic Jul 14, 2026
8f354ae
Fix warnings-as-errors and custom cflags for CheriBSD build
Flowdalic Jul 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions pycheribuild/projects/build_qemu.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,8 @@ def setup(self):
cflags = self.default_compiler_flags("c") + self.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:

@arichardson arichardson Jul 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Flowdalic Flowdalic Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  1. glibc started to use the _Generic macro on things like strchr(), causing incompatible pointer type
  2. clang 22 (?) upgraded incompatible pointer type to 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.

@arichardson arichardson Jul 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@Flowdalic Flowdalic Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless it's an easy fix?

More like tons of easy fixes. strchr is used a lot. :)

@jrtc27 jrtc27 Jul 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those should just be fixed, though I'm a bit surprised QEMU doesn't pass -std= upstream.

cflags += ["-Werror=implicit-function-declaration", "-Werror=incompatible-pointer-types"]
self.configure_args.append("--extra-cflags=" + self.commandline_to_str(cflags))
ldflags = self.default_ldflags + self.LDFLAGS
if self.config.portable_build:
Expand All @@ -323,7 +324,8 @@ def setup(self):
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"]

@arichardson arichardson Jul 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we just delete this and only do it for clang?

Not sure why it's here twice...

if self.warnings_as_errors:
cflags += ["-Werror=implicit-function-declaration", "-Werror=incompatible-pointer-types"]
cxxflags = self.default_compiler_flags("c++") + self.CXXFLAGS
# QEMU's configure script adds --extra-cflags to CXXFLAGS so remove duplicates
cxxflags = [flag for flag in cxxflags if flag not in cflags]
Expand Down
45 changes: 40 additions & 5 deletions pycheribuild/projects/cross/cheribsd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1127,9 +1127,12 @@ def _setup_cross_toolchain_config(self) -> None:
xccinfo = self.get_compiler_info(self.CC)
if not xccinfo.is_clang:
self.ask_for_confirmation("Cross compiler is not clang, are you sure you want to continue?")
xcc = self._get_compiler_with_flags("xcc", str(self.CC), self.cflags)
xcxx = self._get_compiler_with_flags("xcxx", str(self.CXX), self.cxxflags)

self.cross_toolchain_config.set_env(
XCC=self.CC,
XCXX=self.CXX,
XCC=xcc,
XCXX=xcxx,
XCPP=self.CPP,
X_COMPILER_TYPE=xccinfo.compiler, # This is needed otherwise the build assumes it should build with $CC
)
Expand Down Expand Up @@ -1604,19 +1607,51 @@ def rewrite_passwd(old):
if is_jenkins_build():
self._copykernel(kernconfs=kernconfs, rootfs_dir=self.install_dir, dest_dir=self.config.output_root)

def _get_compiler_with_flags(self, name: str, compiler: str, flags: "list[str]") -> str:
if not flags and self.warnings_as_errors:
return str(compiler)

self.build_dir.mkdir(parents=True, exist_ok=True)
wrapper = self.build_dir / f"{name}-wrapper.py"

script = [
f"#!{sys.executable}",
"import sys",
"import os",
f"compiler = {repr(str(compiler))}",
f"flags_to_add = {repr(list(flags))}",
"strip_werror = " + str(not self.warnings_as_errors),
"new_args = []",
"for arg in sys.argv[1:]:",
" if strip_werror and arg.startswith('-Werror'):",
" continue",
" new_args.append(arg)",
"os.execvp(compiler, [compiler] + new_args + flags_to_add)",
]

wrapper.write_text("\n".join(script) + "\n")
wrapper.chmod(0o755)
return str(wrapper)

def add_cross_build_options(self) -> None:
assert self.crossbuild

cc = self._get_compiler_with_flags("host-cc", str(self.host_CC), self.cflags)
cxx = self._get_compiler_with_flags("host-cxx", str(self.host_CXX), self.cxxflags)

self.make_args.set_env(
CC=self.host_CC,
CXX=self.host_CXX,
CC=cc,
CXX=cxx,
CPP=self.host_CPP,
STRIPBIN=shutil.which("strip") or shutil.which("llvm-strip") or "strip",
)
if self.use_bootstrapped_toolchain:
assert "XCC" not in self.make_args.env_vars
# We have to provide the default X* values so that Makefile.inc1 does not disable MK_CLANG_BOOTSTRAP and
# doesn't try to use the host toolchain for cross-building
self.make_args.set_env(XCC="cc", XCXX="c++", XCPP="cpp", XSTRIPBIN="strip")
xcc = self._get_compiler_with_flags("xcc", "cc", self.cflags)
xcxx = self._get_compiler_with_flags("xcxx", "c++", self.cxxflags)
self.make_args.set_env(XCC=xcc, XCXX=xcxx, XCPP="cpp", XSTRIPBIN="strip")
# We also have to set X_COMPILER_TYPE since the build system is broken and determines it before it's built
# the bootstrap toolchain, so will fall back on the inferred value for COMPILER_TYPE, which is likely gcc
# on Linux.
Expand Down
35 changes: 33 additions & 2 deletions pycheribuild/projects/project.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,28 @@ def check_system_dependencies(self) -> None:
@classmethod
def setup_config_options(cls, install_directory_help="", **kwargs) -> None:
super().setup_config_options(**kwargs)

cls.warnings_as_errors = cls.add_bool_option(
"warnings-as-errors",
help="Treat compiler warnings as errors (disabling this strips -Werror from default flags)",
default=True,
)
cls.cflags = cls.add_list_option(

@arichardson arichardson Jul 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@Flowdalic Flowdalic Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Environment variable would work for me as well. I wonder if it should even prefixed with CHERIBUILD_. Or maybe simply use EXTRA_CFLAGS.

"cflags",
metavar="FLAGS",
help="Additional C/C++ compiler flags to append",
)
cls.cxxflags = cls.add_list_option(
"cxxflags",
metavar="FLAGS",
help="Additional C++ compiler flags to append",
)
cls.ldflags = cls.add_list_option(
"ldflags",
metavar="FLAGS",
help="Additional linker flags to append",
)

# --<target>-<suffix>/build-directory is not inherited from the unsuffixed target (unless there is only one
# supported target).
default_xtarget = cls.default_architecture()
Expand Down Expand Up @@ -843,9 +865,14 @@ def _build_type_basic_compiler_flags(self):
@property
def compiler_warning_flags(self) -> "list[str]":
if self.compiling_for_host():
return self.common_warning_flags + self.host_warning_flags
flags = self.common_warning_flags + self.host_warning_flags
else:
return self.common_warning_flags + self.cross_warning_flags
flags = self.common_warning_flags + self.cross_warning_flags

if not self.warnings_as_errors:
flags = [f for f in flags if not f.startswith("-Werror")]

return flags

def default_compiler_flags(self, lang: CompilerLanguage = "c") -> "list[str]":
assert self._setup_called
Expand Down Expand Up @@ -873,6 +900,7 @@ def default_compiler_flags(self, lang: CompilerLanguage = "c") -> "list[str]":
else:
self.warning("Compiler", compiler.path, "does not support -fsanitize=cheri, please update your SDK")
if self.compiling_for_host():
result.extend(self.cflags if lang == "c" else self.cxxflags)
return result
if self.config.csetbounds_stats:
result.extend(
Expand All @@ -884,6 +912,7 @@ def default_compiler_flags(self, lang: CompilerLanguage = "c") -> "list[str]":
# "-Xclang", "-cheri-bounds=everywhere-unsafe"])
]
)
result.extend(self.cflags if lang == "c" else self.cxxflags)
return result

@property
Expand All @@ -898,6 +927,7 @@ def default_ldflags(self) -> "list[str]":
result.append("-fsanitize=cfi")
result.append("-fsanitize-cfi-cross-dso")
if self.compiling_for_host():
result.extend(self.ldflags)
return result

# Should work fine without linker emulation (the linker should infer it from input files)
Expand All @@ -918,6 +948,7 @@ def default_ldflags(self) -> "list[str]":
# We need to include the constructor even if there is no reference to libstatcounters:
# TODO: always include the .a file?
result += ["-Wl,--whole-archive", "-lstatcounters", "-Wl,--no-whole-archive"]
result.extend(self.ldflags)
return result

def add_asan_flags(self):
Expand Down
4 changes: 4 additions & 0 deletions tests/test_argument_parsing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1293,17 +1293,21 @@ def test_mfs_root_kernel_config_options():
"build_nocaprevoke_kernels",
"build_toolchain",
"build_type",
"cflags",
"cxxflags",
"debug_kernel",
"default_kernel_abi",
"extra_make_args",
"extra_make_env",
"fast_rebuild",
"force_configure",
"kernel_config",
"ldflags",
"mfs_root_image",
"skip_update",
"use_ccache",
"use_lto",
"warnings_as_errors",
"with_clean",
"with_debug_files",
"with_debug_info",
Expand Down