Repository navigation
Consider adding -Werror=unguarded-availability-new to compiler flags for Apple platforms #100384
Description
Feature or enhancement
Runtime crashes like #97897 could be prevented by turning clang's unguaded-availability-new warning into a fatal compile error. This can be done by adding -Werror=unguarded-availability-new to compiler flags.
Essentially what this warning does is cross reference undefined symbols against symbols defined in the targeted Apple SDK. If it sees a symbol introduced in a newer SDK version and that symbol isn't weakly referenced/linked, you get a warning. -Werror upconverts it to a fatal compiler error.
If you add this flag to release builds, the compiler prevents you from shipping binaries that ship unguarded symbol usage for targeted SDK versions. i.e. it prevents run-time crashes when binaries run on older Apple OS versions.
If you enable this setting today, you may find the 3.8 branch isn't properly gating use of mkfifoat and mknodat. (Although this may be an oddity from python-build-standalone and not a CPython bug.)
cc @ned-deily
The advantages of having this additional warning is to catch regressions such as:
- Availability of
ptsname_ris not checked at runtime on macOS #123797 - Availability of utimensat, futimens not checked correctly on macOS #75782
Activity
indygreg commentedon Dec 21, 2022
Ignore my comment about 3.8. 3.8 doesn't have properly weak linking support at all (at least not in
posixmodule.c). Themkfifoatandmknodaterrors were just clang catching these symbols falling through a gap in python-build-standalone's build system, which attempts to disable all optional symbols on 3.8 to work around the lack of weak referencing. I think this demonstrates the value of-Werror=unguarded-availability-new, as without this, I probably would have shipped binaries that segfault at runtime.ronaldoussoren commentedon Dec 21, 2022
3.9 and later should have weak linking support in posixmodule, this was introduced while working on support for arm64 and Universal 2 binaries. That said, newer SDKs can introduce new APIs that require changes to that.
zanieb commentedon Dec 21, 2024
Copying over some of my commentary from #128135
Here's an initial patch 36c7da4
unguarded-availabilityorunguarded-availability-new?clang-only, will this help prevent regressions upstream or just avoid shipping bad code downstream?-Werror=unguarded-availability-newin sysconfig prevents building ofwatchdogastral-sh/python-build-standalone#210, users reported if this is present in theCFLAGSinsysconfigit is used to build extension modules in some packages which causes failures. Would this be a problem here too? Is there existing infrastructure to deal with this problem?picnixz commentedon Dec 21, 2024
What we could do is integrate this new flag as part of our CI without necessarily making it part of the
configurescript. At least it could help without affecting local configurations? cc @hugovkned-deily commentedon Dec 21, 2024
Instead of
CFLAGS, try adding this toCFLAGS_NODIST. "CFLAGS_NODISTis used for building the interpreter and stdlib C extensions. Use it when a compiler flag should not be part ofCFLAGSonce Python is installed."zanieb commentedon Dec 21, 2024
Ah cool, thanks @ned-deily. I was hoping there was something like that.
Do you have any other concerns before I post the patch as a pull request?
I think my most important open question is
Is
clangbeing used in CI?indygreg commentedon Dec 21, 2024
The Xcode C toolchain is clang. There are an extremely small number of people not using clang when targeting Apple platforms. I'm pretty sure CPython's build system implicitly requires clang for Apple targets.
picnixz commentedon Dec 21, 2024
We use clang on build bots sanitizers (UBSan, TSan and ASan) so yes we use
clang. And for the JIT as well since this requires LLVM.hugovk commentedon Dec 21, 2024
Like with this tooling?
https://devguide.python.org/development-tools/warnings/
zanieb commentedon Dec 21, 2024
I think the concern about it affecting users is addressed by using
CFLAGS_NODIST, is there a different concern you have that makes this better as a warning than an error?ned-deily commentedon Dec 22, 2024
If I understand the implications correctly, I think we should stick with
unguarded-availabilityuntil there is a demonstrated need to restrict tounguarded-availability-new.Also we specifically now check and report on supported platform / compiler combinations (see
AC_MSG_CHECKING([for PEP 11 support tier])inconfigure.ac) which for macOS isclangfor tier 1 support. That said, we should be careful to not deliberately breakgccbuilds; presumably theAX_CHECK_COMPILE_FLAG([-Wunguarded-availability] ...test in the patch should take care of that.gh-100384: Error on `unguarded-availability` in macOS builds (#128155)
pythongh-100384: Error on `unguarded-availability` in macOS builds (p…
pythongh-100384: Error on `unguarded-availability` in macOS builds (p…
[cmake] Make unguarded availability an error (canonical#5343)