Skip to content

Commit 5999ec3

Browse files
Rollup merge of #153408 - RalfJung:tag-read-must-be-valid, r=oli-obk
miri: make read_discriminant UB when the tag is not in the validity range of the tag field Arguably, reading an enum discriminant is an operation that uses the "type" of the discriminant field -- and therefore it should fail when the value in that field isn't valid at that type. Therefore, code like this should be UB: ```rust 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 } } ``` However, Miri currently sees no UB here. (MiniRust does see UB.) This is because we never actually check whether the tag we read is in the validity range for its field. So let's add such a check, and a corresponding test. In fact, we have to do this check, since the codegen backend adds range metadata on the discriminant load, as can be seen in [this example](https://play.rust-lang.org/?version=stable&mode=release&edition=2024&gist=02ef5e80fdfe61540e44198dd827b630). In other words, the above code has UB in LLVM IR but not in Miri, which is a critical Miri bug.
2 parents 2263a8e + b840338 commit 5999ec3

11 files changed

Lines changed: 97 additions & 14 deletions

compiler/rustc_const_eval/src/interpret/discriminant.rs

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,23 +111,30 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
111111
.try_to_scalar_int()
112112
.map_err(|dbg_val| err_ub!(InvalidTag(dbg_val)))?
113113
.to_bits(tag_layout.size);
114+
// Ensure the tag is in its layout range. Codegen adds range metadata on the
115+
// discriminant load so we really have to make this UB.
116+
if !tag_scalar_layout.valid_range(self).contains(tag_bits) {
117+
throw_ub!(InvalidTag(Scalar::from_uint(tag_bits, tag_layout.size)))
118+
}
114119
// Cast bits from tag layout to discriminant layout.
115120
// After the checks we did above, this cannot fail, as
116121
// discriminants are int-like.
117122
let discr_val = self.int_to_int_or_float(&tag_val, discr_layout).unwrap();
118123
let discr_bits = discr_val.to_scalar().to_bits(discr_layout.size)?;
119-
// Convert discriminant to variant index, and catch invalid discriminants.
124+
// Convert discriminant to variant index. Since we validated the tag against the
125+
// layout range above, this cannot fail.
120126
let index = match *ty.kind() {
121127
ty::Adt(adt, _) => {
122-
adt.discriminants(*self.tcx).find(|(_, var)| var.val == discr_bits)
128+
adt.discriminants(*self.tcx).find(|(_, var)| var.val == discr_bits).unwrap()
123129
}
124130
ty::Coroutine(def_id, args) => {
125131
let args = args.as_coroutine();
126-
args.discriminants(def_id, *self.tcx).find(|(_, var)| var.val == discr_bits)
132+
args.discriminants(def_id, *self.tcx)
133+
.find(|(_, var)| var.val == discr_bits)
134+
.unwrap()
127135
}
128136
_ => span_bug!(self.cur_span(), "tagged layout for non-adt non-coroutine"),
129-
}
130-
.ok_or_else(|| err_ub!(InvalidTag(Scalar::from_uint(tag_bits, tag_layout.size))))?;
137+
};
131138
// Return the cast value, and the index.
132139
index.0
133140
}
@@ -174,13 +181,22 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
174181
let variants =
175182
ty.ty_adt_def().expect("tagged layout for non adt").variants();
176183
assert!(variant_index < variants.next_index());
184+
// This should imply that the tag is in its layout range.
185+
assert!(tag_scalar_layout.valid_range(self).contains(tag_bits));
186+
177187
if variant_index == untagged_variant {
178188
// The untagged variant can be in the niche range, but even then it
179-
// is not a valid encoding.
189+
// is not a valid encoding. Codegen inserts an `assume` here
190+
// so we really have to make this UB.
180191
throw_ub!(InvalidTag(Scalar::from_uint(tag_bits, tag_layout.size)))
181192
}
182193
variant_index
183194
} else {
195+
// Ensure the tag is in its layout range. Codegen adds range metadata on
196+
// the discriminant load so we really have to make this UB.
197+
if !tag_scalar_layout.valid_range(self).contains(tag_bits) {
198+
throw_ub!(InvalidTag(Scalar::from_uint(tag_bits, tag_layout.size)))
199+
}
184200
untagged_variant
185201
}
186202
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ fn main() {
2020
assert!(Foo::Var1 == mem::transmute(2u8));
2121
assert!(Foo::Var3 == mem::transmute(4u8));
2222

23-
let invalid: Foo = mem::transmute(3u8);
24-
assert!(matches!(invalid, Foo::Var2(_)));
23+
let invalid: *const Foo = mem::transmute(&3u8);
24+
assert!(matches!(*invalid, Foo::Var2(_)));
2525
//~^ ERROR: invalid tag
2626
}
2727
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
error: Undefined Behavior: enum value has invalid tag: 0x03
22
--> tests/fail/enum-untagged-variant-invalid-encoding.rs:LL:CC
33
|
4-
LL | assert!(matches!(invalid, Foo::Var2(_)));
5-
| ^^^^^^^ Undefined Behavior occurred here
4+
LL | assert!(matches!(*invalid, Foo::Var2(_)));
5+
| ^^^^^^^^ Undefined Behavior occurred here
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Make sure we find these even with many checks disabled.
2+
//@compile-flags: -Zmiri-disable-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-disable-validation
3+
#![feature(never_type)]
4+
5+
enum Never {}
6+
7+
// An enum with 4 variants of which only some are uninhabited -- so the uninhabited variants *do*
8+
// have a discriminant.
9+
#[allow(unused)]
10+
enum UninhDiscriminant {
11+
A,
12+
B(!),
13+
C,
14+
D(Never),
15+
}
16+
17+
fn main() {
18+
unsafe {
19+
let x = 3u8;
20+
let x_ptr: *const u8 = &x;
21+
let cast_ptr = x_ptr as *const UninhDiscriminant;
22+
// Reading the discriminant should fail since the tag value is not in the valid
23+
// range for the tag field.
24+
let _val = matches!(*cast_ptr, UninhDiscriminant::A);
25+
//~^ ERROR: invalid tag
26+
}
27+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
error: Undefined Behavior: enum value has invalid tag: 0x03
2+
--> tests/fail/validity/invalid_enum_op_discr_uninhabited.rs:LL:CC
3+
|
4+
LL | let _val = matches!(*cast_ptr, UninhDiscriminant::A);
5+
| ^^^^^^^^^ Undefined Behavior occurred here
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
10+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
11+
12+
error: aborting due to 1 previous error
13+
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Make sure we find these even with many checks disabled.
2+
//@compile-flags: -Zmiri-disable-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-disable-validation
3+
4+
fn main() {
5+
unsafe {
6+
let x = 12u8;
7+
let x_ptr: *const u8 = &x;
8+
let cast_ptr = x_ptr as *const Option<bool>;
9+
// Reading the discriminant should fail since the tag value is not in the valid
10+
// range for the tag field.
11+
let _val = matches!(*cast_ptr, None);
12+
//~^ ERROR: invalid tag
13+
}
14+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
error: Undefined Behavior: enum value has invalid tag: 0x0c
2+
--> tests/fail/validity/invalid_enum_op_niche_out_of_range.rs:LL:CC
3+
|
4+
LL | let _val = matches!(*cast_ptr, None);
5+
| ^^^^^^^^^ Undefined Behavior occurred here
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
10+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
11+
12+
error: aborting due to 1 previous error
13+

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ LL | const BAD_UNINHABITED_VARIANT1: UninhDiscriminant = unsafe { mem::transmute
3131
01 │ .
3232
}
3333

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ LL | const BAD_UNINHABITED_VARIANT1: UninhDiscriminant = unsafe { mem::transmute
3131
01 │ .
3232
}
3333

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ const GOOD_INHABITED_VARIANT2: UninhDiscriminant = unsafe { mem::transmute(2u8)
8383
const BAD_UNINHABITED_VARIANT1: UninhDiscriminant = unsafe { mem::transmute(1u8) };
8484
//~^ ERROR uninhabited enum variant
8585
const BAD_UNINHABITED_VARIANT2: UninhDiscriminant = unsafe { mem::transmute(3u8) };
86-
//~^ ERROR uninhabited enum variant
86+
//~^ ERROR expected a valid enum tag
8787

8888
// # other
8989

0 commit comments

Comments
 (0)