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

Promotion rules #2

Closed
oli-obk opened this issue Jul 26, 2018 · 13 comments
Closed

Promotion rules #2

oli-obk opened this issue Jul 26, 2018 · 13 comments

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Jul 26, 2018

Promotion is the process of allowing let x: &'static u32 = &1; to be valid in Rust. Technically the &1 produces a temporary local variable and references to that don't have the 'static lifetime. But there is a check which figures out expressions that are constant and then another pass inserts an unnameable static for the value and refers to that static instead.

This was introduced in rust-lang/rfcs#1414 and I'm doing a write up for it in https://github.com/rust-rfcs/const-eval/blob/master/promotion.md

Now there are some headaches with promotion. E.g.

let x: &'static usize = &(0 - 1);

which, while being const evaluable, would cause a const eval error due to overflow.
This is fine in the above case, since the user obviously requested promotion by setting the 'static lifetime, but

let x: &usize = &(0 - 1);

did not and would still get promoted and then error. We work around this by leaving in the debug assertions that the compiler has on integer arithmetic, which mean even though we promoted the computation to a static, all the checks are still done at runtime (unless optimized out due to guaranteed uselessness).

So everything is fine in these simple cases, it gets problematic when there's code that can't be evaluated at compile-time but which would also not panic at runtime. E.g.

union Foo { x: &'static i32, y: usize }
let x: &bool = unsafe { Foo { x: &1 }.y == Foo { x: &2 }.y };

This errors during const eval because const eval does not compare pointers. We could do pointer comparisons, and in this case it would even be fine because &1 and &2 can never be the same address, but

union Foo { x: &'static i32, y: usize }
let x: &bool = unsafe { Foo { x: &1 }.y == Foo { x: &1 }.y };

is not so clear-cut. LLVM might decide to put both &1 and the other &1 into the same static. Then the addresses would be the same. Or it would not do that, then the addresses are different. When we move to other operators, the result isn't even decideable by LLVM anymore (only the linker knows this):

union Foo { x: &'static i32, y: usize }
let x: &bool = unsafe { Foo { x: &1 }.y < Foo { x: &2 }.y };

So, the solution was to forbid promoting unions (since pointer to usize casts are already not promotable). But this just shifts the problem

union Foo { x: &'static i32, y: usize }
const A: usize = unsafe { Foo { x: &1 }.y };
const B: usize = unsafe { Foo { x: &2 }.y };
let x: &bool = &(A < B);

which accidentally works on stable 1.27 and does undefined behaviour. It is "fixed" on beta (and the fix will be in stable 1.28), but the fix is only to abort instead of UB.

We now need to figure out the concrete rules around this so we know when and where to error out, and when to not promote.

One such solution is unconst (similar to unsafe, but at compile-time). This means that if you do unconst things wrongly, your compiler might produce an error during monomorphization or codegen or whenever it feels like.

Some rules for unconst:

  • any use of unsafe is also unconst
  • raw pointer -> usize casts via as are unconst

What does it mean when I use unconst?

  1. anything unconst will not get promoted.
  2. unconst is not propagated past the same boundaries that unsafe isn't. So a const fn, const or static that internally uses unconst things does not show up as unconst, because the final value might be perfectly fine, even if the intermediate computation did dangerous things
  3. Using unconst to produce a const or static means you need to make sure the value is actually a value of that type. So if you have a usize, that value needs to be an integer, not an address. Similar how a &u32 needs to not be 0 or generally pointing anywhere but at a u32.

We already have this concept when you compute array lengths or enum variant discriminants. So there's definite precedent for this.

@RalfJung
Copy link
Member

RalfJung commented Aug 29, 2018

Also see #6

Basically, my answer to

union Foo { x: &'static i32, y: usize }
const A: usize = unsafe { Foo { x: &1 }.y };
const B: usize = unsafe { Foo { x: &2 }.y };
let x: &bool = &(A < B);

is that the sanity check should reject A and B for not being const-safe usize.

@RalfJung
Copy link
Member

Things that are still entirely missing from the document:

@eddyb would be great if you could mention other things that restrict promotion. Ideally, the document in this repo should eventually serve as a full spec for that analysis.

@eddyb
Copy link
Member

eddyb commented Sep 21, 2018

"references to other statics" is misleading. References are fine to promote, the hard problem is promoting reads from dereferences (*r), because r could point to anything, and knowing what a value points to is not an analysis we're willing to be doing right now (although we could start to do it in the near future, based on more rigorous dataflow, instead of the mess of flags we have today).

If r points to a global allocation with interior mutability, or an integer pointer, or any other such thing, it's possible that either miri would fail to evaluate or the runtime value could be different.

Note that the limitation around *r in promotion was added before the more recent wave of limitations, so it might not be needed - but I prefer to be conservative here.

@RalfJung
Copy link
Member

RalfJung commented Oct 7, 2018

If r points to a global allocation with interior mutability, or an integer pointer, or any other such thing, it's possible that either miri would fail to evaluate or the runtime value could be different.

How could the runtime value be different? CTFE does not permit mutating statics, even if they contain interior mutability. So I think that would manifest as just another way of miri failing. Is that what you mean?

Also, if we enforce const safety in promoteds, we actually know that all refs are valid. We don't promote if there was a raw-ptr-deref. So I don't think miri could actually fail with a dangling pointer or out-of-bounds read. And since globals cannot be mutated, reading from them is actually guaranteed to give the same result as at run-time, the thing that would make miri fail is an attempt to write to a global. So as long as we only promote reading deref, but not assignment to a pointer, I think we should be fine.

That said, I'm all in for promoting as little as possible until we have figured out that entire story a bit better.

@oli-obk
Copy link
Contributor Author

oli-obk commented Oct 8, 2018

How could the runtime value be different? CTFE does not permit mutating statics, even if they contain interior mutability. So I think that would manifest as just another way of miri failing. Is that what you mean?

You can modify the static at runtime. So any CTFE value would be "out of date".

@RalfJung
Copy link
Member

RalfJung commented Oct 8, 2018

oh I see... because for static and const it is quite clear that these are execute "before main", but for promoteds they should behave "as if" executed later. Thanks.

@RalfJung
Copy link
Member

RalfJung commented Oct 8, 2018

So, we could actually allow reading statics whose backing memory is marked immutable? Might not be worth the risk, though.

@oli-obk
Copy link
Contributor Author

oli-obk commented Oct 8, 2018

statics can contain the address of other statics through which you can read (un-)changed statics. Even if the type does not reveal this, it feels like it would require a lot of mental overhead in all future stabilizations of const features to make sure that we don't also brick this further special case

@RalfJung
Copy link
Member

RalfJung commented Oct 8, 2018

Yeah it would be very restricted and probably easy to screw up. Too easy.

But then, given that statics cannot even be mentioned in a promoted, it does not seem clear to me that *r is still dangerous. We know it cannot point to a static.

@oli-obk
Copy link
Contributor Author

oli-obk commented Oct 8, 2018

The only thing I can think about is that on embedded we could have a &'static Volatile<u32> that is in fact a hardware address.

@eddyb
Copy link
Member

eddyb commented Nov 4, 2018

@RalfJung IIRC it's because const could potentially point to a static but maybe it can't, hmm.
I think this was before we decided that const fns should never mention (unfrozen) statics.

Also, if we enforce const safety in promoteds

Yeah I think that's a much stronger guarantee than I was working with.
So we might be able to reallow *x now. But I'd rather wait for old borrowck to die.

@RalfJung
Copy link
Member

RalfJung commented Nov 5, 2018

Indeed let us not change the promotion rules while old borrowck exists.

And even then we should be careful. I am more trying to understand what we could do than suggesting we actually do it.

@RalfJung
Copy link
Member

RalfJung commented May 9, 2021

We now have pretty good docs of the current promotion rules; is there anything left for this issue?

@oli-obk oli-obk closed this as completed May 9, 2021
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

3 participants