NixOS / nix

Nix, the purely functional package manager
https://nixos.org/
GNU Lesser General Public License v2.1
12.92k stars 1.53k forks source link

Enable C++ Sanitizers for Nix #10969

Open roberth opened 5 months ago

roberth commented 5 months ago

Is your feature request related to a problem? Please describe. I'm always frustrated when debugging subtle and hard-to-diagnose issues in Nix due to undefined behavior and memory errors in the C++ codebase. These errors can cause unpredictable behavior and are difficult to trace and fix.

Describe the solution you'd like I would like the Nix build process to include C++ sanitizers such as AddressSanitizer, UndefinedBehaviorSanitizer, and ThreadSanitizer. Enabling these sanitizers during the build can help detect and report errors related to memory usage, undefined behavior, and threading issues, making the code more robust and easier to maintain.

Describe alternatives you've considered

Additional context Including sanitizers in the build process can be controlled via build options or environment variables, ensuring they are only enabled during development and testing phases, not in production builds. This would allow developers to benefit from the enhanced error detection without affecting production performance.

Priorities

Add :+1: to issues you find important.

Ericson2314 commented 5 months ago

I think Meson might just have some stuff for this.

lf- commented 5 months ago

it does. there's a couple of flags you need to get UBSan to work properly on clang however, see lix meson.build. ASan is likely impossible without getting rid of Boost coroutines because even if you do the stuff they claim to make it work, the memory crimes it does still cause ASan to crash.

I've nonetheless found actual bugs with ASan outside of the region using boost coroutines, it's just there's false positives I've not managed to mark as ignored.

Another thing Lix did that you could absolutely take is the fact that our production builds ban signed overflow completely at no measurable performance cost. See our meson.build for that as well.

Ericson2314 commented 5 months ago

it does. there's a couple of flags you need to get UBSan to work properly on clang however, see lix meson.build

OK We'll look!

ASan is likely impossible without getting rid of Boost coroutines because even if you do the stuff they claim to make it work, the memory crimes it does still cause ASan to crash.

Certainly a good reason to reconsider using them! (I am also apprehensive how they will behave on Windows. Or with "anti virus" software.)

Another thing Lix did that you could absolutely take is the fact that our production builds ban signed overflow completely at no measurable performance cost. See our meson.build for that as well.

Per https://discourse.nixos.org/t/2024-06-26-nix-team-meeting-minutes-156/47740 yes we very much do want it :)

xokdvium commented 3 weeks ago

While we are at it, I think it would be also worthwhile to consider enabling linters (think clang-tidy). Its bugprone-* checks would be a good starting point.

It can also be run with agreed upon modernize-use- flags and -fix for large scale modernization of the code base.

Since now we have proper meson this should be trivial to enable, since it automatically generates a clang-tidy target. Starting with 1.3 it also provides a clang-tidy-fix.

The hard part here would be to agree upon the checks. A lot of the checks are opinionated and I don't think it's worthwhile to look into them. But some checks like bugprone-assert-side-effect are very useful. It would have caught things like #11718.