Auto merge of #137025 - oli-obk:mmap, r=<try>

Lock mmapped files to at least get some safety out of it

Unfortunately this only is a guarantee on windows.
Double-unfortunately I don't know what's gonna happen when rustc segfaults on windows and a file is locked. Possibly the file is now deadlocked.

Linux just locks file as a hint, so only tools that know about file locking will actually respect it, others will just access it.

I guess if we had some sort of story for volatile memory we could use that as we're only reading bytes out of it and not actually mapping data structures to that memory.

Author: bors

Diff for: compiler/rustc_codegen_llvm/src/back/lto.rs

@@ -112,10 +112,7 @@ fn prepare_lto(
                     .extend(exported_symbols[&cnum].iter().filter_map(symbol_filter));
             }
 
-            let archive_data = unsafe {
-                Mmap::map(std::fs::File::open(&path).expect("couldn't open rlib"))
-                    .expect("couldn't map rlib")
-            };
+            let archive_data = unsafe { Mmap::map(&path).expect("couldn't map rlib") };
             let archive = ArchiveFile::parse(&*archive_data).expect("wanted an rlib");
             let obj_files = archive
                 .members()

Diff for: compiler/rustc_codegen_ssa/src/back/archive.rs

@@ -138,11 +138,8 @@ pub trait ArchiveBuilderBuilder {
         bundled_lib_file_names: &FxIndexSet<Symbol>,
     ) -> Result<(), ExtractBundledLibsError<'a>> {
         let archive_map = unsafe {
-            Mmap::map(
-                File::open(rlib)
-                    .map_err(|e| ExtractBundledLibsError::OpenFile { rlib, error: Box::new(e) })?,
-            )
-            .map_err(|e| ExtractBundledLibsError::MmapFile { rlib, error: Box::new(e) })?
+            Mmap::map(rlib)
+                .map_err(|e| ExtractBundledLibsError::MmapFile { rlib, error: Box::new(e) })?
         };
         let archive = ArchiveFile::parse(&*archive_map)
             .map_err(|e| ExtractBundledLibsError::ParseArchive { rlib, error: Box::new(e) })?;
@@ -363,7 +360,7 @@ pub fn try_extract_macho_fat_archive(
     sess: &Session,
     archive_path: &Path,
 ) -> io::Result<Option<PathBuf>> {
-    let archive_map = unsafe { Mmap::map(File::open(&archive_path)?)? };
+    let archive_map = unsafe { Mmap::map(&archive_path)? };
     let target_arch = match sess.target.arch.as_ref() {
         "aarch64" => object::Architecture::Aarch64,
         "x86_64" => object::Architecture::X86_64,
@@ -400,7 +397,7 @@ impl<'a> ArchiveBuilder for ArArchiveBuilder<'a> {
             return Ok(());
         }
 
-        let archive_map = unsafe { Mmap::map(File::open(&archive_path)?)? };
+        let archive_map = unsafe { Mmap::map(&archive_path)? };
         let archive = ArchiveFile::parse(&*archive_map)
             .map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))?;
         let archive_index = self.src_archives.len();
@@ -463,25 +460,22 @@ impl<'a> ArArchiveBuilder<'a> {
         let mut entries = Vec::new();
 
         for (entry_name, entry) in self.entries {
-            let data =
-                match entry {
-                    ArchiveEntry::FromArchive { archive_index, file_range } => {
-                        let src_archive = &self.src_archives[archive_index];
-
-                        let data = &src_archive.1
-                            [file_range.0 as usize..file_range.0 as usize + file_range.1 as usize];
-
-                        Box::new(data) as Box<dyn AsRef<[u8]>>
-                    }
-                    ArchiveEntry::File(file) => unsafe {
-                        Box::new(
-                            Mmap::map(File::open(file).map_err(|err| {
-                                io_error_context("failed to open object file", err)
-                            })?)
+            let data = match entry {
+                ArchiveEntry::FromArchive { archive_index, file_range } => {
+                    let src_archive = &self.src_archives[archive_index];
+
+                    let data = &src_archive.1
+                        [file_range.0 as usize..file_range.0 as usize + file_range.1 as usize];
+
+                    Box::new(data) as Box<dyn AsRef<[u8]>>
+                }
+                ArchiveEntry::File(file) => unsafe {
+                    Box::new(
+                        Mmap::map(file)
                             .map_err(|err| io_error_context("failed to map object file", err))?,
-                        ) as Box<dyn AsRef<[u8]>>
-                    },
-                };
+                    ) as Box<dyn AsRef<[u8]>>
+                },
+            };
 
             entries.push(NewArchiveMember {
                 buf: data,

Diff for: compiler/rustc_codegen_ssa/src/back/link.rs

@@ -1,6 +1,6 @@
 use std::collections::BTreeSet;
 use std::ffi::OsString;
-use std::fs::{File, OpenOptions, read};
+use std::fs::{OpenOptions, read};
 use std::io::{BufWriter, Write};
 use std::ops::{ControlFlow, Deref};
 use std::path::{Path, PathBuf};
@@ -680,8 +680,7 @@ fn link_dwarf_object(sess: &Session, cg_results: &CodegenResults, executable_out
         }
 
         fn read_input(&self, path: &Path) -> std::io::Result<&[u8]> {
-            let file = File::open(&path)?;
-            let mmap = (unsafe { Mmap::map(file) })?;
+            let mmap = (unsafe { Mmap::map(&path) })?;
             Ok(self.alloc_mmap(mmap))
         }
     }

Diff for: compiler/rustc_codegen_ssa/src/back/metadata.rs

@@ -1,7 +1,6 @@
 //! Reading of the rustc metadata for rlibs and dylibs
 
 use std::borrow::Cow;
-use std::fs::File;
 use std::io::Write;
 use std::path::Path;
 
@@ -45,10 +44,7 @@ fn load_metadata_with(
     path: &Path,
     f: impl for<'a> FnOnce(&'a [u8]) -> Result<&'a [u8], String>,
 ) -> Result<OwnedSlice, String> {
-    let file =
-        File::open(path).map_err(|e| format!("failed to open file '{}': {}", path.display(), e))?;
-
-    unsafe { Mmap::map(file) }
+    unsafe { Mmap::map(&path) }
         .map_err(|e| format!("failed to mmap file '{}': {}", path.display(), e))
         .and_then(|mmap| try_slice_owned(mmap, |mmap| f(mmap)))
 }

Diff for: compiler/rustc_codegen_ssa/src/back/write.rs

@@ -2142,11 +2142,9 @@ pub(crate) fn submit_pre_lto_module_to_llvm<B: ExtraBackendMethods>(
 ) {
     let filename = pre_lto_bitcode_filename(&module.name);
     let bc_path = in_incr_comp_dir_sess(tcx.sess, &filename);
-    let file = fs::File::open(&bc_path)
-        .unwrap_or_else(|e| panic!("failed to open bitcode file `{}`: {}", bc_path.display(), e));
 
     let mmap = unsafe {
-        Mmap::map(file).unwrap_or_else(|e| {
+        Mmap::map(&bc_path).unwrap_or_else(|e| {
             panic!("failed to mmap bitcode file `{}`: {}", bc_path.display(), e)
         })
     };

Diff for: compiler/rustc_codegen_ssa/src/lib.rs

@@ -276,24 +276,22 @@ impl CodegenResults {
     ) -> Result<(Self, OutputFilenames), CodegenErrors> {
         // The Decodable machinery is not used here because it panics if the input data is invalid
         // and because its internal representation may change.
-        if !data.starts_with(RLINK_MAGIC) {
+        let Some(data) = data.strip_prefix(RLINK_MAGIC) else {
             return Err(CodegenErrors::WrongFileType);
-        }
-        let data = &data[RLINK_MAGIC.len()..];
-        if data.len() < 4 {
+        };
+
+        let Some((&version_array, data)) = data.split_first_chunk() else {
             return Err(CodegenErrors::EmptyVersionNumber);
-        }
+        };
 
-        let mut version_array: [u8; 4] = Default::default();
-        version_array.copy_from_slice(&data[..4]);
         if u32::from_be_bytes(version_array) != RLINK_VERSION {
             return Err(CodegenErrors::EncodingVersionMismatch {
                 version_array: String::from_utf8_lossy(&version_array).to_string(),
                 rlink_version: RLINK_VERSION,
             });
         }
 
-        let Ok(mut decoder) = MemDecoder::new(&data[4..], 0) else {
+        let Ok(mut decoder) = MemDecoder::new(data, 0) else {
             return Err(CodegenErrors::CorruptFile);
         };
         let rustc_version = decoder.read_str();

Diff for: compiler/rustc_data_structures/src/memmap.rs

@@ -1,13 +1,21 @@
 use std::fs::File;
 use std::io;
 use std::ops::{Deref, DerefMut};
+use std::path::Path;
+
+use crate::flock::Lock;
 
 /// A trivial wrapper for [`memmap2::Mmap`] (or `Vec<u8>` on WASM).
 #[cfg(not(any(miri, target_arch = "wasm32")))]
-pub struct Mmap(memmap2::Mmap);
+pub struct Mmap {
+    map: memmap2::Mmap,
+    _lock: Option<Lock>,
+}
 
 #[cfg(any(miri, target_arch = "wasm32"))]
-pub struct Mmap(Vec<u8>);
+pub struct Mmap {
+    map: Vec<u8>,
+}
 
 #[cfg(not(any(miri, target_arch = "wasm32")))]
 impl Mmap {
@@ -17,27 +25,28 @@ impl Mmap {
     ///
     /// However in practice most callers do not ensure this, so uses of this function are likely unsound.
     #[inline]
-    pub unsafe fn map(file: File) -> io::Result<Self> {
+    pub unsafe fn map(path: impl AsRef<Path>) -> io::Result<Self> {
+        let path = path.as_ref();
+        let file = File::open(path)?;
+        let _lock = Some(Lock::new(path, true, false, false)?);
         // By default, memmap2 creates shared mappings, implying that we could see updates to the
         // file through the mapping. That would violate our precondition; so by requesting a
         // map_copy_read_only we do not lose anything.
         // This mapping mode also improves our support for filesystems such as cacheless virtiofs.
         // For more details see https://github.com/rust-lang/rust/issues/122262
         //
         // SAFETY: The caller must ensure that this is safe.
-        unsafe { memmap2::MmapOptions::new().map_copy_read_only(&file).map(Mmap) }
+
+        let map = unsafe { memmap2::MmapOptions::new().map_copy_read_only(&file)? };
+        Ok(Self { _lock, map })
     }
 }
 
 #[cfg(any(miri, target_arch = "wasm32"))]
 impl Mmap {
     #[inline]
-    pub unsafe fn map(mut file: File) -> io::Result<Self> {
-        use std::io::Read;
-
-        let mut data = Vec::new();
-        file.read_to_end(&mut data)?;
-        Ok(Mmap(data))
+    pub unsafe fn map(path: impl AsRef<Path>) -> io::Result<Self> {
+        Ok(Mmap(std::fs::read(path)?))
     }
 }
 
@@ -46,13 +55,13 @@ impl Deref for Mmap {
 
     #[inline]
     fn deref(&self) -> &[u8] {
-        &self.0
+        &self.map
     }
 }
 
 impl AsRef<[u8]> for Mmap {
     fn as_ref(&self) -> &[u8] {
-        &self.0
+        &self.map
     }
 }
 
@@ -77,8 +86,8 @@ impl MmapMut {
 
     #[inline]
     pub fn make_read_only(self) -> std::io::Result<Mmap> {
-        let mmap = self.0.make_read_only()?;
-        Ok(Mmap(mmap))
+        let map = self.0.make_read_only()?;
+        Ok(Mmap { map, _lock: None })
     }
 }
 
@@ -97,7 +106,7 @@ impl MmapMut {
 
     #[inline]
     pub fn make_read_only(self) -> std::io::Result<Mmap> {
-        Ok(Mmap(self.0))
+        Ok(Mmap { map: self.0 })
     }
 }
 

Diff for: compiler/rustc_incremental/src/persist/file_format.rs

@@ -95,18 +95,17 @@ pub(crate) fn read_file(
     is_nightly_build: bool,
     cfg_version: &'static str,
 ) -> io::Result<Option<(Mmap, usize)>> {
-    let file = match fs::File::open(path) {
-        Ok(file) => file,
-        Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(None),
-        Err(err) => return Err(err),
-    };
     // SAFETY: This process must not modify nor remove the backing file while the memory map lives.
     // For the dep-graph and the work product index, it is as soon as the decoding is done.
     // For the query result cache, the memory map is dropped in save_dep_graph before calling
     // save_in and trying to remove the backing file.
     //
     // There is no way to prevent another process from modifying this file.
-    let mmap = unsafe { Mmap::map(file) }?;
+    let mmap = match unsafe { Mmap::map(path) } {
+        Ok(file) => file,
+        Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(None),
+        Err(err) => return Err(err),
+    };
 
     let mut file = io::Cursor::new(&*mmap);
 

Diff for: compiler/rustc_metadata/src/locator.rs

@@ -822,13 +822,7 @@ fn get_metadata_section<'p>(
         }
         CrateFlavor::Rmeta => {
             // mmap the file, because only a small fraction of it is read.
-            let file = std::fs::File::open(filename).map_err(|_| {
-                MetadataError::LoadFailure(format!(
-                    "failed to open rmeta metadata: '{}'",
-                    filename.display()
-                ))
-            })?;
-            let mmap = unsafe { Mmap::map(file) };
+            let mmap = unsafe { Mmap::map(filename) };
             let mmap = mmap.map_err(|_| {
                 MetadataError::LoadFailure(format!(
                     "failed to mmap rmeta metadata: '{}'",

Diff for: compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -2245,12 +2245,10 @@ pub struct EncodedMetadata {
 impl EncodedMetadata {
     #[inline]
     pub fn from_path(path: PathBuf, temp_dir: Option<MaybeTempDir>) -> std::io::Result<Self> {
-        let file = std::fs::File::open(&path)?;
-        let file_metadata = file.metadata()?;
-        if file_metadata.len() == 0 {
+        if std::fs::metadata(&path)?.len() == 0 {
             return Ok(Self { mmap: None, _temp_dir: None });
         }
-        let mmap = unsafe { Some(Mmap::map(file)?) };
+        let mmap = unsafe { Some(Mmap::map(path)?) };
         Ok(Self { mmap, _temp_dir: temp_dir })
     }
 
@@ -2272,9 +2270,7 @@ impl<D: Decoder> Decodable<D> for EncodedMetadata {
         let len = d.read_usize();
         let mmap = if len > 0 {
             let mut mmap = MmapMut::map_anon(len).unwrap();
-            for _ in 0..len {
-                (&mut mmap[..]).write_all(&[d.read_u8()]).unwrap();
-            }
+            mmap.copy_from_slice(d.read_raw_bytes(len));
             mmap.flush().unwrap();
             Some(mmap.make_read_only().unwrap())
         } else {