warning for UB ! exprs is too light

[Gankra]

So this code produces a warning about the &*input expression being unreachable:

enum Void { };

fn process(input: *const Void) {
   let _input = &*input;
}

warning: unreachable expression

I can imagine this needs to "only" be a warning so someone can write some really janky macro code that very carefully ensures the expression isn't ever evaluated. Even if that's the case, I would appreciate that this get its own error-code, or just stronger language to indicate to the user that they are doing something Very Bad. In particular I believe evaluating this expr is Undefined Behaviour.

[varkor]

I think what we want to be doing is distinguishing between unreachable code due to (control-flow) divergence and due to undefined behaviour in the diagnostics, whereas currently they're treated as the same thing.

[hanna-kruppe]

&*input should never actually dereference anything or create a Void value, it just converts the raw pointer to a reference. Is the warning based on the assumption that &! is (validity-)uninhabited? AFAIK that is not decided yet.

[Gankra]

Ralf seemed to think this was materially different from transmuting to a reference, but I don't know why.

[RalfJung]

Hm, good question actually. I've been thinking that we should assert validity of a value on a dereference (i.e., when evaluating a *, whether or not memory actually gets accessed). I thought that's what is happening here.

But actually, miri only asserts validity when a value is copied at a certain type. That does not happen here. And your example code (after fixing syntax errors) actually has no "unreachable":

    bb0: {                              
        StorageLive(_2);                 // bb0[0]: scope 0 at src/lib.rs:4:8: 4:14
        _2 = &(*_1);                     // bb0[1]: scope 3 at src/lib.rs:4:26: 4:33
        StorageDead(_2);                 // bb0[2]: scope 0 at src/lib.rs:5:1: 5:2
        return;                          // bb0[3]: scope 0 at src/lib.rs:5:2: 5:2
    }

So there is no UB here, that miri sees anyway.

Things are very different if you use ! instead:

    bb0: {                              
        StorageLive(_2);                 // bb0[0]: scope 0 at src/lib.rs:3:8: 3:14
        _2 = &(*_1);                     // bb0[1]: scope 3 at src/lib.rs:3:26: 3:33
        StorageDead(_2);                 // bb0[2]: scope 0 at src/lib.rs:4:1: 4:2
        unreachable;                     // bb0[3]: scope 0 at src/lib.rs:2:29: 4:2
    }

[RalfJung]

I don't see a general principle here, but someone decided that a mere deref-without-actual-access on ! is insta-UB.

I think we should probably discuss this as part of rust-lang/unsafe-code-guidelines#8 in the UCG discussions. IMO, if we decide that references do not have to point to initialized data, then &* should not be UB. But if we decide that references have to point to initialized data, there is UB.

[estebank]

The current output for

enum Void { }

fn process(input: *const Void) {
   let _input = unsafe { &*input };
}

fn main() {}

is

warning: enum is never used: `Void`
 --> file2.rs:1:1
  |
1 | enum Void { }
  | ^^^^^^^^^
  |
  = note: #[warn(dead_code)] on by default

warning: function is never used: `process`
 --> file2.rs:3:1
  |
3 | fn process(input: *const Void) {
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[RalfJung]

The thing is, if you replace *const by &, this is safe code, and there is no UB. The code is just unreachable. So, in principle, the same is true for the raw pointer version.
So it is not clear to me that we want a stronger warning?

[nico-abram]

The code is just unreachable. So, in principle, the same is true for the raw pointer version.

How? Making a raw pointer is safe: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=6beb0e3e63565a4034d51cc342476bee

enum Void { }

fn process(input: *const Void) {
   let _input = unsafe { &*input };
}

fn main() {
    process(std::mem::align_of::<Void>() as *const Void);
}

[RalfJung]

As I said above, it is unclear whether your example code has UB or not. If you run your code in Miri, you can see that it is currently not considered UB.

My current stanza on validity of references is that &! should indeed be uninhabited, which would make this code UB, and then it would be good to have a stronger warning. But I view this as a special case among references. See rust-lang/unsafe-code-guidelines#77 for the full discussion.

[nico-abram]

Oh, yes. I'm sorry, I wanted to say I don't think the pointer parameter case was unreachable, regardless of whether or not it is UB, unlike the reference case, which already requires an instance of a reference to an uninhabited type (So it can be considered unreachable if such a reference were uninhabited AFAIK), and not sure if I was misunderstanding something. Reading back I think by the same is true you might have meant "not UB" instead of "unreachable" so I did misunderstand something

[RalfJung]

TBH, I am not entirely sure any more what I meant more than a year ago. ;)

[TornaxO7]

Hello guys! May I ask what the status of this issue is? What exactly should you do, if you'd like to claim this issue?
@estebank since you gave this issue the E-easy and E-needs-test, may I ask you, what needs to be done for this issue?

[estebank]

@TornaxO7 I no longer recall what my rationale/understanding was back then, and rereading this thread I fear I might have misunderstood back then what the scope of the work is. A clarification of what the desired outcome would be useful regardless :)

[TornaxO7]

Hm... I think that I'm the wrong person to see how much work has to be done to fix this issue, since I'm lookin for a first-contribution xD

[estebank]

@TornaxO7 no worries, sorry for the categorization :)

[scottmcm]

Current status: this doesn't compile as-written on nightly, as the dereferencing is unsafe, even if you change it to let _, thanks to #102256: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=e4c847fa5994b69fd891d34035202051

error[E0133]: dereference of raw pointer is unsafe and requires unsafe function or block
 --> src/lib.rs:4:12
  |
4 |    let _ = &*input;
  |            ^^^^^^^ dereference of raw pointer
  |
  = note: raw pointers may be null, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior

Adding the unsafe block means that it gives no warnings right now: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=75584fc0594c9cbc268fce9b0a787d03.

So here's my guess at what the work here might be:

Add a new lint that mentions that you're doing something very sketchy when you dereference a raw pointer that's statically uninhabited from the place where you're doing the dereference