From c5a0cd0197789b16ec0affc151e658e0b5f8ca6e Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 9 May 2020 13:40:02 +0200 Subject: [PATCH 1/2] Add a feature gate for allowing integers as the address of references in constants --- src/librustc_feature/active.rs | 3 +++ src/librustc_mir/interpret/validity.rs | 20 ++++++++++++------- src/librustc_span/symbol.rs | 1 + src/test/ui/consts/const-eval/ub-ref.stderr | 4 ++-- src/test/ui/consts/const-int-ref.rs | 17 ++++++++++++++++ src/test/ui/consts/const-int-ref.stderr | 15 ++++++++++++++ .../ui/consts/const-int-ref.vanilla.stderr | 11 ++++++++++ 7 files changed, 62 insertions(+), 9 deletions(-) create mode 100644 src/test/ui/consts/const-int-ref.rs create mode 100644 src/test/ui/consts/const-int-ref.stderr create mode 100644 src/test/ui/consts/const-int-ref.vanilla.stderr diff --git a/src/librustc_feature/active.rs b/src/librustc_feature/active.rs index a1dd7a5ca5225..1a0c09eca41fa 100644 --- a/src/librustc_feature/active.rs +++ b/src/librustc_feature/active.rs @@ -565,6 +565,9 @@ declare_features! ( /// Allow conditional compilation depending on rust version (active, cfg_version, "1.45.0", Some(64796), None), + /// Allows constants of reference type to have integers for the address. + (active, const_int_ref, "1.45.0", Some(63197), None), + // ------------------------------------------------------------------------- // feature-group-end: actual feature gates // ------------------------------------------------------------------------- diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index eb743675d91d3..fba2cbbec4771 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -382,14 +382,20 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' // alignment and size determined by the layout (size will be 0, // alignment should take attributes into account). .unwrap_or_else(|| (place.layout.size, place.layout.align.abi)); + let ptr = self.ecx.memory.check_ptr_access_align( + place.ptr, + size, + Some(align), + CheckInAllocMsg::InboundsTest, + ); + if let Err(InterpErrorInfo { kind: err_unsup!(ReadBytesAsPointer), ..}) = ptr { + if self.ecx.tcx.features().const_int_ref { + return Ok(()); + } + } // Direct call to `check_ptr_access_align` checks alignment even on CTFE machines. let ptr: Option<_> = try_validation!( - self.ecx.memory.check_ptr_access_align( - place.ptr, - size, - Some(align), - CheckInAllocMsg::InboundsTest, - ), + ptr, self.path, err_ub!(AlignmentCheckFailed { required, has }) => { @@ -405,7 +411,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' err_ub!(PointerOutOfBounds { .. }) => { "a dangling {} (going beyond the bounds of its allocation)", kind }, err_unsup!(ReadBytesAsPointer) => - { "a dangling {} (created from integer)", kind }, + { "a dangling {} (created from integer). Add `#![feature(const_int_ref)]` to the crate attributes to enable", kind }, // This cannot happen during const-eval (because interning already detects // dangling pointers), but it can happen in Miri. err_ub!(PointerUseAfterFree(..)) => diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs index a61647bfd655f..4305b4f927c75 100644 --- a/src/librustc_span/symbol.rs +++ b/src/librustc_span/symbol.rs @@ -219,6 +219,7 @@ symbols! { const_generics, const_if_match, const_indexing, + const_int_ref, const_in_array_repeat_expressions, const_let, const_loop, diff --git a/src/test/ui/consts/const-eval/ub-ref.stderr b/src/test/ui/consts/const-eval/ub-ref.stderr index 95a83d11acd07..7c6955a5a7062 100644 --- a/src/test/ui/consts/const-eval/ub-ref.stderr +++ b/src/test/ui/consts/const-eval/ub-ref.stderr @@ -58,7 +58,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-ref.rs:33:1 | LL | const USIZE_AS_REF: &'static u8 = unsafe { mem::transmute(1337usize) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a dangling reference (created from integer) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a dangling reference (created from integer). Add `#![feature(const_int_ref)]` to the crate attributes to enable | = 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. @@ -66,7 +66,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-ref.rs:36:1 | LL | const USIZE_AS_BOX: Box = unsafe { mem::transmute(1337usize) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a dangling box (created from integer) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a dangling box (created from integer). Add `#![feature(const_int_ref)]` to the crate attributes to enable | = 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. diff --git a/src/test/ui/consts/const-int-ref.rs b/src/test/ui/consts/const-int-ref.rs new file mode 100644 index 0000000000000..383c310d40d54 --- /dev/null +++ b/src/test/ui/consts/const-int-ref.rs @@ -0,0 +1,17 @@ +// gate-test-const_int_refs +// revisions: gated vanilla + +//[gated] check-pass + +#![cfg_attr(gated, feature(const_int_ref))] + +#[repr(C)] +union Transmute { + int: usize, + ptr: &'static i32 +} + +const GPIO: &i32 = unsafe { Transmute { int: 0x800 }.ptr }; +//[vanilla]~^ ERROR it is undefined behavior + +fn main() {} diff --git a/src/test/ui/consts/const-int-ref.stderr b/src/test/ui/consts/const-int-ref.stderr new file mode 100644 index 0000000000000..954960820aaa9 --- /dev/null +++ b/src/test/ui/consts/const-int-ref.stderr @@ -0,0 +1,15 @@ +error[E0601]: `main` function not found in crate `const_int_ref` + --> $DIR/const-int-ref.rs:3:1 + | +LL | / #[repr(C)] +LL | | union Transmute { +LL | | int: usize, +LL | | ptr: &'static i32 +LL | | } +LL | | +LL | | const GPIO: &i32 = unsafe { Transmute { int: 0x800 }.ptr }; + | |___________________________________________________________^ consider adding a `main` function to `$DIR/const-int-ref.rs` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0601`. diff --git a/src/test/ui/consts/const-int-ref.vanilla.stderr b/src/test/ui/consts/const-int-ref.vanilla.stderr new file mode 100644 index 0000000000000..b4a0bc46c9c1b --- /dev/null +++ b/src/test/ui/consts/const-int-ref.vanilla.stderr @@ -0,0 +1,11 @@ +error[E0080]: it is undefined behavior to use this value + --> $DIR/const-int-ref.rs:14:1 + | +LL | const GPIO: &i32 = unsafe { Transmute { int: 0x800 }.ptr }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a dangling reference (created from integer). Add `#![feature(const_int_ref)]` to the crate attributes to enable + | + = 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 previous error + +For more information about this error, try `rustc --explain E0080`. From 398cca3ec4b2c63021ac3863506c7b6ee9edf2a0 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 11 May 2020 13:56:21 +0200 Subject: [PATCH 2/2] Address review comments --- src/librustc_mir/interpret/validity.rs | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index fba2cbbec4771..35ba24bbe7866 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -382,18 +382,23 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' // alignment and size determined by the layout (size will be 0, // alignment should take attributes into account). .unwrap_or_else(|| (place.layout.size, place.layout.align.abi)); + + // Direct call to `check_ptr_access_align` checks alignment even on CTFE machines. let ptr = self.ecx.memory.check_ptr_access_align( place.ptr, size, Some(align), CheckInAllocMsg::InboundsTest, ); - if let Err(InterpErrorInfo { kind: err_unsup!(ReadBytesAsPointer), ..}) = ptr { - if self.ecx.tcx.features().const_int_ref { - return Ok(()); + // In CTFE we allow integers as the address of references. + // See https://github.com/rust-lang/rust/issues/63197 for details. + if self.ref_tracking_for_consts.is_some() { + if let Err(InterpErrorInfo { kind: err_unsup!(ReadBytesAsPointer), ..}) = ptr { + if self.ecx.tcx.features().const_int_ref { + return Ok(()); + } } } - // Direct call to `check_ptr_access_align` checks alignment even on CTFE machines. let ptr: Option<_> = try_validation!( ptr, self.path, @@ -410,8 +415,15 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' { "a dangling {} (address 0x{:x} is unallocated)", kind, i }, err_ub!(PointerOutOfBounds { .. }) => { "a dangling {} (going beyond the bounds of its allocation)", kind }, - err_unsup!(ReadBytesAsPointer) => - { "a dangling {} (created from integer). Add `#![feature(const_int_ref)]` to the crate attributes to enable", kind }, + err_unsup!(ReadBytesAsPointer) => { + "a dangling {} (created from integer).{}", + kind, + if self.ecx.tcx.sess.opts.unstable_features.is_nightly_build() { + " Add `#![feature(const_int_ref)]` to the crate attributes to enable" + } else { + "" + } + }, // This cannot happen during const-eval (because interning already detects // dangling pointers), but it can happen in Miri. err_ub!(PointerUseAfterFree(..)) =>