Skip to content
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

add graceful shim for the custom O_TMPFILE file opening flag plus test case #2718

Merged
merged 1 commit into from
Dec 9, 2022
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
9 changes: 9 additions & 0 deletions src/shims/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// (Technically we do not support *not* setting this flag, but we ignore that.)
mirror |= o_cloexec;
}
if this.tcx.sess.target.os == "linux" {
let o_tmpfile = this.eval_libc_i32("O_TMPFILE")?;
if flag & o_tmpfile != 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you noted, O_TMPFILE = 0x410000, which is more than one bit. This is for historical reasons, since the open() syscall ignores unrecognized flags: https://lwn.net/Articles/588444/

So the right check should be flag & o_tmpfile == o_tmpfile.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! Since this PR is already merged, I will address your feedback within my other related PR #2720

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it looks like #2720 might be a bit more tricky, it'd be good to have a separate PR for this.

// if the flag contains `O_TMPFILE` then we return a graceful error
let eopnotsupp = this.eval_libc("EOPNOTSUPP")?;
this.set_last_error(eopnotsupp)?;
return Ok(-1);
}
}
// If `flag` is not equal to `mirror`, there is an unsupported option enabled in `flag`,
// then we throw an error.
if flag != mirror {
Expand Down
32 changes: 31 additions & 1 deletion tests/pass-dep/shims/libc-fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

use std::convert::TryInto;
use std::ffi::CString;
use std::fs::{canonicalize, remove_file, File};
use std::fs::{canonicalize, remove_dir_all, remove_file, File};
use std::io::{Error, ErrorKind, Write};
use std::os::unix::ffi::OsStrExt;
use std::path::PathBuf;
Expand All @@ -18,6 +18,8 @@ fn main() {
test_file_open_unix_allow_two_args();
test_file_open_unix_needs_three_args();
test_file_open_unix_extra_third_arg();
#[cfg(target_os = "linux")]
test_o_tmpfile_flag();
}

fn tmp() -> PathBuf {
Expand Down Expand Up @@ -45,6 +47,15 @@ fn prepare(filename: &str) -> PathBuf {
path
}

/// Prepare directory: compute directory name and make sure it does not exist.
#[allow(unused)]
fn prepare_dir(dirname: &str) -> PathBuf {
Pointerbender marked this conversation as resolved.
Show resolved Hide resolved
let path = tmp().join(&dirname);
// Clean the directory for robustness.
remove_dir_all(&path).ok();
path
}

/// Prepare like above, and also write some initial content to the file.
fn prepare_with_content(filename: &str, content: &[u8]) -> PathBuf {
let path = prepare(filename);
Expand Down Expand Up @@ -135,3 +146,22 @@ fn test_readlink() {
assert_eq!(res, -1);
assert_eq!(Error::last_os_error().kind(), ErrorKind::NotFound);
}

#[cfg(target_os = "linux")]
fn test_o_tmpfile_flag() {
Pointerbender marked this conversation as resolved.
Show resolved Hide resolved
use std::fs::{create_dir, OpenOptions};
use std::os::unix::fs::OpenOptionsExt;
let dir_path = prepare_dir("miri_test_fs_dir");
create_dir(&dir_path).unwrap();
// test that the `O_TMPFILE` custom flag gracefully errors instead of stopping execution
assert_eq!(
Some(libc::EOPNOTSUPP),
OpenOptions::new()
.read(true)
.write(true)
.custom_flags(libc::O_TMPFILE)
.open(dir_path)
.unwrap_err()
.raw_os_error(),
);
}