Rust: Polonius (but not ordinary NLL) accepts code which segfaults

[KiChjang]

Based on this tweet https://twitter.com/bhuztez/status/1026061344980828160.

Commit that worked around the segfault: https://github.com/bhuztez/porus/commit/d28ebdac301c0d81284bbfb93e0eb0c2d48ddb5b

@bhuztez 不知道你能不能看英语，能否提供有关这个bug的详细资料吗？

[KiChjang]

Removing the soundness label until further details are provided.

[upsuper]

I suggest we close this bug. The problem basically roots in this file: https://github.com/bhuztez/porus/blob/5a733ac564146f20bb1b2a65ff98d3ca9234a1ab/src/ptr.rs

The codebase exposes unsafe pointer operations as safe functions without any soundness check, and those "safe" operations are used to build some basic data structures, which are then used in the code in question. There is nothing Rust compiler can promise with that on soundness, because the code basically bypasses any soundness check.

[kennytm]

This may be a Polonius bug. Note that the project invokes rustc with -Z borrowck=mir -Z polonius.

Compiling these files directly on commit af6ecbc without NLL gives the following errors:

ALDS1_6_B.rs

error[E0597]: borrowed value does not live long enough
  --> ALDS1_6_B.rs:14:5
   |
14 | /     writelnf!(
15 | |         "{}[{:d}]{}",
16 | |         join(f!(""), list::iter(slice!(a, [, pivot])).map(|e| f!("{e:d} "))),
17 | |         a[pivot],
18 | |         join(f!(""), list::iter(slice!(a, [pivot+1, ])).map(|e| f!(" {e:d}"))));
   | |                                                                                ^
   | |                                                                                |
   | |________________________________________________________________________________temporary value dropped here while still borrowed
   |                                                                                  temporary value does not live long enough
   |
   = note: values in a scope are dropped in the opposite order they are created
   = note: consider using a `let` binding to increase its lifetime
   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

error[E0597]: borrowed value does not live long enough
  --> ALDS1_6_B.rs:14:5
   |
14 | /     writelnf!(
15 | |         "{}[{:d}]{}",
16 | |         join(f!(""), list::iter(slice!(a, [, pivot])).map(|e| f!("{e:d} "))),
17 | |         a[pivot],
18 | |         join(f!(""), list::iter(slice!(a, [pivot+1, ])).map(|e| f!(" {e:d}"))));
   | |                                                                                ^
   | |                                                                                |
   | |________________________________________________________________________________temporary value dropped here while still borrowed
   |                                                                                  temporary value does not live long enough
   |
   = note: values in a scope are dropped in the opposite order they are created
   = note: consider using a `let` binding to increase its lifetime
   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

error[E0499]: cannot borrow `*porus_sink` as mutable more than once at a time
  --> ALDS1_6_B.rs:14:5
   |
14 | /     writelnf!(
15 | |         "{}[{:d}]{}",
16 | |         join(f!(""), list::iter(slice!(a, [, pivot])).map(|e| f!("{e:d} "))),
17 | |         a[pivot],
18 | |         join(f!(""), list::iter(slice!(a, [pivot+1, ])).map(|e| f!(" {e:d}"))));
   | |                                                                                ^
   | |                                                                                |
   | |________________________________________________________________________________mutable borrow ends here
   |                                                                                  mutable borrow starts here in previous iteration of loop

error: aborting due to 3 previous errors

Some errors occurred: E0499, E0597.
For more information about an error, try `rustc --explain E0499`.

ITP1_6_A.rs

error[E0597]: borrowed value does not live long enough
  --> ITP1_6_A.rs:11:5
   |
11 |     writelnf!("{}", join(f!(" "), list::iter(slice!(a, [,,-1])).map(|e| f!("{e:d}"))));
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
   |     |                                                                                 |
   |     |                                                                                 temporary value dropped here while still borrowed
   |     temporary value does not live long enough
   |
   = note: values in a scope are dropped in the opposite order they are created
   = note: consider using a `let` binding to increase its lifetime
   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

(Note: I haven't checked the source in detail whether the segfault is caused by the invalid "safe" code in https://github.com/rust-lang/rust/issues/53142#issuecomment-410933624)

[KiChjang]

cc @rust-lang/wg-compiler-nll

[nikomatsakis]

It sounds like it would be helpful if someone (@bhuztez, perhaps?) could try to "narrow this down" to an isolated example. I'm going to mark it as NLL-deferred for now, since Polonius is not high priority just now.

[matthewjasper]

-Z polonius _never_ reports conflict errors.

[bhuztez]

compile the following code with rustc -Z borrowck=mir -Z polonius -C opt-level=2 and run. It does not print 300

struct X<'a, T: 'a> {
    a: &'a T,
    b: isize,
    c: isize,
}

struct Y<'a, T: 'a> {
    a: &'a T,
    b: usize,
}

fn new_y<'a, T: 'a>(x: &'a T) -> Y<'a, T> {
    Y {
        a: x,
        b: 100,
    }
}

fn main() {
    let a = &200isize;
    let y = new_y(&X{a: a, b: 300, c:400});
    let f = || { println!("{}", y.a.b); };
    (|| { f() })();
}

[nikomatsakis]

I think that — quite likely — the optimization work has broken polonius mode. We might consider removing it for now.

[nikomatsakis]

I know some of the changes I am doing are likely to break it. =)

[bjorn3]

Is this still an issue?

[Mark-Simulacrum]

I'm going to go ahead and close this as it seems likely that polonius has since changed and in any case it doesn't seem helpful to keep a bug open about it :)

If anything it could be moved to the polonius repo :)