Open RalfJung opened 5 years ago
Hmm, that's true... Are there cases where NLL or Polonius makes it impossible to define where in the code the lifetime ends (perhaps split over multiple branches)?
Also this would require putting type information into the Stacked Borrows stack items... I think we should only do this if we have solid evidence that it actually is useful (for optimizations).
In a world where you explicitly pop the Uniq
tag, that's not necessarily true. At the "end of lifetime" for a Uniq
, you know the expected validity for the value at the location, because you're in the process of killing the &mut T
.
I still don't particularly like that solution, but it would work without typing the borrow stack. I still prefer defining the borrow stack on uses rather than requiring a statically known end-of-lifetime in the model.
It's important to note that SB isn't necessarily trying to formalize what the borrow checker is doing. It's formalizing what the Rust virtual machine permits, of which the borrow checker restricts you to a statically enforceable subset.
Yes I was referring to the version that detects "end of lifetime" through the relevant borrow stack item being popped.
I am not sure if making "end of lifetime" explicit in the code is even feasible with NLL, let alone Polonius. I also think it would be bad to have an "end of lifetime" operation that is both inserted automatically via some extremely hard to predict rules, and has important side-effects. I deliberately avoided this in Stacked Borrows.
So, this seems like basically talking about if providence is limited to bit-level precision, or not. If I recall, at one point it was talked about how reading other fields from a struct through a pointer to an inner portion is undefined behavior, since otherwise structs being split apart in memory would be an illegal optimization. I would feel inclined to say that formally speaking, NonZeroU8 is not an 8 bit type. It is a 7.99435[...] bit type, which when represented on its own winds up with 0.00564[...] bits of padding. Wrapping something in an Option adds at most one bit to the value, and in this particular case winds up adding exactly the size of the padding bits. So, just like rearranging a struct to minimize padding, it is safe to rearrange this to reduce it from 8.00564[...] bits (or rounded up in practice) to a mere 8 bits. This model however says that writing as though the reference to a NonZeroU8 is a 8 bit type is as bad as writing as though it is a 16 bit type, which can obviously change other code and is, if I recall, undefined behavior. The optimization that assumes the discriminate can't suddenly change is then no different from an optimization that assumes that if you only give away a pointer to a field of a struct, the other fields of the struct won't suddenly change on you. Now, potentially one could say it isn't instantly undefined behavior to write a u8 into a NonZeroU8, for example is it legal to write something a bit too big for the pointer if it happens to be the case that the extra write applies only to padding? Similarly, since unless the u8 is zero, it only writes exactly what is already there to out of providence memory. However, this is made a bit tricky, since when we unwrap a NonZeroU8 as an implementation detail it simply copies out the bits. Including, awkwardly, 'padding' bits. This could be avoided, by once you read all zeros being able to infer the last one must be a one, but then it ceases to be a simple memcopy. So clearly NonZeroU8 winds up imposing a restriction on its padding. Though conveniently, one compatible with the option optimization. But here the padding has clearly been optimized out, and it is writing to something that is not part of the providence of the original pointer, unless it is declared that providence rounds according to some set of rules. The only reason it doesn't instantly break invariants is that what it writes also happens to prevent other ways of getting at the inner value. This would get far more complicated as well if we have multiple fields. For example, consider an enum that contains either a NonZeroU8 and a reference, or a NonZeroU8 and a usize. This can be done in the size of a u8 + a usize, storing the discriminant in the u8. It is currently in the usize mode. We take a mutable reference to both fields. Is writing a zero to the NonZeroU8 now defined if and only if the usize happens to be a valid reference?
I would say 'no'. Providence lets you set memory in certain ways. This may or may not divide smoothly into bits. It often does, but I do not think it is best to guarantee that providences are always integer counts of continuous bytes. Or even integer counts of continuous bits. Nor should they guarantee that you can set all bits independently.
Tracking this may get more expensive, though I think in practice it probably wouldn't have heavy overhead because the integer counts of continuous bytes case can be taken as the happy path, but widening providences is a heavy cost that hinders optimization and makes tracking what is allowed to tamper with what more confusing, and does not seem justified here.
For example, consider an enum that contains either a NonZeroU8 and a reference, or a NonZeroU8 and a usize. This can be done in the size of a u8 + a usize, storing the discriminant in the u8.
No it can't. If you store { Option<NonZeroU8>, union { &T, usize } }
, one of the variants doesn't get a NonZeroU8 (because it's zero to mark which variant it is).
A core rule of niche optimizations is that &T
must be able to write all bits of T
to any valid bit representation of T
, including padding. Niche optimizations do not get to function by mandating a value in the padding; rather they must mandate a bit value which is not bit valid for T
. (And byte padding may hold any value, 0..=255
or undef
/poison
.)
Copies don't not write to padding, they always write to padding. They just always write undef
/poison
to the padding.
Bit level validity is a completely disjoint concept to padding.
Oh, right. Yes, that particular case doesn't work. It does work if we change it to different pattern of niches. EvenU8
and OddU8
for example.
And while I agree there is an important sense in which it is completely disjoint, I think that is in part constructed by splitting up the space of concerns. And some optimizations, like niche optimization, are going to want to rearrange the handling of concerns.
And more generally, I think insisting that there are exactly two particular arranging of the concerns, one for 'regular' and one for 'niche optimization' is closing off things we don't want to close off.
Having explicit padding bits that have nothing to do with bit level validity is helpful for reasoning about things, that way of splitting up concerns is helpful, but I don't always want that for my code.
That we can implement turning a NonZeroU8
to a u8
by simply coping out 8 bits from a NonZeroU8
is a consequence of a property of the type that, when you look into how it works with niche optimization, involves blending the lines between padding, validity, and who owns what bits. There is another possible type which has a more expensive conversion to u8
, but doesn't require any checking when copying a u8
to it, by making it sometimes have one bit which acts rather like padding.
But as soon as we have lifetimes we have already given up the idea that things can be simply checked by their bits. What safe bit values for a reference are depends on where in the program you are.
I would also object to the statement that a &T
must be able to write all bits of T
to any valid bit representation of T
. If the compiler observes that only some fields are used, and the reference never escapes, and so on, it can rewrite it however it wants, including skipping ever having a 'proper' reference. If it observes that there are only two possible things a reference can point to at this time, and it is only used for writing the second field of a struct, then I think it is a legal optimization to change it to a bitflag that gets converted into a pointer directly to the second field. I think it would also be acceptable if the compiler managed to prove this simply by looking at the constraints on the lifetimes, such that even with unsafe code feeding things in, it managed to prove that the only way to have the unsafe code feed in legal values would be if it was one of those two. While this optimization may seem difficult for the compiler, similar optimizations by hand could ever come up, and those should also be legal.
If the compiler manages to observe that all but one of the bits of a NonZeroU8
are zero, it may safely not allocate any memory for it (assuming it isn't mutated) and simply hard code the remaining value.
All of these are based on the fact that bits are not independent portions of the full statespace.
And for not only the compiler, but my own unsafe code, I would prefer that providence not get 'rounded up' in this fashion.
Because what does providence being rounded up here buy us? Maybe in MIRI it not tracking providence this precisely buys us efficiency, but MIRI already simply loses track of providence from time to time with things like integer-pointer conversions for similar efficiency reasons. But in the spec itself? What does it buy us there?
'Rounding up' makes figuring out exactly what providence I'm leaking harder, especially because I'm pretty sure I can construct cases where the rounding amplifies the providence leaks arbitrarily, like with the combination NonZeroU8
and NonOneU8
. Even if the compiler won't do the niche optimization for a case like that on its own (or will it? I haven't tested), I could implement it myself using unions, and I'd like ideally MIRI to catch if some other bit of code tries to write something invalid there right away, rather than later blaming my (in my view perfectly innocent) code for later accessing dangling pointers and such.
I think it would also be acceptable if the compiler managed to prove this simply by looking at the constraints on the lifetimes, such that even with unsafe code feeding things in, it managed to prove that the only way to have the unsafe code feed in legal values would be if it was one of those two. While this optimization may seem difficult for the compiler, similar optimizations by hand could ever come up, and those should also be legal.
I am pretty sure this is not a legal optimization. Unsafe code is allowed to lie about lifetimes; often it is not possible to write correct lifetimes in unsafe code in the first place because the language isn't expressive enough. That's why stacked borrows relies on runtime memory checking for UB and does not validate lifetimes when entering a function, even though it has enough information to do so (at least if it were run on pre-lifetime-erasure MIR).
Oh, right. Yes, that particular case doesn't work. It does work if we change it to a
NonOneU8
for one of them though.
No it doesn't. Which variant is it if the byte is 2
? Both, it seems. The overlapping values must be 100% disjoint.
What would potentially work is NegativeI8
and NonNegativeI8
. Those are completely disjoint and could niche into a single byte.
If you want, switch the cutoff point[^1] so there isn't a clean "this bit indicates the variant." That's not the issue, the issue is that your niche doesn't allow differentiating all values of the two types. If it's niched into a single byte, there must be at most 2⁸ different representable states between all types and other information niched into the byte. You don't need a full free bit for the discriminant, but the values do have to be disjoint.
[^1]: it's not expressible with today's compiler, but you could even do EvenU8
with OddU8
, or U8Mod3Is0
and U8Mod3IsNot0
.
What safe bit values for a reference are depends on where in the program you are.
The validity invariant (a typed copy of this value is not instant UB) and the safety invariant (it is impossible to cause UB with safe code and this value) are two disjoint concerns. Validity is checked on typed copy by Miri; safety is an operational concern that has no impact on the Abstract Machine's operation save for if UB is actually encountered.
I would also object to the statement that a &T must be able to write all bits of T to any valid bit representation of T. If the compiler observes that only some fields are used, and the reference never escapes, and so on, it can rewrite it however it wants,
Then this is not talking about operational properties of the abstract machine. The abstract machine is concerned with operations you can do with a value. If the compiled machine code gets away with knowing you don't actually do some operations, it doesn't have to support them, but the abstract machine supports them, and thus if you actually do them, the compiled code must as well.
Niche optimizations are allowed to impact field layout (including niching discriminant fields into other inactive fields' memory space), but they can only impact layout, which is minimally guaranteed by repr(Rust)
. All other operational properties of the rest of the language (e.g. references) must stay valid.
MIRI already simply loses track of providence from time to time with things like integer-pointer conversions for similar efficiency reasons
You don't want to bring that up -- PNVI is coming, complicated, and under strict provenance, ptr2int isn't even allowed anymore. So, under strict provenance, Miri loses track of provenance because it's actually gone.
But I actually do agree with the underlying point, just not your supporting arguments: it's a cleaner model for the code author if writing to a field is not allowed to write to a logically disjoint field, even if said field overlaps it in memory thanks to niche optimizations.
The super annoying part is that what value you're allowed to write under such a model is a very nonlocal concern. Consider the following:
enum E {
A {
discriminant = 0u8,
payload: NonZeroU8,
}
B {
discriminant = 1u8,
payload: u8,
}
}
let mut value = E::B { payload: 1 };
let value_ptr: *mut E = &raw mut value;
let discriminant_ptr: *mut u8 = &raw mut (*value_ptr).discriminant;
let payload_ptr: *mut u8 = &raw mut (*value_ptr).payload;
// any byte write to payload is fine
*payload_ptr = 2;
// let's change which variant it is
*discriminant_ptr = 0;
// now I can't write 0 anymore
*payload_ptr = 0;
(If the explicit discriminant annoys you, pretend it's #[repr(C)]
so the enum layout is guaranteed. My point still applies.)
This example is technically different from the OP example, but it's a related question, and it nicely illustrates the issue with just "knowing" the real written to type to check its validity.
If you want even more fun, write to individual bytes of a larger type, where only certain byte combinations are invalid, but all individual bytes are valid with other configurations. ("Parity" mod three of the bytes is a fun one.)
Oh, that's a very interesting example. What happens if there is a mutable borrow out on the discriminant byte when you write to payload_ptr
? In that case it's not safe even for the model to sneak a peek at the discriminant to decide whether the write is valid since it might be accessed concurrently on another thread or something; this would induce a data race in the AM.
I think the current answer to this is that when you write to discriminant_ptr
and payload_ptr
the writes are both totally valid and independent and are just using the enum's memory as storage space for two u8
's; it is only if you read the enum later via a match
or dropping it or something that a read at type E
is made that reasserts validity of the bytes that have been placed there.
I think it would also be acceptable if the compiler managed to prove this simply by looking at the constraints on the lifetimes, such that even with unsafe code feeding things in, it managed to prove that the only way to have the unsafe code feed in legal values would be if it was one of those two. While this optimization may seem difficult for the compiler, similar optimizations by hand could ever come up, and those should also be legal.
I am pretty sure this is not a legal optimization. Unsafe code is allowed to lie about lifetimes; often it is not possible to write correct lifetimes in unsafe code in the first place because the language isn't expressive enough. That's why stacked borrows relies on runtime memory checking for UB and does not validate lifetimes when entering a function, even though it has enough information to do so (at least if it were run on pre-lifetime-erasure MIR).
I believe the following is safe and legal code:
pub struct Token<'a> {
marker: PhantomData<Cell<&'a ()>>,
}
pub fn fun_to_bool<F>(f: F) -> bool
where
for<'a, 'b> F: FnOnce(&'b Token<'a>, &'b Token<'a>) -> &'b Token<'a>,
{
let left: &mut Token<'static> = &mut Token {
marker: PhantomData,
};
let right: &mut Token<'static> = &mut Token {
marker: PhantomData,
};
let res: &Token<'static> = f(left, right);
if std::ptr::eq(res, left) {
return true;
}
if std::ptr::eq(res, right) {
return false;
}
unsafe { unreachable_unchecked() }
}
The lifetimes there imply that there are only two valid pointers to the tokens, at least as far as the called function is concerned.
No it doesn't. Which variant is it if the byte is
2
? Both, it seems. The overlapping values must be 100% disjoint.
Yes. I had hoped I had fixed it fast enough, but apparently not... I also similarly considered a PrimeU8
and NonPrimeU8
. I'm also unclear on if it is actually possible to implement this using the rustc_layout_scalar_valid_range_start
and friends, but...
MIRI already simply loses track of providence from time to time with things like integer-pointer conversions for similar efficiency reasons You don't want to bring that up -- PNVI is coming, complicated, and under strict provenance, ptr2int isn't even allowed anymore. So, under strict provenance, Miri loses track of provenance because it's actually gone.
I don't see how this is counter to my point, rather than supporting it. It is one thing for MIRI to lose track of providence due to efficiency concerns, that can be viewed as a bug or an indication that perhaps we want to change the spec to exclude some things. But that doesn't mean we want to have in the spec itself lots of rounding to broader provenances!
The validity invariant (a typed copy of this value is not instant UB) and the safety invariant (it is impossible to cause UB with safe code and this value) are two disjoint concerns. Validity is checked on typed copy by Miri; safety is an operational concern that has no impact on the Abstract Machine's operation save for if UB is actually encountered.
This is fair, though I think trying to arrange so that many invariants can be safely 'promoted' to validity invariants is a worthwhile goal. The primary reasons for having them as disjoint concerns is that some cannot be promoted due to them needing to be unobservably violated from time to time. If it doesn't need to be unobservably violated, then the difference between them is if I understand correctly purely making it so that there are optimizations we can write that the compiler is necessarily handicapped about—a pure downside.
We could make this a pure safety invariant, but why not let the compiler join in on the fun?
The super annoying part is that what value you're allowed to write under such a model is a very nonlocal concern. Consider the following:
Yes, that is what I mean by it being something that can be potentially escalated unlimitedly. I think the only way of making it not explode is to say that the write to discriminant_ptr
is the problem. That previously you had a OnlyOneU8
, and you wrote something illegal to it.
I believe the following is safe and legal code: [...]
Yes, I agree that is safe code. Users can write code in this style and prevent misuse and UB through the type system, and this can be used in safe functions. However the compiler is not permitted to optimize with this information; this is exactly what the difference between safety and validity invariants is about. The reason is because even though that function is safe, it is still legal to use it outside its safety contract with unsafe code and produce programs with defined behavior. For example this (playground) is defined behavior if you swap out unreachable_unchecked
for an observable effect:
fn main() {
fun_to_bool(|a, b| {
fun_to_bool(|_, _| unsafe { transmute(a) });
b
});
}
The compiler cannot replace the println!("wut")
with unreachable_unchecked()
because it would introduce UB in this code.
It is one thing for MIRI to lose track of providence due to efficiency concerns, that can be viewed as a bug or an indication that perhaps we want to change the spec to exclude some things. But that doesn't mean we want to have in the spec itself lots of rounding to broader provenances!
It is desirable to have a dynamic checker that can report all UB. This is something sorely missing from C and Rust is in a decent position to catch almost all UB dynamically with Miri. For this reason, we try to avoid having divergence between what the spec says is UB and what Miri can feasibly check to be UB - anything involving "angelic nondeterminism" is a hard sell for this reason. So if Miri has to lose provenance because it is too hard to track through arbitrary integer operations (not to mention the teachability implications of such a model), then we are better off making the AM also lose provenance at that point.
This is fair, though I think trying to arrange so that many invariants can be safely 'promoted' to validity invariants is a worthwhile goal. The primary reasons for having them as disjoint concerns is that some cannot be promoted due to them needing to be unobservably violated from time to time. If it doesn't need to be unobservably violated, then the difference between them is if I understand correctly purely making it so that there are optimizations we can write that the compiler is necessarily handicapped about—a pure downside.
We could make this a pure safety invariant, but why not let the compiler join in on the fun?
Within the constraints of UB needing to be deterministic / executable by a dynamic checker like Miri, more UB can be a good thing, but it is also a weapon that can be used to break users' code, especially the ones that like to write tricky hacks using unsafe code (usually C expats). So it's certainly not a pure downside, and I think we should probably err on the other side: unless there is a really compelling optimization reason to have a validity invariant we should try to keep things as loose as possible so that unsafe code performs predictably and is understandable by low level developers.
Yes, that is what I mean by it being something that can be potentially escalated unlimitedly. I think the only way of making it not explode is to say that the write to discriminant_ptr is the problem. That previously you had a OnlyOneU8, and you wrote something illegal to it.
This is typed memory, and Rust doesn't have it. discriminant_ptr
is just a pointer to a u8
, there is no memory of the enum that was once there. Besides, the point I made about holding a borrow on discriminant_ptr
applies just as well in the other direction: if you have a mutable borrow on payload_ptr
then it is impossible to tell if the write to discriminant_ptr
is valid or not.
I believe the following is safe and legal code: [...]
No, you can trigger UB fairly easily with that code: playground. But ignoring the implementation bugs, I agree that users can write code in this general style and prevent misuse and UB through the type system, and this can be used in safe functions. However the compiler is not permitted to optimize with this information; this is exactly what the difference between safety and validity invariants is about.
I meant using only the pub
portions, yes. I didn't write out the module wrapping.
It is one thing for MIRI to lose track of providence due to efficiency concerns, that can be viewed as a bug or an indication that perhaps we want to change the spec to exclude some things. But that doesn't mean we want to have in the spec itself lots of rounding to broader provenances!
It is desirable to have a dynamic checker that can report all UB. This is something sorely missing from C and Rust is in a decent position to catch almost all UB dynamically with Miri. For this reason, we try to avoid having divergence between what the spec says is UB and what Miri can feasibly check to be UB - anything involving "angelic nondeterminism" is a hard sell for this reason. So if Miri has to lose provenance because it is too hard to track through arbitrary integer operations (not to mention the teachability implications of such a model), then we are better off making the AM also lose provenance at that point.
Agreed, though I think it is also reasonable for some parts to be a bit slower and controlled by flags. Note that checking for writing an illegal NonZeroU8
is very checkable!
This is fair, though I think trying to arrange so that many invariants can be safely 'promoted' to validity invariants is a worthwhile goal. The primary reasons for having them as disjoint concerns is that some cannot be promoted due to them needing to be unobservably violated from time to time. If it doesn't need to be unobservably violated, then the difference between them is if I understand correctly purely making it so that there are optimizations we can write that the compiler is necessarily handicapped about—a pure downside. We could make this a pure safety invariant, but why not let the compiler join in on the fun?
Within the constraints of UB needing to be deterministic / executable by a dynamic checker like Miri, more UB can be a good thing, but it is also a weapon that can be used to break users' code, especially the ones that like to write tricky hacks using unsafe code (usually C expats). So it's certainly not a pure downside, and I think we should probably err on the other side: unless there is a really compelling optimization reason to have a validity invariant we should try to keep things as loose as possible so that unsafe code performs predictably and is understandable by low level developers.
I believe at least one optimization has been listed here-being able to leak the inner value without leaking the discriminant. In general, this is, while a more obscure example of it, just providence checking, which is one of the central examples of things that enable optimizations!
Yes, that is what I mean by it being something that can be potentially escalated unlimitedly. I think the only way of making it not explode is to say that the write to discriminant_ptr is the problem. That previously you had a OnlyOneU8, and you wrote something illegal to it.
This is typed memory, and Rust doesn't have it.
discriminant_ptr
is just a pointer to au8
, there is no memory of the enum that was once there. Besides, the point I made about holding a borrow ondiscriminant_ptr
applies just as well in the other direction: if you have a mutable borrow onpayload_ptr
then it is impossible to tell if the write todiscriminant_ptr
is valid or not.
It isn't typed memory, it is limited providence. Specifically, it is providence where you are limited to writing exactly 00000000
to the location, no fundamentally different than saying that you aren't allowed to write 0000000000000000
there.
I meant using only the
pub
portions, yes. I didn't write out the module wrapping.
(FYI I noticed this after writing the original comment and rewrote this section, see the edit.)
Note that checking for writing an illegal
NonZeroU8
is very checkable!
Indeed it is, and I would expect Miri to notice such things assuming it appears as language UB. In the particular case of NonZeroU8
I believe it is language UB because the niche optimization is exposed to the compiler; without that I would expect it to be library UB and hence not caught by Miri unless there was a debug_assert or similar in new_unchecked
.
It isn't typed memory, it is limited providence. Specifically, it is providence where you are limited to writing exactly
00000000
to the location, no fundamentally different than saying that you aren't allowed to write0000000000000000
there.
That's not how provenance works (at least in the current implementation). There is no pointer provenance that says you can't write 0 to a location. If you have p: &mut NonZeroU8
then it is undefined behavior to do *p = NonZeroU8::new_unchecked(0)
because this is doing a typed copy at type NonZeroU8
with value 0. However, you can still write 0 to that location:
let p: &mut NonZeroU8 = &mut NonZeroU8::new(1);
// *p = NonZeroU8::new_unchecked(0); // UB
// *(p as *mut NonZeroU8) = NonZeroU8::new_unchecked(0); // still UB
*(p as *mut NonZeroU8 as *mut u8) = 0; // DB
// println!("{}", *p); // UB, because we are reading 0 at type NonZeroU8
The validity invariant depends only on the type you are writing at, as determined by the static type of the things on each side of the =
or the type of the pointer in ptr::write
or ptr::read
. When we write 0 to a &mut NonZeroU8
like this, it becomes UB to read (or drop the underlying memory, although that doesn't matter in this case), so this is usually UB fairly soon afterward, but the important part is that the write itself isn't UB, because *p
is just a byte in memory, with no provenance that remembers that 0 shouldn't be placed in that location.
And my example shows that pointer provenance remembering what type it's "really" writing to isn't tractable, either.
The problem with it is that raw pointer operations necessarily don't change provenance.
If you have *mut E
(for my example), it's perfectly valid to write 0x00_01
or 0x01_01
to it. But when you split the pointer into *mut E.discriminant
and *mut E.value
, what happens? All that these pointers "know" from their provenance is what byte of E
they're pointing to; notably not which discriminant is currently in memory.
Essentially, this is the example where you have *mut Option<NonZero>
, just written out a bit more.
I also strongly expect that bytewise memcpy should be implementable inside the abstract machine (given a byte
type exists). memcpy existing means that a) a typed copy can be replaced with a bytewise untyped copy, and b) it must be valid to write temporarily invalid bytes to memory via such. (As an example, consider writing 0x0000
on top of 0x0101
"at type E
.")
There are two "solutions" to this I know of so far, neither of which are particularly nice.
So, TL;DR of that: there isn't even a known way to say the OP example is UB without a major restructuring of how SB's model of the AM semantics works.
(And if you want to go earn your PhD making a formally sound competing model to SB, then I say glhf!)
Rust memory is completely untyped, and this is a nice property to maintain. Putting the typing into the pointers is a cute trick to avoid typed memory basically in name only.
So, I would actually disagree with this being typed memory "basically in name only". I think it more resembles a "capability" model. #316 I think in part comes down to how permissions get restricted and unrestricted as passed through various phases of the memory lifecycle. The memory is untyped and very general in providence, but the allocator may impose restrictions when the memory leaves, which the allocator itself doesn't have to obey once it is passed back, as long as it was the allocator that imposed those restrictions. The memory remaining untyped for the entire program while the pointers carry further restrictions plays a valuable role.
However, I think the memcopy problem is a pretty big one here. If we have a NonZeroU64
, then with a handwritten memcopy and the loop unrolled, we might do a sequence of 8-bit writes which may involve removing all the ones before we write back a single zero. While calling another function that gets to observe the NonZeroU64
would have problems, this seems to clearly show we can't say that the illegality happens at the moment of writing.
However, this code having radically different meaning based on T
still feels extremely uncomfortable to me:
fn func1<T, F: FnOnce(&mut T)>(mut v: Option<T>, f: F) -> bool {
match &mut v {
Some(ref mut b) => f(b),
None => (),
}
return true;
}
fn func2<T, F: FnOnce(&mut T)>(mut v: Option<T>, f: F) -> bool {
match v {
Some(ref mut b) => f(b),
None => return true,
};
return v.is_some();
}
When T
is u8
, there is I believe no way to get these functions to differ without undefined behavior. Even if I extrapolate out the location and try to write a pointer there, MIRI complains. However, when T
is something like a NonZeroU8, suddenly func2
can start returning false
without MIRI batting an eye.
We can also imagine a 'hard mode' memcopy, where it spreads the writes across multiple threads for some reason. I do feel even this should be legal.
But when I think about the actual reasoning I think justifies it, it comes out as something between the two options given. Something like "this write is allowed through this pointer, when this pointer promised to obey certain rules, because by the time it will be observed (see: popping tags) I promise that someone will have fixed this up".
When I try to imagine formalizing this, it seems like the most general way would be something like... when you hand out a pointer, you get to set the rules that then get triggered when tags are popped and such, and even if it is transmuted or such that can only narrow the rules further. This... also seems like it helps with the nested allocator case in some ways, but... the level of generality of this sort of reasoning makes me worry about actually implementing it into a checker.
It seems like the general pattern here would be saying that pointers/references get to encode generators for predicates that get written every time they are used to write, and then get checked on read. Having functions generating functions as a basic building block for providence checking at least sounds bad.
But the actually relevant ones there seem... at lot easier to handle. Owning an allocation seems the most complicated in a lot of ways, but is the primary thing stacked borrows was made to solve.. Leaving the selected constructor in the same state that it was when the inner reference was made seems downright trivial in comparison.
When T is u8, there is I believe no way to get these functions to differ without undefined behavior. Even if I extrapolate out the location and try to write a pointer there, MIRI complains. However, when T is something like a NonZeroU8, suddenly func2 can start returning false without MIRI batting an eye.
I view this as a consequence of doing layout optimizations. It's not surprising that for code doing byte-wise accesses, this has consequences. Reordering fields also changes some otherwise 'generic' properties in code working on (T1, T2, T3)
.
But the actually relevant ones there seem... at lot easier to handle. Owning an allocation seems the most complicated in a lot of ways, but is the primary thing stacked borrows was made to solve.. Leaving the selected constructor in the same state that it was when the inner reference was made seems downright trivial in comparison.
It doesn't seem trivial to me. :shrug: Stacked Borrows doesn't really do anything with 'ownership', it 'just' checks that the sequence of pointer accesses made to a byte of memory makes sense (and it does that independently for each byte).
The 'selected constructor' is a very subtle concept -- it basically doesn't exist on the Abstract Machine, it is an emergent property that comes up after interpreting a bunch of bytes using a given type, and with types like Option<NonZeroU64>
it non-trivially relies on the correlation of a whole bunch of neighboring bytes. The C memory model tries to impose type-driven structure on memory (values are stored in memory as a 'tree' of nested structs/arrays) whole also supporting byte-level accesses and that is complicated. I don't think we should do any of that for Rust.
(Ignoring the opsem,) I would like to argue that asserting validity at expiry typically matches developer reasoning better, and better behaves like safety invariant does.
I argue that semantically, the OP example^1 is similar to e.g. the practice of as_bytes_mut(&mut str) -> &mut [u8]
or &mut T -> &mut MaybeUninit<T>
. You know the memory exists and is writable, so you temporarily allow writing more bytes than the type otherwise would allow, but you need to clean up and put valid safe bytes in before returning control to the parent safe type.
```rust
fn main() { unsafe {
let mut x = Some(&0);
match x {
Some(ref mut b) => {
let u = b as *mut &i32 as *mut usize;
// Just writing into a *mut usize
*u = 0;
}
None => unreachable!(),
}
assert!(x.is_some());
} }
```
</details>
Obviously the safe interface still requires restoring the type-valid (and type-safe) bytes. This is about the validity requirement. For that, I still hold my previously stated position that we have and should keep untyped memory, and that means only doing type-dependent assertions when doing a typed operation (copy, maybe retag; pop is untyped).
The observation is that restoring the invariants before exiting through the invariant-holder matches the usual patterns, and diverging from this for unsafe opsem is a difference that will need to be learned.
So, TL;DR of that: there isn't even a known way to say the OP example is UB without a major restructuring of how SB's model of the AM semantics works.
I have a proposal for an opsem which could disallow the OP example (at least as written): use a protected tag for match
produced references, like the protectors used for function arguments.
Aside: This of course presumes that function protectors assert something about the referenced memory. This is currently not the case, but doing a read on retag/protector (therefore asserting byte-validity of the referee) is at least possible.
Restated: this would make
unsafe fn oops(x: &mut &i32) { (x as *mut &usize as *mut usize).write(0); } fn main() { unsafe { let mut z = Some(&0); match z { Some(ref mut b) => oops(b), None => unreachable!(), } assert!(z.is_none()); } }
the same opsem as without the function.
There is an important caveat to inserting protectors on match
-introduced references, though: unlike in functions, NLL allows invalidating the reference before it is lexically inaccessible, e.g.
fn main() { unsafe {
let mut z = Some(&0);
match z {
Some(ref mut b) => {
(x as *mut &i32 as *mut usize).write(0);
assert!(z.is_none());
}
None => unreachable!(),
}
} }
Issuing protectors here thus requires removing the protector when the borrow is invalidated non-lexically. This information may not be available in the current structure of MIR, and would need to be added during MIR borrowck.
This limits the viability of the approach somewhat.
(Protectors could additionally be used on any introduced reference with this semantic of scheduling the removal of the protector.)
Also to note: this is not to advocate for this approach, just to note it as an approach.
noted later: there's a significant flaw with this approach: using a parent raw pointer does not cause the child reference to be known to be invalidated.
@CAD97 so you are arguing that we should not have UB for "invalid values on tag invalidation", but you are also proposing an opsem for achieving exactly that thing you don't want? That's a bit confusing so I am trying to make sure I understand you correctly here. :)
My position is roughly summarized as
match
protectors would make a parent raw pointer behave differently than a parent reference^2[^1]: as in, invalidated by access through a tag before it on the borrow stack, rather than preventing use of tags before it until actively popped/unprotected
```rust
let mut place = 0i32;
let ptr = &mut raw place;
match *ptr {
ref mut x => {
*ptr = 1; // here
}
}
```
here this ***must*** be valid if `ptr: &mut i32`, but placing a protector on `x` would make this UB with `ptr: *mut i32`, as the lifetime of `x` is not statically known to be invalidated.
An interesting variant of invalid-not-used-again values came up on Zulip w.r.t. MIR drop elaboration:
// only ever instantiated at <T=bool>
unsafe fn drop_uninit<T>() {
let mut x = std::mem::zeroed::<T>();
let p = addr_of_mut!(x) as *mut MaybeUninit<u8>;
p.write(MaybeUninit::uninit());
// Drop(x) <- current MIR
// drop_in_place(&mut x); // <- proposal
}
fn main() {
unsafe { drop_uninit::<bool>(); }
}
This is very similar to an example in the OP
let mut x = true;
let xptr = &mut x as *mut bool as *mut u8;
*xptr = 2;
but perhaps slightly more interesting due to writing undef
and the framing w.r.t. generic MIR rather than concrete.
FWIW the Drop
in current MIR takes a place, not a value, so it is pretty much the same as drop_in_place(&mut x)
.
FWIW https://github.com/rust-lang/rfcs/blob/master/text/2195-really-tagged-unions.md contains an example that expects at least temporary violation of the validity invariant of an &mut T
to be allowed.
(Taken from https://github.com/rust-rfcs/unsafe-code-guidelines/issues/69#issuecomment-459445822.)
How do we feel about this code -- UB or not?
This "magically" changes the discriminant. OTOH, it is very similar to
which is definitely allowed (it makes assumptions about layout, but we do guarantee layout of
Option<&T>
.Other examples: