Rollup merge of #63657 - RalfJung:invalid_value, r=Centril

Crank up invalid value lint

* Warn against uninit `bool` and `char`.
* Warn against 0-init `NonNull` and friends
* Detect transmute-from-0 as zero-initialization ([seen in the wild](glium/glium#1775 (comment)))

Author: Centril

src/librustc_lint/builtin.rs

@@ -1876,25 +1876,101 @@ declare_lint_pass!(InvalidValue => [INVALID_VALUE]);
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &hir::Expr) {
 
-        const ZEROED_PATH: &[Symbol] = &[sym::core, sym::mem, sym::zeroed];
-        const UININIT_PATH: &[Symbol] = &[sym::core, sym::mem, sym::uninitialized];
+        #[derive(Debug, Copy, Clone, PartialEq)]
+        enum InitKind { Zeroed, Uninit };
 
         /// Information about why a type cannot be initialized this way.
         /// Contains an error message and optionally a span to point at.
         type InitError = (String, Option<Span>);
 
+        /// Test if this constant is all-0.
+        fn is_zero(expr: &hir::Expr) -> bool {
+            use hir::ExprKind::*;
+            use syntax::ast::LitKind::*;
+            match &expr.node {
+                Lit(lit) =>
+                    if let Int(i, _) = lit.node {
+                        i == 0
+                    } else {
+                        false
+                    },
+                Tup(tup) =>
+                    tup.iter().all(is_zero),
+                _ =>
+                    false
+            }
+        }
+
+        /// Determine if this expression is a "dangerous initialization".
+        fn is_dangerous_init(cx: &LateContext<'_, '_>, expr: &hir::Expr) -> Option<InitKind> {
+            const ZEROED_PATH: &[Symbol] = &[sym::core, sym::mem, sym::zeroed];
+            const UININIT_PATH: &[Symbol] = &[sym::core, sym::mem, sym::uninitialized];
+            // `transmute` is inside an anonymous module (the `extern` block?);
+            // `Invalid` represents the empty string and matches that.
+            const TRANSMUTE_PATH: &[Symbol] =
+                &[sym::core, sym::intrinsics, kw::Invalid, sym::transmute];
+
+            if let hir::ExprKind::Call(ref path_expr, ref args) = expr.node {
+                if let hir::ExprKind::Path(ref qpath) = path_expr.node {
+                    let def_id = cx.tables.qpath_res(qpath, path_expr.hir_id).opt_def_id()?;
+
+                    if cx.match_def_path(def_id, ZEROED_PATH) {
+                        return Some(InitKind::Zeroed);
+                    }
+                    if cx.match_def_path(def_id, UININIT_PATH) {
+                        return Some(InitKind::Uninit);
+                    }
+                    if cx.match_def_path(def_id, TRANSMUTE_PATH) {
+                        if is_zero(&args[0]) {
+                            return Some(InitKind::Zeroed);
+                        }
+                    }
+                    // FIXME: Also detect `MaybeUninit::zeroed().assume_init()` and
+                    // `MaybeUninit::uninit().assume_init()`.
+                }
+            }
+
+            None
+        }
+
         /// Return `Some` only if we are sure this type does *not*
         /// allow zero initialization.
-        fn ty_find_init_error<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Option<InitError> {
+        fn ty_find_init_error<'tcx>(
+            tcx: TyCtxt<'tcx>,
+            ty: Ty<'tcx>,
+            init: InitKind,
+        ) -> Option<InitError> {
             use rustc::ty::TyKind::*;
             match ty.sty {
                 // Primitive types that don't like 0 as a value.
                 Ref(..) => Some((format!("References must be non-null"), None)),
                 Adt(..) if ty.is_box() => Some((format!("`Box` must be non-null"), None)),
                 FnPtr(..) => Some((format!("Function pointers must be non-null"), None)),
                 Never => Some((format!("The never type (`!`) has no valid value"), None)),
-                // Recurse for some compound types.
+                // Primitive types with other constraints.
+                Bool if init == InitKind::Uninit =>
+                    Some((format!("Booleans must be `true` or `false`"), None)),
+                Char if init == InitKind::Uninit =>
+                    Some((format!("Characters must be a valid unicode codepoint"), None)),
+                // Recurse and checks for some compound types.
                 Adt(adt_def, substs) if !adt_def.is_union() => {
+                    // First check f this ADT has a layout attribute (like `NonNull` and friends).
+                    use std::ops::Bound;
+                    match tcx.layout_scalar_valid_range(adt_def.did) {
+                        // We exploit here that `layout_scalar_valid_range` will never
+                        // return `Bound::Excluded`.  (And we have tests checking that we
+                        // handle the attribute correctly.)
+                        (Bound::Included(lo), _) if lo > 0 =>
+                            return Some((format!("{} must be non-null", ty), None)),
+                        (Bound::Included(_), _) | (_, Bound::Included(_))
+                        if init == InitKind::Uninit =>
+                            return Some((
+                                format!("{} must be initialized inside its custom valid range", ty),
+                                None,
+                            )),
+                        _ => {}
+                    }
+                    // Now, recurse.
                     match adt_def.variants.len() {
                         0 => Some((format!("0-variant enums have no valid value"), None)),
                         1 => {
@@ -1905,6 +1981,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue {
                                 ty_find_init_error(
                                     tcx,
                                     field.ty(tcx, substs),
+                                    init,
                                 ).map(|(mut msg, span)| if span.is_none() {
                                     // Point to this field, should be helpful for figuring
                                     // out where the source of the error is.
@@ -1918,57 +1995,48 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue {
                                 })
                             })
                         }
+                        // Multi-variant enums are tricky: if all but one variant are
+                        // uninhabited, we might actually do layout like for a single-variant
+                        // enum, and then even leaving them uninitialized could be okay.
                         _ => None, // Conservative fallback for multi-variant enum.
                     }
                 }
                 Tuple(..) => {
                     // Proceed recursively, check all fields.
-                    ty.tuple_fields().find_map(|field| ty_find_init_error(tcx, field))
+                    ty.tuple_fields().find_map(|field| ty_find_init_error(tcx, field, init))
                 }
-                // FIXME: Would be nice to also warn for `NonNull`/`NonZero*`.
-                // FIXME: *Only for `mem::uninitialized`*, we could also warn for `bool`,
-                //        `char`, and any multivariant enum.
                 // Conservative fallback.
                 _ => None,
             }
         }
 
-        if let hir::ExprKind::Call(ref path_expr, ref _args) = expr.node {
-            if let hir::ExprKind::Path(ref qpath) = path_expr.node {
-                if let Some(def_id) = cx.tables.qpath_res(qpath, path_expr.hir_id).opt_def_id() {
-                    if cx.match_def_path(def_id, &ZEROED_PATH) ||
-                        cx.match_def_path(def_id, &UININIT_PATH)
-                    {
-                        // This conjures an instance of a type out of nothing,
-                        // using zeroed or uninitialized memory.
-                        // We are extremely conservative with what we warn about.
-                        let conjured_ty = cx.tables.expr_ty(expr);
-                        if let Some((msg, span)) = ty_find_init_error(cx.tcx, conjured_ty) {
-                            let mut err = cx.struct_span_lint(
-                                INVALID_VALUE,
-                                expr.span,
-                                &format!(
-                                    "the type `{}` does not permit {}",
-                                    conjured_ty,
-                                    if cx.match_def_path(def_id, &ZEROED_PATH) {
-                                        "zero-initialization"
-                                    } else {
-                                        "being left uninitialized"
-                                    }
-                                ),
-                            );
-                            err.span_label(expr.span,
-                                "this code causes undefined behavior when executed");
-                            err.span_label(expr.span, "help: use `MaybeUninit<T>` instead");
-                            if let Some(span) = span {
-                                err.span_note(span, &msg);
-                            } else {
-                                err.note(&msg);
-                            }
-                            err.emit();
-                        }
-                    }
+        if let Some(init) = is_dangerous_init(cx, expr) {
+            // This conjures an instance of a type out of nothing,
+            // using zeroed or uninitialized memory.
+            // We are extremely conservative with what we warn about.
+            let conjured_ty = cx.tables.expr_ty(expr);
+            if let Some((msg, span)) = ty_find_init_error(cx.tcx, conjured_ty, init) {
+                let mut err = cx.struct_span_lint(
+                    INVALID_VALUE,
+                    expr.span,
+                    &format!(
+                        "the type `{}` does not permit {}",
+                        conjured_ty,
+                        match init {
+                            InitKind::Zeroed => "zero-initialization",
+                            InitKind::Uninit => "being left uninitialized",
+                        },
+                    ),
+                );
+                err.span_label(expr.span,
+                    "this code causes undefined behavior when executed");
+                err.span_label(expr.span, "help: use `MaybeUninit<T>` instead");
+                if let Some(span) = span {
+                    err.span_note(span, &msg);
+                } else {
+                    err.note(&msg);
                 }
+                err.emit();
             }
         }
     }

src/test/ui/consts/const-eval/ub-nonnull.rs

@@ -1,5 +1,5 @@
 #![feature(rustc_attrs, const_transmute)]
-#![allow(const_err)] // make sure we cannot allow away the errors tested here
+#![allow(const_err, invalid_value)] // make sure we cannot allow away the errors tested here
 
 use std::mem;
 use std::ptr::NonNull;

src/test/ui/consts/const-eval/ub-ref.rs

@@ -1,6 +1,6 @@
 // ignore-tidy-linelength
 #![feature(const_transmute)]
-#![allow(const_err)] // make sure we cannot allow away the errors tested here
+#![allow(const_err, invalid_value)] // make sure we cannot allow away the errors tested here
 
 use std::mem;
 

src/test/ui/consts/const-eval/ub-upvars.rs

@@ -1,5 +1,5 @@
 #![feature(const_transmute)]
-#![allow(const_err)] // make sure we cannot allow away the errors tested here
+#![allow(const_err, invalid_value)] // make sure we cannot allow away the errors tested here
 
 use std::mem;
 

src/test/ui/lint/uninitialized-zeroed.rs

@@ -2,11 +2,13 @@
 // This test checks that calling `mem::{uninitialized,zeroed}` with certain types results
 // in a lint.
 
-#![feature(never_type)]
+#![feature(never_type, rustc_attrs)]
 #![allow(deprecated)]
 #![deny(invalid_value)]
 
 use std::mem::{self, MaybeUninit};
+use std::ptr::NonNull;
+use std::num::NonZeroU32;
 
 enum Void {}
 
@@ -16,6 +18,11 @@ struct RefPair((&'static i32, i32));
 struct Wrap<T> { wrapped: T }
 enum WrapEnum<T> { Wrapped(T) }
 
+#[rustc_layout_scalar_valid_range_start(0)]
+#[rustc_layout_scalar_valid_range_end(128)]
+#[repr(transparent)]
+pub(crate) struct NonBig(u64);
+
 #[allow(unused)]
 fn generic<T: 'static>() {
     unsafe {
@@ -29,6 +36,7 @@ fn generic<T: 'static>() {
 
 fn main() {
     unsafe {
+        // Things that cannot even be zero.
         let _val: ! = mem::zeroed(); //~ ERROR: does not permit zero-initialization
         let _val: ! = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized
 
@@ -56,11 +64,28 @@ fn main() {
         let _val: Wrap<(RefPair, i32)> = mem::zeroed(); //~ ERROR: does not permit zero-initialization
         let _val: Wrap<(RefPair, i32)> = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized
 
-        // Some types that should work just fine.
+        let _val: NonNull<i32> = mem::zeroed(); //~ ERROR: does not permit zero-initialization
+        let _val: NonNull<i32> = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized
+
+        // Things that can be zero, but not uninit.
+        let _val: bool = mem::zeroed();
+        let _val: bool = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized
+
+        let _val: Wrap<char> = mem::zeroed();
+        let _val: Wrap<char> = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized
+
+        let _val: NonBig = mem::zeroed();
+        let _val: NonBig = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized
+
+        // Transmute-from-0
+        let _val: &'static i32 = mem::transmute(0usize); //~ ERROR: does not permit zero-initialization
+        let _val: &'static [i32] = mem::transmute((0usize, 0usize)); //~ ERROR: does not permit zero-initialization
+        let _val: NonZeroU32 = mem::transmute(0); //~ ERROR: does not permit zero-initialization
+
+        // Some more types that should work just fine.
         let _val: Option<&'static i32> = mem::zeroed();
         let _val: Option<fn()> = mem::zeroed();
         let _val: MaybeUninit<&'static i32> = mem::zeroed();
-        let _val: bool = mem::zeroed();
         let _val: i32 = mem::zeroed();
     }
 }