Skip to content

Commit

Permalink
Auto merge of rust-lang#93670 - erikdesjardins:noundef, r=nikic
Browse files Browse the repository at this point in the history
Apply noundef attribute to &T, &mut T, Box<T>, bool

This doesn't handle `char` because it's a bit awkward to distinguish it from `u32` at this point in codegen.

Note that this _does not_ change whether or not it is UB for `&`, `&mut`, or `Box` to point to undef. It only applies to the pointer itself, not the pointed-to memory.

Fixes (partially) rust-lang#74378.

r? `@nikic` cc `@RalfJung`
  • Loading branch information
bors committed Feb 13, 2022
2 parents 3cfa4de + 4013077 commit 5c30d65
Show file tree
Hide file tree
Showing 13 changed files with 94 additions and 91 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl ArgAttributeExt for ArgAttribute {
where
F: FnMut(llvm::Attribute),
{
for_each_kind!(self, f, NoAlias, NoCapture, NonNull, ReadOnly, InReg)
for_each_kind!(self, f, NoAlias, NoCapture, NonNull, ReadOnly, InReg, NoUndef)
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ pub enum Attribute {
StackProtectReq = 30,
StackProtectStrong = 31,
StackProtect = 32,
NoUndef = 33,
}

/// LLVMIntPredicate
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_llvm/llvm-wrapper/LLVMWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ enum LLVMRustAttribute {
StackProtectReq = 30,
StackProtectStrong = 31,
StackProtect = 32,
NoUndef = 33,
};

typedef struct OpaqueRustString *RustStringRef;
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,8 @@ static Attribute::AttrKind fromRust(LLVMRustAttribute Kind) {
return Attribute::StackProtectStrong;
case StackProtect:
return Attribute::StackProtect;
case NoUndef:
return Attribute::NoUndef;
}
report_fatal_error("bad AttributeKind");
}
Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3051,9 +3051,10 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
layout: TyAndLayout<'tcx>,
offset: Size,
is_return: bool| {
// Booleans are always an i1 that needs to be zero-extended.
// Booleans are always a noundef i1 that needs to be zero-extended.
if scalar.is_bool() {
attrs.ext(ArgExtension::Zext);
attrs.set(ArgAttribute::NoUndef);
return;
}

Expand All @@ -3078,6 +3079,11 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
_ => pointee.size,
};

// `Box`, `&T`, and `&mut T` cannot be undef.
// Note that this only applies to the value of the pointer itself;
// this attribute doesn't make it UB for the pointed-to data to be undef.
attrs.set(ArgAttribute::NoUndef);

// `Box` pointer parameters never alias because ownership is transferred
// `&mut` pointer parameters never alias other parameters,
// or mutable global data
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_target/src/abi/call/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ mod attr_impl {
// or not to actually emit the attribute. It can also be controlled
// with the `-Zmutable-noalias` debugging option.
const NoAliasMutRef = 1 << 6;
const NoUndef = 1 << 7;
}
}
}
Expand Down Expand Up @@ -495,7 +496,11 @@ impl<'a, Ty> ArgAbi<'a, Ty> {
// For non-immediate arguments the callee gets its own copy of
// the value on the stack, so there are no aliases. It's also
// program-invisible so can't possibly capture
attrs.set(ArgAttribute::NoAlias).set(ArgAttribute::NoCapture).set(ArgAttribute::NonNull);
attrs
.set(ArgAttribute::NoAlias)
.set(ArgAttribute::NoCapture)
.set(ArgAttribute::NonNull)
.set(ArgAttribute::NoUndef);
attrs.pointee_size = layout.size;
// FIXME(eddyb) We should be doing this, but at least on
// i686-pc-windows-msvc, it results in wrong stack offsets.
Expand Down
55 changes: 10 additions & 45 deletions src/test/codegen/fastcall-inreg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,52 +2,17 @@
// as "inreg" like the C/C++ compilers for the platforms.
// x86 only.

// ignore-aarch64
// ignore-aarch64_be
// ignore-arm
// ignore-armeb
// ignore-avr
// ignore-bpfel
// ignore-bpfeb
// ignore-hexagon
// ignore-mips
// ignore-mips64
// ignore-msp430
// ignore-powerpc64
// ignore-powerpc64le
// ignore-powerpc
// ignore-r600
// ignore-riscv64
// ignore-amdgcn
// ignore-sparc
// ignore-sparc64
// ignore-sparcv9
// ignore-sparcel
// ignore-s390x
// ignore-tce
// ignore-thumb
// ignore-thumbeb
// ignore-x86_64
// ignore-xcore
// ignore-nvptx
// ignore-nvptx64
// ignore-le32
// ignore-le64
// ignore-amdil
// ignore-amdil64
// ignore-hsail
// ignore-hsail64
// ignore-spir
// ignore-spir64
// ignore-kalimba
// ignore-shave
// ignore-wasm32
// ignore-wasm64
// ignore-emscripten

// compile-flags: -C no-prepopulate-passes
// compile-flags: --target i686-unknown-linux-gnu -C no-prepopulate-passes
// needs-llvm-components: x86

#![crate_type = "lib"]
#![no_core]
#![feature(no_core, lang_items)]

#[lang = "sized"]
trait Sized {}
#[lang = "copy"]
trait Copy {}

pub mod tests {
// CHECK: @f1(i32 inreg %_1, i32 inreg %_2, i32 %_3)
Expand All @@ -70,7 +35,7 @@ pub mod tests {
#[no_mangle]
pub extern "fastcall" fn f5(_: i64, _: i32) {}

// CHECK: @f6(i1 inreg zeroext %_1, i32 inreg %_2, i32 %_3)
// CHECK: @f6(i1 inreg noundef zeroext %_1, i32 inreg %_2, i32 %_3)
#[no_mangle]
pub extern "fastcall" fn f6(_: bool, _: i32, _: i32) {}
}
63 changes: 43 additions & 20 deletions src/test/codegen/function-arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#![crate_type = "lib"]
#![feature(rustc_attrs)]

use std::mem::MaybeUninit;

pub struct S {
_field: [i32; 8],
}
Expand All @@ -11,68 +13,79 @@ pub struct UnsafeInner {
_field: std::cell::UnsafeCell<i16>,
}

// CHECK: zeroext i1 @boolean(i1 zeroext %x)
// CHECK: noundef zeroext i1 @boolean(i1 noundef zeroext %x)
#[no_mangle]
pub fn boolean(x: bool) -> bool {
x
}

// CHECK: @readonly_borrow(i32* noalias readonly align 4 dereferenceable(4) %_1)
// CHECK: i8 @maybeuninit_boolean(i8 %x)
#[no_mangle]
pub fn maybeuninit_boolean(x: MaybeUninit<bool>) -> MaybeUninit<bool> {
x
}

// CHECK: @readonly_borrow(i32* noalias noundef readonly align 4 dereferenceable(4) %_1)
// FIXME #25759 This should also have `nocapture`
#[no_mangle]
pub fn readonly_borrow(_: &i32) {
}

// CHECK: @static_borrow(i32* noalias readonly align 4 dereferenceable(4) %_1)
// CHECK: @static_borrow(i32* noalias noundef readonly align 4 dereferenceable(4) %_1)
// static borrow may be captured
#[no_mangle]
pub fn static_borrow(_: &'static i32) {
}

// CHECK: @named_borrow(i32* noalias readonly align 4 dereferenceable(4) %_1)
// CHECK: @named_borrow(i32* noalias noundef readonly align 4 dereferenceable(4) %_1)
// borrow with named lifetime may be captured
#[no_mangle]
pub fn named_borrow<'r>(_: &'r i32) {
}

// CHECK: @unsafe_borrow(i16* align 2 dereferenceable(2) %_1)
// CHECK: @unsafe_borrow(i16* noundef align 2 dereferenceable(2) %_1)
// unsafe interior means this isn't actually readonly and there may be aliases ...
#[no_mangle]
pub fn unsafe_borrow(_: &UnsafeInner) {
}

// CHECK: @mutable_unsafe_borrow(i16* noalias align 2 dereferenceable(2) %_1)
// CHECK: @mutable_unsafe_borrow(i16* noalias noundef align 2 dereferenceable(2) %_1)
// ... unless this is a mutable borrow, those never alias
#[no_mangle]
pub fn mutable_unsafe_borrow(_: &mut UnsafeInner) {
}

// CHECK: @mutable_borrow(i32* noalias align 4 dereferenceable(4) %_1)
// CHECK: @mutable_borrow(i32* noalias noundef align 4 dereferenceable(4) %_1)
// FIXME #25759 This should also have `nocapture`
#[no_mangle]
pub fn mutable_borrow(_: &mut i32) {
}

// CHECK: @indirect_struct(%S* noalias nocapture dereferenceable(32) %_1)
// CHECK: @indirect_struct(%S* noalias nocapture noundef dereferenceable(32) %_1)
#[no_mangle]
pub fn indirect_struct(_: S) {
}

// CHECK: @borrowed_struct(%S* noalias readonly align 4 dereferenceable(32) %_1)
// CHECK: @borrowed_struct(%S* noalias noundef readonly align 4 dereferenceable(32) %_1)
// FIXME #25759 This should also have `nocapture`
#[no_mangle]
pub fn borrowed_struct(_: &S) {
}

// CHECK: @raw_struct(%S* %_1)
#[no_mangle]
pub fn raw_struct(_: *const S) {
}

// `Box` can get deallocated during execution of the function, so it should
// not get `dereferenceable`.
// CHECK: noalias nonnull align 4 i32* @_box(i32* noalias nonnull align 4 %x)
// CHECK: noalias noundef nonnull align 4 i32* @_box(i32* noalias noundef nonnull align 4 %x)
#[no_mangle]
pub fn _box(x: Box<i32>) -> Box<i32> {
x
}

// CHECK: @struct_return(%S* noalias nocapture sret(%S) dereferenceable(32){{( %0)?}})
// CHECK: @struct_return(%S* noalias nocapture noundef sret(%S) dereferenceable(32){{( %0)?}})
#[no_mangle]
pub fn struct_return() -> S {
S {
Expand All @@ -86,48 +99,58 @@ pub fn struct_return() -> S {
pub fn helper(_: usize) {
}

// CHECK: @slice([0 x i8]* noalias nonnull readonly align 1 %_1.0, [[USIZE]] %_1.1)
// CHECK: @slice([0 x i8]* noalias noundef nonnull readonly align 1 %_1.0, [[USIZE]] %_1.1)
// FIXME #25759 This should also have `nocapture`
#[no_mangle]
pub fn slice(_: &[u8]) {
}

// CHECK: @mutable_slice([0 x i8]* noalias nonnull align 1 %_1.0, [[USIZE]] %_1.1)
// CHECK: @mutable_slice([0 x i8]* noalias noundef nonnull align 1 %_1.0, [[USIZE]] %_1.1)
// FIXME #25759 This should also have `nocapture`
#[no_mangle]
pub fn mutable_slice(_: &mut [u8]) {
}

// CHECK: @unsafe_slice([0 x i16]* nonnull align 2 %_1.0, [[USIZE]] %_1.1)
// CHECK: @unsafe_slice([0 x i16]* noundef nonnull align 2 %_1.0, [[USIZE]] %_1.1)
// unsafe interior means this isn't actually readonly and there may be aliases ...
#[no_mangle]
pub fn unsafe_slice(_: &[UnsafeInner]) {
}

// CHECK: @str([0 x i8]* noalias nonnull readonly align 1 %_1.0, [[USIZE]] %_1.1)
// CHECK: @raw_slice([0 x i8]* %_1.0, [[USIZE]] %_1.1)
#[no_mangle]
pub fn raw_slice(_: *const [u8]) {
}

// CHECK: @str([0 x i8]* noalias noundef nonnull readonly align 1 %_1.0, [[USIZE]] %_1.1)
// FIXME #25759 This should also have `nocapture`
#[no_mangle]
pub fn str(_: &[u8]) {
}

// CHECK: @trait_borrow({}* nonnull align 1 %_1.0, [3 x [[USIZE]]]* noalias readonly align {{.*}} dereferenceable({{.*}}) %_1.1)
// CHECK: @trait_borrow({}* noundef nonnull align 1 %_1.0, [3 x [[USIZE]]]* noalias noundef readonly align {{.*}} dereferenceable({{.*}}) %_1.1)
// FIXME #25759 This should also have `nocapture`
#[no_mangle]
pub fn trait_borrow(_: &Drop) {
}

// CHECK: @trait_box({}* noalias nonnull align 1{{( %0)?}}, [3 x [[USIZE]]]* noalias readonly align {{.*}} dereferenceable({{.*}}){{( %1)?}})
// CHECK: @trait_raw({}* %_1.0, [3 x [[USIZE]]]* noalias noundef readonly align {{.*}} dereferenceable({{.*}}) %_1.1)
#[no_mangle]
pub fn trait_raw(_: *const Drop) {
}

// CHECK: @trait_box({}* noalias noundef nonnull align 1{{( %0)?}}, [3 x [[USIZE]]]* noalias noundef readonly align {{.*}} dereferenceable({{.*}}){{( %1)?}})
#[no_mangle]
pub fn trait_box(_: Box<Drop>) {
}

// CHECK: { i8*, i8* } @trait_option(i8* noalias align 1 %x.0, i8* %x.1)
// CHECK: { i8*, i8* } @trait_option(i8* noalias noundef align 1 %x.0, i8* %x.1)
#[no_mangle]
pub fn trait_option(x: Option<Box<Drop>>) -> Option<Box<Drop>> {
x
}

// CHECK: { [0 x i16]*, [[USIZE]] } @return_slice([0 x i16]* noalias nonnull readonly align 2 %x.0, [[USIZE]] %x.1)
// CHECK: { [0 x i16]*, [[USIZE]] } @return_slice([0 x i16]* noalias noundef nonnull readonly align 2 %x.0, [[USIZE]] %x.1)
#[no_mangle]
pub fn return_slice(x: &[u16]) -> &[u16] {
x
Expand All @@ -139,7 +162,7 @@ pub fn enum_id_1(x: Option<Result<u16, u16>>) -> Option<Result<u16, u16>> {
x
}

// CHECK: { i8, i8 } @enum_id_2(i1 zeroext %x.0, i8 %x.1)
// CHECK: { i8, i8 } @enum_id_2(i1 noundef zeroext %x.0, i8 %x.1)
#[no_mangle]
pub fn enum_id_2(x: Option<u8>) -> Option<u8> {
x
Expand Down
4 changes: 2 additions & 2 deletions src/test/codegen/packed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub struct BigPacked2 {
#[no_mangle]
pub fn call_pkd1(f: fn() -> Array) -> BigPacked1 {
// CHECK: [[ALLOCA:%[_a-z0-9]+]] = alloca %Array
// CHECK: call void %{{.*}}(%Array* noalias nocapture sret{{.*}} dereferenceable(32) [[ALLOCA]])
// CHECK: call void %{{.*}}(%Array* noalias nocapture noundef sret{{.*}} dereferenceable(32) [[ALLOCA]])
// CHECK: call void @llvm.memcpy.{{.*}}(i8* align 1 %{{.*}}, i8* align 4 %{{.*}}, i{{[0-9]+}} 32, i1 false)
// check that calls whose destination is a field of a packed struct
// go through an alloca rather than calling the function with an
Expand All @@ -64,7 +64,7 @@ pub fn call_pkd1(f: fn() -> Array) -> BigPacked1 {
#[no_mangle]
pub fn call_pkd2(f: fn() -> Array) -> BigPacked2 {
// CHECK: [[ALLOCA:%[_a-z0-9]+]] = alloca %Array
// CHECK: call void %{{.*}}(%Array* noalias nocapture sret{{.*}} dereferenceable(32) [[ALLOCA]])
// CHECK: call void %{{.*}}(%Array* noalias nocapture noundef sret{{.*}} dereferenceable(32) [[ALLOCA]])
// CHECK: call void @llvm.memcpy.{{.*}}(i8* align 2 %{{.*}}, i8* align 4 %{{.*}}, i{{[0-9]+}} 32, i1 false)
// check that calls whose destination is a field of a packed struct
// go through an alloca rather than calling the function with an
Expand Down
Loading

0 comments on commit 5c30d65

Please sign in to comment.