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

Strip noalias from pointer derived from mutable ref #327

Closed
NicholasGorski opened this issue Apr 12, 2022 · 9 comments
Closed

Strip noalias from pointer derived from mutable ref #327

NicholasGorski opened this issue Apr 12, 2022 · 9 comments

Comments

@NicholasGorski
Copy link

This example is a stripped down version of a scenario I hit in a larger program. The behavior of this example changes depending on if a certain function is inlined or not:

#[derive(Debug, Default)]
struct Foo {
    data: i32,
}

#[inline(always)] // Prints nothing, `foo.data` is cached.
//#[inline(never)] // Prints "hooray", `foo.data` is fetched.
// Miri always prints "hooray" and believes this to be well-defined.
fn update(foo: *mut Foo, common: *mut Foo) {
    let data = {
        let foo = unsafe { &mut *foo };

        foo.data += 1;
        foo.data
    };
    {
        let common = unsafe { &mut *common };

        common.data += data;
    }
    {
        let foo = unsafe { &mut *foo };

        if foo.data > 2 {
            println!("hooray");
        }
    }
}

#[inline(never)]
fn update_ref(foo: &mut Foo, common: *mut Foo) {
    // The pointer to `foo` "erroneously" has noalias. On one hand
    // this is obviously correct per the mutable ref, but on the other
    // the pointer args for `update` are not noalias.
    update(foo as *mut _, common)
}

fn main() {
    let mut f1 = Foo::default();
    let mut f2 = Foo::default();
    let common = std::ptr::addr_of_mut!(f2);
    
    update_ref(&mut f1, common);
    update_ref(&mut f2, common);
}

Playground.

If the program invokes UB then it makes sense — but I think the intention in Stacked Borrows is that this is well-defined. Is that the case? I did see #86, but I'm not sure this program is quite in the same direction.

To workaround this I tried various ways of stripping provenance from the foo pointer, but ultimately I just had to pointer-ify most of the surrounding code until I hit an unaliased pointer base case. (Not exactly the prettiest, but fair enough!)


For what it's worth, my actual initial function was more like:

fn update(foo: &mut Foo, common: &mut *mut Foo) {
    // ...
}

And I had to pointer-ify the arguments to avoid an obvious double &mut scenario.

More generally, it would be nice to be able to temporarily relinquish a mutable reference in exchange for an aliasable pointer; when the scope ends the mutable reference is reestablished with a noalias status from the aliasable pointer:

fn update(foo: &mut Foo, common: &mut *mut Foo) {
    foo.do_stuff();

    {
        // Shadows foo; foo is now an aliasable *mut Foo.
        std::ptr::alias!(foo);

        let common = unsafe { &mut **common };
        common.do_cool_stuff();

        if /* .. */ { *common = foo; }
    }  // Compiler magic Drop; outer foo is &mut Foo again via inner &mut *foo.

    // Once again we are definitely noalias.
    foo.do_more_stuff();
}

If the topmost snippet is UB, then something like this could provide a nice escape hatch to demote a Unique to a SharedReadWrite (IIUC). But, I am unsure if this is even the appropriate direction to go for resolving such aliasing cases. Thanks!

@RalfJung
Copy link
Member

Miri always prints "hooray" and believes this to be well-defined.

Have you tried Miri with -Zmiri-tag-raw-pointers? Miri by default is more permissive than we might want.

@NicholasGorski
Copy link
Author

Ah, that makes more sense then, thanks! I'll remember to add these additional flags by default.

In my real code, this ended up being a pretty significant change to rule out references throughout the entire main loop, even though only exactly one spot could possibly alias. (The alternative was a self-check, but this tanked the performance.) I am still curious about introducing ergonomics to address the asymmetry in uniqueness (given a pointer you can become Unique, but given Unique you can never become a pointer) but I will read the latest updates to the aliasing model first.

@RalfJung
Copy link
Member

Having a "noalias" promise in signatures like fn update_ref(foo: &mut Foo, common: *mut Foo) is pretty much the most basic thing a mutable reference is intended to do, so I don't see any way that we could allow this code and still get any analysis/optimization benefits out of mutable references -- sorry.

I am still curious about introducing ergonomics to address the asymmetry in uniqueness (given a pointer you can become Unique, but given Unique you can never become a pointer) but I will read the latest updates to the aliasing model first.

I don't quite know what you mean here. Given &mut T you can use as *mut T to become a pointer, but that is clearly not what you mean.

There has been some talk about having a way to "opt-out" of uniqueness of mutable references in a vein similar to how UnsafeCell opts-out of shared references being immutable. That would also help self-referential generators and other, similar data structures. Cc #148

@NicholasGorski
Copy link
Author

I definitely do not mean to suggest noalias be wholesale stripped from mut references, sorry for the confusion!

I think am suggesting more along the lines of #148, especially something like UnsafeAlias. (Though I was thinking syntactical and not a wrapper.) In my head it feels feasible to allow the programmer to decide to require noalias (i.e., &mut) from the caller, but internally limit the scope of noalias. One could imagine that this:

fn test(p1: &mut u32, p2: *mut u32) {
    // ..
}

is translated (for exposition) under the hood into:

fn test(p1_ptr: *mut u32, p2: *mut u32) {
    let p1 = unsafe { &mut *p1_ptr };  // Attach noalias to p1.
    // ..
}

And the body of the function would be none the wiser. If such a translation took place, then the following could be expressed:

fn test(p1: &mut u32, p2: *mut u32) {
    *p1 = 0;
    {
        std::ptr::alias!(p1);
        let r = unsafe { &mut *p2};
        *r = 1;
    }
    println!("{}", *p1);
}

And be translated under the hood into:

fn test(p1_ptr: *mut u32, p2: *mut u32) {
    let p1 = unsafe { &mut *p1 };  // Attach noalias to p1.

    *p1 = 0;
    {
        // Due to std::ptr::alias!(p1) -> p1 is moved. E.g.:
        std::convert::identity(p1);

        let r = unsafe { &mut *p2};
        *r = 1;
    }
    // Scope of std::ptr::alias! is gone, re-establish noalias.
    let p1 = unsafe { &mut *p1_ptr};

    println!("{}", *p1);
}

I am temporarily handing off my unique access to allow some other pointer to have it for a region. This translation is fake so in actuality this may just be an entirely wrong model to flesh out.

(Of course, this final translation is viable as a real hand-written work around, but it's really easy to get wrong if someone accidentally inserts a &mut somewhere in the mut-free quarantine zone.)

@RalfJung
Copy link
Member

The precedent with UnsafeCell is to use type wrappers, not annotations. But yeah, there are many different ways to express this.

@NicholasGorski
Copy link
Author

Thanks for all the info! Closing in favor of #148 re: UnsafeAlias.

@RalfJung
Copy link
Member

Sure. :)
This made me wonder whether an attribute macro could "transform" a function signature to use different types and thus remove the aliasing assumptions... 🤔

@NicholasGorski
Copy link
Author

Heh. :) That would be mighty convenient. Do you mean the function still accepts &mut but actually behaves with &mut UnsafeAlias?

@RalfJung
Copy link
Member

Maybe. I don't actually have anything very concrete here.

But making sense of operations like std::ptr::alias is rather tricky, so this could be easier.^^ (Assuming we have UnsafeAlias, which we probably want either way.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants