]> git.lizzy.rs Git - rust.git/commitdiff
Validate constants during `const_eval_raw`
authorOliver Scherer <github35764891676564198441@oli-obk.de>
Thu, 30 Jul 2020 15:58:39 +0000 (17:58 +0200)
committerOliver Scherer <github35764891676564198441@oli-obk.de>
Sat, 19 Sep 2020 08:36:36 +0000 (10:36 +0200)
compiler/rustc_mir/src/const_eval/eval_queries.rs
compiler/rustc_mir/src/interpret/validity.rs
src/test/ui/consts/const-eval/double_check2.rs
src/test/ui/consts/const-eval/double_check2.stderr [deleted file]

index 72151df7230be2df821d4720625863bc59a671c6..dd2731fb0a0166b459c7e777eaca264a379afd8a 100644 (file)
@@ -193,21 +193,7 @@ fn validate_and_turn_into_const<'tcx>(
     let ecx = mk_eval_cx(tcx, tcx.def_span(key.value.instance.def_id()), key.param_env, is_static);
     let val = (|| {
         let mplace = ecx.raw_const_to_mplace(constant)?;
-
-        // FIXME do not validate promoteds until a decision on
-        // https://github.com/rust-lang/rust/issues/67465 is made
-        if cid.promoted.is_none() {
-            let mut ref_tracking = RefTracking::new(mplace);
-            while let Some((mplace, path)) = ref_tracking.todo.pop() {
-                ecx.const_validate_operand(
-                    mplace.into(),
-                    path,
-                    &mut ref_tracking,
-                    /*may_ref_to_static*/ ecx.memory.extra.can_access_statics,
-                )?;
-            }
-        }
-        // Now that we validated, turn this into a proper constant.
+        // Turn this into a proper constant.
         // Statics/promoteds are always `ByRef`, for the rest `op_to_const` decides
         // whether they become immediates.
         if is_static || cid.promoted.is_some() {
@@ -221,6 +207,7 @@ fn validate_and_turn_into_const<'tcx>(
         }
     })();
 
+    // FIXME: Can this ever be an error and not be a compiler bug or can we just ICE here?
     val.map_err(|error| {
         let err = ConstEvalErr::new(&ecx, error, None);
         err.struct_error(ecx.tcx, "it is undefined behavior to use this value", |mut diag| {
@@ -319,7 +306,6 @@ pub fn const_eval_raw_provider<'tcx>(
 
     let res = ecx.load_mir(cid.instance.def, cid.promoted);
     res.and_then(|body| eval_body_using_ecx(&mut ecx, cid, &body))
-        .map(|place| RawConst { alloc_id: place.ptr.assert_ptr().alloc_id, ty: place.layout.ty })
         .map_err(|error| {
             let err = ConstEvalErr::new(&ecx, error, None);
             // errors in statics are always emitted as fatal errors
@@ -397,4 +383,37 @@ pub fn const_eval_raw_provider<'tcx>(
                 err.report_as_error(ecx.tcx.at(ecx.cur_span()), "could not evaluate constant")
             }
         })
+        .and_then(|mplace| {
+            // Since evaluation had no errors, valiate the resulting constant:
+            let validation = try {
+                // FIXME do not validate promoteds until a decision on
+                // https://github.com/rust-lang/rust/issues/67465 is made
+                if cid.promoted.is_none() {
+                    let mut ref_tracking = RefTracking::new(mplace);
+                    while let Some((mplace, path)) = ref_tracking.todo.pop() {
+                        ecx.const_validate_operand(
+                            mplace.into(),
+                            path,
+                            &mut ref_tracking,
+                            /*may_ref_to_static*/ ecx.memory.extra.can_access_statics,
+                        )?;
+                    }
+                }
+            };
+            if let Err(error) = validation {
+                // Validation failed, report an error
+                let err = ConstEvalErr::new(&ecx, error, None);
+                Err(err.struct_error(
+                    ecx.tcx,
+                    "it is undefined behavior to use this value",
+                    |mut diag| {
+                        diag.note(note_on_undefined_behavior_error());
+                        diag.emit();
+                    },
+                ))
+            } else {
+                // Convert to raw constant
+                Ok(RawConst { alloc_id: mplace.ptr.assert_ptr().alloc_id, ty: mplace.layout.ty })
+            }
+        })
 }
index ca62f0347ffacc6aef380e43e0f3c611d53ee80e..7d9507c08fa1849637ae0432415057bd400c9f98 100644 (file)
@@ -425,26 +425,28 @@ fn check_safe_pointer(
                 let alloc_kind = self.ecx.tcx.get_global_alloc(ptr.alloc_id);
                 if let Some(GlobalAlloc::Static(did)) = alloc_kind {
                     assert!(!self.ecx.tcx.is_thread_local_static(did));
+                    assert!(self.ecx.tcx.is_static(did));
                     // See const_eval::machine::MemoryExtra::can_access_statics for why
                     // this check is so important.
                     // This check is reachable when the const just referenced the static,
                     // but never read it (so we never entered `before_access_global`).
                     // We also need to do it here instead of going on to avoid running
                     // into the `before_access_global` check during validation.
-                    if !self.may_ref_to_static && self.ecx.tcx.is_static(did) {
+                    if !self.may_ref_to_static {
                         throw_validation_failure!(self.path,
                             { "a {} pointing to a static variable", kind }
                         );
                     }
-                    // `extern static` cannot be validated as they have no body.
-                    // FIXME: Statics from other crates are also skipped.
-                    // They might be checked at a different type, but for now we
-                    // want to avoid recursing too deeply.  We might miss const-invalid data,
+                    // We skip checking other statics. These statics must be sound by themselves,
+                    // and the only way to get broken statics here is by using unsafe code.
+                    // The reasons we don't check other statics is twofold. For one, in all sound
+                    // cases, the static was already validated on its own, and second, we trigger
+                    // cycle errors if we try to compute the value of the other static and that
+                    // static refers back to us.
+                    // We might miss const-invalid data,
                     // but things are still sound otherwise (in particular re: consts
                     // referring to statics).
-                    if !did.is_local() || self.ecx.tcx.is_foreign_item(did) {
-                        return Ok(());
-                    }
+                    return Ok(());
                 }
             }
             // Proceed recursively even for ZST, no reason to skip them!
index 8402d628856648f0d9ec022fc9a50a77e17e14cb..e1f3e5bb27a8f730631c9df4a64053e9a0b07b16 100644 (file)
@@ -1,3 +1,11 @@
+// check-pass
+
+// This test exhibits undefined behavior, but it is impossible to prevent generally during
+// const eval, even if possible to prevent in the cases here. The reason it's impossible in general
+// is that we run into query cycles even *without* UB, just because we're checking for UB.
+// We do not detect it if you create references to statics
+// in ways that are UB.
+
 enum Foo {
     A = 5,
     B = 42,
@@ -13,11 +21,14 @@ union Union {
     u8: &'static u8,
 }
 static BAR: u8 = 5;
-static FOO: (&Foo, &Bar) = unsafe {( //~ undefined behavior
-    Union { u8: &BAR }.foo,
-    Union { u8: &BAR }.bar,
-)};
-static FOO2: (&Foo, &Bar) = unsafe {(std::mem::transmute(&BAR), std::mem::transmute(&BAR))};
-//~^ undefined behavior
+static FOO: (&Foo, &Bar) = unsafe {
+    (
+        // undefined behavior
+        Union { u8: &BAR }.foo,
+        Union { u8: &BAR }.bar,
+    )
+};
+static FOO2: (&Foo, &Bar) = unsafe { (std::mem::transmute(&BAR), std::mem::transmute(&BAR)) };
+//^ undefined behavior
 
 fn main() {}
diff --git a/src/test/ui/consts/const-eval/double_check2.stderr b/src/test/ui/consts/const-eval/double_check2.stderr
deleted file mode 100644 (file)
index 84f6080..0000000
+++ /dev/null
@@ -1,22 +0,0 @@
-error[E0080]: it is undefined behavior to use this value
-  --> $DIR/double_check2.rs:16:1
-   |
-LL | / static FOO: (&Foo, &Bar) = unsafe {(
-LL | |     Union { u8: &BAR }.foo,
-LL | |     Union { u8: &BAR }.bar,
-LL | | )};
-   | |___^ type validation failed: encountered 0x05 at .1.<deref>.<enum-tag>, but expected a valid enum tag
-   |
-   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
-
-error[E0080]: it is undefined behavior to use this value
-  --> $DIR/double_check2.rs:20:1
-   |
-LL | static FOO2: (&Foo, &Bar) = unsafe {(std::mem::transmute(&BAR), std::mem::transmute(&BAR))};
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0x05 at .1.<deref>.<enum-tag>, but expected a valid enum tag
-   |
-   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
-
-error: aborting due to 2 previous errors
-
-For more information about this error, try `rustc --explain E0080`.