Repository navigation
Disabling particular unsafe precondition checks under -Cdebug-assertions=y #120969
Description
From #85122 (comment) and #85122 (comment).
After #120594 (1.78), unsafe precondition checks always apply under -Cdebug-assertions=y, but there is currently no way to disable particular cases in debug builds:
They will make things too slow for somebody sooner or later -- it is not uncommon in projects to have a "fast debug" build where a few checks need to be explicitly bypassed in hot paths. And it is not just the branch/call itself, it can also break other optimizations like vectorization: https://godbolt.org/z/MMTordabn.
I really like the unsafe precondition checks, but I would like to make sure there is an escape hatch for
-Cdebug-assertions=ybuilds. Perhaps we need acore::hint::unreachable_unchecked_even_in_debug-like (even if it does not solve the "there is no checked API" case, but I don't know if that is an issue). For me,core::hint::unreachable_uncheckedwas that in the past, and things likeunwrap_unchecked()were the "checked in debug" ones.I also just noticed that using the intrinsic for slice indexing is not enough to remove the check in a debug build and get vectorization back (included in the link).
Activity
ojeda commentedon Feb 12, 2024
Thanks Ben -- I see you want things like
get_uncheckedto use intrinsics, which would solve (I guess) the slice indexing bit I mention above, right?clarfonthey commentedon Feb 12, 2024
I think that it's very fair to separate out unsafe preconditions from standard debug assertions, since these are operations where the developer is stating they've already met the preconditions and the extra checks are only there for, well, cases where they missed up.
Perhaps a separate
unsafe_preconditionsflag could be added and the assertions could be exposed to end users who wish to add them to their own crates? The default would be to enable them alongside debug assertions, but they could be enabled separately. Perhaps they could even be enabled with debug assertions off, since in some ways, the two checks are usually redundant.Another important thing to consider here is that methods on top of the intrinsics using these checks are done intentionally so that less logic actually has to be included in the intrinsics themselves. From this perspective, it's actually less desirable to push people to use the intrinsics directly, since those are unstable whereas the other methods are intended to eventually become stable.
the8472 commentedon Feb 12, 2024
Afaik
debug-assertsworks on a per-crate level. So one option is to move the hot paths into a separate crate and applying package overrides to it when building the binary.Generally I wouldn't expect vectorization to work with debug asserts. Practically no effort is made to make this work. E.g. none of the codegen tests run when std debug asserts are enabled.
saethlin commentedon Feb 12, 2024
@the8472 These checks specifically do not work that way. Please see my PR that implemented the system they use: #120594
the8472 commentedon Feb 12, 2024
Hrrm, that seems problematic because it renders the usual advice to do that kind of splitting ineffective.
saethlin commentedon Feb 12, 2024
No. My wish/goal is that if a user of the standard library calls any function in a manner which is locally invalid (i.e. the kinds of things ubsan finds, not asan/msan/tsan), that erroneous call is detected at runtime, by the default
cargo run/cargo test. I mentioned using an intrinsic so that we can have a single check, and a helpful error message fromget_uncheckedinstead of catching the problem in a callee, which does prevent the UB but has a generic and less-helpful message.Since you were getting vectorization before, are you compiling with
-Copt-level=1in your debug builds? If you are, I think your use case is better satisfied by one of these items in the issue I linked:Or we could also toggle some of the checks off at
-Copt-level=1.ojeda commentedon Feb 12, 2024
I was not talking about avoiding the check completely. What I meant is that, from the quick test I did (in the CE link) with
get+ intrinsics, at least one check is not getting removed, which is what I guessed broke vectorization. I was hoping it was related to the duplicated checks somehow, and therefore that by removing them (i.e. simplifying) the compiler would be able to vectorize again. But from what are saying, the checks are done in a way that they are not currently inlinable, which sounds like a more likely reason. :)Please see the CE link I mentioned in OP -- it uses
-O. It was just an example of projects with "fast debug" builds (or "release with asserts"), which may be compiled with-Copt-level=2or even-Copt-level=3, and where a decrease of performance may make the debug build unusable for certain use cases because there may be some real-time constraints. In those projects, you can typically keep the vast majority of checks enabled, but in the hot parts of the code you may need to avoid a few of them.That is why keeping the ability to selectively remove the checks in certain cases for
-Cdebug-assertions=ybuilds is important (i.e. you don't want to remove all the checks either), and thus thecore::hint::unreachable_unchecked_even_in_debug-like ideas.ojeda commentedon Feb 12, 2024
From what I can tell, you would still do the checks even with those items done, right? i.e. you are just removing the duplicated checks and making them inlinable etc., but the check would still need to be emitted in cases it cannot be proven to not happen. So since the checks would still be there in those cases, it is likely not enough, unless combined with a
core::hint::unreachable_unchecked_even_in_debugor similar.Ideally, the users would pick what they need to disable, ideally per-call (i.e. not even per check, but per instance of the check, e.g. I may want 99% of the
get_uncheckedcalls to be checked, but not particular ones in the hot parts). That is why I mentioned thecore::hint::unreachable_unchecked_even_in_debug(which would be like the intrinsic) or similar.saethlin commentedon Feb 12, 2024
Currently, all of the checks regardless of complexity are hidden behind a
#[inline(never)]function call. I'm going to move the slice bounds check condition out of the function very soon because my benchmarking indicates that this doesn't have a detrimental effect on compile time.Of course that doesn't get vectorization in your example code, but it should help on larger programs.
I don't currently know how to implement this. I am very wary of increasing the library maintenance burden or adding language features in the form of in-source annotations to support this. Maybe someone else who knows the library or compiler better sees how this could be done.
the8472 commentedon Feb 12, 2024
I think in-source annotations would be of limited use since those checks can be buried behind several layers of abstraction in a library. Iterators are an obvious example. Unless they applied to the call-tree, but we don't have many of those.
17 remaining items
RalfJung commentedon Feb 17, 2024
I've sometimes wanted a way to enable/disable arithmetic overflow checks in a particular region of the code. This issue (the original description) sounds similar. But such a mechanism seems pretty tricky to design so I don't think we are anywhere close to having something like that.
asquared31415 commentedon Feb 17, 2024
data point: C# has
checkedanduncheckedblocks (that look syntactically like Rust'sunsafeblocks): https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/checked-and-uncheckedThese control the behavior of integer overflow at a block-scoped level. I don't think that such a design is particularly useful for configuring extra UB checks, but it may be something to think about.
saethlin commentedon Feb 18, 2024
The obstacle here is not coming up with speculative designs for such a thing, but implementing any of them. I'm sure there will be an incredible amount of bikeshedding, but maybe we can at least postone that.
The nearest design to this that I can think of is
#[rustc_inherit_overflow_checks], but crucially that is handled in MIR building, where "which function did this Rvalue come from" is obvious. ButRvalue::DebugAssertionsis lowered after MIR inlining and other such optimizations have happened.rust: upgrade to Rust 1.78.0
rust: upgrade to Rust 1.78.0
rust: upgrade to Rust 1.78.0
rust: upgrade to Rust 1.78.0
saethlin commentedon Aug 28, 2024
In #129498 I'm experimenting with ways to get the precondition checks for
ptr::{read,write}enabled. One of the things I'm trying is inventing an attribute currently called#[rustc_no_ubchecks]which makes the MIR optimization pipeline delete checks that got inlined into the body of the indicated function. I don't think this idea is particularly good or possible to stabilize, but it is easy to implement. If it lands, it would be a thing for RFL to play around with, just in case you happen to find a use for it.This could probably be easily extended to (again) a crude general mechanism by putting the attribute on a closure to disable checks in a few lines of code instead of "a whole function". If the inliner cooperates.
wolfired commentedon Jan 15, 2025
bjorn3 commentedon Jan 15, 2025
wolfired commentedon Jan 16, 2025
saethlin commentedon Jan 16, 2025
It's great that you're enjoying Rust and RISC-V, but what you're trying to do with
--emit=objis not supported, and this is the wrong place to ask for it to be supported. File a new issue or go to a community discussion forum: https://www.rust-lang.org/communityrust: upgrade to Rust 1.78.0
rust: upgrade to Rust 1.78.0