RalfJung
commented
5 years ago Recently @eddyb said something like "turns out Miri catches errors in const qualif, that's good news" -- basically referring to this issue. I take this as confirmation that this is a goal worth pursuing.
With https://github.com/rust-lang/rust/pull/58351, a bunch of "holes" in these checks are fixed. But I think I need help to figure out what is left. Here is an incomplete list, with examples that we could add as -Zunleash-miri tests:
-
We have to check that no destructors are called. I think we already do, through the "no non-const-fn-call" check, but we should add a test case for this.
-
We have to check that const/promoteds do not access statics.
How would we implement this? We could tell CTFE whether it is currently evaluating a
staticor other initializer, and then we could reject reads/writes to/from "global" (already interned) allocations that are marked mutable. That would actually correspond to the more refined analysis that does allow the above code, but does not allow the variant with added interior mutability. Intuitively, if only immutable memory has been read, then clearly the end result must be the same no matter when evaluation happens.use std::cell::Cell; static STATIC: usize = 42; static mut STATIC_MUT: usize = 42; static mut STATIC_CELL: Cell<usize> = Cell::new(42); const SCARY_BUT_OKAY: usize = STATIC+1; const BAD_MUT: usize = unsafe { STATIC_MUT+1 }; const BAD_CELL: usize = unsafe { *(&STATIC_CELL as *const _ as *const usize as *mut usize) +1 };@eddyb is there any deep reason to forbid
SCARY_BUT_OKAY? -
We have to check that the final value of a const/promoted does not add any "new" mutable memory to the global state. I think we do some checks here already with @oli-obk's PR mentioned above, but do we check enough? What about raw pointers / other pointer values in consts?
const BAD: &Cell<i32> = &Cell::new(42); // Inlining `BAD` everywhere clearly is not the same as them all pointing to the same thing. const BAD_RAW: *mut i32 = &Cell::new(42) as *const _ as *mut _; const BAD_USIZE: usize = &Cell::new(42) as *const _ as usize;It seems to me like we are actually fine with constants pointing to already existing mutable memory? After all, then evaluating that constant many time would just make it point to the same memory each time. So, conceivably we could accept something like
static STATIC: Cell<i32> = Cell::new(42); const SCARY_BUT_OKAY: *mut i32 = &STATIC as *const _ as *mut _; const BAD: i32 = unsafe { *SCARY_BUT_OKAY };@eddyb is there anything inherently wrong with
SCARY_BUT_OKAY, if we still forbidBAD? -
Can we do any dynamic check for non-
Syncreferences in constants? -
For promoteds on top of the above, the result must not require dropping. Is there something we can check here?
Also, is this list complete? This mostly reflects what we have documented as required constraints in this repo. We already check that only const fn are called, and for promoteds most of the complexity is avoiding dynamic errors, not figuring out what the dynamic error conditions are.
eddyb
vertexclique
oli-obk
ecstatic-morse
With https://github.com/rust-lang/rust/pull/56123, we can bypass const qualification, so we can e.g. write constants that read from interior mutable statics, and probably other things I forgot to think about.
Could we
Ideally, these restrictions would be sufficient, so the soundness criterion for the static checks could be fully described as "statically check for the absence of the following miri errors" -- and such that @oli-obk's feature would actually be sound!
For example, I could imagine tweaking the CTFE instance of miri such that reading from a static is disallowed (and not just disallowing writing to a static like now), implementing a dynamic check that would reject code like what I described above.
For the UnsafeCell and Drop checks, we'd need some value-based analysis, which @oli-obk wanted in the long run anyway.
Checklist:
dropcalls during initializer evaluation.Sync? See https://github.com/rust-lang/rust/issues/49206.The problem with promotion is tracked separately at https://github.com/rust-lang/const-eval/issues/53.
Cc @eddyb