Mmap::map is always used on a freshly opened file, merge those two operations

Author: oli-obk

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()

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,

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))
         }
     }

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)))
 }

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)
         })
     };

compiler/rustc_data_structures/src/memmap.rs

@@ -1,6 +1,7 @@
 use std::fs::File;
 use std::io;
 use std::ops::{Deref, DerefMut};
+use std::path::Path;
 
 /// A trivial wrapper for [`memmap2::Mmap`] (or `Vec<u8>` on WASM).
 #[cfg(not(any(miri, target_arch = "wasm32")))]
@@ -17,27 +18,24 @@ 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 file = File::open(path)?;
         // 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) }
+        unsafe { Ok(Self(memmap2::MmapOptions::new().map_copy_read_only(&file)?)) }
     }
 }
 
 #[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)?))
     }
 }
 

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);
 

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: '{}'",

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 })
     }