[WIP] Crater rollup of 3 PRs (#63809, #63812, #63831).

src/libproc_macro/bridge/handle.rs

@@ -26,6 +26,12 @@ impl<T> OwnedStore<T> {
     }
 }
 
+impl<T> Drop for OwnedStore<T> {
+    fn drop(&mut self) {
+        assert!(self.data.is_empty(), "{} `proc_macro` handles were leaked", self.data.len());
+    }
+}
+
 impl<T> OwnedStore<T> {
     pub(super) fn alloc(&mut self, x: T) -> Handle {
         let counter = self.counter.fetch_add(1, Ordering::SeqCst);
@@ -63,6 +69,13 @@ pub(super) struct InternedStore<T: 'static> {
     interner: HashMap<T, Handle>,
 }
 
+impl<T> Drop for InternedStore<T> {
+    fn drop(&mut self) {
+        // HACK(eddyb) turn off the leak-checking for interned handles.
+        self.owned.data.clear();
+    }
+}
+
 impl<T: Copy + Eq + Hash> InternedStore<T> {
     pub(super) fn new(counter: &'static AtomicUsize) -> Self {
         InternedStore {

src/librustc_mir/transform/qualify_consts.rs

@@ -206,6 +206,9 @@ trait Qualif {
             ProjectionElem::ConstantIndex { .. } |
             ProjectionElem::Downcast(..) => qualif,
 
+            // FIXME(eddyb) shouldn't this be masked *after* including the
+            // index local? Then again, it's `usize` which is neither
+            // `HasMutInterior` nor `NeedsDrop`.
             ProjectionElem::Index(local) => qualif || Self::in_local(cx, local),
         }
     }
@@ -439,6 +442,7 @@ impl Qualif for IsNotPromotable {
             StaticKind::Promoted(_, _) => unreachable!(),
             StaticKind::Static => {
                 // Only allow statics (not consts) to refer to other statics.
+                // FIXME(eddyb) does this matter at all for promotion?
                 let allowed = cx.mode == Mode::Static || cx.mode == Mode::StaticMut;
 
                 !allowed ||
@@ -667,6 +671,7 @@ struct Checker<'a, 'tcx> {
 
     temp_promotion_state: IndexVec<Local, TempState>,
     promotion_candidates: Vec<Candidate>,
+    unchecked_promotion_candidates: Vec<Candidate>,
 }
 
 macro_rules! unleash_miri {
@@ -690,7 +695,8 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
     fn new(tcx: TyCtxt<'tcx>, def_id: DefId, body: &'a Body<'tcx>, mode: Mode) -> Self {
         assert!(def_id.is_local());
         let mut rpo = traversal::reverse_postorder(body);
-        let temps = promote_consts::collect_temps(body, &mut rpo);
+        let (temps, unchecked_promotion_candidates) =
+            promote_consts::collect_temps_and_candidates(tcx, body, &mut rpo);
         rpo.reset();
 
         let param_env = tcx.param_env(def_id);
@@ -728,7 +734,8 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
             def_id,
             rpo,
             temp_promotion_state: temps,
-            promotion_candidates: vec![]
+            promotion_candidates: vec![],
+            unchecked_promotion_candidates,
         }
     }
 
@@ -802,6 +809,10 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
                 } else if let BorrowKind::Mut { .. } | BorrowKind::Shared = kind {
                     // Don't promote BorrowKind::Shallow borrows, as they don't
                     // reach codegen.
+                    // FIXME(eddyb) the two other kinds of borrow (`Shallow` and `Unique`)
+                    // aren't promoted here but *could* be promoted as part of a larger
+                    // value because `IsNotPromotable` isn't being set for them,
+                    // need to figure out what is the intended behavior.
 
                     // We might have a candidate for promotion.
                     let candidate = Candidate::Ref(location);
@@ -930,6 +941,7 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
 
         let mut seen_blocks = BitSet::new_empty(body.basic_blocks().len());
         let mut bb = START_BLOCK;
+        let mut has_controlflow_error = false;
         loop {
             seen_blocks.insert(bb.index());
 
@@ -969,6 +981,7 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
                     bb = target;
                 }
                 _ => {
+                    has_controlflow_error = true;
                     self.not_const();
                     break;
                 }
@@ -979,9 +992,17 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
         // Collect all the temps we need to promote.
         let mut promoted_temps = BitSet::new_empty(self.temp_promotion_state.len());
 
-        debug!("qualify_const: promotion_candidates={:?}", self.promotion_candidates);
-        for candidate in &self.promotion_candidates {
-            match *candidate {
+        // HACK(eddyb) don't try to validate promotion candidates if any
+        // parts of the control-flow graph were skipped due to an error.
+        let promotion_candidates = if has_controlflow_error {
+            self.tcx.sess.delay_span_bug(body.span, "check_const: expected control-flow error(s)");
+            self.promotion_candidates.clone()
+        } else {
+            self.valid_promotion_candidates()
+        };
+        debug!("qualify_const: promotion_candidates={:?}", promotion_candidates);
+        for candidate in promotion_candidates {
+            match candidate {
                 Candidate::Repeat(Location { block: bb, statement_index: stmt_idx }) => {
                     if let StatementKind::Assign(_, box Rvalue::Repeat(
                         Operand::Move(Place {
@@ -1018,6 +1039,51 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
 
         (qualifs.encode_to_bits(), self.tcx.arena.alloc(promoted_temps))
     }
+
+    /// Get the subset of `unchecked_promotion_candidates` that are eligible
+    /// for promotion.
+    // FIXME(eddyb) replace the old candidate gathering with this.
+    fn valid_promotion_candidates(&self) -> Vec<Candidate> {
+        // Sanity-check the promotion candidates.
+        let candidates = promote_consts::validate_candidates(
+            self.tcx,
+            self.body,
+            self.def_id,
+            &self.temp_promotion_state,
+            &self.per_local.0[HasMutInterior::IDX],
+            &self.per_local.0[NeedsDrop::IDX],
+            &self.unchecked_promotion_candidates,
+        );
+
+        if candidates != self.promotion_candidates {
+            let report = |msg, candidate| {
+                let span = match candidate {
+                    Candidate::Ref(loc) |
+                    Candidate::Repeat(loc) => self.body.source_info(loc).span,
+                    Candidate::Argument { bb, .. } => {
+                        self.body[bb].terminator().source_info.span
+                    }
+                };
+                self.tcx.sess.span_err(span, &format!("{}: {:?}", msg, candidate));
+            };
+
+            for &c in &self.promotion_candidates {
+                if !candidates.contains(&c) {
+                    report("invalidated old candidate", c);
+                }
+            }
+
+            for &c in &candidates {
+                if !self.promotion_candidates.contains(&c) {
+                    report("extra new candidate", c);
+                }
+            }
+
+            bug!("promotion candidate validation mismatches (see above)");
+        }
+
+        candidates
+    }
 }
 
 impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> {
@@ -1609,29 +1675,33 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> {
             let (temps, candidates) = {
                 let mut checker = Checker::new(tcx, def_id, body, mode);
                 if let Mode::ConstFn = mode {
-                    if tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you {
-                        checker.check_const();
-                    } else if tcx.is_min_const_fn(def_id) {
+                    let use_min_const_fn_checks =
+                        !tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you &&
+                        tcx.is_min_const_fn(def_id);
+                    if use_min_const_fn_checks {
                         // Enforce `min_const_fn` for stable `const fn`s.
                         use super::qualify_min_const_fn::is_min_const_fn;
                         if let Err((span, err)) = is_min_const_fn(tcx, def_id, body) {
                             error_min_const_fn_violation(tcx, span, err);
-                        } else {
-                            // this should not produce any errors, but better safe than sorry
-                            // FIXME(#53819)
-                            checker.check_const();
+                            return;
                         }
-                    } else {
-                        // Enforce a constant-like CFG for `const fn`.
-                        checker.check_const();
+
+                        // `check_const` should not produce any errors, but better safe than sorry
+                        // FIXME(#53819)
+                        // NOTE(eddyb) `check_const` is actually needed for promotion inside
+                        // `min_const_fn` functions.
                     }
+
+                    // Enforce a constant-like CFG for `const fn`.
+                    checker.check_const();
                 } else {
                     while let Some((bb, data)) = checker.rpo.next() {
                         checker.visit_basic_block_data(bb, data);
                     }
                 }
 
-                (checker.temp_promotion_state, checker.promotion_candidates)
+                let promotion_candidates = checker.valid_promotion_candidates();
+                (checker.temp_promotion_state, promotion_candidates)
             };
 
             // Do the actual promotion, now that we know what's viable.
@@ -1648,6 +1718,19 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> {
             remove_drop_and_storage_dead_on_promoted_locals(body, promoted_temps);
         }
 
+        // HACK(eddyb) try to prevent global mutable state in proc macros.
+        // (this is not perfect and could also have false positives)
+        if mode == Mode::Static || mode == Mode::StaticMut {
+            use rustc::session::config::CrateType;
+            if tcx.sess.crate_types.borrow().contains(&CrateType::ProcMacro) {
+                let ty = body.return_ty();
+                let param_env = ty::ParamEnv::empty();
+                if mode == Mode::StaticMut || !ty.is_freeze(tcx, param_env, DUMMY_SP) {
+                    tcx.sess.span_err(body.span, "mutable global state in a proc-macro");
+                }
+            }
+        }
+
         if mode == Mode::Static && !tcx.has_attr(def_id, sym::thread_local) {
             // `static`s (not `static mut`s) which are not `#[thread_local]` must be `Sync`.
             check_static_is_sync(tcx, body, hir_id);

src/test/ui/proc-macro/global-mut-state.rs

@@ -0,0 +1,19 @@
+// force-host
+// no-prefer-dynamic
+
+#![crate_type = "proc-macro"]
+#![allow(warnings)]
+
+use std::cell::Cell;
+use std::sync::atomic::AtomicBool;
+
+static mut FOO: u8 = 0;
+//~^ ERROR mutable global state in a proc-macro
+
+static BAR: AtomicBool = AtomicBool::new(false);
+//~^ ERROR mutable global state in a proc-macro
+
+thread_local!(static BAZ: Cell<String> = Cell::new(String::new()));
+//~^ ERROR mutable global state in a proc-macro
+
+static FROZEN: &str = "snow";

src/test/ui/proc-macro/global-mut-state.stderr

@@ -0,0 +1,22 @@
+error: mutable global state in a proc-macro
+  --> $DIR/global-mut-state.rs:10:1
+   |
+LL | static mut FOO: u8 = 0;
+   | ^^^^^^^^^^^^^^^^^^^^^^^
+
+error: mutable global state in a proc-macro
+  --> $DIR/global-mut-state.rs:13:1
+   |
+LL | static BAR: AtomicBool = AtomicBool::new(false);
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: mutable global state in a proc-macro
+  --> $DIR/global-mut-state.rs:16:1
+   |
+LL | thread_local!(static BAZ: Cell<String> = Cell::new(String::new()));
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
+
+error: aborting due to 3 previous errors
+