New Default Compiler Options for Safety

In an effort to make CPython safer I have been working to introduce new compiler options recommended by OpenSSF for C/C++ projects. The parent issue for this effort can be found here.

In this thread I would like to have a more in-depth discussion about which compiler options should be added. Right now there is an open PR to add “-Wformat-nonliteral”. There is some code that emits warnings but appears to be safe.

I would suggest that in this instance instead of adding the files to the tooling’s warning ignore list that we indicate that the warning should be ignored in the file at the smallest scope possible with a clear explanation.

3 Likes

I have also implemented tooling that runs as a GitHub Action to check for new warnings for both Ubuntu and macOS build and test jobs.

4 Likes

@sethmlarson and I have met and came up with a plan for the next steps relating to the compiler hardening effort.

  • Create Documentation
    • Update docs related to --disable-safety and --enable-slower-safety and submit a PR
    • Submit a devguide PR for the compiler warnings tool and how to “accept new warnings”
  • Remove --enable-slower-safety
    • Benchmarks have shown that -D_FORITIFY_SOURCE=3 did not impact performance in any measurable way. Most platforms use level 2 by default. Can either move fortify source 3 to --disable-safety or remove the option all together
  • Add Compiler Options that Generate Warnings
    • Create a PR that enables warning emitting compiler options, add all offending files to the warning ignore file
    • Plan is for incremental adoption without blocking core developers from adding new warnings.
  • Create separate GitHub issues per warning class.
    • Goal is to reduce warnings and fix issues, if any. Confirm there is no security vulnerability in those places.

If you have any questions about the sections of this plan let me know!

5 Likes

For visibility, cc-ing @corona10 iirc you were involved in the new flags for configure, would be good to have your thoughts on the above plan for those flags.

My view on the new configure flags is they are a separate effort to the adoption of the compiler options tooling for warnings which have no effect on performance and thus can continue (assuming there’s no disagreement on this front if core developers are able to “opt-in” to new warnings).

1 Like

On platforms where we only release source code, compiler options have traditionally been set by the distributor, with CPython mainly providing the necessary flags – include directories and such.
Such redistributors have their own sets of flags, which might potentially conflict.
So, at least for Linux, these should be treated as “recommended options” or “default options for CPython’s CI” (and for Mac, “options used for the cpython.org builds”). And the docs should spend some words on how to merge these with existing options, or evaluate and cherry-pick individual ones. (AFAIK, OpenSSF has extensive docs on the recommended options? But to make this useful for redistributors, we should probably document the individual options and how they relate to CPython.)

With that, the decision to add -Wformat-nonliteral, with ignore files or one or two #pragma blocks to ignore warnings, should be easier. If we want it for our CI & releases, it’s in!


Consider wrapping compiler-specific directives in macros like the existing _Py_COMP_DIAG_PUSH, to make things easier to port to different compilers.

When suppressing a warning, consider adding a comment not only about why the code is safe, but also why it can’t be rewritten to not emit the warning at all.

3 Likes

If the only performance-related option is -D_FORITIFY_SOURCE=3 and no other performance-related options remain, I am fine with removing it.

1 Like

Thank you for the response.

I have a PR out to update the documentation that might be closer to your suggestions that would help redistributors. Each of the compiler options now links directly to the relevant section of the OpenSSF guidance document.

With the documentation and the inclusion of the --disable-safety configure option we give redistributors the ability to “evaluate and cherry-pick” (really just remove the new suggested) compiler options should conflicts arise.