rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.78k stars 12.5k forks source link

Stabilize -Z sanitize #47174

Open Firstyear opened 6 years ago

Firstyear commented 6 years ago

Hi,

I would really like to see -Z sanitize options available in stable rust.

Today they are available in nightly. I have used them for 6 months in the 389 Directory Server project in a production ldap environment as part of the code IO event path. I have exercise the leak and address sanitize options extensively, used them to find many FFI safety issues, and tested multiple build types (exec, dylib, more).

The sanitizer support is very important for FFI assertions during development where a code base uses Rust and C in some form. It allows strict analysis of behaviour, and finding where items are not droped for example.

An important reason for inclusion in stable is distributions. As a project, we often have the requirement to build debug builds with the compiler that ships in a distribution to reproduce problems. That means rustc stable that ships in products like fedora or RHEL.

If -Z sanitize isn't in stable, we can't achieve this - and this adds a barrier to adoption. It adds a hurdle to our community inclusiveness as potential developers have more complex steps to follow to create a dev environment. I want to say "dnf install -y cargo rustc ..." and someone should be able to work.

Many people have put a lot of time into sanitizer support, and today it's locked behind nightly. :(

I would love to see this in stable as it brings great run-time analysis features to rust especially for FFI use cases, and having paths to stabilise debugging options into stable rustc will strongly aid adoptions of rust in distributions of linux. It's an advertised feature of the language on email lists, so to have this accessible to anyone would be a huge benefit.

I'm happy to do the coding to enable this, as well as documentation and examples, but I need some help and guidance to achieve this,

Thanks,

See also:

https://github.com/rust-lang/rfcs/issues/670 https://github.com/rust-lang/rust/pull/42711 https://users.rust-lang.org/t/howto-sanitize-your-rust-code/9378

cuviper commented 6 years ago

Sorry this has been neglected -- let's try to flag a team: cc @rust-lang/compiler

nikomatsakis commented 6 years ago

@Firstyear you make a strong case for stable sanitizers. However, I don't really feel comfortable stabilizing -Zsanitize as it stands now, because -- to be quite honest -- I have no idea what it does! It was merged by @japaric in https://github.com/rust-lang/rust/pull/38699 on an explicitly experimental basis. At the time, I wrote this:

I have to say that while I support the aims of this design, I would like to see some sort of process around these kinds of ad-hoc additions, even though they are unstable. At minimum some kind of decision to experiment in the area (with the goal of an eventual RFC). (Or do we just never expect this process to be stable? Which seems...plausible.)

I think that considering stabilization is premature. What I would like best is if we could write-up an RFC describing how the feature works, what the edge cases are, and if there are any open design questions to be addressed. Then we can decide whether to accept the RFC -- once accepted, we can decide whether to stabilize. If the RFC that gets accepted matches what exists in tree, that second decision could presumably go quite quickly!

Another important consideration is maintenance. I don't know that anyone who is currently active in the Rust compiler understands how the existing sanitizer support works. More documentation and a knowledge of who to ping would be great. =)

cc @japaric @rust-lang/dev-tools -- this seems to lie at the intersection of the two teams

kennytm commented 6 years ago

I'd like to see at least Windows support for ASan before calling for stabilization.

codedcosmos commented 3 years ago

It looks like asan is now properly supported in windows: https://devblogs.microsoft.com/cppblog/address-sanitizer-for-msvc-now-generally-available/

Would now be the time to consider stabilizing address sanitization?

nikomatsakis commented 3 years ago

There is documentation of the current sanitizer support here:

https://doc.rust-lang.org/beta/unstable-book/compiler-flags/sanitizer.html

Based on a quick read, I feel pretty positive about the current status of sanitizers. The user workflow seems fairly reasonable.

One caveat is that I think this support should be integrated into cargo, but stabilizing the rustc flags would allow that to happen in the ecosystem as a first step.

There are some questions though that arise in my mind:

Mark-Simulacrum commented 3 years ago

I'll also add that I believe the current sanitizer interface is pretty closely tied to LLVM's sanitizers. Other codegen backends may not have the same naming conventions, at least, so it may be worth considering whether we want some abstraction atop the raw names (or at least choose them a little carefully).

It also looks like there is some amount of configuration, both at runtime and compile time, for sanitizers. Just based on our documentation, I see a mention of ASAN_OPTIONS=detect_stack_use_after_return=1, and -Zsanitizer-memory-track-origins as well. We should consider whether these are something we can stabilize (does LLVM keep them stable? I'd guess no). If not, we'll want to either decide that breakage here is acceptable or try to prevent their usage on stable, perhaps.

nikomatsakis commented 3 years ago

I think making names "more generic" is a good idea, but I also think that I wouldn't want to let perfect be enemy of the good. It seems "ok" to me to ship support for LLVM-based sanitizers, and for them not to work if people opt to use different backends.

Firstyear commented 3 years ago

I agree with @nikomatsakis here, that it's probably best to just ship this "as is" for LLVM, as other backends may or may not have sanitisers available in the same way. Over time those backends would have to increase their support for various features, and sanitisers could be one of them. So my input would be to aim for stabilising this with LLVM as the codegen, and disable it when using a non-llvm codegen.

wesleywiser commented 2 years ago

I think that considering stabilization is premature. What I would like best is if we could write-up an RFC describing how the feature works, what the edge cases are, and if there are any open design questions to be addressed. Then we can decide whether to accept the RFC -- once accepted, we can decide whether to stabilize. If the RFC that gets accepted matches what exists in tree, that second decision could presumably go quite quickly!

I would just like to second this point from Niko. I think before we stabilize, we need to have a plan for how the sanitizers fit into the broader Rust ecosystem:

cuviper commented 2 years ago
  • Should we support targets bringing their own sanitizer libraries (for instance, MSVC's support of ASAN bundles ASAN libs with the MS C++ libraries) or should we always use the sanitizer libraries we've built as part of LLVM?

This should also consider downstream builds, like Linux distributions that build rustc with their system LLVM. For example, Fedora does build the sanitizers in its compiler-rt package, so I'll want a way configure rustc to use those.

nagisa commented 2 years ago

I don't see sanitizer support to be all that different from other already stable codegen-backend specific flags (e.g. -Ctarget-features, -Ctarget-cpu). As enabling a different codegen backend requires specifying additional CLI arguments, a natural consequence is that other codegen backends not supporting specific values for these flags doesn't constitute a breaking change.


Are we going to stabilize all sanitizers at once or only a subset? What is the stabilization process when LLVM (or other backends) add new sanitizers?

Sanitizers should be stabilized on a per-sanitizer basis. See e.g. #89652.

Noah-Kennedy commented 5 months ago

Has there been any movement on this since 2021?

What are the current blockers to stabilization?