-
-
Notifications
You must be signed in to change notification settings - Fork 18k
cc-wrapper: add glibcxxassertions hardening flag
#414987
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
Can’t we check for libc++ by checking for Considering that it’s possible to use libc++ on Linux (which uses libstdc++ by default) and libstdc++ on Darwin (which uses libc++), the place to check would be in the wrapper (assuming it is desired to check and toggle these assertions on our off based on the target C++ stdlib). |
|
So.. we probably could, but that would involve putting the logic into cc-wrapper's |
So maybe putting it there if/when GCC gets split into separate packages? |
no platforms "unsupported" because we don't have a nice mechanism for determining a platform's c++ lib and the flag should be harmlessly ignored by a other c++ libs
2529a05 to
1293be8
Compare
|
What's new in these changes Robert @risicle? Note that GCC in multiple packages landed |
|
It looks like Apart from a straight rebase, this PR hasn't changed since posting. |
|
Someone would be John @Ericson2314 or Tristan @RossComputerGuy for |
philiptaron
left a comment
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.
I'm fine with trusting @risicle on this. Randy do you object enough to close the PR until another way is found?
This adds another flag included in the 2025 openssf hardening guide: https://best.openssf.org/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C++.html#precondition-checks-for-c-standard-library-calls
As ever, as a new hardening flag I have disabled it by default except in
pkgsExtraHardening. Because of the possibility of slowdowns, this is one of the flags I probably won't advocate moving to default-enabled any time soon.This hardening flag is a little bit odd because it only has an effect when compiling against glibc's
libstdc++, but note how I haven't included any mechanism to "unsupport" it on platforms that uselibc++. This is because we don't currently have any nice mechanism to detect what c++ standard library a platform is using - and whether we're using clang or gcc is a poor approximation of this. Instead this PR will happily add the-D_GLIBCXX_ASSERTIONSdefinition on all platforms, whether usinglibstdc++or not, with the idea that other c++ standard libraries will just ignore it.Clang's
libc++does have a similar feature that I'm planning on introducing in a separate PR. Why not put both flags under a common generic hardening flag name? Firstly I'm not convinced that the assertions of each library will have similar enough package-breaking characteristics, and secondlylibc++'s hardening feature has multiple levels that I think are worth exposing separately. Once we get into multiple levels of strictness, ensuring sensible parity between the exposed levels between the two implementations feels like it would be a fool's errand.I built quite a lot of c++ packages on aarch64-linux with this default-enabled and only found
arrow-cpp's build to be affected by it.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.