-
Notifications
You must be signed in to change notification settings - Fork 59
SB: Allowing function argument references to dangle under some circumstances #252
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
Comments
The key problem as I understand it is that SB is strictly operational. Using the previous example, fn foo(x: &mut i32) {
*x = 13;
bar(); // some function we know nothing about, except that it is nounwind
*x = 27;
} We cannot be guaranteed that This is a secondhand remembered interpretation, don't give it more weight than that. |
Here's a variation on the example in 100% safe code: fn bar(y: &mut i32) -> ! {
print!("{}", y);
std::process::exit(0)
}
fn foo(y: &mut i32) {
let x = &mut *y;
*x = 13; // no way we can move this down
bar(y);
*x = 27;
}
fn main() { foo(&mut 0); } Here |
@digama0 I don't really understand how that relates to this issue? @RalfJung was giving an example where, with the right SB rules, it would be possible to move the assignment below the function call. The fact that this optimisation would be enabled was an argument for disallowing dangling references that were passed into the function. |
@Diggsey (in the other thread)
I don't understand the question here. For local variables we simply cannot do this as we have no guarantee for how long they live. Basically what protectors do is they operationally codify the rule "references passed to a function outlive that function", a rule that is also present in the Rust's borrow checker (where lifetime parameters always outlive the function). There is no pendant of this for local variables, so we cannot do anything similar there.
We don't know when a reference is "last used", so I don't understand this proposal. Also there are no references on the call stack. I am confused.^^ But it seems you moved on to another proposal, one that I actually understand:
This is an interesting proposal. It removes an asymmetry between deallocation and writes that I added mostly based on a gut feeling that deallocation is "more destructive". I don't think this extra assumption played any role in our optimization proofs... for the protector-based proofs, we need to prevent the callee from writing (and sometimes even reading) the relevant location(s), and even with your new proposal "calle cannot write (without causing UB)" still implies "callee cannot deallocate (without causing UB)", so this should all still work. That would mean the optimizations we lose are only ones we have not considered yet. (To be fair, we have only considered the most obvious optimizations that first came to our mind.) In fact, this is the key new property that unsafe code would get with your proposal: if you are allowed to use a pointer for writes to some memory, you are also allowed to deallocate that memory. This would, however, still check protectors of other pointers that are invalidated by this write. We will have to carefully go over each of the relevant cases where protectors/ One thing that this definitely means is that we can no longer emit LLVM's On the other hand, this could mean that we can set protectors for @digama0 the original example relied on not passing |
Here's an optimization that this proposal loses: fn foo(x: &mut i32) -> i32 {
let n = bar(&mut *x);
let mut sum = 0;
for i in 0..n { sum += *x; }
sum
} With your proposal we can no longer hoist the I am not saying that this is a particularly interesting optimization or that it would apply particularly often -- but this is what it means to have to use |
I probably didn't explain very well: originally translated into SB, my suggestion would have been to retain the protectors but convert all the references in the stack to "Disabled". Initially I thought that removing the protectors might somehow make some of the formal proofs more difficult as it would violate the rule that protectors exist until the stack frame is removed. However, I quickly realised that the behaviour would be identical to just clearing the stack anyway, so that's what I proposed here.
I don't think that's true, as dereferenceable in LLVM was relaxed to only validate the reference on entry to the function. Wouldn't it just mean we couldn't emit "nofree" or "global_dereferenceable"?
Makes sense! |
Oh, did that change already happen? I thought this was all still being discussed. So to be clear, with Stacked Borrows unchanged we can emit |
why don't we introduce an |
@RalfJung couldn't that function be optimized to fn foo(x: &mut i32) -> i32 {
let n = bar(&mut *x);
let mut sum = 0;
if n != 0 {
let x = *x;
for i in 0..n { sum += x; }
}
sum
} Thus lifting the dereference out of the loop, but only performing the dereference if the loop would have been run? A similar optimization could always work, fn foo(x: &mut i32) -> i32 {
before(x);
while some_condition() { body(*x) }
after()
} Could "optimize" to fn foo(x: &mut i32) -> i32 {
before(x);
if some_condition() {
let x = *x;
loop {
body(x);
if some_condition() { break }
}
}
after()
} |
@RustyYato sure, we can always do loop hoisting if we are willing to split out the first iteration of the loop. |
While I like this model, it would mean degrading not only the optimizations we can do on So my current thinking is we should instead make SharedReadWrite items not protected. This means even fewer optimizations on |
Continuing discussion from rust-lang/rust#55005 (comment)
Currently references in function arguments are special in that they must remain valid even after their last use. @RalfJung gave an example of a powerful optimization that relies on this behaviour.
Quoting the SB page:
What would be the impact of changing this to:
It seems like the specific optimisation @RalfJung mentioned should still be valid, but perhaps there are other optimizations that would be prevented?
The text was updated successfully, but these errors were encountered: