Skip to content

Ignore close errors in read-only files. #1130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 31, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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