bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #99033 - 5225225:interpreter-validity-checks, r=oli-obk

Browse source 
Use constant eval to do strict mem::uninit/zeroed validity checks

I'm not sure about the code organisation here, I just dumped the check in rustc_const_eval at the root. Not hard to move it elsewhere, in any case.

Also, this means cranelift codegen intrinsics lose the strict checks, since they don't seem to depend on rustc_const_eval, and I didn't see a point in keeping around two copies.

I also left comments in the is_zero_valid methods about "uhhh help how do i do this", those apply to both methods equally.

Also rustc_codegen_ssa now depends on rustc_const_eval... is this okay?

Pinging `@RalfJung` since you were the one who mentioned this to me, so I'm assuming you're interested.

Haven't had a chance to run full tests on this since it's really warm, and it's 1AM, I'll check out any failures/comments in the morning :)
This commit is contained in:
bors 2022-07-17 19:28:01 +00:00
commit 263edd43c5
13 changed files with 161 additions and 94 deletions
Download patch file Download diff file Expand all files Collapse all files

38
compiler/rustc_target/src/abi/mod.rs
View file

@ -1372,7 +1372,7 @@ pub struct PointeeInfo {
/// Used in `might_permit_raw_init` to indicate the kind of initialisation
/// that is checked to be valid
#[derive(Copy, Clone, Debug)]
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum InitKind {
Zero,
Uninit,
@ -1487,14 +1487,18 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
///
/// `init_kind` indicates if the memory is zero-initialized or left uninitialized.
///
/// `strict` is an opt-in debugging flag added in #97323 that enables more checks.
/// This code is intentionally conservative, and will not detect
/// * zero init of an enum whose 0 variant does not allow zero initialization
/// * making uninitialized types who have a full valid range (ints, floats, raw pointers)
/// * Any form of invalid value being made inside an array (unless the value is uninhabited)
///
/// This is conservative: in doubt, it will answer `true`.
/// A strict form of these checks that uses const evaluation exists in
/// `rustc_const_eval::might_permit_raw_init`, and a tracking issue for making these checks
/// stricter is <https://github.com/rust-lang/rust/issues/66151>.
///
/// FIXME: Once we removed all the conservatism, we could alternatively
/// create an all-0/all-undef constant and run the const value validator to see if
/// this is a valid value for the given type.
pub fn might_permit_raw_init<C>(self, cx: &C, init_kind: InitKind, strict: bool) -> bool
/// FIXME: Once all the conservatism is removed from here, and the checks are ran by default,
/// we can use the const evaluation checks always instead.
pub fn might_permit_raw_init<C>(self, cx: &C, init_kind: InitKind) -> bool
where
Self: Copy,
Ty: TyAbiInterface<'a, C>,
@ -1507,13 +1511,8 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
s.valid_range(cx).contains(0)
}
InitKind::Uninit => {
if strict {
// The type must be allowed to be uninit (which means "is a union").
s.is_uninit_valid()
} else {
// The range must include all values.
s.is_always_valid(cx)
}
// The range must include all values.
s.is_always_valid(cx)
}
}
};
@ -1534,19 +1533,12 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
// If we have not found an error yet, we need to recursively descend into fields.
match &self.fields {
FieldsShape::Primitive | FieldsShape::Union { .. } => {}
FieldsShape::Array { count, .. } => {
FieldsShape::Array { .. } => {
// FIXME(#66151): For now, we are conservative and do not check arrays by default.
if strict
&& *count > 0
&& !self.field(cx, 0).might_permit_raw_init(cx, init_kind, strict)
{
// Found non empty array with a type that is unhappy about this kind of initialization
return false;
}
}
FieldsShape::Arbitrary { offsets, .. } => {
for idx in 0..offsets.len() {
if !self.field(cx, idx).might_permit_raw_init(cx, init_kind, strict) {
if !self.field(cx, idx).might_permit_raw_init(cx, init_kind) {
// We found a field that is unhappy with this kind of initialization.
return false;
}