Skip to content
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

Stabilize -Z sanitize #47174

Open
Firstyear opened this issue Jan 4, 2018 · 12 comments
Open

Stabilize -Z sanitize #47174

Firstyear opened this issue Jan 4, 2018 · 12 comments
Labels
A-sanitizers Area: Sanitizers for correctness and code quality. C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Firstyear
Copy link
Contributor

Firstyear commented Jan 4, 2018

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:

rust-lang/rfcs#670
#42711
https://users.rust-lang.org/t/howto-sanitize-your-rust-code/9378

@cuviper
Copy link
Member

cuviper commented Jan 8, 2018

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

@pietroalbini pietroalbini added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 6, 2018
@nikomatsakis
Copy link
Contributor

@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 #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
Copy link
Member

kennytm commented Feb 15, 2018

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

@codedcosmos
Copy link

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
Copy link
Contributor

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:

  • What happens when sanitizers are used on a platform that is not supported?
  • What kind of external dependencies (if any) must be installed to use the sanitizers? Is everything included with operating systems etc at this point?
  • What about projects that use C code? Can we see some guidance on how to develop "mixed projects" where the Rust and C code is both being sanitized?

@Mark-Simulacrum
Copy link
Member

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Member

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:

  • Cargo support is obviously desirable but what does this look like exactly? At a minimum Cargo should build your dependencies with whatever sanitizer you have enabled as well. cargo test support would be nice as well.
  • Can cc-rs learn that we're building with a sanitizer enabled and build C code with the same sanitizer? (Perhaps Cargo sets an environment variable?) How does cc-rs find the appropriate sanitizer library to link to?
  • 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?
  • 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?
  • If the codegen backend doesn't support the specified sanitizer, does this generate an error or warning?
  • If the codegen backend supports the specified sanitizer but the target doesn't, does that generate an error or warning?
  • If a target or codegen backend drops support for a sanitizer, I assume we will have to as well. We should probably note that the set of supported sanitizers is not part of the stability guarantee and can change over time.
  • Do we need anything else to support build systems other than Cargo?
  • Most of the sanitizers currently require the use of -Zbuild-std. Will this continue to be the case or can we do something so that a fully stable toolchain can use the sanitizers?

@cuviper
Copy link
Member

cuviper commented Sep 24, 2021

  • 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
Copy link
Member

nagisa commented Oct 8, 2021

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
Copy link

Noah-Kennedy commented Apr 4, 2024

Has there been any movement on this since 2021?

What are the current blockers to stabilization?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sanitizers Area: Sanitizers for correctness and code quality. C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests