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

Do not allow moving out of thread local under ast borrowck #55150

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
29 changes: 21 additions & 8 deletions src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ use util::nodemap::ItemLocalSet;
#[derive(Clone, Debug, PartialEq)]
pub enum Categorization<'tcx> {
Rvalue(ty::Region<'tcx>), // temporary val, argument is its scope
ThreadLocal(ty::Region<'tcx>), // value that cannot move, but still restricted in scope
StaticItem,
Upvar(Upvar), // upvar referenced by closure env
Local(ast::NodeId), // local variable
Expand Down Expand Up @@ -268,6 +269,7 @@ impl<'tcx> cmt_<'tcx> {
Categorization::Deref(ref base_cmt, _) => {
base_cmt.immutability_blame()
}
Categorization::ThreadLocal(..) |
Categorization::StaticItem => {
// Do we want to do something here?
None
Expand Down Expand Up @@ -715,17 +717,23 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
}

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(hir_id, span, expr_ty));
}
}
// `#[thread_local]` statics may not outlive the current function, but
// they also cannot be moved out of.
let is_thread_local = self.tcx.get_attrs(def_id)[..]
.iter()
.any(|attr| attr.check_name("thread_local"));

let cat = if is_thread_local {
let re = self.temporary_scope(hir_id.local_id);
Categorization::ThreadLocal(re)
} else {
Categorization::StaticItem
};

Ok(cmt_ {
hir_id,
span:span,
cat:Categorization::StaticItem,
span,
cat,
mutbl: if mutbl { McDeclared } else { McImmutable},
ty:expr_ty,
note: NoteNone
Expand Down Expand Up @@ -1408,6 +1416,7 @@ impl<'tcx> cmt_<'tcx> {
match self.cat {
Categorization::Rvalue(..) |
Categorization::StaticItem |
Categorization::ThreadLocal(..) |
Categorization::Local(..) |
Categorization::Deref(_, UnsafePtr(..)) |
Categorization::Deref(_, BorrowedPtr(..)) |
Expand Down Expand Up @@ -1439,6 +1448,7 @@ impl<'tcx> cmt_<'tcx> {
}

Categorization::Rvalue(..) |
Categorization::ThreadLocal(..) |
Categorization::Local(..) |
Categorization::Upvar(..) |
Categorization::Deref(_, UnsafePtr(..)) => { // yes, it's aliasable, but...
Expand Down Expand Up @@ -1485,6 +1495,9 @@ impl<'tcx> cmt_<'tcx> {
Categorization::StaticItem => {
"static item".into()
}
Categorization::ThreadLocal(..) => {
"thread-local static item".into()
}
Categorization::Rvalue(..) => {
"non-place".into()
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_borrowck/borrowck/check_loans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
// by-move upvars, which is local data for generators
Categorization::Upvar(..) => true,

Categorization::ThreadLocal(region) |
Categorization::Rvalue(region) => {
// Rvalues promoted to 'static are no longer local
if let RegionKind::ReStatic = *region {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ fn check_and_get_illegal_move_origin<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
match cmt.cat {
Categorization::Deref(_, mc::BorrowedPtr(..)) |
Categorization::Deref(_, mc::UnsafePtr(..)) |
Categorization::ThreadLocal(..) |
Categorization::StaticItem => {
Some(cmt.clone())
}
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_borrowck/borrowck/gather_loans/lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ impl<'a, 'tcx> GuaranteeLifetimeContext<'a, 'tcx> {

match cmt.cat {
Categorization::Rvalue(..) |
Categorization::ThreadLocal(..) |
Categorization::Local(..) | // L-Local
Categorization::Upvar(..) |
Categorization::Deref(_, mc::BorrowedPtr(..)) | // L-Deref-Borrowed
Expand Down Expand Up @@ -105,6 +106,7 @@ impl<'a, 'tcx> GuaranteeLifetimeContext<'a, 'tcx> {
//! rooting etc, and presuming `cmt` is not mutated.

match cmt.cat {
Categorization::ThreadLocal(temp_scope) |
Categorization::Rvalue(temp_scope) => {
temp_scope
}
Expand Down
7 changes: 6 additions & 1 deletion src/librustc_borrowck/borrowck/gather_loans/move_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ fn report_cannot_move_out_of<'a, 'tcx>(bccx: &'a BorrowckCtxt<'a, 'tcx>,
match move_from.cat {
Categorization::Deref(_, mc::BorrowedPtr(..)) |
Categorization::Deref(_, mc::UnsafePtr(..)) |
Categorization::Deref(_, mc::Unique) |
Categorization::ThreadLocal(..) |
Categorization::StaticItem => {
bccx.cannot_move_out_of(
move_from.span, &move_from.descriptive_string(bccx.tcx), Origin::Ast)
Expand All @@ -166,7 +168,10 @@ fn report_cannot_move_out_of<'a, 'tcx>(bccx: &'a BorrowckCtxt<'a, 'tcx>,
}
}
}
_ => {

Categorization::Rvalue(..) |
Categorization::Local(..) |
Categorization::Upvar(..) => {
span_bug!(move_from.span, "this path should not cause illegal move");
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/librustc_borrowck/borrowck/gather_loans/restrictions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ impl<'a, 'tcx> RestrictionsContext<'a, 'tcx> {
RestrictionResult::Safe
}

Categorization::ThreadLocal(..) => {
// Thread-locals are statics that have a scope, with
// no underlying structure to provide restrictions.
RestrictionResult::Safe
}

Categorization::Local(local_id) => {
// R-Variable, locally declared
let lp = new_lp(LpVar(local_id));
Expand Down
1 change: 1 addition & 0 deletions src/librustc_borrowck/borrowck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,7 @@ pub fn opt_loan_path_is_field<'tcx>(cmt: &mc::cmt_<'tcx>) -> (Option<Rc<LoanPath

match cmt.cat {
Categorization::Rvalue(..) |
Categorization::ThreadLocal(..) |
Categorization::StaticItem => {
(None, false)
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_passes/rvalue_promotion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,7 @@ impl<'a, 'gcx, 'tcx> euv::Delegate<'tcx> for CheckCrateVisitor<'a, 'gcx> {
let mut cur = cmt;
loop {
match cur.cat {
Categorization::ThreadLocal(..) |
Categorization::Rvalue(..) => {
if loan_cause == euv::MatchDiscriminant {
// Ignore the dummy immutable borrow created by EUV.
Expand Down
1 change: 1 addition & 0 deletions src/librustc_typeck/check/regionck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1243,6 +1243,7 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> {
| Categorization::StaticItem
| Categorization::Upvar(..)
| Categorization::Local(..)
| Categorization::ThreadLocal(..)
| Categorization::Rvalue(..) => {
// These are all "base cases" with independent lifetimes
// that are not subject to inference
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_typeck/check/upvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ impl<'a, 'gcx, 'tcx> InferBorrowKind<'a, 'gcx, 'tcx> {

Categorization::Deref(_, mc::UnsafePtr(..)) |
Categorization::StaticItem |
Categorization::ThreadLocal(..) |
Categorization::Rvalue(..) |
Categorization::Local(_) |
Categorization::Upvar(..) => {
Expand Down Expand Up @@ -431,6 +432,7 @@ impl<'a, 'gcx, 'tcx> InferBorrowKind<'a, 'gcx, 'tcx> {

Categorization::Deref(_, mc::UnsafePtr(..)) |
Categorization::StaticItem |
Categorization::ThreadLocal(..) |
Categorization::Rvalue(..) |
Categorization::Local(_) |
Categorization::Upvar(..) => {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ error[E0597]: borrowed value does not live long enough
--> $DIR/borrowck-thread-local-static-borrow-outlives-fn.rs:21:21
|
LL | assert_static(&FOO); //[ast]~ ERROR [E0597]
| ^^^ - temporary value only lives until here
| ^^^ - borrowed value only lives until here
| |
| temporary value does not live long enough
| borrowed value does not live long enough
|
= note: borrowed value must be valid for the static lifetime...

Expand Down
12 changes: 12 additions & 0 deletions src/test/ui/borrowck/issue-47215-ice-from-drop-elab.nll.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0507]: cannot move out of static item
--> $DIR/issue-47215-ice-from-drop-elab.rs:17:21
|
LL | let mut x = X; //~ ERROR cannot move out of thread-local static item [E0507]
| ^
| |
| cannot move out of static item
| help: consider borrowing here: `&X`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0507`.
20 changes: 20 additions & 0 deletions src/test/ui/borrowck/issue-47215-ice-from-drop-elab.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// rust-lang/rust#47215: at one time, the compiler categorized
// thread-local statics as a temporary rvalue, as a way to enforce
// that they are only valid for a given lifetime.
//
// The problem with this is that you cannot move out of static items,
// but you *can* move temporary rvalues. I.e., the categorization
// above only solves half of the problem presented by thread-local
// statics.

#![feature(thread_local)]

#[thread_local]
static mut X: ::std::sync::atomic::AtomicUsize = ::std::sync::atomic::ATOMIC_USIZE_INIT;

fn main() {
unsafe {
let mut x = X; //~ ERROR cannot move out of thread-local static item [E0507]
let _y = x.get_mut();
}
}
12 changes: 12 additions & 0 deletions src/test/ui/borrowck/issue-47215-ice-from-drop-elab.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0507]: cannot move out of thread-local static item
--> $DIR/issue-47215-ice-from-drop-elab.rs:17:21
|
LL | let mut x = X; //~ ERROR cannot move out of thread-local static item [E0507]
| ^
| |
| cannot move out of thread-local static item
| help: consider using a reference instead: `&X`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0507`.
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-17954.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@ fn main() {
println!("{}", a);
});
}
//~^ NOTE temporary value only lives until here
//~^ NOTE borrowed value only lives until here
4 changes: 2 additions & 2 deletions src/test/ui/issues/issue-17954.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ error[E0597]: borrowed value does not live long enough
--> $DIR/issue-17954.rs:17:14
|
LL | let a = &FOO;
| ^^^ temporary value does not live long enough
| ^^^ borrowed value does not live long enough
...
LL | }
| - temporary value only lives until here
| - borrowed value only lives until here
|
= note: borrowed value must be valid for the static lifetime...

Expand Down