Skip to content

Ignore borrowck for static lvalues and allow assignment to static muts #46032

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 3 commits into from
Nov 18, 2017
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
6 changes: 4 additions & 2 deletions src/librustc_mir/borrow_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,10 +640,12 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> MirBorrowckCtxt<'c, 'b, 'a, 'gcx, 'tcx>
Mutability::Mut => return,
}
}
Lvalue::Static(_) => {
Lvalue::Static(ref static_) => {
// mutation of non-mut static is always illegal,
// independent of dataflow.
self.report_assignment_to_static(context, (lvalue, span));
if !self.tcx.is_static_mut(static_.def_id) {
self.report_assignment_to_static(context, (lvalue, span));
}
return;
}
}
Expand Down
58 changes: 50 additions & 8 deletions src/librustc_mir/dataflow/impls/borrows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

use rustc::mir::{self, Location, Mir};
use rustc::mir::visit::Visitor;
use rustc::ty::{Region, TyCtxt};
use rustc::ty::{self, Region, TyCtxt};
use rustc::ty::RegionKind;
use rustc::ty::RegionKind::ReScope;
use rustc::util::nodemap::{FxHashMap, FxHashSet};
Expand Down Expand Up @@ -71,10 +71,14 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
mir: &'a Mir<'tcx>,
nonlexical_regioncx: Option<&'a RegionInferenceContext<'tcx>>)
-> Self {
let mut visitor = GatherBorrows { idx_vec: IndexVec::new(),
location_map: FxHashMap(),
region_map: FxHashMap(),
region_span_map: FxHashMap()};
let mut visitor = GatherBorrows {
tcx,
mir,
idx_vec: IndexVec::new(),
location_map: FxHashMap(),
region_map: FxHashMap(),
region_span_map: FxHashMap()
};
visitor.visit_mir(mir);
return Borrows { tcx: tcx,
mir: mir,
Expand All @@ -84,17 +88,22 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
region_span_map: visitor.region_span_map,
nonlexical_regioncx };

struct GatherBorrows<'tcx> {
struct GatherBorrows<'a, 'gcx: 'tcx, 'tcx: 'a> {
tcx: TyCtxt<'a, 'gcx, 'tcx>,
mir: &'a Mir<'tcx>,
idx_vec: IndexVec<BorrowIndex, BorrowData<'tcx>>,
location_map: FxHashMap<Location, BorrowIndex>,
region_map: FxHashMap<Region<'tcx>, FxHashSet<BorrowIndex>>,
region_span_map: FxHashMap<RegionKind, Span>,
}
impl<'tcx> Visitor<'tcx> for GatherBorrows<'tcx> {

impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
fn visit_rvalue(&mut self,
rvalue: &mir::Rvalue<'tcx>,
location: mir::Location) {
if let mir::Rvalue::Ref(region, kind, ref lvalue) = *rvalue {
if is_unsafe_lvalue(self.tcx, self.mir, lvalue) { return; }

let borrow = BorrowData {
location: location, kind: kind, region: region, lvalue: lvalue.clone(),
};
Expand Down Expand Up @@ -197,7 +206,8 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
}

mir::StatementKind::Assign(_, ref rhs) => {
if let mir::Rvalue::Ref(region, _, _) = *rhs {
if let mir::Rvalue::Ref(region, _, ref lvalue) = *rhs {
if is_unsafe_lvalue(self.tcx, self.mir, lvalue) { return; }
let index = self.location_map.get(&location).unwrap_or_else(|| {
panic!("could not find BorrowIndex for location {:?}", location);
});
Expand Down Expand Up @@ -248,3 +258,35 @@ impl<'a, 'gcx, 'tcx> DataflowOperator for Borrows<'a, 'gcx, 'tcx> {
false // bottom = no Rvalue::Refs are active by default
}
}

fn is_unsafe_lvalue<'a, 'gcx: 'tcx, 'tcx: 'a>(
tcx: TyCtxt<'a, 'gcx, 'tcx>,
mir: &'a Mir<'tcx>,
lvalue: &mir::Lvalue<'tcx>
) -> bool {
use self::mir::Lvalue::*;
use self::mir::ProjectionElem;

match *lvalue {
Local(_) => false,
Static(ref static_) => tcx.is_static_mut(static_.def_id),
Projection(ref proj) => {
match proj.elem {
ProjectionElem::Field(..) |
ProjectionElem::Downcast(..) |
ProjectionElem::Subslice { .. } |
ProjectionElem::ConstantIndex { .. } |
ProjectionElem::Index(_) => {
is_unsafe_lvalue(tcx, mir, &proj.base)
}
ProjectionElem::Deref => {
let ty = proj.base.ty(mir, tcx).to_ty(tcx);
match ty.sty {
ty::TyRawPtr(..) => true,
_ => is_unsafe_lvalue(tcx, mir, &proj.base),
}
}
}
}
}
}
34 changes: 0 additions & 34 deletions src/test/compile-fail/borrowck/borrowck-describe-lvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,6 @@ impl Baz {
}
}

static mut sfoo : Foo = Foo{x: 23 };
static mut sbar : Bar = Bar(23);
static mut stuple : (i32, i32) = (24, 25);
static mut senum : Baz = Baz::X(26);
static mut sunion : U = U { a: 0 };

fn main() {
// Local and field from struct
{
Expand Down Expand Up @@ -96,34 +90,6 @@ fn main() {
//[mir]~^ ERROR cannot use `u.a` because it was mutably borrowed (Ast)
//[mir]~| ERROR cannot use `u.a` because it was mutably borrowed (Mir)
}
// Static and field from struct
unsafe {
let _x = sfoo.x();
sfoo.x; //[mir]~ ERROR cannot use `sfoo.x` because it was mutably borrowed (Mir)
}
// Static and field from tuple-struct
unsafe {
let _0 = sbar.x();
sbar.0; //[mir]~ ERROR cannot use `sbar.0` because it was mutably borrowed (Mir)
}
// Static and field from tuple
unsafe {
let _0 = &mut stuple.0;
stuple.0; //[mir]~ ERROR cannot use `stuple.0` because it was mutably borrowed (Mir)
}
// Static and field from enum
unsafe {
let _e0 = senum.x();
match senum {
Baz::X(value) => value
//[mir]~^ ERROR cannot use `senum.0` because it was mutably borrowed (Mir)
};
}
// Static and field from union
unsafe {
let _ra = &mut sunion.a;
sunion.a; //[mir]~ ERROR cannot use `sunion.a` because it was mutably borrowed (Mir)
}
// Deref and field from struct
{
let mut f = Box::new(Foo { x: 22 });
Expand Down
23 changes: 23 additions & 0 deletions src/test/run-pass/borrowck/borrowck-assignment-to-static-mut.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Test taken from #45641 (https://github.com/rust-lang/rust/issues/45641)

// ignore-tidy-linelength
// revisions: ast mir
//[mir]compile-flags: -Z emit-end-regions -Z borrowck-mir

static mut Y: u32 = 0;

unsafe fn should_ok() {
Y = 1;
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// revisions: ast mir
//[mir]compile-flags: -Z emit-end-regions -Z borrowck-mir

// Test file taken from issue 45129 (https://github.com/rust-lang/rust/issues/45129)
Copy link
Contributor

@arielb1 arielb1 Nov 16, 2017

Choose a reason for hiding this comment

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

This code is UB. Please don't have it as a run-pass test, because verified compilation will correctly complain.

to make it non-UB, e.g. use an array and access different array indices:

struct Foo { x: [usize; 2] }

static mut SFOO: Foo = Foo { x: [23, 32] }; 

impl Foo {
    fn x(&mut self) -> &mut usize { &mut self.x[0] }
}

fn main() {
    unsafe {
        let x = SFOO.x();
        SFOO.x[1] += 1;
        *x += 1; 
    }
}


struct Foo { x: [usize; 2] }

static mut SFOO: Foo = Foo { x: [23, 32] };

impl Foo {
fn x(&mut self) -> &mut usize { &mut self.x[0] }
}

fn main() {
unsafe {
let sfoo: *mut Foo = &mut SFOO;
let x = (*sfoo).x();
(*sfoo).x[1] += 1;
*x += 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good! =)

}
}