miri: make read_discriminant UB when the tag is not in the validity range of the tag field

Author: RalfJung

compiler/rustc_const_eval/src/interpret/discriminant.rs

@@ -111,23 +111,30 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                     .try_to_scalar_int()
                     .map_err(|dbg_val| err_ub!(InvalidTag(dbg_val)))?
                     .to_bits(tag_layout.size);
+                // Ensure the tag is in its layout range. Codegen adds range metadata on the
+                // discriminant load so we really have to make this UB.
+                if !tag_scalar_layout.valid_range(self).contains(tag_bits) {
+                    throw_ub!(InvalidTag(Scalar::from_uint(tag_bits, tag_layout.size)))
+                }
                 // Cast bits from tag layout to discriminant layout.
                 // After the checks we did above, this cannot fail, as
                 // discriminants are int-like.
                 let discr_val = self.int_to_int_or_float(&tag_val, discr_layout).unwrap();
                 let discr_bits = discr_val.to_scalar().to_bits(discr_layout.size)?;
-                // Convert discriminant to variant index, and catch invalid discriminants.
+                // Convert discriminant to variant index. Since we validated the tag against the
+                // layout range above, this cannot fail.
                 let index = match *ty.kind() {
                     ty::Adt(adt, _) => {
-                        adt.discriminants(*self.tcx).find(|(_, var)| var.val == discr_bits)
+                        adt.discriminants(*self.tcx).find(|(_, var)| var.val == discr_bits).unwrap()
                     }
                     ty::Coroutine(def_id, args) => {
                         let args = args.as_coroutine();
-                        args.discriminants(def_id, *self.tcx).find(|(_, var)| var.val == discr_bits)
+                        args.discriminants(def_id, *self.tcx)
+                            .find(|(_, var)| var.val == discr_bits)
+                            .unwrap()
                     }
                     _ => span_bug!(self.cur_span(), "tagged layout for non-adt non-coroutine"),
-                }
-                .ok_or_else(|| err_ub!(InvalidTag(Scalar::from_uint(tag_bits, tag_layout.size))))?;
+                };
                 // Return the cast value, and the index.
                 index.0
             }
@@ -174,13 +181,22 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                             let variants =
                                 ty.ty_adt_def().expect("tagged layout for non adt").variants();
                             assert!(variant_index < variants.next_index());
+                            // This should imply that the tag is in its layout range.
+                            assert!(tag_scalar_layout.valid_range(self).contains(tag_bits));
+
                             if variant_index == untagged_variant {
                                 // The untagged variant can be in the niche range, but even then it
-                                // is not a valid encoding.
+                                // is not a valid encoding. Codegen inserts an `assume` here
+                                // so we really have to make this UB.
                                 throw_ub!(InvalidTag(Scalar::from_uint(tag_bits, tag_layout.size)))
                             }
                             variant_index
                         } else {
+                            // Ensure the tag is in its layout range. Codegen adds range metadata on
+                            // the discriminant load so we really have to make this UB.
+                            if !tag_scalar_layout.valid_range(self).contains(tag_bits) {
+                                throw_ub!(InvalidTag(Scalar::from_uint(tag_bits, tag_layout.size)))
+                            }
                             untagged_variant
                         }
                     }

src/tools/miri/tests/fail/enum-untagged-variant-invalid-encoding.rs

@@ -20,8 +20,8 @@ fn main() {
         assert!(Foo::Var1 == mem::transmute(2u8));
         assert!(Foo::Var3 == mem::transmute(4u8));
 
-        let invalid: Foo = mem::transmute(3u8);
-        assert!(matches!(invalid, Foo::Var2(_)));
+        let invalid: *const Foo = mem::transmute(&3u8);
+        assert!(matches!(*invalid, Foo::Var2(_)));
         //~^ ERROR: invalid tag
     }
 }

src/tools/miri/tests/fail/enum-untagged-variant-invalid-encoding.stderr

@@ -1,8 +1,8 @@
 error: Undefined Behavior: enum value has invalid tag: 0x03
   --> tests/fail/enum-untagged-variant-invalid-encoding.rs:LL:CC
    |
-LL |         assert!(matches!(invalid, Foo::Var2(_)));
-   |                          ^^^^^^^ Undefined Behavior occurred here
+LL |         assert!(matches!(*invalid, Foo::Var2(_)));
+   |                          ^^^^^^^^ Undefined Behavior occurred here
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

src/tools/miri/tests/fail/validity/invalid_enum_op_discr_uninhabited.rs

@@ -0,0 +1,27 @@
+// Make sure we find these even with many checks disabled.
+//@compile-flags: -Zmiri-disable-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-disable-validation
+#![feature(never_type)]
+
+enum Never {}
+
+// An enum with 4 variants of which only some are uninhabited -- so the uninhabited variants *do*
+// have a discriminant.
+#[allow(unused)]
+enum UninhDiscriminant {
+    A,
+    B(!),
+    C,
+    D(Never),
+}
+
+fn main() {
+    unsafe {
+        let x = 3u8;
+        let x_ptr: *const u8 = &x;
+        let cast_ptr = x_ptr as *const UninhDiscriminant;
+        // Reading the discriminant should fail since the tag value is not in the valid
+        // range for the tag field.
+        let _val = matches!(*cast_ptr, UninhDiscriminant::A);
+        //~^ ERROR: invalid tag
+    }
+}

src/tools/miri/tests/fail/validity/invalid_enum_op_discr_uninhabited.stderr

@@ -0,0 +1,13 @@
+error: Undefined Behavior: enum value has invalid tag: 0x03
+  --> tests/fail/validity/invalid_enum_op_discr_uninhabited.rs:LL:CC
+   |
+LL |         let _val = matches!(*cast_ptr, UninhDiscriminant::A);
+   |                             ^^^^^^^^^ Undefined Behavior occurred here
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

src/tools/miri/tests/fail/validity/invalid_enum_op_niche_out_of_range.rs

@@ -0,0 +1,14 @@
+// Make sure we find these even with many checks disabled.
+//@compile-flags: -Zmiri-disable-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-disable-validation
+
+fn main() {
+    unsafe {
+        let x = 12u8;
+        let x_ptr: *const u8 = &x;
+        let cast_ptr = x_ptr as *const Option<bool>;
+        // Reading the discriminant should fail since the tag value is not in the valid
+        // range for the tag field.
+        let _val = matches!(*cast_ptr, None);
+        //~^ ERROR: invalid tag
+    }
+}

src/tools/miri/tests/fail/validity/invalid_enum_op_niche_out_of_range.stderr

@@ -0,0 +1,13 @@
+error: Undefined Behavior: enum value has invalid tag: 0x0c
+  --> tests/fail/validity/invalid_enum_op_niche_out_of_range.rs:LL:CC
+   |
+LL |         let _val = matches!(*cast_ptr, None);
+   |                             ^^^^^^^^^ Undefined Behavior occurred here
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

tests/ui/consts/const-eval/raw-bytes.64bit.stderr

@@ -31,7 +31,7 @@ LL | const BAD_UNINHABITED_VARIANT1: UninhDiscriminant = unsafe { mem::transmute
                01                                              │ .
            }
 
-error[E0080]: constructing invalid value at .<enum-tag>: encountered an uninhabited enum variant
+error[E0080]: constructing invalid value at .<enum-tag>: encountered 0x03, but expected a valid enum tag
   --> $DIR/raw-bytes.rs:47:1
    |
 LL | const BAD_UNINHABITED_VARIANT2: UninhDiscriminant = unsafe { mem::transmute(3u8) };

tests/ui/consts/const-eval/ub-enum.rs

@@ -83,7 +83,7 @@ const GOOD_INHABITED_VARIANT2: UninhDiscriminant = unsafe { mem::transmute(2u8)
 const BAD_UNINHABITED_VARIANT1: UninhDiscriminant = unsafe { mem::transmute(1u8) };
 //~^ ERROR uninhabited enum variant
 const BAD_UNINHABITED_VARIANT2: UninhDiscriminant = unsafe { mem::transmute(3u8) };
-//~^ ERROR uninhabited enum variant
+//~^ ERROR expected a valid enum tag
 
 // # other
 

tests/ui/consts/const-eval/ub-enum.stderr

@@ -86,7 +86,7 @@ LL | const BAD_UNINHABITED_VARIANT1: UninhDiscriminant = unsafe { mem::transmute
                HEX_DUMP
            }
 
-error[E0080]: constructing invalid value at .<enum-tag>: encountered an uninhabited enum variant
+error[E0080]: constructing invalid value at .<enum-tag>: encountered 0x03, but expected a valid enum tag
   --> $DIR/ub-enum.rs:85:1
    |
 LL | const BAD_UNINHABITED_VARIANT2: UninhDiscriminant = unsafe { mem::transmute(3u8) };