Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add -Zextra-const-ub-checks to enable more UB checking in const-eval #100229

Merged
merged 3 commits into from
Aug 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,16 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,

const PANIC_ON_ALLOC_FAIL: bool = false; // will be raised as a proper error

#[inline(always)]
fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
ecx.tcx.sess.opts.unstable_opts.extra_const_ub_checks
}

#[inline(always)]
fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
ecx.tcx.sess.opts.unstable_opts.extra_const_ub_checks
}
RalfJung marked this conversation as resolved.
Show resolved Hide resolved

fn load_mir(
ecx: &InterpCx<'mir, 'tcx, Self>,
instance: ty::InstanceDef<'tcx>,
Expand Down
12 changes: 0 additions & 12 deletions compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,24 +436,12 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
type AllocExtra = ();
type FrameExtra = ();

#[inline(always)]
fn enforce_alignment(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool {
// We do not check for alignment to avoid having to carry an `Align`
// in `ConstValue::ByRef`.
false
}

#[inline(always)]
fn force_int_for_alignment_check(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool {
// We do not support `force_int`.
false
}

#[inline(always)]
fn enforce_validity(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool {
false // for now, we don't enforce validity
}

#[inline(always)]
fn enforce_number_init(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool {
true
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
/// It will error if the bits at the destination do not match the ones described by the layout.
#[inline(always)]
pub fn validate_operand(&self, op: &OpTy<'tcx, M::Provenance>) -> InterpResult<'tcx> {
// Note that we *could* actually be in CTFE here with `-Zextra-const-ub-checks`, but it's
// still correct to not use `ctfe_mode`: that mode is for validation of the final constant
// value, it rules out things like `UnsafeCell` in awkward places. It also can make checking
// recurse through references which, for now, we don't want here, either.
self.validate_operand_internal(op, vec![], None, None)
}
}
12 changes: 12 additions & 0 deletions compiler/rustc_mir_transform/src/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,18 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx>

type MemoryKind = !;

#[inline(always)]
fn enforce_alignment(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
// We do not check for alignment to avoid having to carry an `Align`
// in `ConstValue::ByRef`.
false
}

#[inline(always)]
fn enforce_validity(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
false // for now, we don't enforce validity
}

fn load_mir(
_ecx: &InterpCx<'mir, 'tcx, Self>,
_instance: ty::InstanceDef<'tcx>,
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1310,6 +1310,8 @@ options! {
"emit the bc module with thin LTO info (default: yes)"),
export_executable_symbols: bool = (false, parse_bool, [TRACKED],
"export symbols from executables, as if they were dynamic libraries"),
extra_const_ub_checks: bool = (false, parse_bool, [TRACKED],
"turns on more checks to detect const UB, which can be slow (default: no)"),
#[cfg_attr(not(bootstrap), rustc_lint_opt_deny_field_access("use `Session::fewer_names` instead of this field"))]
fewer_names: Option<bool> = (None, parse_opt_bool, [TRACKED],
"reduce memory use by retaining fewer names within compilation artifacts (LLVM-IR) \
Expand Down
1 change: 1 addition & 0 deletions src/test/rustdoc-ui/z-help.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
-Z emit-stack-sizes=val -- emit a section containing stack size metadata (default: no)
-Z emit-thin-lto=val -- emit the bc module with thin LTO info (default: yes)
-Z export-executable-symbols=val -- export symbols from executables, as if they were dynamic libraries
-Z extra-const-ub-checks=val -- turns on more checks to detect const UB, which can be slow (default: no)
-Z fewer-names=val -- reduce memory use by retaining fewer names within compilation artifacts (LLVM-IR) (default: no)
-Z force-unstable-if-unmarked=val -- force all crates to be `rustc_private` unstable (default: no)
-Z fuel=val -- set the optimization fuel quota for a crate
Expand Down
45 changes: 45 additions & 0 deletions src/test/ui/consts/extra-const-ub/detect-extra-ub.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// revisions: no_flag with_flag
// [no_flag] check-pass
// [with_flag] compile-flags: -Zextra-const-ub-checks
#![feature(const_ptr_read)]

use std::mem::transmute;

const INVALID_BOOL: () = unsafe {
let _x: bool = transmute(3u8);
//[with_flag]~^ ERROR: evaluation of constant value failed
//[with_flag]~| invalid value
};

const INVALID_PTR_IN_INT: () = unsafe {
let _x: usize = transmute(&3u8);
//[with_flag]~^ ERROR: evaluation of constant value failed
//[with_flag]~| invalid value
};

const INVALID_SLICE_TO_USIZE_TRANSMUTE: () = unsafe {
let x: &[u8] = &[0; 32];
let _x: (usize, usize) = transmute(x);
//[with_flag]~^ ERROR: evaluation of constant value failed
//[with_flag]~| invalid value
};

const UNALIGNED_PTR: () = unsafe {
let _x: &u32 = transmute(&[0u8; 4]);
//[with_flag]~^ ERROR: evaluation of constant value failed
//[with_flag]~| invalid value
};

const UNALIGNED_READ: () = {
INNER; //[with_flag]~ERROR any use of this value will cause an error
//[with_flag]~| previously accepted
// There is an error here but its span is in the standard library so we cannot match it...
// so we have this in a *nested* const, such that the *outer* const fails to use it.
const INNER: () = unsafe {
let x = &[0u8; 4];
let ptr = x.as_ptr().cast::<u32>();
ptr.read();
};
};

fn main() {}
71 changes: 71 additions & 0 deletions src/test/ui/consts/extra-const-ub/detect-extra-ub.with_flag.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
error[E0080]: evaluation of constant value failed
--> $DIR/detect-extra-ub.rs:9:20
|
LL | let _x: bool = transmute(3u8);
| ^^^^^^^^^^^^^^ constructing invalid value: encountered 0x03, but expected a boolean

error[E0080]: evaluation of constant value failed
--> $DIR/detect-extra-ub.rs:15:21
|
LL | let _x: usize = transmute(&3u8);
| ^^^^^^^^^^^^^^^ constructing invalid value: encountered (potentially part of) a pointer, but expected plain (non-pointer) bytes

error[E0080]: evaluation of constant value failed
--> $DIR/detect-extra-ub.rs:22:30
|
LL | let _x: (usize, usize) = transmute(x);
| ^^^^^^^^^^^^ constructing invalid value at .0: encountered (potentially part of) a pointer, but expected plain (non-pointer) bytes

error[E0080]: evaluation of constant value failed
--> $DIR/detect-extra-ub.rs:28:20
|
LL | let _x: &u32 = transmute(&[0u8; 4]);
| ^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered an unaligned reference (required 4 byte alignment but found 1)

error[E0080]: evaluation of constant value failed
--> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
|
LL | copy_nonoverlapping(src, tmp.as_mut_ptr(), 1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| accessing memory with alignment 1, but alignment 4 is required
| inside `std::ptr::read::<u32>` at $SRC_DIR/core/src/ptr/mod.rs:LL:COL
|
::: $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
|
LL | unsafe { read(self) }
| ---------- inside `ptr::const_ptr::<impl *const u32>::read` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
|
::: $DIR/detect-extra-ub.rs:41:9
|
LL | ptr.read();
| ---------- inside `INNER` at $DIR/detect-extra-ub.rs:41:9

error: any use of this value will cause an error
--> $DIR/detect-extra-ub.rs:34:5
|
LL | const UNALIGNED_READ: () = {
| ------------------------
LL | INNER;
| ^^^^^ referenced constant has errors
|
= note: `#[deny(const_err)]` on by default
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #71800 <https://github.com/rust-lang/rust/issues/71800>

error: aborting due to 6 previous errors

For more information about this error, try `rustc --explain E0080`.
Future incompatibility report: Future breakage diagnostic:
error: any use of this value will cause an error
--> $DIR/detect-extra-ub.rs:34:5
|
LL | const UNALIGNED_READ: () = {
| ------------------------
LL | INNER;
| ^^^^^ referenced constant has errors
|
= note: `#[deny(const_err)]` on by default
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #71800 <https://github.com/rust-lang/rust/issues/71800>