Skip to content

Commit f65d3b5

Browse files
RalfJungRalfJung
committed
switch validation of scalars to be type-driven
This does not actually regress anything. It would regress NonNull, but we didn't handle that correctly previously either.
1 parent ff5a245 commit f65d3b5

12 files changed

+117
-135
lines changed

‎src/librustc_mir/interpret/validity.rs

Lines changed: 63 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use rustc::ty::layout::{self, Size};
1515
use rustc::ty::{self, Ty};
1616
use rustc_data_structures::fx::FxHashSet;
1717
use rustc::mir::interpret::{
18-
Scalar, AllocType, EvalResult, EvalErrorKind, PointerArithmetic
18+
Scalar, AllocType, EvalResult, EvalErrorKind
1919
};
2020

2121
use super::{
@@ -50,6 +50,13 @@ macro_rules! validation_failure {
5050
}
5151

5252
macro_rules! try_validation {
53+
($e:expr, $what:expr, $where:expr, $details:expr) => {{
54+
match $e {
55+
Ok(x) => x,
56+
Err(_) => return validation_failure!($what, $where, $details),
57+
}
58+
}};
59+
5360
($e:expr, $what:expr, $where:expr) => {{
5461
match $e {
5562
Ok(x) => x,
@@ -121,114 +128,65 @@ fn path_format(path: &Vec<PathElem>) -> String {
121128
out
122129
}
123130

131+
fn scalar_format(value: ScalarMaybeUndef) -> String {
132+
match value {
133+
ScalarMaybeUndef::Undef =>
134+
"uninitialized bytes".to_owned(),
135+
ScalarMaybeUndef::Scalar(Scalar::Ptr(_)) =>
136+
"a pointer".to_owned(),
137+
ScalarMaybeUndef::Scalar(Scalar::Bits { bits, .. }) =>
138+
bits.to_string(),
139+
}
140+
}
141+
124142
impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
125143
fn validate_scalar(
126144
&self,
127145
value: ScalarMaybeUndef,
128146
size: Size,
129-
scalar: &layout::Scalar,
130147
path: &Vec<PathElem>,
131148
ty: Ty,
132149
) -> EvalResult<'tcx> {
133-
trace!("validate scalar: {:#?}, {:#?}, {:#?}, {}", value, size, scalar, ty);
134-
let (lo, hi) = scalar.valid_range.clone().into_inner();
135-
136-
let value = match value {
137-
ScalarMaybeUndef::Scalar(scalar) => scalar,
138-
ScalarMaybeUndef::Undef => return validation_failure!("undefined bytes", path),
139-
};
140-
141-
let bits = match value {
142-
Scalar::Bits { bits, size: value_size } => {
143-
assert_eq!(value_size as u64, size.bytes());
144-
bits
145-
},
146-
Scalar::Ptr(_) => {
147-
match ty.sty {
148-
ty::Bool |
149-
ty::Char |
150-
ty::Float(_) |
151-
ty::Int(_) |
152-
ty::Uint(_) => {
153-
return validation_failure!(
154-
"a pointer",
155-
path,
156-
format!("the type {}", ty.sty)
157-
);
158-
}
159-
ty::RawPtr(_) |
160-
ty::Ref(_, _, _) |
161-
ty::FnPtr(_) => {}
162-
_ => { unreachable!(); }
163-
}
150+
trace!("validate scalar: {:#?}, {:#?}, {}", value, size, ty);
164151

165-
let ptr_size = self.pointer_size();
166-
let ptr_max = u128::max_value() >> (128 - ptr_size.bits());
167-
return if lo > hi {
168-
if lo - hi == 1 {
169-
// no gap, all values are ok
170-
Ok(())
171-
} else if hi < ptr_max || lo > 1 {
172-
let max = u128::max_value() >> (128 - size.bits());
173-
validation_failure!(
174-
"pointer",
175-
path,
176-
format!("something in the range {:?} or {:?}", 0..=lo, hi..=max)
177-
)
178-
} else {
179-
Ok(())
180-
}
181-
} else if hi < ptr_max || lo > 1 {
182-
validation_failure!(
183-
"pointer",
184-
path,
185-
format!("something in the range {:?}", scalar.valid_range)
186-
)
187-
} else {
188-
Ok(())
189-
};
190-
},
191-
};
192-
193-
// char gets a special treatment, because its number space is not contiguous so `TyLayout`
194-
// has no special checks for chars
152+
// Go over all the primitive types
195153
match ty.sty {
154+
ty::Bool => {
155+
try_validation!(value.to_bool(),
156+
scalar_format(value), path, "a boolean");
157+
},
196158
ty::Char => {
197-
debug_assert_eq!(size.bytes(), 4);
198-
if ::std::char::from_u32(bits as u32).is_none() {
199-
return validation_failure!(
200-
"character",
201-
path,
202-
"a valid unicode codepoint"
203-
);
204-
}
159+
try_validation!(value.to_char(),
160+
scalar_format(value), path, "a valid unicode codepoint");
161+
},
162+
ty::Float(_) | ty::Int(_) | ty::Uint(_) => {
163+
// Must be scalar bits
164+
try_validation!(value.to_bits(size),
165+
scalar_format(value), path, "initialized plain bits");
205166
}
206-
_ => {},
207-
}
208-
209-
use std::ops::RangeInclusive;
210-
let in_range = |bound: RangeInclusive<u128>| bound.contains(&bits);
211-
if lo > hi {
212-
if in_range(0..=hi) || in_range(lo..=u128::max_value()) {
213-
Ok(())
214-
} else {
215-
validation_failure!(
216-
bits,
217-
path,
218-
format!("something in the range {:?} or {:?}", ..=hi, lo..)
219-
)
167+
ty::RawPtr(_) => {
168+
// Anything but undef goes
169+
try_validation!(value.not_undef(),
170+
scalar_format(value), path, "a raw pointer");
171+
},
172+
ty::Ref(..) => {
173+
// This is checked by the recursive reference handling, nothing to do here.
174+
debug_assert!(ty.builtin_deref(true).is_some() && !ty.is_unsafe_ptr());
220175
}
221-
} else {
222-
if in_range(scalar.valid_range.clone()) {
223-
Ok(())
224-
} else {
225-
validation_failure!(
226-
bits,
227-
path,
228-
format!("something in the range {:?}", scalar.valid_range)
229-
)
176+
ty::FnPtr(_sig) => {
177+
let ptr = try_validation!(value.to_ptr(),
178+
scalar_format(value), path, "a pointer");
179+
let _fn = try_validation!(self.memory.get_fn(ptr),
180+
scalar_format(value), path, "a function pointer");
181+
// TODO: Check if the signature matches
182+
}
183+
ty::FnDef(..) => {
184+
// This is a zero-sized type with all relevant data sitting in the type.
185+
// There is nothing to validate.
230186
}
187+
_ => bug!("Unexpected primitive type {}", ty)
231188
}
189+
Ok(())
232190
}
233191

234192
/// Validate a reference, potentially recursively. `place` is assumed to already be
@@ -240,10 +198,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
240198
ref_tracking: Option<&mut RefTracking<'tcx>>,
241199
) -> EvalResult<'tcx> {
242200
// Before we do anything else, make sure this is entirely in-bounds.
201+
let (size, align) = self.size_and_align_of(place.extra, place.layout)?;
202+
try_validation!(self.memory.check_align(place.ptr, align),
203+
"unaligned reference", path);
243204
if !place.layout.is_zst() {
244205
let ptr = try_validation!(place.ptr.to_ptr(),
245206
"integer pointer in non-ZST reference", path);
246-
let size = self.size_and_align_of(place.extra, place.layout)?.0;
247207
try_validation!(self.memory.check_bounds(ptr, size, false),
248208
"dangling reference (not entirely in bounds)", path);
249209
// Skip recursion for some external statics
@@ -277,6 +237,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
277237
/// It will error if the bits at the destination do not match the ones described by the layout.
278238
/// The `path` may be pushed to, but the part that is present when the function
279239
/// starts must not be changed!
240+
///
241+
/// `ref_tracking` can be None to avoid recursive checking below references.
242+
/// This also toggles between "run-time" (no recursion) and "compile-time" (with recursion)
243+
/// validation (e.g., pointer values are fine in integers at runtime).
280244
pub fn validate_operand(
281245
&self,
282246
dest: OpTy<'tcx>,
@@ -346,12 +310,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
346310
return validation_failure!("a value of an uninhabited type", path),
347311
// check that the scalar is a valid pointer or that its bit range matches the
348312
// expectation.
349-
layout::Abi::Scalar(ref scalar_layout) => {
350-
let size = scalar_layout.value.size(self);
313+
layout::Abi::Scalar(_) => {
351314
let value = try_validation!(self.read_value(dest),
352315
"uninitialized or unrepresentable data", path);
353316
let scalar = value.to_scalar_or_undef();
354-
self.validate_scalar(scalar, size, scalar_layout, &path, dest.layout.ty)?;
317+
self.validate_scalar(scalar, dest.layout.size, &path, dest.layout.ty)?;
355318
// Recursively check *safe* references
356319
if dest.layout.ty.builtin_deref(true).is_some() &&
357320
!dest.layout.ty.is_unsafe_ptr()
@@ -365,7 +328,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
365328
layout::FieldPlacement::Arbitrary { .. }
366329
if dest.layout.ty.builtin_deref(true).is_some() =>
367330
{
368-
// This is a fat pointer.
331+
// This is a fat pointer. We also check fat raw pointers, their metadata must
332+
// be valid!
369333
let ptr = try_validation!(self.read_value(dest.into()),
370334
"undefined location in fat pointer", path);
371335
let ptr = try_validation!(self.ref_to_mplace(ptr),

‎src/test/ui/consts/const-eval/const-pointer-values-in-various-types.stderr

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error[E0080]: this constant likely exhibits undefined behavior
22
--> $DIR/const-pointer-values-in-various-types.rs:24:5
33
|
44
LL | const I32_REF_USIZE_UNION: usize = unsafe { Nonsense { int_32_ref: &3 }.u };
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected the type usize
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits
66
|
77
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
88

@@ -36,7 +36,7 @@ error[E0080]: this constant likely exhibits undefined behavior
3636
--> $DIR/const-pointer-values-in-various-types.rs:36:5
3737
|
3838
LL | const I32_REF_U64_UNION: u64 = unsafe { Nonsense { int_32_ref: &3 }.uint_64 };
39-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected the type u64
39+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits
4040
|
4141
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
4242

@@ -74,7 +74,7 @@ error[E0080]: this constant likely exhibits undefined behavior
7474
--> $DIR/const-pointer-values-in-various-types.rs:51:5
7575
|
7676
LL | const I32_REF_I64_UNION: i64 = unsafe { Nonsense { int_32_ref: &3 }.int_64 };
77-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected the type i64
77+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits
7878
|
7979
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
8080

@@ -96,7 +96,7 @@ error[E0080]: this constant likely exhibits undefined behavior
9696
--> $DIR/const-pointer-values-in-various-types.rs:60:5
9797
|
9898
LL | const I32_REF_F64_UNION: f64 = unsafe { Nonsense { int_32_ref: &3 }.float_64 };
99-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected the type f64
99+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits
100100
|
101101
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
102102

@@ -144,7 +144,7 @@ error[E0080]: this constant likely exhibits undefined behavior
144144
--> $DIR/const-pointer-values-in-various-types.rs:78:5
145145
|
146146
LL | const STR_U64_UNION: u64 = unsafe { Nonsense { stringy: "3" }.uint_64 };
147-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected the type u64
147+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits
148148
|
149149
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
150150

@@ -184,7 +184,7 @@ error[E0080]: this constant likely exhibits undefined behavior
184184
--> $DIR/const-pointer-values-in-various-types.rs:93:5
185185
|
186186
LL | const STR_I64_UNION: i64 = unsafe { Nonsense { stringy: "3" }.int_64 };
187-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected the type i64
187+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits
188188
|
189189
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
190190

@@ -208,7 +208,7 @@ error[E0080]: this constant likely exhibits undefined behavior
208208
--> $DIR/const-pointer-values-in-various-types.rs:102:5
209209
|
210210
LL | const STR_F64_UNION: f64 = unsafe { Nonsense { stringy: "3" }.float_64 };
211-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected the type f64
211+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits
212212
|
213213
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
214214

‎src/test/ui/consts/const-eval/transmute-const.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,5 @@ use std::mem;
1414

1515
static FOO: bool = unsafe { mem::transmute(3u8) };
1616
//~^ ERROR this static likely exhibits undefined behavior
17-
//~^^ type validation failed: encountered 3, but expected something in the range 0..=1
1817

1918
fn main() {}

‎src/test/ui/consts/const-eval/transmute-const.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error[E0080]: this static likely exhibits undefined behavior
22
--> $DIR/transmute-const.rs:15:1
33
|
44
LL | static FOO: bool = unsafe { mem::transmute(3u8) };
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3, but expected something in the range 0..=1
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3, but expected a boolean
66
|
77
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
88

‎src/test/ui/consts/const-eval/ub-enum.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ error[E0080]: this constant likely exhibits undefined behavior
1818
--> $DIR/ub-enum.rs:45:1
1919
|
2020
LL | const BAD_ENUM_CHAR : Option<(char, char)> = Some(('x', unsafe { TransmuteChar { a: !0 }.b }));
21-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered character at .Some.0.1, but expected a valid unicode codepoint
21+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 4294967295 at .Some.0.1, but expected a valid unicode codepoint
2222
|
2323
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
2424

‎src/test/ui/consts/const-eval/ub-usize-in-ref.rs renamed to ‎src/test/ui/consts/const-eval/ub-ref.rs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,17 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
union Foo {
12-
a: &'static u8,
13-
b: usize,
14-
}
11+
#![feature(const_transmute)]
1512

16-
const USIZE_AS_STATIC_REF: &'static u8 = unsafe { Foo { b: 1337 }.a};
13+
use std::mem;
14+
15+
const UNALIGNED: &u16 = unsafe { mem::transmute(&[0u8; 4]) };
16+
//~^ ERROR this constant likely exhibits undefined behavior
17+
18+
const REF_AS_USIZE: usize = unsafe { mem::transmute(&0) };
19+
//~^ ERROR this constant likely exhibits undefined behavior
20+
21+
const USIZE_AS_REF: &'static u8 = unsafe { mem::transmute(1337usize) };
1722
//~^ ERROR this constant likely exhibits undefined behavior
1823

19-
fn main() {
20-
}
24+
fn main() {}

‎src/test/ui/consts/const-eval/ub-ref.stderr

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
error[E0080]: this constant likely exhibits undefined behavior
2+
--> $DIR/ub-ref.rs:15:1
3+
|
4+
LL | const UNALIGNED: &u16 = unsafe { mem::transmute(&[0u8; 4]) };
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered unaligned reference
6+
|
7+
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
8+
9+
error[E0080]: this constant likely exhibits undefined behavior
10+
--> $DIR/ub-ref.rs:18:1
11+
|
12+
LL | const REF_AS_USIZE: usize = unsafe { mem::transmute(&0) };
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits
14+
|
15+
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
16+
17+
error[E0080]: this constant likely exhibits undefined behavior
18+
--> $DIR/ub-ref.rs:21:1
19+
|
20+
LL | const USIZE_AS_REF: &'static u8 = unsafe { mem::transmute(1337usize) };
21+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered integer pointer in non-ZST reference
22+
|
23+
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior
24+
25+
error: aborting due to 3 previous errors
26+
27+
For more information about this error, try `rustc --explain E0080`.

0 commit comments

Comments
 (0)