about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2018-02-17 19:32:25 +0000
committerbors <bors@rust-lang.org>2018-02-17 19:32:25 +0000
commit5313e8728f028cb7914f3c9f02804158a5732b52 (patch)
tree51870e1177c943cd3011fd0a8e3c26dc34067156
parentf6216b2d556afba2fdab906409582cb5471eafe0 (diff)
parent121499a11990b9e8d7359f7f51a7bafb1a266002 (diff)
downloadrust-5313e8728f028cb7914f3c9f02804158a5732b52.tar.gz
rust-5313e8728f028cb7914f3c9f02804158a5732b52.zip
Auto merge of #47408 - eddyb:deref-danger, r=nikomatsakis
Don't promote to 'static the result of dereferences.

This is a **breaking change**, removing copies out of dereferences from rvalue-to-`'static` promotion.

With miri we won't easily know whether the dereference itself would see the same value at runtime as miri (e.g. after mutating a `static`) or even if it can be interpreted (e.g. integer pointers).
One alternative to this ban is defining at least *some* of those situations as UB, i.e. you shouldn't have a reference in the first place, and you should work through raw pointers instead, to avoid promotion.

**EDIT**: The other *may seem* to be to add some analysis which whitelists references-to-constant-values and assume any values produced by arbitrary computation to not be safe to promote dereferences thereof - but that means producing a reference from an associated constant or `const fn` would necessarily obscure it, and in the former case, this could still impact code that runs on stable today. What we do today to track "references to statics" only works because we restrict taking a reference to a `static` at all to other `static`s (which, again, are currently limited in that they can't be read at compile-time) and to runtime-only `fn`s (*not* `const fn`s).

I'm primarily opening this PR with a conservative first approximation (e.g. `&(*r).a` is not allowed, only reborrows are, and in the old borrow only implicit ones from adjustments, at that) for cratering.

r? @nikomatsakis
-rw-r--r--src/librustc_mir/transform/qualify_consts.rs36
-rw-r--r--src/librustc_passes/consts.rs25
2 files changed, 46 insertions, 15 deletions
diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs
index 2b9ee223b01..a8a7831c823 100644
--- a/src/librustc_mir/transform/qualify_consts.rs
+++ b/src/librustc_mir/transform/qualify_consts.rs
@@ -526,9 +526,10 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
                                 this.add(Qualif::STATIC);
                             }
 
+                            this.add(Qualif::NOT_CONST);
+
                             let base_ty = proj.base.ty(this.mir, this.tcx).to_ty(this.tcx);
                             if let ty::TyRawPtr(_) = base_ty.sty {
-                                this.add(Qualif::NOT_CONST);
                                 if this.mode != Mode::Fn {
                                     let mut err = struct_span_err!(
                                         this.tcx.sess,
@@ -617,7 +618,38 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
 
     fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, location: Location) {
         // Recurse through operands and places.
-        self.super_rvalue(rvalue, location);
+        if let Rvalue::Ref(region, kind, ref place) = *rvalue {
+            let mut is_reborrow = false;
+            if let Place::Projection(ref proj) = *place {
+                if let ProjectionElem::Deref = proj.elem {
+                    let base_ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx);
+                    if let ty::TyRef(..) = base_ty.sty {
+                        is_reborrow = true;
+                    }
+                }
+            }
+
+            if is_reborrow {
+                self.nest(|this| {
+                    this.super_place(place, PlaceContext::Borrow {
+                        region,
+                        kind
+                    }, location);
+                    if !this.try_consume() {
+                        return;
+                    }
+
+                    if this.qualif.intersects(Qualif::STATIC_REF) {
+                        this.qualif = this.qualif - Qualif::STATIC_REF;
+                        this.add(Qualif::STATIC);
+                    }
+                });
+            } else {
+                self.super_rvalue(rvalue, location);
+            }
+        } else {
+            self.super_rvalue(rvalue, location);
+        }
 
         match *rvalue {
             Rvalue::Use(_) |
diff --git a/src/librustc_passes/consts.rs b/src/librustc_passes/consts.rs
index 59864182a7e..60637bb6182 100644
--- a/src/librustc_passes/consts.rs
+++ b/src/librustc_passes/consts.rs
@@ -362,14 +362,9 @@ fn check_expr<'a, 'tcx>(v: &mut CheckCrateVisitor<'a, 'tcx>, e: &hir::Expr, node
         hir::ExprBox(_) => {
             v.promotable = false;
         }
-        hir::ExprUnary(op, ref inner) => {
-            match v.tables.node_id_to_type(inner.hir_id).sty {
-                ty::TyRawPtr(_) => {
-                    assert!(op == hir::UnDeref);
-
-                    v.promotable = false;
-                }
-                _ => {}
+        hir::ExprUnary(op, _) => {
+            if op == hir::UnDeref {
+                v.promotable = false;
             }
         }
         hir::ExprBinary(op, ref lhs, _) => {
@@ -558,7 +553,8 @@ fn check_expr<'a, 'tcx>(v: &mut CheckCrateVisitor<'a, 'tcx>, e: &hir::Expr, node
 fn check_adjustments<'a, 'tcx>(v: &mut CheckCrateVisitor<'a, 'tcx>, e: &hir::Expr) {
     use rustc::ty::adjustment::*;
 
-    for adjustment in v.tables.expr_adjustments(e) {
+    let mut adjustments = v.tables.expr_adjustments(e).iter().peekable();
+    while let Some(adjustment) = adjustments.next() {
         match adjustment.kind {
             Adjust::NeverToAny |
             Adjust::ReifyFnPointer |
@@ -568,11 +564,14 @@ fn check_adjustments<'a, 'tcx>(v: &mut CheckCrateVisitor<'a, 'tcx>, e: &hir::Exp
             Adjust::Borrow(_) |
             Adjust::Unsize => {}
 
-            Adjust::Deref(ref overloaded) => {
-                if overloaded.is_some() {
-                    v.promotable = false;
-                    break;
+            Adjust::Deref(_) => {
+                if let Some(next_adjustment) = adjustments.peek() {
+                    if let Adjust::Borrow(_) = next_adjustment.kind {
+                        continue;
+                    }
                 }
+                v.promotable = false;
+                break;
             }
         }
     }