mirror of
https://github.com/rust-lang/rust.git
synced 2024-10-30 22:12:15 +00:00
MaybeUninit::assume_init_read
should have noundef
load metadata
I was looking into `array::IntoIter` optimization, and noticed that it wasn't annotating the loads with `noundef` for simple things like `array::IntoIter<i32, N>`. Turned out to be a more general problem as `MaybeUninit::assume_init_read` isn't marking the load as initialized (<https://rust.godbolt.org/z/Mxd8TPTnv>), which is unfortunate since that's basically its reason to exist. This PR lowers `ptr::read(p)` to `copy *p` in MIR, which fortuitiously also improves the IR we give to LLVM for things like `mem::replace`.
This commit is contained in:
parent
19c53768af
commit
b2c717fa33
@ -363,6 +363,8 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) {
|
||||
sym::likely => (0, vec![tcx.types.bool], tcx.types.bool),
|
||||
sym::unlikely => (0, vec![tcx.types.bool], tcx.types.bool),
|
||||
|
||||
sym::read_via_copy => (1, vec![tcx.mk_imm_ptr(param(0))], param(0)),
|
||||
|
||||
sym::discriminant_value => {
|
||||
let assoc_items = tcx.associated_item_def_ids(
|
||||
tcx.require_lang_item(hir::LangItem::DiscriminantKind, None),
|
||||
|
@ -1026,12 +1026,13 @@ declare_lint! {
|
||||
/// ### Example
|
||||
///
|
||||
/// ```rust,compile_fail
|
||||
/// #![feature(const_ptr_read)]
|
||||
/// #![feature(const_mut_refs)]
|
||||
/// const FOO: () = unsafe {
|
||||
/// let x = &[0_u8; 4];
|
||||
/// let y = x.as_ptr().cast::<u32>();
|
||||
/// y.read(); // the address of a `u8` array is unknown and thus we don't know if
|
||||
/// // it is aligned enough for reading a `u32`.
|
||||
/// let mut z = 123;
|
||||
/// y.copy_to_nonoverlapping(&mut z, 1); // the address of a `u8` array is unknown
|
||||
/// // and thus we don't know if it is aligned enough for copying a `u32`.
|
||||
/// };
|
||||
/// ```
|
||||
///
|
||||
|
@ -149,6 +149,30 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics {
|
||||
terminator.kind = TerminatorKind::Goto { target };
|
||||
}
|
||||
}
|
||||
sym::read_via_copy => {
|
||||
let Ok([arg]) = <[_; 1]>::try_from(std::mem::take(args)) else {
|
||||
span_bug!(terminator.source_info.span, "Wrong number of arguments");
|
||||
};
|
||||
let derefed_place =
|
||||
if let Some(place) = arg.place() && let Some(local) = place.as_local() {
|
||||
tcx.mk_place_deref(local.into())
|
||||
} else {
|
||||
span_bug!(terminator.source_info.span, "Only passing a local is supported");
|
||||
};
|
||||
block.statements.push(Statement {
|
||||
source_info: terminator.source_info,
|
||||
kind: StatementKind::Assign(Box::new((
|
||||
*destination,
|
||||
Rvalue::Use(Operand::Copy(derefed_place)),
|
||||
))),
|
||||
});
|
||||
if let Some(target) = *target {
|
||||
terminator.kind = TerminatorKind::Goto { target };
|
||||
} else {
|
||||
// Reading something uninhabited means this is unreachable.
|
||||
terminator.kind = TerminatorKind::Unreachable;
|
||||
}
|
||||
}
|
||||
sym::discriminant_value => {
|
||||
if let (Some(target), Some(arg)) = (*target, args[0].place()) {
|
||||
let arg = tcx.mk_place_deref(arg);
|
||||
|
@ -1153,6 +1153,7 @@ symbols! {
|
||||
read_enum_variant_arg,
|
||||
read_struct,
|
||||
read_struct_field,
|
||||
read_via_copy,
|
||||
readonly,
|
||||
realloc,
|
||||
reason,
|
||||
|
@ -2020,6 +2020,20 @@ extern "rust-intrinsic" {
|
||||
#[rustc_safe_intrinsic]
|
||||
pub fn saturating_sub<T: Copy>(a: T, b: T) -> T;
|
||||
|
||||
/// This is a *typed* read, `copy *p` in MIR.
|
||||
///
|
||||
/// The stabilized form of this intrinsic is [`crate::ptr::read`], so
|
||||
/// that can be implemented without needing to do an *untyped* copy
|
||||
/// via [`copy_nonoverlapping`], and thus can get proper metadata.
|
||||
///
|
||||
/// This intrinsic can *only* be called with a copy or move of a local.
|
||||
/// (It allows neither constants nor projections.)
|
||||
///
|
||||
/// To avoid introducing any `noalias` requirements, it just takes a pointer.
|
||||
#[cfg(not(bootstrap))]
|
||||
#[rustc_const_unstable(feature = "const_ptr_read", issue = "80377")]
|
||||
pub fn read_via_copy<T>(p: *const T) -> T;
|
||||
|
||||
/// Returns the value of the discriminant for the variant in 'v';
|
||||
/// if `T` has no discriminant, returns `0`.
|
||||
///
|
||||
|
@ -1137,26 +1137,34 @@ pub const unsafe fn replace<T>(dst: *mut T, mut src: T) -> T {
|
||||
pub const unsafe fn read<T>(src: *const T) -> T {
|
||||
// We are calling the intrinsics directly to avoid function calls in the generated code
|
||||
// as `intrinsics::copy_nonoverlapping` is a wrapper function.
|
||||
#[cfg(bootstrap)]
|
||||
extern "rust-intrinsic" {
|
||||
#[rustc_const_stable(feature = "const_intrinsic_copy", since = "1.63.0")]
|
||||
fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: usize);
|
||||
}
|
||||
|
||||
let mut tmp = MaybeUninit::<T>::uninit();
|
||||
// SAFETY: the caller must guarantee that `src` is valid for reads.
|
||||
// `src` cannot overlap `tmp` because `tmp` was just allocated on
|
||||
// the stack as a separate allocated object.
|
||||
//
|
||||
// Also, since we just wrote a valid value into `tmp`, it is guaranteed
|
||||
// to be properly initialized.
|
||||
unsafe {
|
||||
assert_unsafe_precondition!(
|
||||
"ptr::read requires that the pointer argument is aligned and non-null",
|
||||
[T](src: *const T) => is_aligned_and_not_null(src)
|
||||
);
|
||||
|
||||
#[cfg(bootstrap)]
|
||||
{
|
||||
let mut tmp = MaybeUninit::<T>::uninit();
|
||||
copy_nonoverlapping(src, tmp.as_mut_ptr(), 1);
|
||||
tmp.assume_init()
|
||||
}
|
||||
#[cfg(not(bootstrap))]
|
||||
{
|
||||
// This uses a dedicated intrinsic, not `copy_nonoverlapping`,
|
||||
// so that it gets a *typed* copy, not an *untyped* one.
|
||||
crate::intrinsics::read_via_copy(src)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Reads the value from `src` without moving it. This leaves the
|
||||
|
@ -12,13 +12,44 @@ pub fn replace_byte(dst: &mut u8, src: u8) -> u8 {
|
||||
std::mem::replace(dst, src)
|
||||
}
|
||||
|
||||
#[repr(C, align(8))]
|
||||
pub struct Big([u64; 7]);
|
||||
pub fn replace_big(dst: &mut Big, src: Big) -> Big {
|
||||
// Before the `read_via_copy` intrinsic, this emitted six `memcpy`s.
|
||||
std::mem::replace(dst, src)
|
||||
}
|
||||
|
||||
// NOTE(eddyb) the `CHECK-NOT`s ensure that the only calls of `@llvm.memcpy` in
|
||||
// the entire output, are the two direct calls we want, from `ptr::replace`.
|
||||
// the entire output, are the direct calls we want, from `ptr::replace`.
|
||||
|
||||
// CHECK-NOT: call void @llvm.memcpy
|
||||
// CHECK: ; core::mem::replace
|
||||
|
||||
// For a large type, we expect exactly three `memcpy`s
|
||||
// CHECK-LABEL: define internal void @{{.+}}mem{{.+}}replace{{.+}}sret(%Big)
|
||||
// CHECK-NOT: alloca
|
||||
// CHECK: alloca %Big
|
||||
// CHECK-NOT: alloca
|
||||
// CHECK-NOT: call void @llvm.memcpy
|
||||
// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 8 %{{.*}}, {{i8\*|ptr}} align 8 %{{.*}}, i{{.*}} 56, i1 false)
|
||||
// CHECK-NOT: call void @llvm.memcpy
|
||||
// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 8 %{{.*}}, {{i8\*|ptr}} align 8 %{{.*}}, i{{.*}} 56, i1 false)
|
||||
// CHECK-NOT: call void @llvm.memcpy
|
||||
// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 8 %{{.*}}, {{i8\*|ptr}} align 8 %{{.*}}, i{{.*}} 56, i1 false)
|
||||
// CHECK-NOT: call void @llvm.memcpy
|
||||
|
||||
// CHECK-NOT: call void @llvm.memcpy
|
||||
// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 1 %{{.*}}, {{i8\*|ptr}} align 1 %{{.*}}, i{{.*}} 1, i1 false)
|
||||
// CHECK-NOT: call void @llvm.memcpy
|
||||
// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 1 %{{.*}}, {{i8\*|ptr}} align 1 %{{.*}}, i{{.*}} 1, i1 false)
|
||||
|
||||
// For a small type, we expect one each of `load`/`store`/`memcpy` instead
|
||||
// CHECK-LABEL: define internal noundef i8 @{{.+}}mem{{.+}}replace
|
||||
// CHECK-NOT: alloca
|
||||
// CHECK: alloca i8
|
||||
// CHECK-NOT: alloca
|
||||
// CHECK-NOT: call void @llvm.memcpy
|
||||
// CHECK: load i8
|
||||
// CHECK-NOT: call void @llvm.memcpy
|
||||
// CHECK: store i8
|
||||
// CHECK-NOT: call void @llvm.memcpy
|
||||
// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 1 %{{.*}}, {{i8\*|ptr}} align 1 %{{.*}}, i{{.*}} 1, i1 false)
|
||||
// CHECK-NOT: call void @llvm.memcpy
|
||||
|
||||
// CHECK-NOT: call void @llvm.memcpy
|
||||
|
51
tests/codegen/read-noundef-metadata.rs
Normal file
51
tests/codegen/read-noundef-metadata.rs
Normal file
@ -0,0 +1,51 @@
|
||||
// compile-flags: -O -Z merge-functions=disabled
|
||||
// no-system-llvm
|
||||
// ignore-debug (the extra assertions get in the way)
|
||||
|
||||
#![crate_type = "lib"]
|
||||
|
||||
// Ensure that various forms of reading pointers correctly annotate the `load`s
|
||||
// with `!noundef` metadata to enable extra optimization. The functions return
|
||||
// `MaybeUninit` to keep it from being inferred from the function type.
|
||||
|
||||
use std::mem::MaybeUninit;
|
||||
|
||||
// CHECK-LABEL: define i8 @copy_byte(
|
||||
#[no_mangle]
|
||||
pub unsafe fn copy_byte(p: *const u8) -> MaybeUninit<u8> {
|
||||
// CHECK-NOT: load
|
||||
// CHECK: load i8, ptr %p, align 1
|
||||
// CHECK-SAME: !noundef !
|
||||
// CHECK-NOT: load
|
||||
MaybeUninit::new(*p)
|
||||
}
|
||||
|
||||
// CHECK-LABEL: define i8 @read_byte(
|
||||
#[no_mangle]
|
||||
pub unsafe fn read_byte(p: *const u8) -> MaybeUninit<u8> {
|
||||
// CHECK-NOT: load
|
||||
// CHECK: load i8, ptr %p, align 1
|
||||
// CHECK-SAME: !noundef !
|
||||
// CHECK-NOT: load
|
||||
MaybeUninit::new(p.read())
|
||||
}
|
||||
|
||||
// CHECK-LABEL: define i8 @read_byte_maybe_uninit(
|
||||
#[no_mangle]
|
||||
pub unsafe fn read_byte_maybe_uninit(p: *const MaybeUninit<u8>) -> MaybeUninit<u8> {
|
||||
// CHECK-NOT: load
|
||||
// CHECK: load i8, ptr %p, align 1
|
||||
// CHECK-NOT: noundef
|
||||
// CHECK-NOT: load
|
||||
p.read()
|
||||
}
|
||||
|
||||
// CHECK-LABEL: define i8 @read_byte_assume_init(
|
||||
#[no_mangle]
|
||||
pub unsafe fn read_byte_assume_init(p: &MaybeUninit<u8>) -> MaybeUninit<u8> {
|
||||
// CHECK-NOT: load
|
||||
// CHECK: load i8, ptr %p, align 1
|
||||
// CHECK-SAME: !noundef !
|
||||
// CHECK-NOT: load
|
||||
MaybeUninit::new(p.assume_init_read())
|
||||
}
|
@ -0,0 +1,27 @@
|
||||
- // MIR for `read_via_copy_primitive` before LowerIntrinsics
|
||||
+ // MIR for `read_via_copy_primitive` after LowerIntrinsics
|
||||
|
||||
fn read_via_copy_primitive(_1: &i32) -> i32 {
|
||||
debug r => _1; // in scope 0 at $DIR/lower_intrinsics.rs:+0:32: +0:33
|
||||
let mut _0: i32; // return place in scope 0 at $DIR/lower_intrinsics.rs:+0:44: +0:47
|
||||
let mut _2: *const i32; // in scope 0 at $DIR/lower_intrinsics.rs:+1:46: +1:47
|
||||
scope 1 {
|
||||
}
|
||||
|
||||
bb0: {
|
||||
StorageLive(_2); // scope 1 at $DIR/lower_intrinsics.rs:+1:46: +1:47
|
||||
_2 = &raw const (*_1); // scope 1 at $DIR/lower_intrinsics.rs:+1:46: +1:47
|
||||
- _0 = read_via_copy::<i32>(move _2) -> bb1; // scope 1 at $DIR/lower_intrinsics.rs:+1:14: +1:48
|
||||
- // mir::Constant
|
||||
- // + span: $DIR/lower_intrinsics.rs:85:14: 85:45
|
||||
- // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(*const i32) -> i32 {read_via_copy::<i32>}, val: Value(<ZST>) }
|
||||
+ _0 = (*_2); // scope 1 at $DIR/lower_intrinsics.rs:+1:14: +1:48
|
||||
+ goto -> bb1; // scope 1 at $DIR/lower_intrinsics.rs:+1:14: +1:48
|
||||
}
|
||||
|
||||
bb1: {
|
||||
StorageDead(_2); // scope 1 at $DIR/lower_intrinsics.rs:+1:47: +1:48
|
||||
return; // scope 0 at $DIR/lower_intrinsics.rs:+2:2: +2:2
|
||||
}
|
||||
}
|
||||
|
@ -0,0 +1,22 @@
|
||||
- // MIR for `read_via_copy_uninhabited` before LowerIntrinsics
|
||||
+ // MIR for `read_via_copy_uninhabited` after LowerIntrinsics
|
||||
|
||||
fn read_via_copy_uninhabited(_1: &Never) -> Never {
|
||||
debug r => _1; // in scope 0 at $DIR/lower_intrinsics.rs:+0:34: +0:35
|
||||
let mut _0: Never; // return place in scope 0 at $DIR/lower_intrinsics.rs:+0:48: +0:53
|
||||
let mut _2: *const Never; // in scope 0 at $DIR/lower_intrinsics.rs:+1:46: +1:47
|
||||
scope 1 {
|
||||
}
|
||||
|
||||
bb0: {
|
||||
StorageLive(_2); // scope 1 at $DIR/lower_intrinsics.rs:+1:46: +1:47
|
||||
_2 = &raw const (*_1); // scope 1 at $DIR/lower_intrinsics.rs:+1:46: +1:47
|
||||
- _0 = read_via_copy::<Never>(move _2); // scope 1 at $DIR/lower_intrinsics.rs:+1:14: +1:48
|
||||
- // mir::Constant
|
||||
- // + span: $DIR/lower_intrinsics.rs:90:14: 90:45
|
||||
- // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(*const Never) -> Never {read_via_copy::<Never>}, val: Value(<ZST>) }
|
||||
+ _0 = (*_2); // scope 1 at $DIR/lower_intrinsics.rs:+1:14: +1:48
|
||||
+ unreachable; // scope 1 at $DIR/lower_intrinsics.rs:+1:14: +1:48
|
||||
}
|
||||
}
|
||||
|
@ -79,3 +79,15 @@ pub fn with_overflow(a: i32, b: i32) {
|
||||
let _y = core::intrinsics::sub_with_overflow(a, b);
|
||||
let _z = core::intrinsics::mul_with_overflow(a, b);
|
||||
}
|
||||
|
||||
// EMIT_MIR lower_intrinsics.read_via_copy_primitive.LowerIntrinsics.diff
|
||||
pub fn read_via_copy_primitive(r: &i32) -> i32 {
|
||||
unsafe { core::intrinsics::read_via_copy(r) }
|
||||
}
|
||||
|
||||
// EMIT_MIR lower_intrinsics.read_via_copy_uninhabited.LowerIntrinsics.diff
|
||||
pub fn read_via_copy_uninhabited(r: &Never) -> Never {
|
||||
unsafe { core::intrinsics::read_via_copy(r) }
|
||||
}
|
||||
|
||||
pub enum Never {}
|
||||
|
@ -1,7 +1,7 @@
|
||||
error[E0080]: evaluation of constant value failed
|
||||
--> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
|
||||
|
|
||||
= note: memory access failed: alloc5 has size 4, so pointer to 4 bytes starting at offset 4 is out-of-bounds
|
||||
= note: dereferencing pointer failed: alloc5 has size 4, so pointer to 4 bytes starting at offset 4 is out-of-bounds
|
||||
|
|
||||
note: inside `std::ptr::read::<u32>`
|
||||
--> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
|
||||
@ -14,7 +14,7 @@ LL | const _READ: u32 = unsafe { ptr::read(PAST_END_PTR) };
|
||||
error[E0080]: evaluation of constant value failed
|
||||
--> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
|
||||
|
|
||||
= note: memory access failed: alloc5 has size 4, so pointer to 4 bytes starting at offset 4 is out-of-bounds
|
||||
= note: dereferencing pointer failed: alloc5 has size 4, so pointer to 4 bytes starting at offset 4 is out-of-bounds
|
||||
|
|
||||
note: inside `std::ptr::read::<u32>`
|
||||
--> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
|
||||
@ -29,7 +29,7 @@ LL | const _CONST_READ: u32 = unsafe { PAST_END_PTR.read() };
|
||||
error[E0080]: evaluation of constant value failed
|
||||
--> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
|
||||
|
|
||||
= note: memory access failed: alloc5 has size 4, so pointer to 4 bytes starting at offset 4 is out-of-bounds
|
||||
= note: dereferencing pointer failed: alloc5 has size 4, so pointer to 4 bytes starting at offset 4 is out-of-bounds
|
||||
|
|
||||
note: inside `std::ptr::read::<u32>`
|
||||
--> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
|
||||
|
@ -148,11 +148,11 @@ LL | const DATA_FN_PTR: fn() = unsafe { mem::transmute(&13) };
|
||||
HEX_DUMP
|
||||
}
|
||||
|
||||
error: accessing memory with alignment 1, but alignment 4 is required
|
||||
error[E0080]: evaluation of constant value failed
|
||||
--> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
|
||||
|
|
||||
= 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 #68585 <https://github.com/rust-lang/rust/issues/104616>
|
||||
= note: accessing memory with alignment 1, but alignment 4 is required
|
||||
|
|
||||
note: inside `std::ptr::read::<u32>`
|
||||
--> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
|
||||
note: inside `ptr::const_ptr::<impl *const u32>::read`
|
||||
@ -162,25 +162,7 @@ note: inside `UNALIGNED_READ`
|
||||
|
|
||||
LL | ptr.read();
|
||||
| ^^^^^^^^^^
|
||||
= note: `#[deny(invalid_alignment)]` on by default
|
||||
|
||||
error: aborting due to 15 previous errors
|
||||
|
||||
For more information about this error, try `rustc --explain E0080`.
|
||||
Future incompatibility report: Future breakage diagnostic:
|
||||
error: accessing memory with alignment 1, but alignment 4 is required
|
||||
--> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
|
||||
|
|
||||
= 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 #68585 <https://github.com/rust-lang/rust/issues/104616>
|
||||
note: inside `std::ptr::read::<u32>`
|
||||
--> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
|
||||
note: inside `ptr::const_ptr::<impl *const u32>::read`
|
||||
--> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
|
||||
note: inside `UNALIGNED_READ`
|
||||
--> $DIR/ub-ref-ptr.rs:67:5
|
||||
|
|
||||
LL | ptr.read();
|
||||
| ^^^^^^^^^^
|
||||
= note: `#[deny(invalid_alignment)]` on by default
|
||||
|
||||
|
@ -1,7 +1,7 @@
|
||||
error[E0080]: evaluation of constant value failed
|
||||
--> $SRC_DIR/core/src/ptr/mod.rs:LL:COL
|
||||
|
|
||||
= note: unable to copy parts of a pointer from memory at ALLOC
|
||||
= note: unable to turn pointer into raw bytes
|
||||
|
|
||||
= help: this code performed an operation that depends on the underlying bytes representing a pointer
|
||||
= help: the absolute address of a pointer is not known at compile-time, so such operations are not supported
|
||||
|
Loading…
Reference in New Issue
Block a user