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

UnsafeCell blocks niches inside its nested type from being available outside #99011

Merged
merged 19 commits into from
Jul 13, 2022
Merged
Show file tree
Hide file tree
Changes from 16 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
8 changes: 3 additions & 5 deletions compiler/rustc_attr/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -856,7 +856,6 @@ pub enum ReprAttr {
ReprSimd,
ReprTransparent,
ReprAlign(u32),
ReprNoNiche,
}

#[derive(Eq, PartialEq, Debug, Copy, Clone)]
Expand Down Expand Up @@ -904,7 +903,6 @@ pub fn parse_repr_attr(sess: &Session, attr: &Attribute) -> Vec<ReprAttr> {
sym::packed => Some(ReprPacked(1)),
sym::simd => Some(ReprSimd),
sym::transparent => Some(ReprTransparent),
sym::no_niche => Some(ReprNoNiche),
sym::align => {
let mut err = struct_span_err!(
diagnostic,
Expand Down Expand Up @@ -943,7 +941,7 @@ pub fn parse_repr_attr(sess: &Session, attr: &Attribute) -> Vec<ReprAttr> {
Ok(literal) => acc.push(ReprPacked(literal)),
Err(message) => literal_error = Some(message),
};
} else if matches!(name, sym::C | sym::simd | sym::transparent | sym::no_niche)
} else if matches!(name, sym::C | sym::simd | sym::transparent)
|| int_type_of_word(name).is_some()
{
recognised = true;
Expand Down Expand Up @@ -1001,7 +999,7 @@ pub fn parse_repr_attr(sess: &Session, attr: &Attribute) -> Vec<ReprAttr> {
} else {
if matches!(
meta_item.name_or_empty(),
sym::C | sym::simd | sym::transparent | sym::no_niche
sym::C | sym::simd | sym::transparent
) || int_type_of_word(meta_item.name_or_empty()).is_some()
{
recognised = true;
Expand Down Expand Up @@ -1039,7 +1037,7 @@ pub fn parse_repr_attr(sess: &Session, attr: &Attribute) -> Vec<ReprAttr> {
.emit();
} else if matches!(
meta_item.name_or_empty(),
sym::C | sym::simd | sym::transparent | sym::no_niche
sym::C | sym::simd | sym::transparent
) || int_type_of_word(meta_item.name_or_empty()).is_some()
{
recognised = true;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/intern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::Memory
}

if let Some(def) = mplace.layout.ty.ty_adt_def() {
if Some(def.did()) == self.ecx.tcx.lang_items().unsafe_cell_type() {
if def.is_unsafe_cell() {
// We are crossing over an `UnsafeCell`, we can mutate again. This means that
// References we encounter inside here are interned as pointing to mutable
// allocations.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
// Special check preventing `UnsafeCell` in the inner part of constants
if let Some(def) = op.layout.ty.ty_adt_def() {
if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { inner: true, .. }))
&& Some(def.did()) == self.ecx.tcx.lang_items().unsafe_cell_type()
&& def.is_unsafe_cell()
{
throw_validation_failure!(self.path, { "`UnsafeCell` in a `const`" });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,13 @@ impl Qualif for HasMutInterior {
}

fn in_adt_inherently<'tcx>(
cx: &ConstCx<'_, 'tcx>,
_cx: &ConstCx<'_, 'tcx>,
adt: AdtDef<'tcx>,
_: SubstsRef<'tcx>,
) -> bool {
// Exactly one type, `UnsafeCell`, has the `HasMutInterior` qualif inherently.
// It arises structurally for all other types.
Some(adt.did()) == cx.tcx.lang_items().unsafe_cell_type()
adt.is_unsafe_cell()
}
}

Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_feature/src/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,6 @@ declare_features! (
(active, intrinsics, "1.0.0", None, None),
/// Allows using `#[lang = ".."]` attribute for linking items to special compiler logic.
(active, lang_items, "1.0.0", None, None),
/// Allows `#[repr(no_niche)]` (an implementation detail of `rustc`,
/// it is not on path for eventual stabilization).
(active, no_niche, "1.42.0", None, None),
/// Allows using `#[omit_gdb_pretty_printer_section]`.
(active, omit_gdb_pretty_printer_section, "1.5.0", None, None),
/// Allows using `#[prelude_import]` on glob `use` items.
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_lint/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -703,9 +703,8 @@ fn ty_is_known_nonnull<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, mode: CItemKi
return true;
}

// Types with a `#[repr(no_niche)]` attribute have their niche hidden.
// The attribute is used by the UnsafeCell for example (the only use so far).
if def.repr().hide_niche() {
// `UnsafeCell` has its niche hidden.
if def.is_unsafe_cell() {
return false;
}

Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_middle/src/ty/adt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ bitflags! {
/// Indicates whether the variant list of this ADT is `#[non_exhaustive]`.
/// (i.e., this flag is never set unless this ADT is an enum).
const IS_VARIANT_LIST_NON_EXHAUSTIVE = 1 << 8;
/// Indicates whether the type is `UnsafeCell`.
const IS_UNSAFE_CELL = 1 << 9;
}
}

Expand Down Expand Up @@ -242,6 +244,9 @@ impl AdtDefData {
if Some(did) == tcx.lang_items().manually_drop() {
flags |= AdtFlags::IS_MANUALLY_DROP;
}
if Some(did) == tcx.lang_items().unsafe_cell_type() {
flags |= AdtFlags::IS_UNSAFE_CELL;
}

AdtDefData { did, variants, flags, repr }
}
Expand Down Expand Up @@ -328,6 +333,12 @@ impl<'tcx> AdtDef<'tcx> {
self.flags().contains(AdtFlags::IS_BOX)
}

/// Returns `true` if this is UnsafeCell<T>.
#[inline]
pub fn is_unsafe_cell(self) -> bool {
self.flags().contains(AdtFlags::IS_UNSAFE_CELL)
}

/// Returns `true` if this is `ManuallyDrop<T>`.
#[inline]
pub fn is_manually_drop(self) -> bool {
Expand Down
43 changes: 30 additions & 13 deletions compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,14 +542,12 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
debug!("univariant offset: {:?} field: {:#?}", offset, field);
offsets[i as usize] = offset;

if !repr.hide_niche() {
if let Some(mut niche) = field.largest_niche {
let available = niche.available(dl);
if available > largest_niche_available {
largest_niche_available = available;
niche.offset += offset;
largest_niche = Some(niche);
}
if let Some(mut niche) = field.largest_niche {
let available = niche.available(dl);
if available > largest_niche_available {
largest_niche_available = available;
niche.offset += offset;
largest_niche = Some(niche);
}
}

Expand Down Expand Up @@ -1078,6 +1076,29 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {

let mut st = self.univariant_uninterned(ty, &variants[v], &def.repr(), kind)?;
st.variants = Variants::Single { index: v };

if def.is_unsafe_cell() {
let hide_niches = |scalar: &mut _| match scalar {
Scalar::Initialized { value, valid_range } => {
*valid_range = WrappingRange::full(value.size(dl))
}
// Already doesn't have any niches
Scalar::Union { .. } => {}
};
match &mut st.abi {
Abi::Uninhabited => {}
Abi::Scalar(scalar) => hide_niches(scalar),
Abi::ScalarPair(a, b) => {
hide_niches(a);
hide_niches(b);
}
Abi::Vector { element, count: _ } => hide_niches(element),
Abi::Aggregate { sized: _ } => {}
}
st.largest_niche = None;
return Ok(tcx.intern_layout(st));
}

let (start, end) = self.tcx.layout_scalar_valid_range(def.did());
match st.abi {
Abi::Scalar(ref mut scalar) | Abi::ScalarPair(ref mut scalar, _) => {
Expand Down Expand Up @@ -1106,11 +1127,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
}

// Update `largest_niche` if we have introduced a larger niche.
let niche = if def.repr().hide_niche() {
None
} else {
Niche::from_scalar(dl, Size::ZERO, *scalar)
};
let niche = Niche::from_scalar(dl, Size::ZERO, *scalar);
if let Some(niche) = niche {
match st.largest_niche {
Some(largest_niche) => {
Expand Down
10 changes: 1 addition & 9 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1719,11 +1719,9 @@ bitflags! {
const IS_TRANSPARENT = 1 << 2;
// Internal only for now. If true, don't reorder fields.
const IS_LINEAR = 1 << 3;
// If true, don't expose any niche to type's context.
const HIDE_NICHE = 1 << 4;
// If true, the type's layout can be randomized using
// the seed stored in `ReprOptions.layout_seed`
const RANDOMIZE_LAYOUT = 1 << 5;
const RANDOMIZE_LAYOUT = 1 << 4;
// Any of these flags being set prevent field reordering optimisation.
const IS_UNOPTIMISABLE = ReprFlags::IS_C.bits
| ReprFlags::IS_SIMD.bits
Expand Down Expand Up @@ -1780,7 +1778,6 @@ impl ReprOptions {
ReprFlags::empty()
}
attr::ReprTransparent => ReprFlags::IS_TRANSPARENT,
attr::ReprNoNiche => ReprFlags::HIDE_NICHE,
attr::ReprSimd => ReprFlags::IS_SIMD,
attr::ReprInt(i) => {
size = Some(i);
Expand Down Expand Up @@ -1833,11 +1830,6 @@ impl ReprOptions {
self.flags.contains(ReprFlags::IS_LINEAR)
}

#[inline]
pub fn hide_niche(&self) -> bool {
self.flags.contains(ReprFlags::HIDE_NICHE)
}

/// Returns the discriminant type, given these `repr` options.
/// This must only be called on enums!
pub fn discr_type(&self) -> attr::IntType {
Expand Down
21 changes: 2 additions & 19 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1808,21 +1808,6 @@ impl CheckAttrVisitor<'_> {
_ => ("a", "struct, enum, or union"),
}
}
sym::no_niche => {
if !self.tcx.features().enabled(sym::no_niche) {
feature_err(
&self.tcx.sess.parse_sess,
sym::no_niche,
hint.span(),
"the attribute `repr(no_niche)` is currently unstable",
)
.emit();
}
match target {
Target::Struct | Target::Enum => continue,
_ => ("a", "struct or enum"),
}
}
sym::i8
| sym::u8
| sym::i16
Expand Down Expand Up @@ -1870,10 +1855,8 @@ impl CheckAttrVisitor<'_> {
// This is not ideal, but tracking precisely which ones are at fault is a huge hassle.
let hint_spans = hints.iter().map(|hint| hint.span());

// Error on repr(transparent, <anything else apart from no_niche>).
let non_no_niche = |hint: &&NestedMetaItem| hint.name_or_empty() != sym::no_niche;
let non_no_niche_count = hints.iter().filter(non_no_niche).count();
if is_transparent && non_no_niche_count > 1 {
// Error on repr(transparent, <anything else>).
if is_transparent && hints.len() > 1 {
let hint_spans: Vec<_> = hint_spans.clone().collect();
struct_span_err!(
self.tcx.sess,
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -979,7 +979,6 @@ symbols! {
no_link,
no_main,
no_mangle,
no_niche,
no_sanitize,
no_stack_check,
no_start,
Expand Down Expand Up @@ -1152,7 +1151,6 @@ symbols! {
repr128,
repr_align,
repr_align_enum,
repr_no_niche,
repr_packed,
repr_simd,
repr_transparent,
Expand Down
1 change: 0 additions & 1 deletion library/core/src/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1856,7 +1856,6 @@ impl<T: ?Sized + fmt::Display> fmt::Display for RefMut<'_, T> {
#[lang = "unsafe_cell"]
#[stable(feature = "rust1", since = "1.0.0")]
#[repr(transparent)]
#[repr(no_niche)] // rust-lang/rust#68303.
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
pub struct UnsafeCell<T: ?Sized> {
value: T,
}
Expand Down
1 change: 0 additions & 1 deletion library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@
#![feature(never_type)]
#![feature(no_core)]
#![feature(no_coverage)] // rust-lang/rust#84605
#![feature(no_niche)] // rust-lang/rust#68303
#![feature(platform_intrinsics)]
#![feature(prelude_import)]
#![feature(repr_simd)]
Expand Down
66 changes: 52 additions & 14 deletions src/test/ui/layout/unsafe-cell-hides-niche.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,68 @@
// test checks that an `Option<UnsafeCell<NonZeroU32>>` has the same
// size in memory as an `Option<UnsafeCell<u32>>` (namely, 8 bytes).

// run-pass
// check-pass
// compile-flags: --crate-type=lib

#![feature(no_niche)]
#![feature(repr_simd)]

use std::cell::UnsafeCell;
use std::cell::{UnsafeCell, RefCell, Cell};
use std::mem::size_of;
use std::num::NonZeroU32 as N32;
use std::sync::{Mutex, RwLock};

struct Wrapper<T>(T);

#[repr(transparent)]
struct Transparent<T>(T);

#[repr(no_niche)]
struct NoNiche<T>(T);
struct NoNiche<T>(UnsafeCell<T>);

fn main() {
assert_eq!(size_of::<Option<Wrapper<u32>>>(), 8);
assert_eq!(size_of::<Option<Wrapper<N32>>>(), 4);
assert_eq!(size_of::<Option<Transparent<u32>>>(), 8);
assert_eq!(size_of::<Option<Transparent<N32>>>(), 4);
assert_eq!(size_of::<Option<NoNiche<u32>>>(), 8);
assert_eq!(size_of::<Option<NoNiche<N32>>>(), 8);
struct Size<const S: usize>;

assert_eq!(size_of::<Option<UnsafeCell<u32>>>(), 8);
assert_eq!(size_of::<Option<UnsafeCell<N32>>>(), 8);
macro_rules! check_sizes {
(check_one_specific_size: $ty:ty, $size:expr) => {
const _: Size::<{$size}> = Size::<{size_of::<$ty>()}>;
};
($ty:ty, $size:expr, $optioned_size:expr) => {
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
check_sizes!(check_one_specific_size: $ty, $size);
check_sizes!(check_one_specific_size: Option<$ty>, $optioned_size);
check_sizes!(check_no_niche_opt: $size != $optioned_size, $ty);
};
($ty:ty) => {
check_sizes!(check_no_niche_opt: true, $ty);
};
(check_no_niche_opt: $no_niche_opt:expr, $ty:ty) => {
const _: () = if $no_niche_opt { assert!(size_of::<$ty>() < size_of::<Option<$ty>>()); };
};
}

const PTR_SIZE: usize = std::mem::size_of::<*const ()>();

check_sizes!(Wrapper<u32>, 4, 8);
check_sizes!(Wrapper<N32>, 4, 4); // (✓ niche opt)
check_sizes!(Transparent<u32>, 4, 8);
check_sizes!(Transparent<N32>, 4, 4); // (✓ niche opt)
check_sizes!(NoNiche<u32>, 4, 8);
check_sizes!(NoNiche<N32>, 4, 8);

check_sizes!(UnsafeCell<u32>, 4, 8);
check_sizes!(UnsafeCell<N32>, 4, 8);

check_sizes!(UnsafeCell<&()> , PTR_SIZE, PTR_SIZE * 2);
check_sizes!( Cell<&()> , PTR_SIZE, PTR_SIZE * 2);
check_sizes!( RefCell<&()> , PTR_SIZE * 2, PTR_SIZE * 3);

check_sizes!(RwLock<&()>);
check_sizes!(Mutex<&()>);

check_sizes!(UnsafeCell<&[i32]> , PTR_SIZE * 2, PTR_SIZE * 3);
check_sizes!(UnsafeCell<(&(), &())> , PTR_SIZE * 2, PTR_SIZE * 3);
oli-obk marked this conversation as resolved.
Show resolved Hide resolved

trait Trait {}
check_sizes!(UnsafeCell<&dyn Trait> , PTR_SIZE * 2, PTR_SIZE * 3);

#[repr(simd)]
pub struct Vec4<T>([T; 4]);

check_sizes!(UnsafeCell<Vec4<N32>> , 16, 32);
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 1 addition & 3 deletions src/test/ui/lint/clashing-extern-fn.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// check-pass
// aux-build:external_extern_fn.rs
#![crate_type = "lib"]
#![feature(no_niche)]
#![warn(clashing_extern_declarations)]

mod redeclared_different_signature {
Expand Down Expand Up @@ -400,9 +399,8 @@ mod hidden_niche {
#[repr(transparent)]
struct Transparent { x: NonZeroUsize }

#[repr(no_niche)]
#[repr(transparent)]
struct TransparentNoNiche { y: NonZeroUsize }
struct TransparentNoNiche { y: UnsafeCell<NonZeroUsize> }

extern "C" {
fn hidden_niche_transparent() -> Option<Transparent>;
Expand Down
Loading