about summary refs log tree commit diff
path: root/src/tools
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2024-10-06 11:06:57 +0200
committerGitHub <noreply@github.com>2024-10-06 11:06:57 +0200
commit9aaebd481a63df4673691dfd35f9a7eda5c51724 (patch)
treedfaafcfcfc9b8e1f1bc95d48394bfca2cec64097 /src/tools
parent7d53688b25d52d822c7094ee80db42b2b2f2a8d3 (diff)
parente8d5eb2a2b6cdddd6725501e1b54a33c278d9697 (diff)
downloadrust-9aaebd481a63df4673691dfd35f9a7eda5c51724.tar.gz
rust-9aaebd481a63df4673691dfd35f9a7eda5c51724.zip
Rollup merge of #129392 - compiler-errors:raw-ref-op-doesnt-diverge-but-more, r=lcnr
Do not consider match/let/ref of place that evaluates to `!` to diverge, disallow coercions from them too

Fixes #117288.

This PR implements a heuristic which disables two things that are currently being performed on the HIR when we have **expressions that involve place-like expressions that point to `!`**. Specifically, it will (in certain cases explained below):

### (1.) Disable the `NeverToAny` coercion we implicitly insert for `!`.

Which fixes this inadvertent, sneaky unsoundness:

```
unsafe {
    let x: *const ! = &0 as *const u8 as *const !;
    let _: () = *x;
}
```

which is UB because currently rust emits an *implicit* NeverToAny coercion even though we really shouldn't be, since there's no read of the value pointed by `x`.

### (2.) Disable the logic which considers expression which evaluate to `!` to diverge, which affects the type returned by the containing block.

Which fixes this unsoundness:

```
fn make_up_a_value<T>() -> T {
    unsafe {
        let x: *const ! = &0 as *const u8 as *const !;
        let _ = *x;
    }
}
```

We disable these two operations **if** the expression is a place-like expression (locals, statics, field projections, index operations, and deref operations), and if the parent expression is either:
(1.) the LHS of an assignment
(2.) AddrOf
(3.) A match or let **unless** all of the *patterns consitute a read*, which is explained below:

And finally, a pattern currently is considered to constitute a read **unless** it is a wildcard, or an OR pattern. An OR pattern is considered to constitute a read if all of its subpatterns constitute a read, to remain as conservative as possible in cases like `_ | subpat` or `subpat | _`.

All other patterns are considered currently to constitute a read. Specifically, because `NeverToAny` is a coercion performed on a *value* and not a *place*, `Struct { .. }` on a `!` type must be a coercion currently, and we currently rely on this behavior to allow us to perform coercions like `let _: i32 = x;` where `x: !`.

This is already considered UB by [miri](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=daf3a2246433fe43fdc07d1389c276c9), but also means it does not affect the preexisting UB in this case:

```
let Struct { .. } = *never_ptr;
```

Even though it's likely up for debate since we're not actually reading any data out of the struct, it almost certainly causes inference changes which I do *NOT* want to fix in this PR.
Diffstat (limited to 'src/tools')
-rw-r--r--src/tools/miri/tests/pass/underscore_pattern.rs51
1 files changed, 49 insertions, 2 deletions
diff --git a/src/tools/miri/tests/pass/underscore_pattern.rs b/src/tools/miri/tests/pass/underscore_pattern.rs
index f0afe558954..f59bb9f5c82 100644
--- a/src/tools/miri/tests/pass/underscore_pattern.rs
+++ b/src/tools/miri/tests/pass/underscore_pattern.rs
@@ -1,5 +1,7 @@
 // Various tests ensuring that underscore patterns really just construct the place, but don't check its contents.
 #![feature(strict_provenance)]
+#![feature(never_type)]
+
 use std::ptr;
 
 fn main() {
@@ -9,6 +11,7 @@ fn main() {
     invalid_let();
     dangling_let_type_annotation();
     invalid_let_type_annotation();
+    never();
 }
 
 fn dangling_match() {
@@ -34,6 +37,13 @@ fn invalid_match() {
             _ => {}
         }
     }
+
+    unsafe {
+        let x: Uninit<!> = Uninit { uninit: () };
+        match x.value {
+            _ => {}
+        }
+    }
 }
 
 fn dangling_let() {
@@ -41,6 +51,11 @@ fn dangling_let() {
         let ptr = ptr::without_provenance::<bool>(0x40);
         let _ = *ptr;
     }
+
+    unsafe {
+        let ptr = ptr::without_provenance::<!>(0x40);
+        let _ = *ptr;
+    }
 }
 
 fn invalid_let() {
@@ -49,6 +64,12 @@ fn invalid_let() {
         let ptr = ptr::addr_of!(val).cast::<bool>();
         let _ = *ptr;
     }
+
+    unsafe {
+        let val = 3u8;
+        let ptr = ptr::addr_of!(val).cast::<!>();
+        let _ = *ptr;
+    }
 }
 
 // Adding a type annotation used to change how MIR is generated, make sure we cover both cases.
@@ -57,6 +78,11 @@ fn dangling_let_type_annotation() {
         let ptr = ptr::without_provenance::<bool>(0x40);
         let _: bool = *ptr;
     }
+
+    unsafe {
+        let ptr = ptr::without_provenance::<!>(0x40);
+        let _: ! = *ptr;
+    }
 }
 
 fn invalid_let_type_annotation() {
@@ -65,7 +91,28 @@ fn invalid_let_type_annotation() {
         let ptr = ptr::addr_of!(val).cast::<bool>();
         let _: bool = *ptr;
     }
+
+    unsafe {
+        let val = 3u8;
+        let ptr = ptr::addr_of!(val).cast::<!>();
+        let _: ! = *ptr;
+    }
 }
 
-// FIXME: we should also test `!`, not just `bool` -- but that s currently buggy:
-// https://github.com/rust-lang/rust/issues/117288
+// Regression test from <https://github.com/rust-lang/rust/issues/117288>.
+fn never() {
+    unsafe {
+        let x = 3u8;
+        let x: *const ! = &x as *const u8 as *const _;
+        let _: ! = *x;
+    }
+
+    // Without a type annotation, make sure we don't implicitly coerce `!` to `()`
+    // when we do the noop `*x` (as that would require a `!` *value*, creating
+    // which is UB).
+    unsafe {
+        let x = 3u8;
+        let x: *const ! = &x as *const u8 as *const _;
+        let _ = *x;
+    }
+}