Output deterministic AllocIds when emitting MIR

compiler/rustc_middle/src/mir/pretty.rs

@@ -1,5 +1,7 @@
+use std::cell::RefCell;
 use std::collections::BTreeSet;
 use std::fmt::{Display, Write as _};
+use std::num::NonZero;
 use std::path::{Path, PathBuf};
 use std::{fs, io};
 
@@ -10,10 +12,10 @@ use ty::print::PrettyPrinter;
 
 use super::graphviz::write_mir_fn_graphviz;
 use crate::mir::interpret::{
-    AllocBytes, AllocId, Allocation, ConstAllocation, GlobalAlloc, Pointer, Provenance,
-    alloc_range, read_target_uint,
+    AllocBytes, AllocId, Allocation, ConstAllocation, CtfeProvenance, GlobalAlloc, Pointer,
+    Provenance, alloc_range, read_target_uint,
 };
-use crate::mir::visit::Visitor;
+use crate::mir::visit::{MutVisitor, Visitor};
 use crate::mir::*;
 use crate::ty::CoroutineArgsExt;
 
@@ -318,6 +320,10 @@ pub fn write_mir_pretty<'tcx>(tcx: TyCtxt<'tcx>, w: &mut dyn io::Write) -> io::R
     writeln!(w, "// WARNING: This output format is intended for human consumers only")?;
     writeln!(w, "// and is subject to change without notice. Knock yourself out.")?;
     writeln!(w, "// HINT: See also -Z dump-mir for MIR at specific points during compilation.")?;
+    writeln!(
+        w,
+        "// WARNING: Allocation ids were remapped for deterministic output, they may be differ from real ones."
+    )?;
 
     let mut first = true;
     for &def_id in tcx.mir_keys(()) {
@@ -330,7 +336,6 @@ pub fn write_mir_pretty<'tcx>(tcx: TyCtxt<'tcx>, w: &mut dyn io::Write) -> io::R
 
         let render_body = |w: &mut dyn io::Write, body| -> io::Result<()> {
             writer.write_mir_fn(body, w)?;
-
             for body in tcx.promoted_mir(def_id) {
                 writeln!(w)?;
                 writer.write_mir_fn(body, w)?;
@@ -367,15 +372,42 @@ pub struct MirWriter<'a, 'tcx> {
     tcx: TyCtxt<'tcx>,
     extra_data: &'a dyn Fn(PassWhere, &mut dyn io::Write) -> io::Result<()>,
     options: PrettyPrintMirOptions,
+    alloc_map: RefCell<FxHashMap<AllocId, AllocId>>,
+    reverse_alloc_map: RefCell<FxHashMap<AllocId, AllocId>>,
 }
 
 impl<'a, 'tcx> MirWriter<'a, 'tcx> {
     pub fn new(tcx: TyCtxt<'tcx>) -> Self {
-        MirWriter { tcx, extra_data: &|_, _| Ok(()), options: PrettyPrintMirOptions::from_cli(tcx) }
+        MirWriter {
+            tcx,
+            extra_data: &|_, _| Ok(()),
+            options: PrettyPrintMirOptions::from_cli(tcx),
+            alloc_map: Default::default(),
+            reverse_alloc_map: Default::default(),
+        }
+    }
+
+    fn remap_alloc_id(&self, alloc_id: AllocId) -> AllocId {
+        let next_remap_id = self.alloc_map.borrow().len() as u64 + 1;
+
+        *self.alloc_map.borrow_mut().entry(alloc_id).or_insert_with(|| {
+            let remapped_id = AllocId(NonZero::new(next_remap_id).expect("can't be zero"));
+
+            self.reverse_alloc_map.borrow_mut().insert(remapped_id, alloc_id);
+
+            remapped_id
+        })
     }
 
     /// Write out a human-readable textual representation for the given function.
     pub fn write_mir_fn(&self, body: &Body<'tcx>, w: &mut dyn io::Write) -> io::Result<()> {
+        let mut body = body.clone();
+
+        let mut visitor = AllocIdsRemapper { writer: self, ids: Default::default() };
+        visitor.visit_body(&mut body);
+
+        let body = &body;
+
         write_mir_intro(self.tcx, body, w, self.options)?;
         for block in body.basic_blocks.indices() {
             (self.extra_data)(PassWhere::BeforeBlock(block), w)?;
@@ -387,7 +419,7 @@ impl<'a, 'tcx> MirWriter<'a, 'tcx> {
 
         writeln!(w, "}}")?;
 
-        write_allocations(self.tcx, body, w)?;
+        write_allocations(self, body, w, visitor.ids)?;
 
         Ok(())
     }
@@ -1565,66 +1597,93 @@ fn comment(tcx: TyCtxt<'_>, SourceInfo { span, scope }: SourceInfo) -> String {
 ///////////////////////////////////////////////////////////////////////////
 // Allocations
 
+/// Remaps allocation ids for deterministic output. Despite the fact that allocation
+/// ids are not deterministic, we can remap them into deterministic output for serialization,
+/// as serialization order is deterministic.
+struct AllocIdsRemapper<'a, 'b, 'tcx> {
+    writer: &'a MirWriter<'b, 'tcx>,
+    ids: BTreeSet<AllocId>,
+}
+
+impl AllocIdsRemapper<'_, '_, '_> {
+    fn remap_alloc_id(&mut self, alloc_id: AllocId) -> AllocId {
+        let remapped_id = self.writer.remap_alloc_id(alloc_id);
+        self.ids.insert(remapped_id);
+
+        remapped_id
+    }
+}
+
+impl<'tcx> MutVisitor<'tcx> for AllocIdsRemapper<'_, '_, 'tcx> {
+    fn tcx<'a>(&'a self) -> TyCtxt<'tcx> {
+        self.writer.tcx
+    }
+
+    fn visit_const_operand(&mut self, constant: &mut ConstOperand<'tcx>, _: Location) {
+        match &mut constant.const_ {
+            Const::Val(const_value, _) => {
+                match const_value {
+                    ConstValue::Scalar(Scalar::Ptr(pointer, ..)) => {
+                        let mut parts = pointer.provenance.into_parts();
+                        parts.0 = self.remap_alloc_id(parts.0);
+
+                        pointer.provenance = CtfeProvenance::from_parts(parts);
+                    }
+                    ConstValue::Slice { alloc_id, .. } | ConstValue::Indirect { alloc_id, .. } => {
+                        // FIXME: we don't actually want to print all of these, since some are printed nicely directly as values inline in MIR.
+                        // Really we'd want `pretty_print_const_value` to decide which allocations to print, instead of having a separate visitor.
+                        *alloc_id = self.remap_alloc_id(*alloc_id);
+                    }
+                    ConstValue::Scalar(Scalar::Int { .. }) | ConstValue::ZeroSized => {}
+                };
+            }
+            Const::Ty(_, _) | Const::Unevaluated(..) => {}
+        }
+    }
+}
+
 /// Find all `AllocId`s mentioned (recursively) in the MIR body and print their corresponding
 /// allocations.
 pub fn write_allocations<'tcx>(
-    tcx: TyCtxt<'tcx>,
+    mir_writer: &MirWriter<'_, 'tcx>,
     body: &Body<'_>,
     w: &mut dyn io::Write,
+    initial_alloc_ids: BTreeSet<AllocId>,
 ) -> io::Result<()> {
     fn alloc_ids_from_alloc(
         alloc: ConstAllocation<'_>,
     ) -> impl DoubleEndedIterator<Item = AllocId> {
         alloc.inner().provenance().ptrs().values().map(|p| p.alloc_id())
     }
 
-    fn alloc_id_from_const_val(val: ConstValue) -> Option<AllocId> {
-        match val {
-            ConstValue::Scalar(interpret::Scalar::Ptr(ptr, _)) => Some(ptr.provenance.alloc_id()),
-            ConstValue::Scalar(interpret::Scalar::Int { .. }) => None,
-            ConstValue::ZeroSized => None,
-            ConstValue::Slice { alloc_id, .. } | ConstValue::Indirect { alloc_id, .. } => {
-                // FIXME: we don't actually want to print all of these, since some are printed nicely directly as values inline in MIR.
-                // Really we'd want `pretty_print_const_value` to decide which allocations to print, instead of having a separate visitor.
-                Some(alloc_id)
-            }
-        }
-    }
-    struct CollectAllocIds(BTreeSet<AllocId>);
-
-    impl<'tcx> Visitor<'tcx> for CollectAllocIds {
-        fn visit_const_operand(&mut self, c: &ConstOperand<'tcx>, _: Location) {
-            match c.const_ {
-                Const::Ty(_, _) | Const::Unevaluated(..) => {}
-                Const::Val(val, _) => {
-                    if let Some(id) = alloc_id_from_const_val(val) {
-                        self.0.insert(id);
-                    }
-                }
-            }
-        }
-    }
-
-    let mut visitor = CollectAllocIds(Default::default());
-    visitor.visit_body(body);
+    let tcx = mir_writer.tcx;
 
     // `seen` contains all seen allocations, including the ones we have *not* printed yet.
     // The protocol is to first `insert` into `seen`, and only if that returns `true`
     // then push to `todo`.
-    let mut seen = visitor.0;
+    let mut seen = initial_alloc_ids;
     let mut todo: Vec<_> = seen.iter().copied().collect();
+
+    // Invariant: all ids in this loop are remapped.
     while let Some(id) = todo.pop() {
-        let mut write_allocation_track_relocs =
-            |w: &mut dyn io::Write, alloc: ConstAllocation<'tcx>| -> io::Result<()> {
-                // `.rev()` because we are popping them from the back of the `todo` vector.
-                for id in alloc_ids_from_alloc(alloc).rev() {
-                    if seen.insert(id) {
-                        todo.push(id);
-                    }
+        let mut write_allocation_track_relocs = |mir_writer: &MirWriter<'_, 'tcx>,
+                                                 w: &mut dyn io::Write,
+                                                 alloc: ConstAllocation<'tcx>|
+         -> io::Result<()> {
+            // `.rev()` because we are popping them from the back of the `todo` vector.
+            for id in alloc_ids_from_alloc(alloc).rev() {
+                let mapped_id = mir_writer.remap_alloc_id(id);
+                if seen.insert(mapped_id) {
+                    todo.push(mapped_id);
                 }
-                write!(w, "{}", display_allocation(tcx, alloc.inner()))
-            };
+            }
+            write!(w, "{}", display_allocation(tcx, alloc.inner()))
+        };
+
         write!(w, "\n{id:?}")?;
+
+        let id = mir_writer.reverse_alloc_map.borrow()[&id];
+
         match tcx.try_get_global_alloc(id) {
             // This can't really happen unless there are bugs, but it doesn't cost us anything to
             // gracefully handle it and allow buggy rustc to be debugged via allocation printing.
@@ -1651,7 +1710,7 @@ pub fn write_allocations<'tcx>(
                     match tcx.eval_static_initializer(did) {
                         Ok(alloc) => {
                             write!(w, ", ")?;
-                            write_allocation_track_relocs(w, alloc)?;
+                            write_allocation_track_relocs(mir_writer, w, alloc)?;
                         }
                         Err(_) => write!(w, ", error during initializer evaluation)")?,
                     }
@@ -1662,7 +1721,7 @@ pub fn write_allocations<'tcx>(
             }
             Some(GlobalAlloc::Memory(alloc)) => {
                 write!(w, " (")?;
-                write_allocation_track_relocs(w, alloc)?
+                write_allocation_track_relocs(mir_writer, w, alloc)?
             }
         }
         writeln!(w)?;

compiler/rustc_span/src/hygiene.rs

@@ -1302,9 +1302,49 @@ pub struct HygieneEncodeContext {
     serialized_expns: Lock<FxHashSet<ExpnId>>,
 
     latest_expns: Lock<FxHashSet<ExpnId>>,
+
+    /// Maps every `SyntaxContext` into its encoding index.
+    /// Earlier the `ctxt.0` was used when writing metadata, however,
+    /// this results into non-deterministic metadata (see #129094).
+    /// The non-determinism is encountered when decoding syntax contexts
+    /// in `decode_syntax_context` function below. The syntax contexts from
+    /// other crate metadata can be decoded in different order, which results
+    /// into different ids assigned to decoded syntax contexts.
+    /// First invocation:
+    /// (ALLOC - syntax context id, ORIG - original id of decoded syntax context:
+    /// `raw_id` in `decode_syntax_context`)
+    /// ALLOC: #3, ORIG: 1
+    /// ALLOC: #9, ORIG: 18769
+    /// ALLOC: #10, ORIG: 25868
+    /// ALLOC: #11, ORIG: 18822
+    /// ALLOC: #12, ORIG: 23092
+    ///
+    /// Second invocation:
+    /// ALLOC: #3, ORIG: 1
+    /// ALLOC: #9, ORIG: 25868
+    /// ALLOC: #10, ORIG: 18769
+    /// ALLOC: #11, ORIG: 18822
+    /// ALLOC: #12, ORIG: 23092
+    ///
+    /// We see that `18769` and `25868` assigned different syntax context ids,
+    /// however, the order of encoding is deterministic, so we can remap allocated
+    /// syntax context ids into encoding indices and use them, thus outputting
+    /// same metadata.
+    encoding_indices: Lock<FxHashMap<SyntaxContext, u32>>,
 }
 
 impl HygieneEncodeContext {
+    fn get_encoding_index(&self, ctxt: SyntaxContext) -> u32 {
+        if ctxt.is_root() {
+            return 0;
+        }
+
+        let mut map = self.encoding_indices.lock();
+        // Zero is taken by root syntax context.
+        let encoding_index = map.len() + 1;
+        *map.entry(ctxt).or_insert(encoding_index as u32)
+    }
+
     /// Record the fact that we need to serialize the corresponding `ExpnData`.
     pub fn schedule_expn_data_for_encoding(&self, expn: ExpnId) {
         if !self.serialized_expns.lock().contains(&expn) {
@@ -1329,18 +1369,19 @@ impl HygieneEncodeContext {
 
             // Consume the current round of syntax contexts.
             // Drop the lock() temporary early.
-            // It's fine to iterate over a HashMap, because the serialization of the table
-            // that we insert data into doesn't depend on insertion order.
             #[allow(rustc::potential_query_instability)]
             let latest_ctxts = { mem::take(&mut *self.latest_ctxts.lock()) }.into_iter();
-            let all_ctxt_data: Vec<_> = HygieneData::with(|data| {
+            let mut all_ctxt_data: Vec<_> = HygieneData::with(|data| {
                 latest_ctxts
                     .map(|ctxt| (ctxt, data.syntax_context_data[ctxt.0 as usize].key()))
                     .collect()
             });
+
+            all_ctxt_data.sort_by_key(|&(ctxt, _)| self.get_encoding_index(ctxt));
+
             for (ctxt, ctxt_key) in all_ctxt_data {
                 if self.serialized_ctxts.lock().insert(ctxt) {
-                    encode_ctxt(encoder, ctxt.0, &ctxt_key);
+                    encode_ctxt(encoder, self.get_encoding_index(ctxt), &ctxt_key);
                 }
             }
 
@@ -1488,7 +1529,8 @@ pub fn raw_encode_syntax_context(
     if !context.serialized_ctxts.lock().contains(&ctxt) {
         context.latest_ctxts.lock().insert(ctxt);
     }
-    ctxt.0.encode(e);
+
+    context.get_encoding_index(ctxt).encode(e);
 }
 
 /// Updates the `disambiguator` field of the corresponding `ExpnData`

tests/run-make/parallel-reproducible-build/derives-issue-129094.rs

@@ -0,0 +1,5 @@
+#![crate_type = "lib"]
+#[derive(Clone, Copy, Hash, PartialEq, PartialOrd)]
+struct PackedPoint {
+    x: u32,
+}

tests/run-make/parallel-reproducible-build/mir-alloc-ids-issue-154278.rs

@@ -0,0 +1,24 @@
+pub struct A<T> {
+    pub v: T,
+}
+pub struct B<T> {
+    pub v: T,
+}
+
+pub mod test {
+    pub struct A<T> {
+        pub v: T,
+    }
+
+    impl<T> A<T> {
+        pub fn foo(&self) -> isize {
+            static a: isize = 5;
+            return a;
+        }
+
+        pub fn bar(&self) -> isize {
+            static a: isize = 6;
+            return a;
+        }
+    }
+}