fix validating fat pointers to user-defined unsized types

Author: RalfJung

src/librustc_mir/interpret/place.rs

@@ -774,6 +774,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
                 let (size, align) = self.read_size_and_align_from_vtable(vtable)?;
                 assert_eq!(size, layout.size);
                 assert_eq!(align.abi(), layout.align.abi()); // only ABI alignment is preserved
+                // FIXME: More checks for the vtable? We could make sure it is exactly
+                // the one one would expect for this type.
                 // Done!
                 layout
             },

src/librustc_mir/interpret/validity.rs

@@ -9,7 +9,7 @@ use rustc::mir::interpret::{
 };
 
 use super::{
-    MPlaceTy, PlaceExtra, Machine, EvalContext
+    MPlaceTy, Machine, EvalContext
 };
 
 macro_rules! validation_failure{
@@ -281,59 +281,32 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
             },
             layout::FieldPlacement::Arbitrary { ref offsets, .. } => {
                 // Fat pointers need special treatment.
-                match dest.layout.ty.builtin_deref(true).map(|tam| &tam.ty.sty) {
-                    | Some(ty::TyStr)
-                    | Some(ty::TySlice(_)) => {
-                        // check the length (for nicer error messages); must be valid even
-                        // for a raw pointer.
-                        let len_mplace = self.mplace_field(dest, 1)?;
-                        let len = self.read_scalar(len_mplace.into())?;
-                        let len = match len.to_bits(len_mplace.layout.size) {
-                            Err(_) => return validation_failure!("length is not a valid integer", path),
-                            Ok(len) => len as u64,
-                        };
-                        // for safe ptrs, get the fat ptr, and recursively check it
-                        if !dest.layout.ty.is_unsafe_ptr() {
-                            let ptr = self.ref_to_mplace(self.read_value(dest.into())?)?;
-                            assert_eq!(ptr.extra, PlaceExtra::Length(len));
-                            let unpacked_ptr = self.unpack_unsized_mplace(ptr)?;
-                            if seen.insert(unpacked_ptr) {
-                                todo.push((unpacked_ptr, path_clone_and_deref(path)));
-                            }
-                        }
-                    },
-                    Some(ty::TyDynamic(..)) => {
-                        // check the vtable (for nicer error messages); must be valid even for a
-                        // raw ptr.
-                        let vtable = self.read_scalar(self.mplace_field(dest, 1)?.into())?;
-                        let vtable = match vtable.to_ptr() {
-                            Err(_) => return validation_failure!("vtable address is not a pointer", path),
-                            Ok(vtable) => vtable,
-                        };
-                        // for safe ptrs, get the fat ptr, and recursively check it
-                        if !dest.layout.ty.is_unsafe_ptr() {
-                            let ptr = self.ref_to_mplace(self.read_value(dest.into())?)?;
-                            assert_eq!(ptr.extra, PlaceExtra::Vtable(vtable));
-                            let unpacked_ptr = self.unpack_unsized_mplace(ptr)?;
-                            if seen.insert(unpacked_ptr) {
-                                todo.push((unpacked_ptr, path_clone_and_deref(path)));
-                            }
-                            // FIXME: More checks for the vtable... making sure it is exactly
-                            // the one one would expect for this type.
-                        }
-                    },
-                    Some(ty) =>
-                        bug!("Unexpected fat pointer target type {:?}", ty),
-                    None => {
-                        // Not a pointer, perform regular aggregate handling below
-                        for i in 0..offsets.len() {
-                            let field = self.mplace_field(dest, i as u64)?;
-                            path.push(self.aggregate_field_path_elem(dest.layout.ty, variant, i));
-                            self.validate_mplace(field, path, seen, todo)?;
-                            path.truncate(path_len);
+                if dest.layout.ty.builtin_deref(true).is_some() {
+                    // This is a fat pointer.
+                    let ptr = match self.ref_to_mplace(self.read_value(dest.into())?) {
+                        Ok(ptr) => ptr,
+                        Err(ReadPointerAsBytes) =>
+                            return validation_failure!("fat pointer length is not a valid integer", path),
+                        Err(ReadBytesAsPointer) =>
+                            return validation_failure!("fat pointer vtable is not a valid pointer", path),
+                        Err(err) => return Err(err),
+                    };
+                    let unpacked_ptr = self.unpack_unsized_mplace(ptr)?;
+                    // for safe ptrs, recursively check it
+                    if !dest.layout.ty.is_unsafe_ptr() {
+                        if seen.insert(unpacked_ptr) {
+                            todo.push((unpacked_ptr, path_clone_and_deref(path)));
                         }
-                        // FIXME: For a TyStr, check that this is valid UTF-8.
-                    },
+                    }
+                } else {
+                    // Not a pointer, perform regular aggregate handling below
+                    for i in 0..offsets.len() {
+                        let field = self.mplace_field(dest, i as u64)?;
+                        path.push(self.aggregate_field_path_elem(dest.layout.ty, variant, i));
+                        self.validate_mplace(field, path, seen, todo)?;
+                        path.truncate(path_len);
+                    }
+                    // FIXME: For a TyStr, check that this is valid UTF-8.
                 }
             }
         }

src/test/ui/union-ub-fat-ptr.rs

@@ -37,6 +37,7 @@ union SliceTransmute {
     bad: BadSliceRepr,
     slice: &'static [u8],
     str: &'static str,
+    my_str: &'static Str,
 }
 
 #[repr(C)]
@@ -70,6 +71,8 @@ union DynTransmute {
 trait Trait {}
 impl Trait for bool {}
 
+struct Str(str);
+
 // OK
 const A: &str = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: 1 } }.str};
 // bad str
@@ -78,14 +81,17 @@ const B: &str = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: 999 }
 // bad str
 const C: &str = unsafe { SliceTransmute { bad: BadSliceRepr { ptr: &42, len: &3 } }.str};
 //~^ ERROR this constant likely exhibits undefined behavior
+// bad str in Str
+const C2: &Str = unsafe { SliceTransmute { bad: BadSliceRepr { ptr: &42, len: &3 } }.my_str};
+//~^ ERROR this constant likely exhibits undefined behavior
 
 // OK
 const A2: &[u8] = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: 1 } }.slice};
 // bad slice
 const B2: &[u8] = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: 999 } }.slice};
 //~^ ERROR this constant likely exhibits undefined behavior
 // bad slice
-const C2: &[u8] = unsafe { SliceTransmute { bad: BadSliceRepr { ptr: &42, len: &3 } }.slice};
+const C3: &[u8] = unsafe { SliceTransmute { bad: BadSliceRepr { ptr: &42, len: &3 } }.slice};
 //~^ ERROR this constant likely exhibits undefined behavior
 
 // bad trait object

src/test/ui/union-ub-fat-ptr.stderr

@@ -1,75 +1,83 @@
 error[E0080]: this constant likely exhibits undefined behavior
-  --> $DIR/union-ub-fat-ptr.rs:76:1
+  --> $DIR/union-ub-fat-ptr.rs:79:1
    |
 LL | const B: &str = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: 999 } }.str};
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access at offset N, outside bounds of allocation N which has size N
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
 error[E0080]: this constant likely exhibits undefined behavior
-  --> $DIR/union-ub-fat-ptr.rs:79:1
+  --> $DIR/union-ub-fat-ptr.rs:82:1
    |
 LL | const C: &str = unsafe { SliceTransmute { bad: BadSliceRepr { ptr: &42, len: &3 } }.str};
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered length is not a valid integer
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered fat pointer length is not a valid integer
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
 error[E0080]: this constant likely exhibits undefined behavior
   --> $DIR/union-ub-fat-ptr.rs:85:1
    |
+LL | const C2: &Str = unsafe { SliceTransmute { bad: BadSliceRepr { ptr: &42, len: &3 } }.my_str};
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered fat pointer length is not a valid integer
+   |
+   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
+
+error[E0080]: this constant likely exhibits undefined behavior
+  --> $DIR/union-ub-fat-ptr.rs:91:1
+   |
 LL | const B2: &[u8] = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: 999 } }.slice};
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access at offset N, outside bounds of allocation N which has size N
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
 error[E0080]: this constant likely exhibits undefined behavior
-  --> $DIR/union-ub-fat-ptr.rs:88:1
+  --> $DIR/union-ub-fat-ptr.rs:94:1
    |
-LL | const C2: &[u8] = unsafe { SliceTransmute { bad: BadSliceRepr { ptr: &42, len: &3 } }.slice};
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered length is not a valid integer
+LL | const C3: &[u8] = unsafe { SliceTransmute { bad: BadSliceRepr { ptr: &42, len: &3 } }.slice};
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered fat pointer length is not a valid integer
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
 error[E0080]: this constant likely exhibits undefined behavior
-  --> $DIR/union-ub-fat-ptr.rs:92:1
+  --> $DIR/union-ub-fat-ptr.rs:98:1
    |
 LL | const D: &Trait = unsafe { DynTransmute { repr: DynRepr { ptr: &92, vtable: &3 } }.rust};
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ tried to access memory with alignment N, but alignment N is required
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
 error[E0080]: this constant likely exhibits undefined behavior
-  --> $DIR/union-ub-fat-ptr.rs:95:1
+  --> $DIR/union-ub-fat-ptr.rs:101:1
    |
 LL | const E: &Trait = unsafe { DynTransmute { repr2: DynRepr2 { ptr: &92, vtable: &3 } }.rust};
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ a memory access tried to interpret some bytes as a pointer
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
 error[E0080]: this constant likely exhibits undefined behavior
-  --> $DIR/union-ub-fat-ptr.rs:98:1
+  --> $DIR/union-ub-fat-ptr.rs:104:1
    |
 LL | const F: &Trait = unsafe { DynTransmute { bad: BadDynRepr { ptr: &92, vtable: 3 } }.rust};
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered vtable address is not a pointer
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered fat pointer length is not a valid integer
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
 error[E0080]: this constant likely exhibits undefined behavior
-  --> $DIR/union-ub-fat-ptr.rs:102:1
+  --> $DIR/union-ub-fat-ptr.rs:108:1
    |
 LL | const G: &Trait = &unsafe { BoolTransmute { val: 3 }.bl };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .<deref>, but expected something in the range 0..=1
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
 error[E0080]: this constant likely exhibits undefined behavior
-  --> $DIR/union-ub-fat-ptr.rs:106:1
+  --> $DIR/union-ub-fat-ptr.rs:112:1
    |
 LL | const H: &[bool] = &[unsafe { BoolTransmute { val: 3 }.bl }];
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .<deref>[0], but expected something in the range 0..=1
    |
    = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
 
-error: aborting due to 9 previous errors
+error: aborting due to 10 previous errors
 
 For more information about this error, try `rustc --explain E0080`.