Skip to content

Commit

Permalink
Auto merge of rust-lang#1130 - christianpoveda:ignore-close-read-only…
Browse files Browse the repository at this point in the history
…, r=RalfJung

Ignore close errors in read-only files.

this fixes rust-lang#999

r? @RalfJung
  • Loading branch information
bors committed Dec 31, 2019
2 parents 9f79aa9 + a40a99d commit 39146c4
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 15 deletions.
8 changes: 1 addition & 7 deletions src/shims/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
31 changes: 23 additions & 8 deletions src/shims/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use shims::time::system_time_to_duration;
#[derive(Debug)]
pub struct FileHandle {
file: File,
writable: bool,
}

pub struct FileHandler {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
});

Expand Down Expand Up @@ -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()
}
Expand Down

0 comments on commit 39146c4

Please sign in to comment.