Auto merge of #1130 - christianpoveda:ignore-close-read-only, r=RalfJung

Ignore close errors in read-only files.

this fixes #999

r? @RalfJung

Author: bors

src/shims/env.rs

@@ -18,14 +18,8 @@ pub struct EnvVars {
 impl EnvVars {
     pub(crate) fn init<'mir, 'tcx>(
         ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>,
-        mut excluded_env_vars: Vec<String>,
+        excluded_env_vars: Vec<String>,
     ) {
-        // FIXME: this can be removed when we fix the behavior of the `close` shim for macos.
-        if ecx.tcx.sess.target.target.target_os.to_lowercase() != "linux" {
-            // Exclude `TERM` var to avoid terminfo trying to open the termcap file.
-            excluded_env_vars.push("TERM".to_owned());
-        }
-
         if ecx.machine.communicate {
             for (name, value) in env::vars() {
                 if !excluded_env_vars.contains(&name) {

src/shims/fs.rs

@@ -15,6 +15,7 @@ use shims::time::system_time_to_duration;
 #[derive(Debug)]
 pub struct FileHandle {
     file: File,
+    writable: bool,
 }
 
 pub struct FileHandler {
@@ -56,10 +57,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         if (o_rdonly | o_wronly | o_rdwr) & !0b11 != 0 {
             throw_unsup_format!("Access mode flags on this platform are unsupported");
         }
+        let mut writable = true;
+
         // Now we check the access mode
         let access_mode = flag & 0b11;
 
         if access_mode == o_rdonly {
+            writable = false;
             options.read(true);
         } else if access_mode == o_wronly {
             options.write(true);
@@ -105,7 +109,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         let fd = options.open(&path).map(|file| {
             let mut fh = &mut this.machine.file_handler;
             fh.low += 1;
-            fh.handles.insert(fh.low, FileHandle { file }).unwrap_none();
+            fh.handles.insert(fh.low, FileHandle { file, writable }).unwrap_none();
             fh.low
         });
 
@@ -148,13 +152,24 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         let fd = this.read_scalar(fd_op)?.to_i32()?;
 
         if let Some(handle) = this.machine.file_handler.handles.remove(&fd) {
-            // `File::sync_all` does the checks that are done when closing a file. We do this to
-            // to handle possible errors correctly.
-            let result = this.try_unwrap_io_result(handle.file.sync_all().map(|_| 0i32));
-            // Now we actually close the file.
-            drop(handle);
-            // And return the result.
-            result
+            // We sync the file if it was opened in a mode different than read-only.
+            if handle.writable {
+                // `File::sync_all` does the checks that are done when closing a file. We do this to
+                // to handle possible errors correctly.
+                let result = this.try_unwrap_io_result(handle.file.sync_all().map(|_| 0i32));
+                // Now we actually close the file.
+                drop(handle);
+                // And return the result.
+                result
+            } else {
+                // We drop the file, this closes it but ignores any errors produced when closing
+                // it. This is done because `File::sync_call` cannot be done over files like
+                // `/dev/urandom` which are read-only. Check
+                // https://github.com/rust-lang/miri/issues/999#issuecomment-568920439 for a deeper
+                // discussion.
+                drop(handle);
+                Ok(0)
+            }
         } else {
             this.handle_not_found()
         }