-
-
Notifications
You must be signed in to change notification settings - Fork 32k
gh-121996: Introduce --disable-safety and --enable-slower-safety options #122054
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.
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
Conversation
if test "$disable_safty" = "no" | ||
then | ||
AX_CHECK_COMPILE_FLAG([-fstack-protector-strong], [BASECFLAGS="$BASECFLAGS -fstack-protector-strong"], [AC_MSG_WARN([-fstack-protector-strong not supported])], [-Werror]) | ||
AX_CHECK_COMPILE_FLAG([-Wtrampolines], [BASECFLAGS="$BASECFLAGS -Wtrampolines"], [AC_MSG_WARN([-Wtrampolines not supported])], [-Werror]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mixing of warning options with an actual codegen option (SSP) doesn't feel ideal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, tbh, pretty much every Linux distro builds with SSP and _F_S=2 at least..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's handle it as the seperate PR cc @nohlson
|
||
if test "$enable_slower_safty" = "yes" | ||
then | ||
AX_CHECK_COMPILE_FLAG([-D_FORTIFY_SOURCE=3], [BASECFLAGS="$BASECFLAGS -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3"], [AC_MSG_WARN([-D_FORTIFY_SOURCE=3 not supported])]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use AX_ADD_FORTIFY_SOURCE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#122054 (comment) ditto,
This PR is not about compiler options but more about how we can enable them and disable them without interuptting @nohlson 's experimentation.
It's not clear to me how this PR interacts with #121997. |
It's not interact, it is just separate suggestion. |
AC_MSG_CHECKING([for --disable-safety]) | ||
AC_ARG_ENABLE([safety], | ||
[AS_HELP_STRING([--disable-safety], [disable usage of the security compiler options with no performance overhead])], | ||
[AS_VAR_IF([enable_safety], [yes], [disable_safety=no], [disable_saftey=yes])], [disable_saftey=no]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: disable_saftey
should be disable_safety
📚 Documentation preview 📚: https://cpython-previews--122054.org.readthedocs.build/