Rollup merge of #71663 - jumbatm:caller-handles-validation-error, r=R…

…alfJung Fix exceeding bitshifts not emitting for assoc. consts (properly this time, I swear!) Fixes #69021 and fixes #71353. As described in #71353 (comment), this PR: - adds a variant of `try_validation!` called `try_validation_pat!` that allows specific failures to be turned into validation failures (but returns the rest, unchanged), and - allows `InvalidProgram` to be returned out of validation r? @RalfJung
rust-lang · May 3, 2020 · 5b17290 · 5b17290
2 parents 6f5de87 + bd18ad4
commit 5b17290
Show file tree

Hide file tree

Showing 6 changed files with 263 additions and 216 deletions.
diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs
@@ -11,6 +11,7 @@ use std::ops::RangeInclusive;
 
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir as hir;
+use rustc_middle::mir::interpret::{InterpError, InterpErrorInfo};
 use rustc_middle::ty;
 use rustc_middle::ty::layout::TyAndLayout;
 use rustc_span::symbol::{sym, Symbol};
@@ -24,43 +25,71 @@ use super::{
 };
 
 macro_rules! throw_validation_failure {
-    ($what:expr, $where:expr, $details:expr) => {{
-        let mut msg = format!("encountered {}", $what);
-        let where_ = &$where;
-        if !where_.is_empty() {
-            msg.push_str(" at ");
-            write_path(&mut msg, where_);
-        }
-        write!(&mut msg, ", but expected {}", $details).unwrap();
-        throw_ub!(ValidationFailure(msg))
-    }};
-    ($what:expr, $where:expr) => {{
+    ($what:expr, $where:expr $(, $expected:expr )?) => {{
         let mut msg = format!("encountered {}", $what);
         let where_ = &$where;
         if !where_.is_empty() {
             msg.push_str(" at ");
             write_path(&mut msg, where_);
         }
+        $( write!(&mut msg, ", but expected {}", $expected).unwrap(); )?
         throw_ub!(ValidationFailure(msg))
     }};
 }
 
+/// Returns a validation failure for any Err value of $e.
+// FIXME: Replace all usages of try_validation! with try_validation_pat!.
 macro_rules! try_validation {
-    ($e:expr, $what:expr, $where:expr, $details:expr) => {{
-        match $e {
-            Ok(x) => x,
-            // We re-throw the error, so we are okay with allocation:
-            // this can only slow down builds that fail anyway.
-            Err(_) => throw_validation_failure!($what, $where, $details),
-        }
+    ($e:expr, $what:expr, $where:expr $(, $expected:expr )?) => {{
+        try_validation_pat!($e, $where, {
+            _ => { "{}", $what } $( expected { "{}", $expected } )?,
+        })
     }};
-
-    ($e:expr, $what:expr, $where:expr) => {{
+}
+/// Like try_validation, but will throw a validation error if any of the patterns in $p are
+/// matched. Other errors are passed back to the caller, unchanged. This lets you use the patterns
+/// as a kind of validation blacklist:
+///
+/// ```
+/// let v = try_validation_pat!(some_fn(), some_path, {
+///     Foo | Bar | Baz => { "some failure" },
+/// });
+/// // Failures that match $p are thrown up as validation errors, but other errors are passed back
+/// // unchanged.
+/// ```
+///
+/// An additional expected parameter can also be added to the failure message:
+///
+/// ```
+/// let v = try_validation_pat!(some_fn(), some_path, {
+///     Foo | Bar | Baz => { "some failure" } expected { "something that wasn't a failure" },
+/// });
+/// ```
+///
+/// An additional nicety is that both parameters actually take format args, so you can just write
+/// the format string in directly:
+///
+/// ```
+/// let v = try_validation_pat!(some_fn(), some_path, {
+///     Foo | Bar | Baz => { "{:?}", some_failure } expected { "{}", expected_value },
+/// });
+/// ```
+///
+macro_rules! try_validation_pat {
+    ($e:expr, $where:expr, { $( $p:pat )|+ =>
+        { $( $what_fmt:expr ),+ } $( expected { $( $expected_fmt:expr ),+ } )? $( , )?}) => {{
         match $e {
             Ok(x) => x,
-            // We re-throw the error, so we are okay with allocation:
-            // this can only slow down builds that fail anyway.
-            Err(_) => throw_validation_failure!($what, $where),
+            // We catch the error and turn it into a validation failure. We are okay with
+            // allocation here as this can only slow down builds that fail anyway.
+            $( Err(InterpErrorInfo { kind: $p, .. }) )|+ =>
+                throw_validation_failure!(
+                    format_args!($( $what_fmt ),+),
+                    $where
+                    $(, format_args!($( $expected_fmt ),+))?
+                ),
+            #[allow(unreachable_patterns)]
+            Err(e) => Err::<!, _>(e)?,
         }
     }};
 }
@@ -492,11 +521,9 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
                 // We are conservative with undef for integers, but try to
                 // actually enforce the strict rules for raw pointers (mostly because
                 // that lets us re-use `ref_to_mplace`).
-                let place = try_validation!(
-                    self.ecx.ref_to_mplace(self.ecx.read_immediate(value)?),
-                    "uninitialized raw pointer",
-                    self.path
-                );
+                let place = try_validation_pat!(self.ecx.ref_to_mplace(self.ecx.read_immediate(value)?), self.path, {
+                    err_ub!(InvalidUndefBytes(..)) => { "uninitialized raw pointer" },
+                });
                 if place.layout.is_unsized() {
                     self.check_wide_ptr_meta(place.meta, place.layout)?;
                 }
@@ -800,7 +827,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
 
                                 throw_validation_failure!("uninitialized bytes", self.path)
                             }
-                            // Other errors shouldn't be possible
+                            // Propagate upwards (that will also check for unexpected errors).
                             _ => return Err(err),
                         }
                     }
@@ -843,9 +870,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         // Run it.
         match visitor.visit_value(op) {
             Ok(()) => Ok(()),
-            // We should only get validation errors here. Avoid other errors as
-            // those do not show *where* in the value the issue lies.
+            // Pass through validation failures.
             Err(err) if matches!(err.kind, err_ub!(ValidationFailure { .. })) => Err(err),
+            // Also pass through InvalidProgram, those just indicate that we could not
+            // validate and each caller will know best what to do with them.
+            Err(err) if matches!(err.kind, InterpError::InvalidProgram(_)) => Err(err),
+            // Avoid other errors as those do not show *where* in the value the issue lies.
             Err(err) => bug!("Unexpected error during validation: {}", err),
         }
     }

diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs
@@ -549,11 +549,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
             return None;
         }
 
-        // FIXME we need to revisit this for #67176
-        if rvalue.needs_subst() {
-            return None;
-        }
-
         // Perform any special handling for specific Rvalue types.
         // Generally, checks here fall into one of two categories:
         //   1. Additional checking to provide useful lints to the user
@@ -594,6 +589,11 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
             _ => {}
         }
 
+        // FIXME we need to revisit this for #67176
+        if rvalue.needs_subst() {
+            return None;
+        }
+
         self.use_ecx(|this| {
             trace!("calling eval_rvalue_into_place(rvalue = {:?}, place = {:?})", rvalue, place);
             this.ecx.eval_rvalue_into_place(rvalue, place)?;

diff --git a/src/test/ui/lint/lint-exceeding-bitshifts.noopt.stderr b/src/test/ui/lint/lint-exceeding-bitshifts.noopt.stderr
@@ -1,146 +1,152 @@
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:22:13
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:17:20
    |
-LL |     let _ = x << 42;
-   |             ^^^^^^^ attempt to shift left with overflow
+LL |     const N: i32 = T::N << 42;
+   |                    ^^^^^^^^^^ attempt to shift left with overflow
    |
 note: the lint level is defined here
-  --> $DIR/lint-exceeding-bitshifts.rs:9:9
+  --> $DIR/lint-exceeding-bitshifts.rs:8:9
    |
-LL | #![deny(arithmetic_overflow, const_err)]
+LL | #![warn(arithmetic_overflow, const_err)]
    |         ^^^^^^^^^^^^^^^^^^^
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:27:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:21:13
+   |
+LL |     let _ = x << 42;
+   |             ^^^^^^^ attempt to shift left with overflow
+
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:26:15
    |
 LL |       let n = 1u8 << 8;
    |               ^^^^^^^^ attempt to shift left with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:29:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:28:15
    |
 LL |       let n = 1u16 << 16;
    |               ^^^^^^^^^^ attempt to shift left with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:31:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:30:15
    |
 LL |       let n = 1u32 << 32;
    |               ^^^^^^^^^^ attempt to shift left with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:33:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:32:15
    |
 LL |       let n = 1u64 << 64;
    |               ^^^^^^^^^^ attempt to shift left with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:35:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:34:15
    |
 LL |       let n = 1i8 << 8;
    |               ^^^^^^^^ attempt to shift left with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:37:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:36:15
    |
 LL |       let n = 1i16 << 16;
    |               ^^^^^^^^^^ attempt to shift left with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:39:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:38:15
    |
 LL |       let n = 1i32 << 32;
    |               ^^^^^^^^^^ attempt to shift left with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:41:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:40:15
    |
 LL |       let n = 1i64 << 64;
    |               ^^^^^^^^^^ attempt to shift left with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:44:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:43:15
    |
 LL |       let n = 1u8 >> 8;
    |               ^^^^^^^^ attempt to shift right with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:46:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:45:15
    |
 LL |       let n = 1u16 >> 16;
    |               ^^^^^^^^^^ attempt to shift right with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:48:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:47:15
    |
 LL |       let n = 1u32 >> 32;
    |               ^^^^^^^^^^ attempt to shift right with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:50:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:49:15
    |
 LL |       let n = 1u64 >> 64;
    |               ^^^^^^^^^^ attempt to shift right with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:52:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:51:15
    |
 LL |       let n = 1i8 >> 8;
    |               ^^^^^^^^ attempt to shift right with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:54:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:53:15
    |
 LL |       let n = 1i16 >> 16;
    |               ^^^^^^^^^^ attempt to shift right with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:56:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:55:15
    |
 LL |       let n = 1i32 >> 32;
    |               ^^^^^^^^^^ attempt to shift right with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:58:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:57:15
    |
 LL |       let n = 1i64 >> 64;
    |               ^^^^^^^^^^ attempt to shift right with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:62:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:61:15
    |
 LL |       let n = n << 8;
    |               ^^^^^^ attempt to shift left with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:64:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:63:15
    |
 LL |       let n = 1u8 << -8;
    |               ^^^^^^^^^ attempt to shift left with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:69:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:68:15
    |
 LL |       let n = 1u8 << (4+4);
    |               ^^^^^^^^^^^^ attempt to shift left with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:71:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:70:15
    |
 LL |       let n = 1i64 >> [64][0];
    |               ^^^^^^^^^^^^^^^ attempt to shift right with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:77:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:76:15
    |
 LL |       let n = 1_isize << BITS;
    |               ^^^^^^^^^^^^^^^ attempt to shift left with overflow
 
-error: this arithmetic operation will overflow
-  --> $DIR/lint-exceeding-bitshifts.rs:78:15
+warning: this arithmetic operation will overflow
+  --> $DIR/lint-exceeding-bitshifts.rs:77:15
    |
 LL |       let n = 1_usize << BITS;
    |               ^^^^^^^^^^^^^^^ attempt to shift left with overflow
 
-error: aborting due to 23 previous errors
+warning: 24 warnings emitted