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

Hide niches under UnsafeCell #68491

Merged
merged 4 commits into from
Feb 11, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion src/ci/docker/dist-various-2/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ RUN /tmp/build-solaris-toolchain.sh sparcv9 sparcv9 solaris-sparc
COPY dist-various-2/build-x86_64-fortanix-unknown-sgx-toolchain.sh /tmp/
# We pass the commit id of the port of LLVM's libunwind to the build script.
# Any update to the commit id here, should cause the container image to be re-built from this point on.
RUN /tmp/build-x86_64-fortanix-unknown-sgx-toolchain.sh "53b586346f2c7870e20b170decdc30729d97c42b"
RUN /tmp/build-x86_64-fortanix-unknown-sgx-toolchain.sh "5125c169b30837208a842f85f7ae44a83533bd0e"

COPY dist-various-2/build-wasi-toolchain.sh /tmp/
RUN /tmp/build-wasi-toolchain.sh
Expand Down
1 change: 1 addition & 0 deletions src/libcore/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1475,6 +1475,7 @@ impl<T: ?Sized + fmt::Display> fmt::Display for RefMut<'_, T> {
#[lang = "unsafe_cell"]
#[stable(feature = "rust1", since = "1.0.0")]
#[repr(transparent)]
#[cfg_attr(not(bootstrap), repr(no_niche))] // rust-lang/rust#68303.
pub struct UnsafeCell<T: ?Sized> {
value: T,
}
Expand Down
1 change: 1 addition & 0 deletions src/libcore/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@
#![feature(const_type_id)]
#![feature(const_caller_location)]
#![feature(assoc_int_consts)]
#![cfg_attr(not(bootstrap), feature(no_niche))] // rust-lang/rust#68303

#[prelude_import]
#[allow(unused)]
Expand Down
25 changes: 18 additions & 7 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,12 +356,14 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
debug!("univariant offset: {:?} field: {:#?}", offset, field);
offsets[i as usize] = offset;

if let Some(mut niche) = field.largest_niche.clone() {
let available = niche.available(dl);
if available > largest_niche_available {
largest_niche_available = available;
niche.offset += offset;
largest_niche = Some(niche);
if !repr.hide_niche() {
if let Some(mut niche) = field.largest_niche.clone() {
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -838,7 +840,11 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
}

// Update `largest_niche` if we have introduced a larger niche.
let niche = Niche::from_scalar(dl, Size::ZERO, scalar.clone());
let niche = if def.repr.hide_niche() {
None
} else {
Niche::from_scalar(dl, Size::ZERO, scalar.clone())
};
if let Some(niche) = niche {
match &st.largest_niche {
Some(largest_niche) => {
Expand All @@ -863,6 +869,11 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
return Ok(tcx.intern_layout(st));
}

// At this point, we have handled all unions and
// structs. (We have also handled univariant enums
// that allow representation optimization.)
assert!(def.is_enum());

// The current code for niche-filling relies on variant indices
// instead of actual discriminants, so dataful enums with
// explicit discriminants (RFC #2363) would misbehave.
Expand Down
8 changes: 7 additions & 1 deletion src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2041,7 +2041,8 @@ 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;
// 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 @@ -2078,6 +2079,7 @@ 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 @@ -2118,6 +2120,10 @@ impl ReprOptions {
pub fn linear(&self) -> bool {
self.flags.contains(ReprFlags::IS_LINEAR)
}
#[inline]
pub fn hide_niche(&self) -> bool {
self.flags.contains(ReprFlags::HIDE_NICHE)
}

pub fn discr_type(&self) -> attr::IntType {
self.int.unwrap_or(attr::SignedInt(ast::IntTy::Isize))
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_attr/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,7 @@ pub enum ReprAttr {
ReprSimd,
ReprTransparent,
ReprAlign(u32),
ReprNoNiche,
}

#[derive(Eq, PartialEq, Debug, RustcEncodable, RustcDecodable, Copy, Clone, HashStable_Generic)]
Expand Down Expand Up @@ -893,6 +894,7 @@ pub fn find_repr_attrs(sess: &ParseSess, attr: &Attribute) -> Vec<ReprAttr> {
sym::packed => Some(ReprPacked(1)),
sym::simd => Some(ReprSimd),
sym::transparent => Some(ReprTransparent),
sym::no_niche => Some(ReprNoNiche),
name => int_type_of_word(name).map(ReprInt),
};

Expand Down
3 changes: 2 additions & 1 deletion src/librustc_builtin_macros/deriving/generic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,8 @@ fn find_repr_type_name(sess: &ParseSess, type_attrs: &[ast::Attribute]) -> &'sta
attr::ReprPacked(_)
| attr::ReprSimd
| attr::ReprAlign(_)
| attr::ReprTransparent => continue,
| attr::ReprTransparent
| attr::ReprNoNiche => continue,

attr::ReprC => "i32",

Expand Down
4 changes: 4 additions & 0 deletions src/librustc_feature/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,10 @@ declare_features! (
/// Added for testing E0705; perma-unstable.
(active, test_2018_feature, "1.31.0", None, Some(Edition::Edition2018)),

/// 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),

// no-tracking-issue-end

// -------------------------------------------------------------------------
Expand Down
24 changes: 21 additions & 3 deletions src/librustc_passes/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::DUMMY_HIR_ID;
use rustc_hir::{self, HirId, Item, ItemKind, TraitItem};
use rustc_session::lint::builtin::{CONFLICTING_REPR_HINTS, UNUSED_ATTRIBUTES};
use rustc_session::parse::feature_err;
use rustc_span::symbol::sym;
use rustc_span::Span;
use syntax::ast::Attribute;
use syntax::ast::{Attribute, NestedMetaItem};
use syntax::attr;

fn target_from_impl_item<'tcx>(tcx: TyCtxt<'tcx>, impl_item: &hir::ImplItem<'_>) -> Target {
Expand Down Expand Up @@ -287,6 +288,21 @@ impl CheckAttrVisitor<'tcx> {
_ => ("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 @@ -314,8 +330,10 @@ impl CheckAttrVisitor<'tcx> {
// 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>).
if is_transparent && hints.len() > 1 {
// 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 {
let hint_spans: Vec<_> = hint_spans.clone().collect();
struct_span_err!(
self.tcx.sess,
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_span/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,7 @@ symbols! {
non_exhaustive,
non_modrs_mods,
no_sanitize,
no_niche,
no_stack_check,
no_start,
no_std,
Expand Down Expand Up @@ -587,6 +588,7 @@ symbols! {
repr128,
repr_align,
repr_align_enum,
repr_no_niche,
repr_packed,
repr_simd,
repr_transparent,
Expand Down
24 changes: 13 additions & 11 deletions src/libstd/sys/sgx/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ pub struct RWLock {
writer: SpinMutex<WaitVariable<bool>>,
}

// Below is to check at compile time, that RWLock has size of 128 bytes.
// Check at compile time that RWLock size matches C definition (see test_c_rwlock_initializer below)
#[allow(dead_code)]
unsafe fn rw_lock_size_assert(r: RWLock) {
mem::transmute::<RWLock, [u8; 128]>(r);
mem::transmute::<RWLock, [u8; 144]>(r);
}

impl RWLock {
Expand Down Expand Up @@ -210,15 +210,17 @@ mod tests {
// be changed too.
#[test]
fn test_c_rwlock_initializer() {
#[rustfmt::skip]
const RWLOCK_INIT: &[u8] = &[
0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x3, 0x0,
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0,
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x3, 0x0, 0x0, 0x0, 0x0, 0x0,
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
/* 0x00 */ 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
/* 0x10 */ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x2, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
/* 0x20 */ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
/* 0x30 */ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
/* 0x40 */ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
/* 0x50 */ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
/* 0x60 */ 0x2, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
/* 0x70 */ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
/* 0x80 */ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
];

#[inline(never)]
Expand All @@ -239,7 +241,7 @@ mod tests {
zero_stack();
let mut init = MaybeUninit::<RWLock>::zeroed();
rwlock_new(&mut init);
assert_eq!(mem::transmute::<_, [u8; 128]>(init.assume_init()).as_slice(), RWLOCK_INIT)
assert_eq!(mem::transmute::<_, [u8; 144]>(init.assume_init()).as_slice(), RWLOCK_INIT)
};
}
}
32 changes: 32 additions & 0 deletions src/test/ui/layout/unsafe-cell-hides-niche.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// For rust-lang/rust#68303: the contents of `UnsafeCell<T>` cannot
// participate in the niche-optimization for enum discriminants. This
// test checks that an `Option<UnsafeCell<NonZeroU32>>` has the same
// size in memory as an `Option<UnsafeCell<u32>>` (namely, 8 bytes).

// run-pass

#![feature(no_niche)]

use std::cell::UnsafeCell;
use std::mem::size_of;
use std::num::NonZeroU32 as N32;

struct Wrapper<T>(T);

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

#[repr(no_niche)]
struct NoNiche<T>(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);

assert_eq!(size_of::<Option<UnsafeCell<u32>>>(), 8);
assert_eq!(size_of::<Option<UnsafeCell<N32>>>(), 8);
}
20 changes: 20 additions & 0 deletions src/test/ui/repr/feature-gate-no-niche.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
use std::num::NonZeroU8 as N8;
use std::num::NonZeroU16 as N16;

#[repr(no_niche)]
pub struct Cloaked(N16);
//~^^ ERROR the attribute `repr(no_niche)` is currently unstable [E0658]

#[repr(transparent, no_niche)]
pub struct Shadowy(N16);
//~^^ ERROR the attribute `repr(no_niche)` is currently unstable [E0658]

#[repr(no_niche)]
pub enum Cloaked1 { _A(N16), }
//~^^ ERROR the attribute `repr(no_niche)` is currently unstable [E0658]

#[repr(no_niche)]
pub enum Cloaked2 { _A(N16), _B(u8, N8) }
//~^^ ERROR the attribute `repr(no_niche)` is currently unstable [E0658]

fn main() { }
35 changes: 35 additions & 0 deletions src/test/ui/repr/feature-gate-no-niche.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
error[E0658]: the attribute `repr(no_niche)` is currently unstable
--> $DIR/feature-gate-no-niche.rs:4:8
|
LL | #[repr(no_niche)]
| ^^^^^^^^
|
= help: add `#![feature(no_niche)]` to the crate attributes to enable

error[E0658]: the attribute `repr(no_niche)` is currently unstable
--> $DIR/feature-gate-no-niche.rs:8:21
|
LL | #[repr(transparent, no_niche)]
| ^^^^^^^^
|
= help: add `#![feature(no_niche)]` to the crate attributes to enable

error[E0658]: the attribute `repr(no_niche)` is currently unstable
--> $DIR/feature-gate-no-niche.rs:12:8
|
LL | #[repr(no_niche)]
| ^^^^^^^^
|
= help: add `#![feature(no_niche)]` to the crate attributes to enable

error[E0658]: the attribute `repr(no_niche)` is currently unstable
--> $DIR/feature-gate-no-niche.rs:16:8
|
LL | #[repr(no_niche)]
| ^^^^^^^^
|
= help: add `#![feature(no_niche)]` to the crate attributes to enable

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0658`.
14 changes: 14 additions & 0 deletions src/test/ui/repr/repr-no-niche-inapplicable-to-unions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#![feature(no_niche)]

use std::num::NonZeroU8 as N8;
use std::num::NonZeroU16 as N16;

#[repr(no_niche)]
pub union Cloaked1 { _A: N16 }
//~^^ ERROR attribute should be applied to struct or enum [E0517]

#[repr(no_niche)]
pub union Cloaked2 { _A: N16, _B: (u8, N8) }
//~^^ ERROR attribute should be applied to struct or enum [E0517]

fn main() { }
19 changes: 19 additions & 0 deletions src/test/ui/repr/repr-no-niche-inapplicable-to-unions.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
error[E0517]: attribute should be applied to struct or enum
--> $DIR/repr-no-niche-inapplicable-to-unions.rs:6:8
|
LL | #[repr(no_niche)]
| ^^^^^^^^
LL | pub union Cloaked1 { _A: N16 }
| ------------------------------ not a struct or enum

error[E0517]: attribute should be applied to struct or enum
--> $DIR/repr-no-niche-inapplicable-to-unions.rs:10:8
|
LL | #[repr(no_niche)]
| ^^^^^^^^
LL | pub union Cloaked2 { _A: N16, _B: (u8, N8) }
| -------------------------------------------- not a struct or enum

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0517`.
Loading