Skip to content

Commit

Permalink
also do not add noalias on not-Unpin Box
Browse files Browse the repository at this point in the history
  • Loading branch information
@RalfJung
RalfJung committed Feb 6, 2023
1 parent ea541bc commit 1ef1687
Show file tree
Hide file tree
Showing 6 changed files with 180 additions and 116 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_abi/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1443,8 +1443,8 @@ pub enum PointerKind {
SharedRef { frozen: bool },
/// Mutable reference. `unpin` indicates the absence of any pinned data.
MutableRef { unpin: bool },
/// Box.
Box,
/// Box. `unpin` indicates the absence of any pinned data.
Box { unpin: bool },
}

/// Note that this information is advisory only, and backends are free to ignore it.
Expand Down
186 changes: 95 additions & 91 deletions compiler/rustc_middle/src/ty/layout.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -818,110 +818,114 @@ where
let tcx = cx.tcx();
let param_env = cx.param_env();

let pointee_info =
match *this.ty.kind() {
ty::RawPtr(mt) if offset.bytes() == 0 => {
tcx.layout_of(param_env.and(mt.ty)).ok().map(|layout| PointeeInfo {
size: layout.size,
align: layout.align.abi,
safe: None,
})
}
ty::FnPtr(fn_sig) if offset.bytes() == 0 => {
tcx.layout_of(param_env.and(tcx.mk_fn_ptr(fn_sig))).ok().map(|layout| {
PointeeInfo { size: layout.size, align: layout.align.abi, safe: None }
})
}
ty::Ref(_, ty, mt) if offset.bytes() == 0 => {
// Use conservative pointer kind if not optimizing. This saves us the
// Freeze/Unpin queries, and can save time in the codegen backend (noalias
// attributes in LLVM have compile-time cost even in unoptimized builds).
let optimize = tcx.sess.opts.optimize != OptLevel::No;
let kind = match mt {
hir::Mutability::Not => PointerKind::SharedRef {
frozen: optimize && ty.is_freeze(tcx, cx.param_env()),
},
hir::Mutability::Mut => PointerKind::MutableRef {
unpin: optimize && ty.is_unpin(tcx, cx.param_env()),
},
};
let pointee_info = match *this.ty.kind() {
ty::RawPtr(mt) if offset.bytes() == 0 => {
tcx.layout_of(param_env.and(mt.ty)).ok().map(|layout| PointeeInfo {
size: layout.size,
align: layout.align.abi,
safe: None,
})
}
ty::FnPtr(fn_sig) if offset.bytes() == 0 => {
tcx.layout_of(param_env.and(tcx.mk_fn_ptr(fn_sig))).ok().map(|layout| PointeeInfo {
size: layout.size,
align: layout.align.abi,
safe: None,
})
}
ty::Ref(_, ty, mt) if offset.bytes() == 0 => {
// Use conservative pointer kind if not optimizing. This saves us the
// Freeze/Unpin queries, and can save time in the codegen backend (noalias
// attributes in LLVM have compile-time cost even in unoptimized builds).
let optimize = tcx.sess.opts.optimize != OptLevel::No;
let kind = match mt {
hir::Mutability::Not => PointerKind::SharedRef {
frozen: optimize && ty.is_freeze(tcx, cx.param_env()),
},
hir::Mutability::Mut => PointerKind::MutableRef {
unpin: optimize && ty.is_unpin(tcx, cx.param_env()),
},
};

tcx.layout_of(param_env.and(ty)).ok().map(|layout| PointeeInfo {
size: layout.size,
align: layout.align.abi,
safe: Some(kind),
})
}
tcx.layout_of(param_env.and(ty)).ok().map(|layout| PointeeInfo {
size: layout.size,
align: layout.align.abi,
safe: Some(kind),
})
}

_ => {
let mut data_variant = match this.variants {
// Within the discriminant field, only the niche itself is
// always initialized, so we only check for a pointer at its
// offset.
//
// If the niche is a pointer, it's either valid (according
// to its type), or null (which the niche field's scalar
// validity range encodes). This allows using
// `dereferenceable_or_null` for e.g., `Option<&T>`, and
// this will continue to work as long as we don't start
// using more niches than just null (e.g., the first page of
// the address space, or unaligned pointers).
Variants::Multiple {
tag_encoding: TagEncoding::Niche { untagged_variant, .. },
tag_field,
..
} if this.fields.offset(tag_field) == offset => {
Some(this.for_variant(cx, untagged_variant))
}
_ => Some(this),
};
_ => {
let mut data_variant = match this.variants {
// Within the discriminant field, only the niche itself is
// always initialized, so we only check for a pointer at its
// offset.
//
// If the niche is a pointer, it's either valid (according
// to its type), or null (which the niche field's scalar
// validity range encodes). This allows using
// `dereferenceable_or_null` for e.g., `Option<&T>`, and
// this will continue to work as long as we don't start
// using more niches than just null (e.g., the first page of
// the address space, or unaligned pointers).
Variants::Multiple {
tag_encoding: TagEncoding::Niche { untagged_variant, .. },
tag_field,
..
} if this.fields.offset(tag_field) == offset => {
Some(this.for_variant(cx, untagged_variant))
}
_ => Some(this),
};

if let Some(variant) = data_variant {
// We're not interested in any unions.
if let FieldsShape::Union(_) = variant.fields {
data_variant = None;
}
if let Some(variant) = data_variant {
// We're not interested in any unions.
if let FieldsShape::Union(_) = variant.fields {
data_variant = None;
}
}

let mut result = None;

if let Some(variant) = data_variant {
// FIXME(erikdesjardins): handle non-default addrspace ptr sizes
// (requires passing in the expected address space from the caller)
let ptr_end = offset + Pointer(AddressSpace::DATA).size(cx);
for i in 0..variant.fields.count() {
let field_start = variant.fields.offset(i);
if field_start <= offset {
let field = variant.field(cx, i);
result = field.to_result().ok().and_then(|field| {
if ptr_end <= field_start + field.size {
// We found the right field, look inside it.
let field_info =
field.pointee_info_at(cx, offset - field_start);
field_info
} else {
None
}
});
if result.is_some() {
break;
let mut result = None;

if let Some(variant) = data_variant {
// FIXME(erikdesjardins): handle non-default addrspace ptr sizes
// (requires passing in the expected address space from the caller)
let ptr_end = offset + Pointer(AddressSpace::DATA).size(cx);
for i in 0..variant.fields.count() {
let field_start = variant.fields.offset(i);
if field_start <= offset {
let field = variant.field(cx, i);
result = field.to_result().ok().and_then(|field| {
if ptr_end <= field_start + field.size {
// We found the right field, look inside it.
let field_info =
field.pointee_info_at(cx, offset - field_start);
field_info
} else {
None
}
});
if result.is_some() {
break;
}
}
}
}

// FIXME(eddyb) This should be for `ptr::Unique<T>`, not `Box<T>`.
if let Some(ref mut pointee) = result {
if let ty::Adt(def, _) = this.ty.kind() {
if def.is_box() && offset.bytes() == 0 {
pointee.safe = Some(PointerKind::Box);
}
// FIXME(eddyb) This should be for `ptr::Unique<T>`, not `Box<T>`.
if let Some(ref mut pointee) = result {
if let ty::Adt(def, _) = this.ty.kind() {
if def.is_box() && offset.bytes() == 0 {
let optimize = tcx.sess.opts.optimize != OptLevel::No;
pointee.safe = Some(PointerKind::Box {
unpin: optimize && this.ty.boxed_ty().is_unpin(tcx, cx.param_env()),
});
}
}

result
}
};

result
}
};

debug!(
"pointee_info_at (offset={:?}, type kind: {:?}) => {:?}",
Expand Down
9 changes: 4 additions & 5 deletions compiler/rustc_ty_utils/src/abi.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ fn adjust_for_rust_scalar<'tcx>(
// <https://github.com/rust-lang/unsafe-code-guidelines/issues/381>). If LLVM had a way
// to say "dereferenceable on entry" we could use it here.
attrs.pointee_size = match kind {
PointerKind::Box
PointerKind::Box { .. }
| PointerKind::SharedRef { frozen: false }
| PointerKind::MutableRef { unpin: false } => Size::ZERO,
PointerKind::SharedRef { frozen: true }
Expand All @@ -278,17 +278,16 @@ fn adjust_for_rust_scalar<'tcx>(
// versions at all anymore. We still support turning it off using -Zmutable-noalias.
let noalias_mut_ref = cx.tcx.sess.opts.unstable_opts.mutable_noalias;

// `&mut` pointer parameters never alias other parameters,
// or mutable global data
//
// `&T` where `T` contains no `UnsafeCell<U>` is immutable, and can be marked as both
// `readonly` and `noalias`, as LLVM's definition of `noalias` is based solely on memory
// dependencies rather than pointer equality. However this only applies to arguments,
// not return values.
//
// `&mut T` and `Box<T>` where `T: Unpin` are unique and hence `noalias`.
let no_alias = match kind {
PointerKind::SharedRef { frozen } => frozen,
PointerKind::MutableRef { unpin } => unpin && noalias_mut_ref,
PointerKind::Box => noalias_for_box,
PointerKind::Box { unpin } => unpin && noalias_for_box,
};
// We can never add `noalias` in return position; that LLVM attribute has some very surprising semantics
// (see <https://github.com/rust-lang/unsafe-code-guidelines/issues/385#issuecomment-1368055745>).
Expand Down
33 changes: 27 additions & 6 deletions src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,32 @@ impl NewPermission {
}
}

fn from_box_ty<'tcx>(
ty: Ty<'tcx>,
kind: RetagKind,
cx: &crate::MiriInterpCx<'_, 'tcx>,
) -> Self {
// `ty` is not the `Box` but the field of the Box with this pointer (due to allocator handling).
let pointee = ty.builtin_deref(true).unwrap().ty;
if pointee.is_unpin(*cx.tcx, cx.param_env()) {
// A regular box. On `FnEntry` this is `noalias`, but not `dereferenceable` (hence only
// a weak protector).
NewPermission::Uniform {
perm: Permission::Unique,
access: Some(AccessKind::Write),
protector: (kind == RetagKind::FnEntry)
.then_some(ProtectorKind::WeakProtector),
}
} else {
// `!Unpin` boxes do not get `noalias` nor `dereferenceable`.
NewPermission::Uniform {
perm: Permission::SharedReadWrite,
access: None,
protector: None,
}
}
}

fn protector(&self) -> Option<ProtectorKind> {
match self {
NewPermission::Uniform { protector, .. } => *protector,
Expand Down Expand Up @@ -914,12 +940,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {

fn visit_box(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> {
// Boxes get a weak protectors, since they may be deallocated.
let new_perm = NewPermission::Uniform {
perm: Permission::Unique,
access: Some(AccessKind::Write),
protector: (self.kind == RetagKind::FnEntry)
.then_some(ProtectorKind::WeakProtector),
};
let new_perm = NewPermission::from_box_ty(place.layout.ty, self.kind, self.ecx);
self.retag_ptr_inplace(place, new_perm, self.retag_cause)
}

Expand Down
54 changes: 44 additions & 10 deletions src/tools/miri/tests/pass/stacked-borrows/future-self-referential.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,19 @@ impl Future for Delay {
}
}

fn mk_waker() -> Waker {
use std::sync::Arc;

struct MyWaker;
impl Wake for MyWaker {
fn wake(self: Arc<Self>) {
unimplemented!()
}
}

Waker::from(Arc::new(MyWaker))
}

async fn do_stuff() {
(&mut Delay::new(1)).await;
}
Expand Down Expand Up @@ -73,16 +86,7 @@ impl Future for DoStuff {
}

fn run_fut<T>(fut: impl Future<Output = T>) -> T {
use std::sync::Arc;

struct MyWaker;
impl Wake for MyWaker {
fn wake(self: Arc<Self>) {
unimplemented!()
}
}

let waker = Waker::from(Arc::new(MyWaker));
let waker = mk_waker();
let mut context = Context::from_waker(&waker);

let mut pinned = pin!(fut);
Expand All @@ -94,7 +98,37 @@ fn run_fut<T>(fut: impl Future<Output = T>) -> T {
}
}

fn self_referential_box() {
let waker = mk_waker();
let cx = &mut Context::from_waker(&waker);

async fn my_fut() -> i32 {
let val = 10;
let val_ref = &val;

let _ = Delay::new(1).await;

*val_ref
}

fn box_poll<F: Future>(
mut f: Pin<Box<F>>,
cx: &mut Context<'_>,
) -> (Pin<Box<F>>, Poll<F::Output>) {
let p = f.as_mut().poll(cx);
(f, p)
}

let my_fut = Box::pin(my_fut());
let (my_fut, p1) = box_poll(my_fut, cx);
assert!(p1.is_pending());
let (my_fut, p2) = box_poll(my_fut, cx);
assert!(p2.is_ready());
drop(my_fut);
}

fn main() {
run_fut(do_stuff());
run_fut(DoStuff::new());
self_referential_box();
}
10 changes: 8 additions & 2 deletions tests/codegen/function-arguments.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,12 @@ pub fn _box(x: Box<i32>) -> Box<i32> {
x
}

// CHECK: noundef nonnull align 4 {{i32\*|ptr}} @notunpin_box({{i32\*|ptr}} noundef nonnull align 4 %x)
#[no_mangle]
pub fn notunpin_box(x: Box<NotUnpin>) -> Box<NotUnpin> {
x
}

// CHECK: @struct_return({{%S\*|ptr}} noalias nocapture noundef sret(%S) dereferenceable(32){{( %0)?}})
#[no_mangle]
pub fn struct_return() -> S {
Expand Down Expand Up @@ -247,12 +253,12 @@ pub fn trait_raw(_: *const dyn Drop) {

// CHECK: @trait_box({{\{\}\*|ptr}} noalias noundef nonnull align 1{{( %0)?}}, {{.+}} noalias noundef readonly align {{.*}} dereferenceable({{.*}}){{( %1)?}})
#[no_mangle]
pub fn trait_box(_: Box<dyn Drop>) {
pub fn trait_box(_: Box<dyn Drop + Unpin>) {
}

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

Expand Down

0 comments on commit 1ef1687

Please sign in to comment.