Skip to content

Commit

Permalink
Auto merge of #43746 - eddyb:sound-thread-local, r=alexcrichton
Browse files Browse the repository at this point in the history
Check #[thread_local] statics correctly in the compiler.

Fixes #43733 by introducing `#[allow_internal_unsafe]` analogous to `#[allow_internal_unstable]`, for letting a macro expand to `unsafe` blocks and functions even in `#![forbid(unsafe_code)]` crates.

Fixes #17954 by not letting references to `#[thread_local]` statics escape the function they're taken in - we can't just use a magical lifetime because Rust has *lifetime parametrism*, so if we added the often-proposed `'thread` lifetime, we'd have no way to check it in generic code.
To avoid potential edge cases in the compiler, the lifetime is actually that of a temporary at the same position, i.e. `&TLS_STATIC` has the same lifetime `&non_const_fn()` would.

Referring to `#[thread_local]` `static`s at compile-time is banned now (as per PR discussion).

Additionally, to remove `unsafe impl Sync` from `std::thread::local::fast::Key`, `#[thread_local]` statics are now not required to implement `Sync`, as they are not shared between threads.
  • Loading branch information
bors committed Aug 11, 2017
2 parents edd82ee + eba7592 commit e957a91
Show file tree
Hide file tree
Showing 30 changed files with 384 additions and 72 deletions.
1 change: 1 addition & 0 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ impl<'a> LoweringContext<'a> {
format: codemap::CompilerDesugaring(Symbol::intern(reason)),
span: Some(span),
allow_internal_unstable: true,
allow_internal_unsafe: false,
},
});
span.ctxt = SyntaxContext::empty().apply_mark(mark);
Expand Down
8 changes: 7 additions & 1 deletion src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,13 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
Ok(self.cat_rvalue_node(id, span, expr_ty))
}

Def::Static(_, mutbl) => {
Def::Static(def_id, mutbl) => {
// `#[thread_local]` statics may not outlive the current function.
for attr in &self.tcx.get_attrs(def_id)[..] {
if attr.check_name("thread_local") {
return Ok(self.cat_rvalue_node(id, span, expr_ty));
}
}
Ok(Rc::new(cmt_ {
id:id,
span:span,
Expand Down
1 change: 1 addition & 0 deletions src/librustc_allocator/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ impl<'a> Folder for ExpandAllocatorDirectives<'a> {
format: MacroAttribute(Symbol::intern(name)),
span: None,
allow_internal_unstable: true,
allow_internal_unsafe: false,
}
});
let span = Span {
Expand Down
25 changes: 17 additions & 8 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,24 +195,35 @@ impl LintPass for UnsafeCode {
}
}

impl UnsafeCode {
fn report_unsafe(&self, cx: &LateContext, span: Span, desc: &'static str) {
// This comes from a macro that has #[allow_internal_unsafe].
if span.allows_unsafe() {
return;
}

cx.span_lint(UNSAFE_CODE, span, desc);
}
}

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnsafeCode {
fn check_expr(&mut self, cx: &LateContext, e: &hir::Expr) {
if let hir::ExprBlock(ref blk) = e.node {
// Don't warn about generated blocks, that'll just pollute the output.
if blk.rules == hir::UnsafeBlock(hir::UserProvided) {
cx.span_lint(UNSAFE_CODE, blk.span, "usage of an `unsafe` block");
self.report_unsafe(cx, blk.span, "usage of an `unsafe` block");
}
}
}

fn check_item(&mut self, cx: &LateContext, it: &hir::Item) {
match it.node {
hir::ItemTrait(hir::Unsafety::Unsafe, ..) => {
cx.span_lint(UNSAFE_CODE, it.span, "declaration of an `unsafe` trait")
self.report_unsafe(cx, it.span, "declaration of an `unsafe` trait")
}

hir::ItemImpl(hir::Unsafety::Unsafe, ..) => {
cx.span_lint(UNSAFE_CODE, it.span, "implementation of an `unsafe` trait")
self.report_unsafe(cx, it.span, "implementation of an `unsafe` trait")
}

_ => return,
Expand All @@ -228,12 +239,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnsafeCode {
_: ast::NodeId) {
match fk {
FnKind::ItemFn(_, _, hir::Unsafety::Unsafe, ..) => {
cx.span_lint(UNSAFE_CODE, span, "declaration of an `unsafe` function")
self.report_unsafe(cx, span, "declaration of an `unsafe` function")
}

FnKind::Method(_, sig, ..) => {
if sig.unsafety == hir::Unsafety::Unsafe {
cx.span_lint(UNSAFE_CODE, span, "implementation of an `unsafe` method")
self.report_unsafe(cx, span, "implementation of an `unsafe` method")
}
}

Expand All @@ -244,9 +255,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnsafeCode {
fn check_trait_item(&mut self, cx: &LateContext, item: &hir::TraitItem) {
if let hir::TraitItemKind::Method(ref sig, hir::TraitMethod::Required(_)) = item.node {
if sig.unsafety == hir::Unsafety::Unsafe {
cx.span_lint(UNSAFE_CODE,
item.span,
"declaration of an `unsafe` method")
self.report_unsafe(cx, item.span, "declaration of an `unsafe` method")
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_mir/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,4 +442,5 @@ static A : &'static u32 = &S.a; // ok!

register_diagnostics! {
E0526, // shuffle indices are not constant
E0625, // thread-local statics cannot be accessed at compile-time
}
20 changes: 19 additions & 1 deletion src/librustc_mir/transform/qualify_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -484,8 +484,20 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
}
}
},
Lvalue::Static(_) => {
Lvalue::Static(ref global) => {
self.add(Qualif::STATIC);

if self.mode != Mode::Fn {
for attr in &self.tcx.get_attrs(global.def_id)[..] {
if attr.check_name("thread_local") {
span_err!(self.tcx.sess, self.span, E0625,
"thread-local statics cannot be \
accessed at compile-time");
return;
}
}
}

if self.mode == Mode::Const || self.mode == Mode::ConstFn {
span_err!(self.tcx.sess, self.span, E0013,
"{}s cannot refer to statics, use \
Expand Down Expand Up @@ -998,6 +1010,12 @@ impl MirPass for QualifyAndPromoteConstants {

// Statics must be Sync.
if mode == Mode::Static {
// `#[thread_local]` statics don't have to be `Sync`.
for attr in &tcx.get_attrs(def_id)[..] {
if attr.check_name("thread_local") {
return;
}
}
let ty = mir.return_ty;
tcx.infer_ctxt().enter(|infcx| {
let param_env = ty::ParamEnv::empty(Reveal::UserFacing);
Expand Down
22 changes: 18 additions & 4 deletions src/librustc_plugin/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,19 @@ impl<'a> Registry<'a> {
panic!("user-defined macros may not be named `macro_rules`");
}
self.syntax_exts.push((name, match extension {
NormalTT(ext, _, allow_internal_unstable) => {
NormalTT {
expander,
def_info: _,
allow_internal_unstable,
allow_internal_unsafe
} => {
let nid = ast::CRATE_NODE_ID;
NormalTT(ext, Some((nid, self.krate_span)), allow_internal_unstable)
NormalTT {
expander,
def_info: Some((nid, self.krate_span)),
allow_internal_unstable,
allow_internal_unsafe
}
}
IdentTT(ext, _, allow_internal_unstable) => {
IdentTT(ext, Some(self.krate_span), allow_internal_unstable)
Expand Down Expand Up @@ -134,8 +144,12 @@ impl<'a> Registry<'a> {
/// It builds for you a `NormalTT` that calls `expander`,
/// and also takes care of interning the macro's name.
pub fn register_macro(&mut self, name: &str, expander: MacroExpanderFn) {
self.register_syntax_extension(Symbol::intern(name),
NormalTT(Box::new(expander), None, false));
self.register_syntax_extension(Symbol::intern(name), NormalTT {
expander: Box::new(expander),
def_info: None,
allow_internal_unstable: false,
allow_internal_unsafe: false,
});
}

/// Register a compiler lint pass.
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ impl<'a> base::Resolver for Resolver<'a> {
fn check_unused_macros(&self) {
for did in self.unused_macros.iter() {
let id_span = match *self.macro_map[did] {
SyntaxExtension::NormalTT(_, isp, _) => isp,
SyntaxExtension::NormalTT { def_info, .. } => def_info,
SyntaxExtension::DeclMacro(.., osp) => osp,
_ => None,
};
Expand Down
1 change: 1 addition & 0 deletions src/libstd/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@
#![feature(allocator_api)]
#![feature(alloc_system)]
#![feature(allocator_internals)]
#![feature(allow_internal_unsafe)]
#![feature(allow_internal_unstable)]
#![feature(asm)]
#![feature(box_syntax)]
Expand Down
62 changes: 31 additions & 31 deletions src/libstd/thread/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,13 @@ pub struct LocalKey<T: 'static> {
//
// Note that the thunk is itself unsafe because the returned lifetime of the
// slot where data lives, `'static`, is not actually valid. The lifetime
// here is actually `'thread`!
// here is actually slightly shorter than the currently running thread!
//
// Although this is an extra layer of indirection, it should in theory be
// trivially devirtualizable by LLVM because the value of `inner` never
// changes and the constant should be readonly within a crate. This mainly
// only runs into problems when TLS statics are exported across crates.
inner: fn() -> Option<&'static UnsafeCell<Option<T>>>,
inner: unsafe fn() -> Option<&'static UnsafeCell<Option<T>>>,

// initialization routine to invoke to create a value
init: fn() -> T,
Expand Down Expand Up @@ -157,12 +157,13 @@ macro_rules! thread_local {
issue = "0")]
#[macro_export]
#[allow_internal_unstable]
#[cfg_attr(not(stage0), allow_internal_unsafe)]
macro_rules! __thread_local_inner {
($(#[$attr:meta])* $vis:vis $name:ident, $t:ty, $init:expr) => {
$(#[$attr])* $vis static $name: $crate::thread::LocalKey<$t> = {
fn __init() -> $t { $init }

fn __getit() -> $crate::option::Option<
unsafe fn __getit() -> $crate::option::Option<
&'static $crate::cell::UnsafeCell<
$crate::option::Option<$t>>>
{
Expand All @@ -178,7 +179,9 @@ macro_rules! __thread_local_inner {
__KEY.get()
}

$crate::thread::LocalKey::new(__getit, __init)
unsafe {
$crate::thread::LocalKey::new(__getit, __init)
}
};
}
}
Expand Down Expand Up @@ -252,8 +255,8 @@ impl<T: 'static> LocalKey<T> {
#[unstable(feature = "thread_local_internals",
reason = "recently added to create a key",
issue = "0")]
pub const fn new(inner: fn() -> Option<&'static UnsafeCell<Option<T>>>,
init: fn() -> T) -> LocalKey<T> {
pub const unsafe fn new(inner: unsafe fn() -> Option<&'static UnsafeCell<Option<T>>>,
init: fn() -> T) -> LocalKey<T> {
LocalKey {
inner: inner,
init: init,
Expand Down Expand Up @@ -391,6 +394,7 @@ pub mod fast {
}
}

#[cfg(stage0)]
unsafe impl<T> ::marker::Sync for Key<T> { }

impl<T> Key<T> {
Expand All @@ -402,14 +406,12 @@ pub mod fast {
}
}

pub fn get(&'static self) -> Option<&'static UnsafeCell<Option<T>>> {
unsafe {
if mem::needs_drop::<T>() && self.dtor_running.get() {
return None
}
self.register_dtor();
pub unsafe fn get(&self) -> Option<&'static UnsafeCell<Option<T>>> {
if mem::needs_drop::<T>() && self.dtor_running.get() {
return None
}
Some(&self.inner)
self.register_dtor();
Some(&*(&self.inner as *const _))
}

unsafe fn register_dtor(&self) {
Expand Down Expand Up @@ -478,26 +480,24 @@ pub mod os {
}
}

pub fn get(&'static self) -> Option<&'static UnsafeCell<Option<T>>> {
unsafe {
let ptr = self.os.get() as *mut Value<T>;
if !ptr.is_null() {
if ptr as usize == 1 {
return None
}
return Some(&(*ptr).value);
pub unsafe fn get(&'static self) -> Option<&'static UnsafeCell<Option<T>>> {
let ptr = self.os.get() as *mut Value<T>;
if !ptr.is_null() {
if ptr as usize == 1 {
return None
}

// If the lookup returned null, we haven't initialized our own
// local copy, so do that now.
let ptr: Box<Value<T>> = box Value {
key: self,
value: UnsafeCell::new(None),
};
let ptr = Box::into_raw(ptr);
self.os.set(ptr as *mut u8);
Some(&(*ptr).value)
return Some(&(*ptr).value);
}

// If the lookup returned null, we haven't initialized our own
// local copy, so do that now.
let ptr: Box<Value<T>> = box Value {
key: self,
value: UnsafeCell::new(None),
};
let ptr = Box::into_raw(ptr);
self.os.set(ptr as *mut u8);
Some(&(*ptr).value)
}
}

Expand Down
16 changes: 11 additions & 5 deletions src/libsyntax/ext/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,10 +532,16 @@ pub enum SyntaxExtension {
/// A normal, function-like syntax extension.
///
/// `bytes!` is a `NormalTT`.
///
/// The `bool` dictates whether the contents of the macro can
/// directly use `#[unstable]` things (true == yes).
NormalTT(Box<TTMacroExpander>, Option<(ast::NodeId, Span)>, bool),
NormalTT {
expander: Box<TTMacroExpander>,
def_info: Option<(ast::NodeId, Span)>,
/// Whether the contents of the macro can
/// directly use `#[unstable]` things (true == yes).
allow_internal_unstable: bool,
/// Whether the contents of the macro can use `unsafe`
/// without triggering the `unsafe_code` lint.
allow_internal_unsafe: bool,
},

/// A function-like syntax extension that has an extra ident before
/// the block.
Expand All @@ -562,7 +568,7 @@ impl SyntaxExtension {
pub fn kind(&self) -> MacroKind {
match *self {
SyntaxExtension::DeclMacro(..) |
SyntaxExtension::NormalTT(..) |
SyntaxExtension::NormalTT { .. } |
SyntaxExtension::IdentTT(..) |
SyntaxExtension::ProcMacro(..) =>
MacroKind::Bang,
Expand Down
1 change: 1 addition & 0 deletions src/libsyntax/ext/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ pub fn add_derived_markers<T>(cx: &mut ExtCtxt, span: Span, traits: &[ast::Path]
format: ExpnFormat::MacroAttribute(Symbol::intern(&pretty_name)),
span: None,
allow_internal_unstable: true,
allow_internal_unsafe: false,
},
});

Expand Down
Loading

0 comments on commit e957a91

Please sign in to comment.