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

How do aliasing model protectors interact with tail calls? #523

Open
RalfJung opened this issue Aug 6, 2024 · 5 comments
Open

How do aliasing model protectors interact with tail calls? #523

RalfJung opened this issue Aug 6, 2024 · 5 comments

Comments

@RalfJung
Copy link
Member

RalfJung commented Aug 6, 2024

In code like this

fn test1(x: &mut i32) {
  become test2(x as *mut i32);
}

fn test2(x: *mut i32) { ... }

x is protected inside test1. Is it also still protected while test2 runs?

If we want become to behave mostly like return, the answer should be "yes". However, test1's stack frame is guaranteed to not exist any more when test2 runs, so maybe it would make sense to say that the protectors associated with that stack frame have also expired?

The answer may be decided for us by LLVM, depending on what they say about the scope of noalias and dereferenceable in a function with a tail call: is the tail call inside or outside the scope of noalias and dereferenceable of a function argument?

@comex
Copy link

comex commented Aug 8, 2024

Assuming that become will turn into LLVM musttail, my intuition is that it wouldn't have much effect on optimizations, so the noalias would still be in scope for the tail-callee.

...Well, grepping the LLVM source for musttail, there are more references than I thought, mostly in order to avoid breaking the musttail invariants in various passes…

But it doesn't seem to affect aliasing.

Here is an example program demonstrating the need for protectors. Since rustc doesn’t seem to be able to generate LLVM IR for become, here is the same program transliterated to C, using [[clang::musttail]] in place of become, and we can see that LLVM does perform the optimization that’s enabled by noalias.

@RalfJung
Copy link
Member Author

RalfJung commented Aug 10, 2024

@comex so in that example, test2 gets inlined and then the noalias on test1 kicks in and moves the load down across the store?

Something is odd about this though, the implicit reborrow of x when it gets passed to test1 should constitute a write that invalidates y. Unfortunately I can't figure out how to even generate MIR for this code, it always ICEs in codegen which is odd since --emit=mir shouldn't codegen anything, I thought? @WaffleLapkin how do I dump the MIR when become is involved...? EDIT: --emit=metadata -Zdump-mir seems to work.

@RalfJung
Copy link
Member Author

Okay the other UB doesn't happen because of a surprising two-phase borrows interaction.

So getting back to @comex's example: test2::x is considered derived from y here, so using first x and then y is fine, but swapping them is not. This indeed needs protectors. Nice example, thanks!

@comex
Copy link

comex commented Aug 11, 2024

Heh. I had no idea that two-phase borrows were involved; I was just fiddling with the code until the version with become commented out produced the right Miri error.

@RalfJung
Copy link
Member Author

FWIW here is a variant that does not rely on two-phase borrows.

#![allow(incomplete_features)]
#![feature(explicit_tail_calls)]
use std::cell::UnsafeCell;

fn test2(x: *mut i32, y: *mut i32, loop_count: i32) -> i32 {
    unsafe {
        let mut val = 0;
        for _ in 0..=loop_count {
            // This load gets moved down:
            val = *x;
            // Conflicting store:
            *y = 200;
        }
        val
    }
}

#[inline(never)]
#[no_mangle]
fn test1(x: &mut i32, y: *mut i32, loop_count: i32) -> i32 {
  // Optionally comment out `become` here.
  become test2(x as *mut i32, y, loop_count)
}


fn main() {
    let x = &mut 100;
    let y = x as *mut _;
    let ret = test1(unsafe { &mut *y }, y, 0);
    assert_eq!(ret, 100);
}

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