Skip to content

Check #[thread_local] statics correctly in the compiler. #43746

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

Merged
merged 2 commits into from
Aug 12, 2017
Merged
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
1 change: 1 addition & 0 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
@@ -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);
8 changes: 7 additions & 1 deletion src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
@@ -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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like 2-space indent here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entire file has broken (read: old-skool match rules) indentation, I'm actually on a multiple of 4 and the Def::Static line above is wrong.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I just mentioned this b/c the line just after these modifications is 4-space indented, but yes it looks like the indentation is off in most of this file.

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,
1 change: 1 addition & 0 deletions src/librustc_allocator/expand.rs
Original file line number Diff line number Diff line change
@@ -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 {
25 changes: 17 additions & 8 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
@@ -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,
@@ -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")
}
}

@@ -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")
}
}
}
1 change: 1 addition & 0 deletions src/librustc_mir/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -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
@@ -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 \
@@ -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);
22 changes: 18 additions & 4 deletions src/librustc_plugin/registry.rs
Original file line number Diff line number Diff line change
@@ -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)
@@ -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.
2 changes: 1 addition & 1 deletion src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
@@ -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,
};
1 change: 1 addition & 0 deletions src/libstd/lib.rs
Original file line number Diff line number Diff line change
@@ -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)]
62 changes: 31 additions & 31 deletions src/libstd/thread/local.rs
Original file line number Diff line number Diff line change
@@ -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,
@@ -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>>>
{
@@ -178,7 +179,9 @@ macro_rules! __thread_local_inner {
__KEY.get()
}

$crate::thread::LocalKey::new(__getit, __init)
unsafe {
$crate::thread::LocalKey::new(__getit, __init)
}
};
}
}
@@ -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,
@@ -391,6 +394,7 @@ pub mod fast {
}
}

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

impl<T> Key<T> {
@@ -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) {
@@ -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)
}
}

16 changes: 11 additions & 5 deletions src/libsyntax/ext/base.rs
Original file line number Diff line number Diff line change
@@ -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.
@@ -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,
1 change: 1 addition & 0 deletions src/libsyntax/ext/derive.rs
Original file line number Diff line number Diff line change
@@ -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,
},
});

Loading