Rust: warning for UB `!` exprs is too light

Created on 24 Nov 2018  路  7Comments  路  Source: rust-lang/rust

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.

A-diagnostics A-lint A-typesystem C-enhancement E-easy E-needs-test T-compiler T-lang

Most helpful comment

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 https://github.com/rust-rfcs/unsafe-code-guidelines/issues/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.

All 7 comments

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.

&*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.

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

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
    }

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 https://github.com/rust-rfcs/unsafe-code-guidelines/issues/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.

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) {
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

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?

Was this page helpful?
0 / 5 - 0 ratings