Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mutable Pointer Aliasing Rules are Unclear for Unsafe Code #19733

Closed
Gankra opened this issue Dec 11, 2014 · 15 comments · Fixed by #23813
Closed

Mutable Pointer Aliasing Rules are Unclear for Unsafe Code #19733

Gankra opened this issue Dec 11, 2014 · 15 comments · Fixed by #23813
Labels
I-needs-decision Issue: In need of a decision. P-medium Medium priority

Comments

@Gankra
Copy link
Contributor

Gankra commented Dec 11, 2014

As I rub against the boundaries of unsafe and undefined behaviour more and more It's becoming less "obvious" to me what is or isn't allowed. To demonstrate this I've whipped up a few examples of safe and unsafe code that does basically the same thing. Some of them are clearly defined or clearly undefined in my mental model, but I really have no idea at this point.

a is clearly defined, as it uses no unsafe code. We re-loan a mutable reference to a different variable temporarily. At any given point there's clear ownership of the value.

fn a() -> u32 {
    // totally safe mutable aliasing
    let a = &mut 1u32;
    {
        let b = &mut *a;
        *b += 1;
    }
    *a += 1;
    *a
}

b does the exact same thing, but through a *mut instead of an &mut. Ownership is still "clear" to the compiler, but we mutate through the *mut, and then later through the &mut. It wouldn't be unreasonable to consider this undefined behaviour. We mutated something "owned" by an &mut through something other than that &mut, and then worked with the value through the &mut.

fn b() -> u32 {
    // same mutable aliasing, but with a raw ptr
    let a = &mut 1u32;
    unsafe {
        let b = a as *mut _;
        *b += 1;
    }
    *a += 1;
    *a
}

c does the exact same things as b, but explicitly constructs an &'static mut to mutate through unsafely. Here we have created two &mut's to the same value, which in my mental mode is clearly invoking undefined behaviour as I understand it. You cannot have two &mut's to the same value.

fn c() -> u32 {
    // same unsafe aliasing, but by explicitly making an &mut from a *mut
    let a = &mut 1u32;
    unsafe {
        let b = &mut *((&mut *a) as *mut _);
        *b += 1;
    }
    *a += 1;
    *a
}

d is basically the same as a, but we've added a box in the way. This adds a rawptr between the &mut and the actual data. Semantically the data is still "owned" by the &mut, but is the *mut in-between important or just an implementation detail? Regardless this is all safe, so this must be defined.

fn d() -> u32 {
    // totally safe mutable aliasing, but through a box 
    // (and therefore a raw ptr)
    let mut a = box 1u32;
    {
        let b = &mut *a;
        *b += 1;
    }
    *a += 1;
    *a
}

e is the same as d, but we've added a *mut again. This time we mutate the data inside the box while the box is owned. However the box is a *mut, so really we've just mutated data behind a rawptr with another rawptr. Now it really matters if the box's representation is defined or not! Critically I believe that the defined-ness of this case effects whether DList is sound or not. It mixes boxes, *muts, and &muts pretty freely. What is or isn't allowed is important.

fn e() -> u32 {
    // same mutable aliasing, but with another raw ptr
    let mut a = box 1u32;
    unsafe {
        let b = (&mut *a) as *mut _;
        *b += 1;
    }
    *a += 1;
    *a
}

Finally f is a special case of unsafe mutable aliasing. Here we construct a *mut to a subfield of a composite structure. Then we capture the whole structure with an &mut. We mutate the subfield while the whole struct is owned, but then only use the ownership to mutate a different subfield. At no point do we read the "unsafely" mutated field. Then we relinquish ownership to the "parent" owner which, presumably, must assume that all fields may have been mutated since it loaned the structure out. Is this defined behaviour? I honestly have no clue.

fn f() -> u32 {
    // mutable aliasing with a raw ptr, but on an unaccessed field
    // until ownership is "returned" to the parent
    let mut x = (1u32, 10u32);
    let b = (&mut x.1) as *mut _;
    {
        let a = &mut x;
        unsafe {
            *b += 1;
        }
        a.0 += 1;
    }
    x.0 + x.1
}
@bstrie
Copy link
Contributor

bstrie commented Dec 11, 2014

Nominating, because we need to either specify this for 1.0 or we need to take the Go route and say that all usage of unsafe carries no guarantee of BC whatsoever.

@thestinger
Copy link
Contributor

The aliasing model is essentially the same as restrict in the C11 memory model, because Rust references the LLVM documentation. If you have an understanding of aliasing, the rules are clear. It is defined based on memory dependencies, not address equality.

@thestinger
Copy link
Contributor

I realize that pointer aliasing is not easy to grasp and that the documentation doesn't attempt to teach this, but that doesn't make it a backwards compatibility issue. The unsafe documentation just needs to clarify that the scoped noalias model (same as restrict) applies to &mut and &. I think it's a bit much to expect us to document the meaning of aliasing without referencing LLVM at this point, because it's hard to explain it to someone who doesn't grasp it already.

@brson brson added A-docs P-medium Medium priority and removed I-nominated labels Dec 11, 2014
@brson
Copy link
Contributor

brson commented Dec 11, 2014

P-high. Would be nice.

@arielb1
Copy link
Contributor

arielb1 commented Dec 11, 2014

@thestinger

Mostly that solves it, but there are still problems with "implicit accesses" (e.g. (f), which may be a problem if for some reason the compiler loads x as 2 words, modifies the lower one, then stores both).

@Gankra
Copy link
Contributor Author

Gankra commented Jan 1, 2015

CC @cmr @zwarich

@aturon
Copy link
Member

aturon commented Jan 1, 2015

CC @huonw @nikomatsakis

@Gankra
Copy link
Contributor Author

Gankra commented Mar 31, 2015

@steveklabnik Sorry but that really doesn't resolve the issue at hand. In particular it's an issue about how * and & are allowed to interact.

@Gankra Gankra reopened this Mar 31, 2015
@steveklabnik
Copy link
Member

Can you give me the specific, exact details that you want, then?

@Gankra
Copy link
Contributor Author

Gankra commented Mar 31, 2015

@steveklabnik This isn't a documentation problem per-se. It's more of a well-defined/concensus problem. I'm fairly certain there is no concensus on exactly what the rules are for what you're allowed to do with a * to values that are held by an &(mut). @zwarich was working on some detailed discussion but accidentally deleted it on their last day at Mozilla (and now can't work on it because of their contract at Apple). :(

Although perhaps you've managed to obtain some level of concensus?

@steveklabnik
Copy link
Member

This was previously described as a doc bug. If it's not, then it's not a big deal to me, at the moment.

@steveklabnik steveklabnik added the I-needs-decision Issue: In need of a decision. label Mar 31, 2015
@steveklabnik
Copy link
Member

@gankro is this still unclear today? it's been a while

@Gankra
Copy link
Contributor Author

Gankra commented Dec 19, 2015

Abso-lutely. Nothing has been done in this space, and there's nothing on the horizon that will change this fact.

@pnkfelix
Copy link
Member

cc rust-lang/rfcs#1447

@brson
Copy link
Contributor

brson commented Aug 25, 2016

Closing in favor of rust-lang/rfcs#1447

@brson brson closed this as completed Aug 25, 2016
dlrobertson pushed a commit to dlrobertson/rust that referenced this issue Nov 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-needs-decision Issue: In need of a decision. P-medium Medium priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants