-
Notifications
You must be signed in to change notification settings - Fork 352
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
&A as *const _ == &A as *const _ not guaranteed for const A
#131
Comments
@nikomatsakis and @eddyb were just talking about a way to change how promoteds work that I think might fix this problem. (I.e. I'm leaning a. However, I think we have other reasons to do b anyway.) |
Specifically, changing promoted fragments to be lvalues instead of constant rvalues and moving the reference out, so they're (inside a function) runtime borrows of static lvalues instead of copies of constant rvalues, and then we also want borrows inside @nikomatsakis likes this and I agree it should work better, at least in the sense that you're encoding the "right" semantics, as you likely want to allow |
I implemented b in 6ffd700 but we'll still need the fix for promoteds. |
Another example from the rustc test suite: use std::{str, string};
const A: [u8; 2] = ['h' as u8, 'i' as u8];
const B: &'static [u8; 2] = &A;
const C: *const u8 = B as *const u8;
pub fn main() {
unsafe {
let foo = &A as *const u8;
assert_eq!(foo, C);
assert_eq!(str::from_utf8_unchecked(&A), "hi");
assert_eq!(*C, A[0]);
assert_eq!(*(&B[0] as *const u8), A[0]);
}
} |
This test also differs between rust (it passes) and miri (it fails): fn main() {
assert!(&A as *const A as *const () == &() as *const _);
assert!(&A as *const A == &A as *const A);
} |
This seems to be just an instance of "we should not really allocate ZSTs", right? I.e., fixing this should also fix the above test. |
We'll still have some zsts whose address would differ, e.g. because they are in a field of another type. I don't feel too strongly about any variant, but I'm not sure what semantics we want in const eval |
Actually this has nothing to do with ZST allocation. rustc somehow makes it so that with
the code
ends up saying that the two pointers are equal. Is it going deduplication of promoted constants or how is that working? |
All immutable constants are deduplicated in |
TBH I do not feel "bug" is the right label for this; we don't guarantee this deduplication (I would think) so what miri does seems reasonable to me. For example, does it happen across crates? We could also make it error out when comparing, because we don't want to predict run-time behavior. Not sure if that would be better. |
So... I've come to the conclusion that we can only compare
without erroring, because any other comparisons might differ between execution and interpretation. Of course pointers into the same allocation can always be compared. |
Even immutable |
Indeed. I have now built the following comparison table that I think represents everything we want:
The reasoning behind |
Ok... it's not as clear cut as I assumed. E.g. slice comparison has an optimization where if the length is the same and the address is the same, we don't compare the contents at all. With the rules in the table above, This is a similar situation to the problems we've had around Can we assume that named and live local variables are guaranteed distinct, even if the compiler can figure out that they are immutable and have the same value? Basically: let x = 42;
let y = foo();
if x == y {
// is it a misoptimization to free the stack slot of either x or y to
// be reused and all further mentions of `x` and `y` refer to the same
// stack slot?
return;
} |
I think we should take a step back and think about the goals here. All these examples here are essentially cases where whether the physical values of the pointers are equal is non-deterministic. Do we want to make miri detect all such possible non-determinism and stop execution? There is at least one more case: Comparing the pointer to the beginning of one allocation and one-past-the-end of another allocation could actually return Right now, I think being more aggressive than we already are about ruling out non-determinism will quickly make us bail out on existing code. I see two options, in principle:
|
Only if we start allowing casting to raw pointers in promoteds or allow calling arbitrary const functions
Right now we bail out for any pointer comparison in CTFE, so we're good and can consider this independently.
Yes I think that's a good solution. Pointers are not equal if there's any optimization that may consider them unequal. This means we can simplify the rules to "if both are |
Long-term we want to allow the latter with explicit promotion, right? So, doesn't that mean it will be a problem?
True.
Do you mean "any possible execution"? I don't think The problem with comparing out-of-bounds pointers of different allocations is that if we declare them never equal, we get strange effects. Basically, the following function will return fn compare_all_offsets(x: *const u8, y: *const u8) -> bool {
(0..=usize::MAX).any(|offset| x.wrapping_offset(offset) == y)
} |
It is not. We should simply ensure that every creation of a function pointer gets a new
On 64 bit that program is basically an infinite loop But I agree, we should keep the code as it is. The obvious issues are addressible at the time the |
Oh right, with function pointers we currently have spurious equality even when they can be inequal at run-time. Seems preferrable to only ever err on one side (spurious inequality). We could also special-case function pointers during comparison, though. Not sure what would work better / be more efficient / more clean. |
I'm addressing this directly on the rustc side: rust-lang/rust@f6b3eb0 |
Switch to hashbrown's RawTable internally
Closing in favor of #1955 |
miri creates two allocations for the
*b"hi"
inA
.see MIR in https://is.gd/bCUD7p
This makes sense, since there's
_1 = A; _2 = &_1
instead of_2 = &A
.Should this
a) be fixed in rustc, or
b) should we cache all literals (we have FIXMEs for that), or
c) should we add some
Cow
-like structure to lvalues, so they can refer to other lvalues "ByVal"?d) is the test ( https://github.com/rust-lang/rust/blob/master/src/test/run-pass/const-str-ptr.rs ) wrong?
@eddyb: any preferences?
Found more evidence for b and against d:
The text was updated successfully, but these errors were encountered: