Skip to content

Commit 1e7d04b

Browse files
Dylan-DPCDylan-DPC
authored
Rollup merge of #99011 - oli-obk:UnsoundCell, r=eddyb
`UnsafeCell` blocks niches inside its nested type from being available outside fixes #87341 This implements the plan by `@eddyb` in #87341 (comment) Somewhat related PR (not strictly necessary, but that cleanup made this PR simpler): #94527
2 parents 0083cd2 + 519c07b commit 1e7d04b

File tree

22 files changed

+139
-513
lines changed

22 files changed

+139
-513
lines changed

‎compiler/rustc_attr/src/builtin.rs

+3-5
Original file line numberDiff line numberDiff line change
@@ -856,7 +856,6 @@ pub enum ReprAttr {
856856
ReprSimd,
857857
ReprTransparent,
858858
ReprAlign(u32),
859-
ReprNoNiche,
860859
}
861860

862861
#[derive(Eq, PartialEq, Debug, Copy, Clone)]
@@ -904,7 +903,6 @@ pub fn parse_repr_attr(sess: &Session, attr: &Attribute) -> Vec<ReprAttr> {
904903
sym::packed => Some(ReprPacked(1)),
905904
sym::simd => Some(ReprSimd),
906905
sym::transparent => Some(ReprTransparent),
907-
sym::no_niche => Some(ReprNoNiche),
908906
sym::align => {
909907
let mut err = struct_span_err!(
910908
diagnostic,
@@ -943,7 +941,7 @@ pub fn parse_repr_attr(sess: &Session, attr: &Attribute) -> Vec<ReprAttr> {
943941
Ok(literal) => acc.push(ReprPacked(literal)),
944942
Err(message) => literal_error = Some(message),
945943
};
946-
} else if matches!(name, sym::C | sym::simd | sym::transparent | sym::no_niche)
944+
} else if matches!(name, sym::C | sym::simd | sym::transparent)
947945
|| int_type_of_word(name).is_some()
948946
{
949947
recognised = true;
@@ -1001,7 +999,7 @@ pub fn parse_repr_attr(sess: &Session, attr: &Attribute) -> Vec<ReprAttr> {
1001999
} else {
10021000
if matches!(
10031001
meta_item.name_or_empty(),
1004-
sym::C | sym::simd | sym::transparent | sym::no_niche
1002+
sym::C | sym::simd | sym::transparent
10051003
) || int_type_of_word(meta_item.name_or_empty()).is_some()
10061004
{
10071005
recognised = true;
@@ -1039,7 +1037,7 @@ pub fn parse_repr_attr(sess: &Session, attr: &Attribute) -> Vec<ReprAttr> {
10391037
.emit();
10401038
} else if matches!(
10411039
meta_item.name_or_empty(),
1042-
sym::C | sym::simd | sym::transparent | sym::no_niche
1040+
sym::C | sym::simd | sym::transparent
10431041
) || int_type_of_word(meta_item.name_or_empty()).is_some()
10441042
{
10451043
recognised = true;

‎compiler/rustc_const_eval/src/interpret/intern.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::Memory
217217
}
218218

219219
if let Some(def) = mplace.layout.ty.ty_adt_def() {
220-
if Some(def.did()) == self.ecx.tcx.lang_items().unsafe_cell_type() {
220+
if def.is_unsafe_cell() {
221221
// We are crossing over an `UnsafeCell`, we can mutate again. This means that
222222
// References we encounter inside here are interned as pointing to mutable
223223
// allocations.

‎compiler/rustc_const_eval/src/interpret/validity.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -821,7 +821,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
821821
// Special check preventing `UnsafeCell` in the inner part of constants
822822
if let Some(def) = op.layout.ty.ty_adt_def() {
823823
if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { inner: true, .. }))
824-
&& Some(def.did()) == self.ecx.tcx.lang_items().unsafe_cell_type()
824+
&& def.is_unsafe_cell()
825825
{
826826
throw_validation_failure!(self.path, { "`UnsafeCell` in a `const`" });
827827
}

‎compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,13 @@ impl Qualif for HasMutInterior {
9696
}
9797

9898
fn in_adt_inherently<'tcx>(
99-
cx: &ConstCx<'_, 'tcx>,
99+
_cx: &ConstCx<'_, 'tcx>,
100100
adt: AdtDef<'tcx>,
101101
_: SubstsRef<'tcx>,
102102
) -> bool {
103103
// Exactly one type, `UnsafeCell`, has the `HasMutInterior` qualif inherently.
104104
// It arises structurally for all other types.
105-
Some(adt.did()) == cx.tcx.lang_items().unsafe_cell_type()
105+
adt.is_unsafe_cell()
106106
}
107107
}
108108

‎compiler/rustc_feature/src/active.rs

-3
Original file line numberDiff line numberDiff line change
@@ -156,9 +156,6 @@ declare_features! (
156156
(active, intrinsics, "1.0.0", None, None),
157157
/// Allows using `#[lang = ".."]` attribute for linking items to special compiler logic.
158158
(active, lang_items, "1.0.0", None, None),
159-
/// Allows `#[repr(no_niche)]` (an implementation detail of `rustc`,
160-
/// it is not on path for eventual stabilization).
161-
(active, no_niche, "1.42.0", None, None),
162159
/// Allows using `#[omit_gdb_pretty_printer_section]`.
163160
(active, omit_gdb_pretty_printer_section, "1.5.0", None, None),
164161
/// Allows using `#[prelude_import]` on glob `use` items.

‎compiler/rustc_lint/src/types.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -703,9 +703,8 @@ fn ty_is_known_nonnull<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, mode: CItemKi
703703
return true;
704704
}
705705

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

‎compiler/rustc_middle/src/ty/adt.rs

+11
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ bitflags! {
5252
/// Indicates whether the variant list of this ADT is `#[non_exhaustive]`.
5353
/// (i.e., this flag is never set unless this ADT is an enum).
5454
const IS_VARIANT_LIST_NON_EXHAUSTIVE = 1 << 8;
55+
/// Indicates whether the type is `UnsafeCell`.
56+
const IS_UNSAFE_CELL = 1 << 9;
5557
}
5658
}
5759

@@ -247,6 +249,9 @@ impl AdtDefData {
247249
if Some(did) == tcx.lang_items().manually_drop() {
248250
flags |= AdtFlags::IS_MANUALLY_DROP;
249251
}
252+
if Some(did) == tcx.lang_items().unsafe_cell_type() {
253+
flags |= AdtFlags::IS_UNSAFE_CELL;
254+
}
250255

251256
AdtDefData { did, variants, flags, repr }
252257
}
@@ -333,6 +338,12 @@ impl<'tcx> AdtDef<'tcx> {
333338
self.flags().contains(AdtFlags::IS_BOX)
334339
}
335340

341+
/// Returns `true` if this is UnsafeCell<T>.
342+
#[inline]
343+
pub fn is_unsafe_cell(self) -> bool {
344+
self.flags().contains(AdtFlags::IS_UNSAFE_CELL)
345+
}
346+
336347
/// Returns `true` if this is `ManuallyDrop<T>`.
337348
#[inline]
338349
pub fn is_manually_drop(self) -> bool {

‎compiler/rustc_middle/src/ty/layout.rs

+30-13
Original file line numberDiff line numberDiff line change
@@ -542,14 +542,12 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
542542
debug!("univariant offset: {:?} field: {:#?}", offset, field);
543543
offsets[i as usize] = offset;
544544

545-
if !repr.hide_niche() {
546-
if let Some(mut niche) = field.largest_niche {
547-
let available = niche.available(dl);
548-
if available > largest_niche_available {
549-
largest_niche_available = available;
550-
niche.offset += offset;
551-
largest_niche = Some(niche);
552-
}
545+
if let Some(mut niche) = field.largest_niche {
546+
let available = niche.available(dl);
547+
if available > largest_niche_available {
548+
largest_niche_available = available;
549+
niche.offset += offset;
550+
largest_niche = Some(niche);
553551
}
554552
}
555553

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

10791077
let mut st = self.univariant_uninterned(ty, &variants[v], &def.repr(), kind)?;
10801078
st.variants = Variants::Single { index: v };
1079+
1080+
if def.is_unsafe_cell() {
1081+
let hide_niches = |scalar: &mut _| match scalar {
1082+
Scalar::Initialized { value, valid_range } => {
1083+
*valid_range = WrappingRange::full(value.size(dl))
1084+
}
1085+
// Already doesn't have any niches
1086+
Scalar::Union { .. } => {}
1087+
};
1088+
match &mut st.abi {
1089+
Abi::Uninhabited => {}
1090+
Abi::Scalar(scalar) => hide_niches(scalar),
1091+
Abi::ScalarPair(a, b) => {
1092+
hide_niches(a);
1093+
hide_niches(b);
1094+
}
1095+
Abi::Vector { element, count: _ } => hide_niches(element),
1096+
Abi::Aggregate { sized: _ } => {}
1097+
}
1098+
st.largest_niche = None;
1099+
return Ok(tcx.intern_layout(st));
1100+
}
1101+
10811102
let (start, end) = self.tcx.layout_scalar_valid_range(def.did());
10821103
match st.abi {
10831104
Abi::Scalar(ref mut scalar) | Abi::ScalarPair(ref mut scalar, _) => {
@@ -1106,11 +1127,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
11061127
}
11071128

11081129
// Update `largest_niche` if we have introduced a larger niche.
1109-
let niche = if def.repr().hide_niche() {
1110-
None
1111-
} else {
1112-
Niche::from_scalar(dl, Size::ZERO, *scalar)
1113-
};
1130+
let niche = Niche::from_scalar(dl, Size::ZERO, *scalar);
11141131
if let Some(niche) = niche {
11151132
match st.largest_niche {
11161133
Some(largest_niche) => {

‎compiler/rustc_middle/src/ty/mod.rs

+1-9
Original file line numberDiff line numberDiff line change
@@ -1720,11 +1720,9 @@ bitflags! {
17201720
const IS_TRANSPARENT = 1 << 2;
17211721
// Internal only for now. If true, don't reorder fields.
17221722
const IS_LINEAR = 1 << 3;
1723-
// If true, don't expose any niche to type's context.
1724-
const HIDE_NICHE = 1 << 4;
17251723
// If true, the type's layout can be randomized using
17261724
// the seed stored in `ReprOptions.layout_seed`
1727-
const RANDOMIZE_LAYOUT = 1 << 5;
1725+
const RANDOMIZE_LAYOUT = 1 << 4;
17281726
// Any of these flags being set prevent field reordering optimisation.
17291727
const IS_UNOPTIMISABLE = ReprFlags::IS_C.bits
17301728
| ReprFlags::IS_SIMD.bits
@@ -1781,7 +1779,6 @@ impl ReprOptions {
17811779
ReprFlags::empty()
17821780
}
17831781
attr::ReprTransparent => ReprFlags::IS_TRANSPARENT,
1784-
attr::ReprNoNiche => ReprFlags::HIDE_NICHE,
17851782
attr::ReprSimd => ReprFlags::IS_SIMD,
17861783
attr::ReprInt(i) => {
17871784
size = Some(i);
@@ -1834,11 +1831,6 @@ impl ReprOptions {
18341831
self.flags.contains(ReprFlags::IS_LINEAR)
18351832
}
18361833

1837-
#[inline]
1838-
pub fn hide_niche(&self) -> bool {
1839-
self.flags.contains(ReprFlags::HIDE_NICHE)
1840-
}
1841-
18421834
/// Returns the discriminant type, given these `repr` options.
18431835
/// This must only be called on enums!
18441836
pub fn discr_type(&self) -> attr::IntType {

‎compiler/rustc_passes/src/check_attr.rs

+2-19
Original file line numberDiff line numberDiff line change
@@ -1808,21 +1808,6 @@ impl CheckAttrVisitor<'_> {
18081808
_ => ("a", "struct, enum, or union"),
18091809
}
18101810
}
1811-
sym::no_niche => {
1812-
if !self.tcx.features().enabled(sym::no_niche) {
1813-
feature_err(
1814-
&self.tcx.sess.parse_sess,
1815-
sym::no_niche,
1816-
hint.span(),
1817-
"the attribute `repr(no_niche)` is currently unstable",
1818-
)
1819-
.emit();
1820-
}
1821-
match target {
1822-
Target::Struct | Target::Enum => continue,
1823-
_ => ("a", "struct or enum"),
1824-
}
1825-
}
18261811
sym::i8
18271812
| sym::u8
18281813
| sym::i16
@@ -1870,10 +1855,8 @@ impl CheckAttrVisitor<'_> {
18701855
// This is not ideal, but tracking precisely which ones are at fault is a huge hassle.
18711856
let hint_spans = hints.iter().map(|hint| hint.span());
18721857

1873-
// Error on repr(transparent, <anything else apart from no_niche>).
1874-
let non_no_niche = |hint: &&NestedMetaItem| hint.name_or_empty() != sym::no_niche;
1875-
let non_no_niche_count = hints.iter().filter(non_no_niche).count();
1876-
if is_transparent && non_no_niche_count > 1 {
1858+
// Error on repr(transparent, <anything else>).
1859+
if is_transparent && hints.len() > 1 {
18771860
let hint_spans: Vec<_> = hint_spans.clone().collect();
18781861
struct_span_err!(
18791862
self.tcx.sess,

‎compiler/rustc_span/src/symbol.rs

-2
Original file line numberDiff line numberDiff line change
@@ -980,7 +980,6 @@ symbols! {
980980
no_link,
981981
no_main,
982982
no_mangle,
983-
no_niche,
984983
no_sanitize,
985984
no_stack_check,
986985
no_start,
@@ -1153,7 +1152,6 @@ symbols! {
11531152
repr128,
11541153
repr_align,
11551154
repr_align_enum,
1156-
repr_no_niche,
11571155
repr_packed,
11581156
repr_simd,
11591157
repr_transparent,

‎library/core/src/cell.rs

-1
Original file line numberDiff line numberDiff line change
@@ -1856,7 +1856,6 @@ impl<T: ?Sized + fmt::Display> fmt::Display for RefMut<'_, T> {
18561856
#[lang = "unsafe_cell"]
18571857
#[stable(feature = "rust1", since = "1.0.0")]
18581858
#[repr(transparent)]
1859-
#[repr(no_niche)] // rust-lang/rust#68303.
18601859
pub struct UnsafeCell<T: ?Sized> {
18611860
value: T,
18621861
}

‎library/core/src/lib.rs

-1
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,6 @@
191191
#![feature(never_type)]
192192
#![feature(no_core)]
193193
#![feature(no_coverage)] // rust-lang/rust#84605
194-
#![feature(no_niche)] // rust-lang/rust#68303
195194
#![feature(platform_intrinsics)]
196195
#![feature(prelude_import)]
197196
#![feature(repr_simd)]

‎src/test/ui/layout/unsafe-cell-hides-niche.rs

+64-14
Original file line numberDiff line numberDiff line change
@@ -3,30 +3,80 @@
33
// test checks that an `Option<UnsafeCell<NonZeroU32>>` has the same
44
// size in memory as an `Option<UnsafeCell<u32>>` (namely, 8 bytes).
55

6-
// run-pass
6+
// check-pass
7+
// compile-flags: --crate-type=lib
8+
// only-x86
79

8-
#![feature(no_niche)]
10+
#![feature(repr_simd)]
911

10-
use std::cell::UnsafeCell;
12+
use std::cell::{UnsafeCell, RefCell, Cell};
1113
use std::mem::size_of;
1214
use std::num::NonZeroU32 as N32;
15+
use std::sync::{Mutex, RwLock};
1316

1417
struct Wrapper<T>(T);
1518

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

19-
#[repr(no_niche)]
20-
struct NoNiche<T>(T);
22+
struct NoNiche<T>(UnsafeCell<T>);
2123

22-
fn main() {
23-
assert_eq!(size_of::<Option<Wrapper<u32>>>(), 8);
24-
assert_eq!(size_of::<Option<Wrapper<N32>>>(), 4);
25-
assert_eq!(size_of::<Option<Transparent<u32>>>(), 8);
26-
assert_eq!(size_of::<Option<Transparent<N32>>>(), 4);
27-
assert_eq!(size_of::<Option<NoNiche<u32>>>(), 8);
28-
assert_eq!(size_of::<Option<NoNiche<N32>>>(), 8);
24+
struct Size<const S: usize>;
2925

30-
assert_eq!(size_of::<Option<UnsafeCell<u32>>>(), 8);
31-
assert_eq!(size_of::<Option<UnsafeCell<N32>>>(), 8);
26+
macro_rules! check_sizes {
27+
(check_one_specific_size: $ty:ty, $size:expr) => {
28+
const _: Size::<{$size}> = Size::<{size_of::<$ty>()}>;
29+
};
30+
// Any tests run on `UnsafeCell` must be the same for `Cell`
31+
(UnsafeCell<$ty:ty>: $size:expr => $optioned_size:expr) => {
32+
check_sizes!(Cell<$ty>: $size => $optioned_size);
33+
check_sizes!(@actual_check: UnsafeCell<$ty>: $size => $optioned_size);
34+
};
35+
($ty:ty: $size:expr => $optioned_size:expr) => {
36+
check_sizes!(@actual_check: $ty: $size => $optioned_size);
37+
};
38+
// This branch does the actual checking logic, the `@actual_check` prefix is here to distinguish
39+
// it from other branches and not accidentally match any.
40+
(@actual_check: $ty:ty: $size:expr => $optioned_size:expr) => {
41+
check_sizes!(check_one_specific_size: $ty, $size);
42+
check_sizes!(check_one_specific_size: Option<$ty>, $optioned_size);
43+
check_sizes!(check_no_niche_opt: $size != $optioned_size, $ty);
44+
};
45+
// only check that there is no niche (size goes up when wrapped in an option),
46+
// don't check actual sizes
47+
($ty:ty) => {
48+
check_sizes!(check_no_niche_opt: true, $ty);
49+
};
50+
(check_no_niche_opt: $no_niche_opt:expr, $ty:ty) => {
51+
const _: () = if $no_niche_opt { assert!(size_of::<$ty>() < size_of::<Option<$ty>>()); };
52+
};
3253
}
54+
55+
const PTR_SIZE: usize = std::mem::size_of::<*const ()>();
56+
57+
check_sizes!(Wrapper<u32>: 4 => 8);
58+
check_sizes!(Wrapper<N32>: 4 => 4); // (✓ niche opt)
59+
check_sizes!(Transparent<u32>: 4 => 8);
60+
check_sizes!(Transparent<N32>: 4 => 4); // (✓ niche opt)
61+
check_sizes!(NoNiche<u32>: 4 => 8);
62+
check_sizes!(NoNiche<N32>: 4 => 8);
63+
64+
check_sizes!(UnsafeCell<u32>: 4 => 8);
65+
check_sizes!(UnsafeCell<N32>: 4 => 8);
66+
67+
check_sizes!(UnsafeCell<&()>: PTR_SIZE => PTR_SIZE * 2);
68+
check_sizes!( RefCell<&()>: PTR_SIZE * 2 => PTR_SIZE * 3);
69+
70+
check_sizes!(RwLock<&()>);
71+
check_sizes!(Mutex<&()>);
72+
73+
check_sizes!(UnsafeCell<&[i32]>: PTR_SIZE * 2 => PTR_SIZE * 3);
74+
check_sizes!(UnsafeCell<(&(), &())>: PTR_SIZE * 2 => PTR_SIZE * 3);
75+
76+
trait Trait {}
77+
check_sizes!(UnsafeCell<&dyn Trait>: PTR_SIZE * 2 => PTR_SIZE * 3);
78+
79+
#[repr(simd)]
80+
pub struct Vec4<T>([T; 4]);
81+
82+
check_sizes!(UnsafeCell<Vec4<N32>>: 16 => 32);

0 commit comments

Comments
 (0)