Stream: t-compiler/const-eval

Topic: unsoundness in constant mutable borrows


Christian Poveda (Dec 16 2019 at 21:51, on Zulip):

I'm opening this to discuss the existing unsoundness bugs for the const_mut_refs feature

Christian Poveda (Dec 16 2019 at 21:51, on Zulip):

@ecstatic-morse has pointed out this one: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=4371a0cf7245a7d235cb2b8809b0bb10

Christian Poveda (Dec 16 2019 at 21:52, on Zulip):

I'm working under the assumption that we don't want to expose mutable references as return values for any constant. So that one must definitively be fixed

Christian Poveda (Dec 16 2019 at 21:53, on Zulip):

However, I was thinking about a case like this:

const fn foo(&mut i32) ->  &mut i32;
const fn bar(&mut i32) -> i32;
const fn baz(x: &mut i32 ) -> i32 {
    bar(foo(x))
}

Is foo disallowed even if it does not used anywhere outside a constant?

ecstatic-morse (Dec 16 2019 at 21:54, on Zulip):

There's also this one which needs to be fixed: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=eb1b6c9bb69dfd8ff18d3c2f50fd66cd

ecstatic-morse (Dec 16 2019 at 21:56, on Zulip):

First, I want to ask if it's feasible to forbid creating new mutable references in a const-context for now. Instead only allowing them to be passed in as parameters

ecstatic-morse (Dec 16 2019 at 21:58, on Zulip):

That would mean that e.g.

const fn demux(a: &mut i32, b: &mut i32, sel: bool) -> &mut i32 {
    if sel { a } else { b }
}

is allowed as well as the case you mentioned above, but that it can't really be used in a const context. So maybe this is a no-go?

ecstatic-morse (Dec 16 2019 at 22:00, on Zulip):

(I say it can't be used since there would be no way to create a and b)

ecstatic-morse (Dec 16 2019 at 22:04, on Zulip):

My thought is that we want to do a local analysis that prevents a new &mut from "escaping" a function, so you could do

const fn foo(a: &mut i32) -> &mut i32 {
    a
}
ecstatic-morse (Dec 16 2019 at 22:05, on Zulip):

but not

const fn foo(a: &mut i32) -> &mut i32 {
    if *a == 0 { a } else { &mut 4 }
}
ecstatic-morse (Dec 16 2019 at 22:06, on Zulip):

Oh well I guess &mut 4 won't get promoted, so this is impossible

ecstatic-morse (Dec 16 2019 at 22:06, on Zulip):

In that case, the problem is only with const and static initializers, right?

ecstatic-morse (Dec 16 2019 at 22:13, on Zulip):

Shifting gears, we probably want to allow code like:

const X: Option<&mut i32> = None;
ecstatic-morse (Dec 16 2019 at 22:15, on Zulip):

This suggests that we want an HasMutPtr qualification (that is related to but distinct from the existing HasInteriorMut qualif).

ecstatic-morse (Dec 16 2019 at 22:17, on Zulip):

We would not need to check for this in the return value of const fns, since the borrow checker already prevents us from returning a reference to local memory.

Christian Poveda (Dec 16 2019 at 22:49, on Zulip):

My thought is that we want to do a local analysis that prevents a new &mut from "escaping" a function

shouldn't we want to prevent &muts to just escape constant contexts?

ecstatic-morse (Dec 16 2019 at 22:50, on Zulip):

A const fn is a const context. The issue will only be with "top-level" const contexts (const and static).

ecstatic-morse (Dec 16 2019 at 22:55, on Zulip):

Since &muts created in a top-level context are 'static and can escape outside of the block.

Christian Poveda (Dec 16 2019 at 23:05, on Zulip):

so basically we allow returning mutable references unless we are in a const or static.

ecstatic-morse (Dec 16 2019 at 23:16, on Zulip):

Yes. There should be no issues with const fns. Remember that structs containing mutable references also must be forbidden.

Christian Poveda (Dec 16 2019 at 23:17, on Zulip):

Yes. There should be no issues with const fns. Remember that structs containing mutable references also must be forbidden.

just to clarify, any type containing a mutable reference must not be exposed via const nor static

ecstatic-morse (Dec 16 2019 at 23:17, on Zulip):

Yes.

Christian Poveda (Dec 16 2019 at 23:17, on Zulip):

Shifting gears, we probably want to allow code like:

const X: Option<&mut i32> = None;

This remembers me the problem with making Vec::new() const safe.

ecstatic-morse (Dec 16 2019 at 23:20, on Zulip):

The problem there was that values of non-Copy types can already appear at the top-level of a const, and they will be bitwise copied anyway with no safe code.

ecstatic-morse (Dec 16 2019 at 23:22, on Zulip):

&mut T is also a non-Copy type, but we would have to forbid it anyways, even if we did not copy the final value of a const wherever it gets used.

ecstatic-morse (Dec 16 2019 at 23:22, on Zulip):

This is (I think) what @oli was referring to when they mentioned that we might need ConstSafe for &mut, not just const heap allocations.

ecstatic-morse (Dec 16 2019 at 23:23, on Zulip):

However, I think as soon as a &mut T appears in the final value of a const, the battle is already lost.

ecstatic-morse (Dec 16 2019 at 23:26, on Zulip):

So, I think a good start would be to implement a TypeVisitor that can find types with a &mut, and use it to forbid certain types from being the final value of a constant.

ecstatic-morse (Dec 16 2019 at 23:26, on Zulip):

(if one does not already exist)

ecstatic-morse (Dec 16 2019 at 23:26, on Zulip):

And separately to fix whatever code allows you to do &mut Cell::new(4).

ecstatic-morse (Dec 16 2019 at 23:27, on Zulip):

And we can worry about the Qualif part of this later

Christian Poveda (Dec 16 2019 at 23:33, on Zulip):

Ok I'll start working on this tomorrow :)

ecstatic-morse (Dec 16 2019 at 23:34, on Zulip):

:heart: Thank you! Might wanna check in with oli if they're available in case I missed something.

Christian Poveda (Dec 16 2019 at 23:34, on Zulip):

This is (I think) what oli was referring to when they mentioned that we might need ConstSafe for &mut, not just const heap allocations.

No, what I mean is that we want to allow some constant values of a type while disallowing other

Christian Poveda (Dec 16 2019 at 23:34, on Zulip):

:heart: Thank you! Might wanna check in with oli if they're available in case I missed something.

Heh yeah that's my main reason for waiting :P

ecstatic-morse (Dec 16 2019 at 23:34, on Zulip):

Yeah, you'll need to implement Qualif to get those semantics.

ecstatic-morse (Dec 16 2019 at 23:35, on Zulip):

Which is not too hard, but also not necessary to fix the soundness issue.

Christian Poveda (Dec 16 2019 at 23:35, on Zulip):

Which is not too hard, but also not necessary to fix the soundness issue.

Well even if it's not neccessary I'd like to come back to it later

oli (Dec 17 2019 at 07:48, on Zulip):

static FOO: &mut u32 = &mut 9; is actually fine

oli (Dec 17 2019 at 07:49, on Zulip):

It's only constants that are problematic, because they are copied implicitly

oli (Dec 17 2019 at 07:51, on Zulip):

The reason I mentioned ConstSafe is that &&mut u32 is fine, but (&str, &mut u32) is not, which is the same as if it were Box

oli (Dec 17 2019 at 07:53, on Zulip):

static FOO: Box<u32> is fine, too

oli (Dec 17 2019 at 07:56, on Zulip):

You can't deallocate or mutate a static

oli (Dec 17 2019 at 07:57, on Zulip):

static FOO: AtomicBox<u32> is not fine though

oli (Dec 17 2019 at 07:58, on Zulip):

Even though static FOO: AtomicMutRef<u32> could be fine

oli (Dec 17 2019 at 07:58, on Zulip):

Ugh

oli (Dec 17 2019 at 07:58, on Zulip):

So let's start with everything is not fine and grow the set slowly

oli (Dec 17 2019 at 08:00, on Zulip):

I think a marker type will be simpler than a type visitor?

oli (Dec 17 2019 at 08:03, on Zulip):

We should also forbid raw pointers via that marker type, so noone goes around doing const FOO: *mut u32 = &mut 7;

oli (Dec 17 2019 at 08:05, on Zulip):

Which... would break Vec::new(), so we need the "no actual pointers" cop out where the marker trait is not checked

oli (Dec 17 2019 at 08:05, on Zulip):

Oh or we figure out a validation based system

oli (Dec 17 2019 at 08:06, on Zulip):

Ha we already have that type visitor @ecstatic-morse

oli (Dec 17 2019 at 08:06, on Zulip):

But it also walks the value at the same time

oli (Dec 17 2019 at 08:07, on Zulip):

Hmm.. we should ask @RalfJ if that is an abuse of validation and whether this deserves its own system

ecstatic-morse (Dec 17 2019 at 21:24, on Zulip):

Ah, we forbid *STATIC = x already.

ecstatic-morse (Dec 17 2019 at 21:26, on Zulip):

Yeah, a marker trait would be better, I wasn't sure if we're still adding new OIBITs

ecstatic-morse (Dec 17 2019 at 21:30, on Zulip):

I think as soon as you allow a mutable reference to be taken during const-eval, you need to forbid ptr-to-int conversions forever. Otherwise there's no way to stop a user from "laundering" the pointer out of const-eval through an integer type. This may already be the plan.

ecstatic-morse (Dec 17 2019 at 21:30, on Zulip):

That's if you want to try to forbid even *mut in the final value, which may not be feasible as you've explained.

Christian Poveda (Dec 17 2019 at 22:59, on Zulip):

static FOO: AtomicBox<u32> is not fine though

I think we are already catching interior mutability so that shouldn't be a problem

Christian Poveda (Dec 17 2019 at 23:02, on Zulip):

I think as soon as you allow a mutable reference to be taken during const-eval, you need to forbid ptr-to-int conversions forever. Otherwise there's no way to stop a user from "laundering" the pointer out of const-eval through an integer type. This may already be the plan.

But even then, you can leak a immutable pointer out of const-eval, right?

ecstatic-morse (Dec 17 2019 at 23:03, on Zulip):

@Christian Poveda yes but it's UB to write to them unless you got them from a &mut

ecstatic-morse (Dec 17 2019 at 23:04, on Zulip):

So we don't have this problem today

Christian Poveda (Dec 17 2019 at 23:04, on Zulip):

Ohh so the problem here is UB not unsafety

Christian Poveda (Dec 17 2019 at 23:08, on Zulip):

and it is not UB to produce a mutable reference from an integer

ecstatic-morse (Dec 17 2019 at 23:09, on Zulip):

It depends on the provenance of the value in the integer.

Christian Poveda (Dec 17 2019 at 23:10, on Zulip):

oh so if it's an integer from an actual reference it is not UB even though it is wrong here

ecstatic-morse (Dec 17 2019 at 23:11, on Zulip):

These rules haven't been formalized yet, but that's the idea yes.

Christian Poveda (Dec 17 2019 at 23:11, on Zulip):

hmmm interesting

oli (Dec 18 2019 at 08:49, on Zulip):

static FOO: AtomicBox<u32> is not fine though

I think we are already catching interior mutability so that shouldn't be a problem

statics may have interior mutability. so static FOO: AtomicUsize works on stable

RalfJ (Dec 22 2019 at 15:42, on Zulip):

ouch, this looks nasty

RalfJ (Dec 22 2019 at 15:44, on Zulip):

It depends on the provenance of the value in the integer.

integers don't actually have provenance though, not if we fully support int-ptr-casts.
I guess we could make them have provenance for CTFE (that's what currently unleash-miri basically does) but I am not sure of that is a good idea.

RalfJ (Dec 22 2019 at 15:45, on Zulip):

I agree this feels very similar to Box/heap-alloc in const -- we can allow anything to happen during the computation, we just have to be careful in what we allow in the final return value.

Christian Poveda (Dec 22 2019 at 22:53, on Zulip):

So after hearing from @RalfJ and @oli I'll start working on this :P

Christian Poveda (Dec 22 2019 at 22:55, on Zulip):

@RalfJ there is no way to expose the information from int-to-ptr casts to the compiler so we can check if an integer comes from a pointer?

oli (Dec 22 2019 at 22:55, on Zulip):

@Christian Poveda an integer cannot come from a pointer in const eval

Christian Poveda (Dec 22 2019 at 22:56, on Zulip):

oh that's great D:

Christian Poveda (Dec 22 2019 at 22:56, on Zulip):

:D

oli (Dec 22 2019 at 22:56, on Zulip):

Though you can have integers as addresses of ZSTs

Christian Poveda (Dec 22 2019 at 22:56, on Zulip):

wait, why?

oli (Dec 22 2019 at 22:57, on Zulip):

&*(1 as *const ()) is sound

oli (Dec 22 2019 at 22:57, on Zulip):

it's a valid address for a ()

oli (Dec 22 2019 at 22:58, on Zulip):

you can get this on stable by using a union to transmute 1usize to &'static ()

Christian Poveda (Dec 22 2019 at 23:00, on Zulip):

I think I'm not seeing the whole picture

Christian Poveda (Dec 22 2019 at 23:02, on Zulip):

I mean, if any integer can be a valid address for a ZST, how does that affect exposing pointers as integers?

oli (Dec 22 2019 at 23:02, on Zulip):

it doesn't. I don't know why we are talking about exposing pointers as integers.

oli (Dec 22 2019 at 23:03, on Zulip):

Side note: I don't know what provenance means

oli (Dec 22 2019 at 23:04, on Zulip):

Ok, wikipedia cleared this up

Christian Poveda (Dec 22 2019 at 23:04, on Zulip):

Side note: I don't know what provenance means

Oxford: ‚Äčthe place that something originally came from

Christian Poveda (Dec 22 2019 at 23:04, on Zulip):

:P

Christian Poveda (Dec 22 2019 at 23:05, on Zulip):

and it is not UB to produce a mutable reference from an integer

I thought we were discussing how to avoid leaking mutable references as integers

oli (Dec 22 2019 at 23:05, on Zulip):

aaah

Christian Poveda (Dec 22 2019 at 23:05, on Zulip):

or am I getting ahead of it?

oli (Dec 22 2019 at 23:06, on Zulip):

so... this feels unrelated, because we have the same "problem" for immutable references

oli (Dec 22 2019 at 23:06, on Zulip):

sec, grabbing the issue id

oli (Dec 22 2019 at 23:06, on Zulip):

https://github.com/rust-lang/rust/issues/63197

oli (Dec 22 2019 at 23:14, on Zulip):

yea, so this is entirely orthogonal to the problem at hand :slight_smile:

oli (Dec 22 2019 at 23:17, on Zulip):

I think as soon as you allow a mutable reference to be taken during const-eval, you need to forbid ptr-to-int conversions forever. Otherwise there's no way to stop a user from "laundering" the pointer out of const-eval through an integer type. This may already be the plan.

we need to forbid this forever anyway, because a) you'd be able to create a pseudorandom number generator that may change its value arbitrarily between compiler runs and b) you'd get different addresses at compile-time than at runtime for the same thing

oli (Dec 22 2019 at 23:17, on Zulip):

I don't see any way that would make it possible for us to have ptr-to-int conversions in const eval without causing a whole lot of (to the best of my knowledge unfixable) problems

Christian Poveda (Dec 23 2019 at 00:19, on Zulip):

damn... so the plan is just forbid mutable references (or types containing mutable references) in const or statics

oli (Dec 23 2019 at 00:22, on Zulip):

just in consts I think

RalfJ (Dec 23 2019 at 09:03, on Zulip):

Christian Poveda an integer cannot come from a pointer in const eval

Hu? We have unions in const-eval...

RalfJ (Dec 23 2019 at 09:03, on Zulip):

or do you mean an integer value cannot come from a ptr? True, but that is a difference in semantics between const and real Rust. I'd rather avoid having to spec two languages.

RalfJ (Dec 23 2019 at 09:15, on Zulip):

FWIW, if we properly forced constants to be Sync, that would already exclude raw pointers and we'd have much fewer problems I think...

RalfJ (Dec 23 2019 at 09:20, on Zulip):

if we need some new type-based scheme for consts anyway, maybe that is also a good opportunity to fix the Sync thing

oli (Dec 23 2019 at 09:48, on Zulip):

I don't see how we can ever make an int-to-ptr conversion (force_bits) legal in const eval without bricking everything. So we have static A: i32 = 42; and static B: usize = (&A as *const i32 as usize) / 2 * 2;. If we compute this integer address at const eval time, it is essentially guaranteed, that llvm will produce a different value. I don't see any way to get this to work. So if you then do *(B as *const i32) at runtime, you'll get a segfault or at least a different value than what is stored in A

oli (Dec 23 2019 at 09:50, on Zulip):

Also it's not much of a semantics difference. Compile time evaluation will always abort $somewhere where runtime can go on just fine

oli (Dec 23 2019 at 09:50, on Zulip):

it's not like behaviour changes between runtime and compile time

oli (Dec 23 2019 at 09:50, on Zulip):

it's just that some things abort compilation at compile time

oli (Dec 23 2019 at 09:51, on Zulip):

Just like you'll never be able to do *(1234 as *const i32) at compile-time, even if that address is totally sane at runtime on your target

oli (Dec 23 2019 at 09:52, on Zulip):

or how you'll always be able to take an address of an extern static but never deref that pointer because it points nowhere at compile time

RalfJ (Dec 23 2019 at 11:31, on Zulip):

I don't see how we can ever make an int-to-ptr conversion (force_bits) legal in const eval without bricking everything.

ah no, that's not at all what I meant

RalfJ (Dec 23 2019 at 11:31, on Zulip):

but we also shouldnt rely on being able to trace the provenance of an integer IMO

RalfJ (Dec 23 2019 at 11:32, on Zulip):

like, if we ever decide that transmuting ptrs to ints is UB, we should do this consistently between runtime and CTFE

RalfJ (Dec 23 2019 at 11:32, on Zulip):

a CTFE that goes on in "best effort" with pointer values at integer type might well be incompatible with the runtime model we end up with

Last update: Apr 05 2020 at 01:00UTC