diff --git a/const.md b/const.md index bd24e4d..2b9133b 100644 --- a/const.md +++ b/const.md @@ -12,6 +12,8 @@ To make this work, we need to ensure [const safety](const_safety.md). Based on this requirement, we allow other constants and [promoteds](promotion.md) to read from constants. This is why the value of a `const` is subject to validity checks. +Constants can also be [used in patterns](patterns.md), which brings its own soundness concerns. + ## References One issue is constants of reference type: diff --git a/patterns.md b/patterns.md new file mode 100644 index 0000000..fa1b344 --- /dev/null +++ b/patterns.md @@ -0,0 +1,66 @@ +# Constants in patterns + +Constants that pass the [structural equality check][struct-eq] can be used in patterns: +```rust +const FOO: i32 = 13; + +fn is_foo(x: i32) -> bool { + match x { + FOO => true, + _ => false, + } +} +``` +However, that check has some loopholes, so e.g. `&T` can be used in a pattern no matter the `T`. + +Any reference type const-pattern is compiled by [calling `PartialEq::eq`][compile-partial-eq] to compare the subject of the `match` with the constant. Const-patterns with other types (enum, struct, tuple, array) are treated as if the constant was inlined as a pattern (and the usual `match` tree is constructed). +[RFC 1445](https://github.com/rust-lang/rfcs/blob/master/text/1445-restrict-constants-in-patterns.md) lays the groundwork for changing this in the future to always using `PartialEq::eq`, but no decision either way has been made yet. + +[struct-eq]: https://github.com/rust-lang/rust/blob/2c28244cf0fc9868f55070e55b8f332d196eaf3f/src/librustc_mir_build/hair/pattern/const_to_pat.rs#L121 +[compile-partial-eq]: https://github.com/rust-lang/rust/blob/2c28244cf0fc9868f55070e55b8f332d196eaf3f/src/librustc_mir_build/build/matches/test.rs#L355 + +## Soundness concerns + +Most of the time there are no extra soundness concerns due to const-patterns; except for surprising behavior nothing can go wrong when the structural equality check gives a wrong answer. +However, there is one exception: some constants participate in exhaustiveness checking. +If a pattern is incorrectly considered exhaustive, that leads to a critical soundness bug. + +Exhaustiveness checking is done for all constants that do not have reference type, as well as `&[u8]` and `&str`. +This means we can write: +```rust +#![feature(exclusive_range_pattern)] +#![feature(half_open_range_patterns)] +const PAT: &[u8] = &[0]; + +pub fn test(x: &[u8]) -> bool { + match x { + PAT => true, + &[] => false, + &[1..] => false, + &[_, _, ..] => false + } +} +``` + +To ensure soundness of exhaustiveness checking, it is crucial that all data considered this check is fully immutable. +In particular, for constants of reference type, it is important that they only point to immutable data. +For this reason, the static const checks reject references to `static` items. +This is a new soundness concern that otherwise does not come up during CTFE. +Note that this is orthogonal to the concern of [*reading from* a mutable `static`](const.md#reading-statics) during const initialization, which is a problem for all `const` (even when they are not used in patterns) because it breaks the "inlining" property. +A more precise check could be possible, but is non-trivial: even an immutable `static` could point to a mutable `static`; that would have to be excluded. +(We could, in principle, also rely on it being UB to have a shared reference to mutable memory, but for now we prefer not to rely on the aliasing model like that---aliasing of global pointers is a tricky subject.) + +*Dynamic check.* +To dynamically ensure constants only point to read-only data, we maintain a global invariant: +pointers to a `static` (including memory that was created as part of interning a `static`) may never "leak" to a non-`static`. +All memory outside of `static`s is marked as immutable. +As a consequence, non-`static`s recursively only point to immutable memory. + +This check is uncomfortably close to the static checks, and fragile due to its reliance on a global invariant. +Longer-term, it would be better to move to a more local check, by making validation check if consts are recursively read-only. +This check (unlike our current validation) would have to descend through pointers to `static`. + +An alternative could be to wait for the [value tree proposal](https://github.com/rust-lang/compiler-team/issues/323) to be implemented. +Then we could check during value tree construction that all memory is read-only (if we use the CTFE machine in `const`-mode, that will happen implicitly), and thus we know for sure we can rely on the value tree to be sound to use for exhaustiveness checking. +How exactly this looks like depends on how we resolve [pattern matching allowing many things that do not fit a value tree](https://github.com/rust-lang/rust/issues/74446#issuecomment-663439899). +However, if only the well-behaved part of the value tree is used for exhaustiveness checking, handling these extra patterns should not interact closely with the soundness concerns discussed here.