Auto merge of #1189 - RalfJung:cleanup, r=RalfJung

bors · bors · commit 29b0bbf85989 · 2020-02-23T18:11:37.000Z
some foreign_items cleanup

Cc @christianpoveda
diff --git a/src/eval.rs b/src/eval.rs
@@ -169,7 +169,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
 pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) -> Option<i64> {
     // FIXME: We always ignore leaks on some platforms where we do not
     // correctly implement TLS destructors.
-    let target_os = tcx.sess.target.target.target_os.to_lowercase();
+    let target_os = tcx.sess.target.target.target_os.as_str();
     let ignore_leaks = config.ignore_leaks || target_os == "windows" || target_os == "macos";
 
     let (mut ecx, ret_place) = match create_ecx(tcx, main_id, config) {
diff --git a/src/helpers.rs b/src/helpers.rs
@@ -359,25 +359,25 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     /// Helper function used inside the shims of foreign functions to check that isolation is
     /// disabled. It returns an error using the `name` of the foreign function if this is not the
     /// case.
-    fn check_no_isolation(&mut self, name: &str) -> InterpResult<'tcx> {
-        if !self.eval_context_mut().machine.communicate {
+    fn check_no_isolation(&self, name: &str) -> InterpResult<'tcx> {
+        if !self.eval_context_ref().machine.communicate {
             throw_unsup_format!(
                 "`{}` not available when isolation is enabled. Pass the flag `-Zmiri-disable-isolation` to disable it.",
-                name
+                name,
             )
         }
         Ok(())
     }
     /// Helper function used inside the shims of foreign functions to assert that the target
     /// platform is `platform`. It panics showing a message with the `name` of the foreign function
     /// if this is not the case.
-    fn assert_platform(&mut self, platform: &str, name: &str) {
+    fn assert_platform(&self, platform: &str, name: &str) {
         assert_eq!(
-            self.eval_context_mut().tcx.sess.target.target.target_os.to_lowercase(),
+            self.eval_context_ref().tcx.sess.target.target.target_os,
             platform,
             "`{}` is only available on the `{}` platform",
             name,
-            platform
+            platform,
         )
     }
 
@@ -389,8 +389,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     }
 
     /// Gets the last error variable.
-    fn get_last_error(&mut self) -> InterpResult<'tcx, Scalar<Tag>> {
-        let this = self.eval_context_mut();
+    fn get_last_error(&self) -> InterpResult<'tcx, Scalar<Tag>> {
+        let this = self.eval_context_ref();
         let errno_place = this.machine.last_error.unwrap();
         this.read_scalar(errno_place.into())?.not_undef()
     }
diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs
@@ -455,7 +455,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 this.write_scalar(Scalar::from_f64(res), dest)?;
             }
 
-            _ => match this.tcx.sess.target.target.target_os.to_lowercase().as_str() {
+            _ => match this.tcx.sess.target.target.target_os.as_str() {
                 "linux" | "macos" => return posix::EvalContextExt::emulate_foreign_item_by_name(this, link_name, args, dest, ret),
                 "windows" => return windows::EvalContextExt::emulate_foreign_item_by_name(this, link_name, args, dest, ret),
                 target => throw_unsup_format!("The {} target platform is not supported", target),
diff --git a/src/shims/foreign_items/posix.rs b/src/shims/foreign_items/posix.rs
@@ -322,7 +322,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             }
 
             _ => {
-                match this.tcx.sess.target.target.target_os.to_lowercase().as_str() {
+                match this.tcx.sess.target.target.target_os.as_str() {
                     "linux" => return linux::EvalContextExt::emulate_foreign_item_by_name(this, link_name, args, dest, ret),
                     "macos" => return macos::EvalContextExt::emulate_foreign_item_by_name(this, link_name, args, dest, ret),
                     _ => unreachable!(),
diff --git a/src/shims/foreign_items/posix/linux.rs b/src/shims/foreign_items/posix/linux.rs
@@ -18,7 +18,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 this.write_scalar(errno_place.to_ref().to_scalar()?, dest)?;
             }
 
-            // File related shims
+            // File related shims (but also see "syscall" below for statx)
 
             // The only reason this is not in the `posix` module is because the `macos` item has a
             // different name.
@@ -59,8 +59,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                         // so skip over it.
                         getrandom(this, &args[1..], dest)?;
                     }
-                    // `statx` is used by `libstd` to retrieve metadata information in `linux`
-                    // instead of using `stat`,`lstat` or `fstat` as in the `macos` platform.
+                    // `statx` is used by `libstd` to retrieve metadata information on `linux`
+                    // instead of using `stat`,`lstat` or `fstat` as on `macos`.
                     id if id == sys_statx => {
                         // The first argument is the syscall id,
                         // so skip over it.
@@ -87,7 +87,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     }
 }
 
-// Shims the linux 'getrandom()' syscall.
+// Shims the linux `getrandom` syscall.
 fn getrandom<'tcx>(
     this: &mut MiriEvalContext<'_, 'tcx>,
     args: &[OpTy<'tcx, Tag>],
diff --git a/src/shims/fs.rs b/src/shims/fs.rs
@@ -63,6 +63,105 @@ impl FileHandler {
     }
 }
 
+impl<'mir, 'tcx> EvalContextExtPrivate<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
+trait EvalContextExtPrivate<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
+    /// Emulate `stat` or `lstat` on the `macos` platform. This function is not intended to be
+    /// called directly from `emulate_foreign_item_by_name`, so it does not check if isolation is
+    /// disabled or if the target platform is the correct one. Please use `macos_stat` or
+    /// `macos_lstat` instead.
+    fn macos_stat_or_lstat(
+        &mut self,
+        follow_symlink: bool,
+        path_op: OpTy<'tcx, Tag>,
+        buf_op: OpTy<'tcx, Tag>,
+    ) -> InterpResult<'tcx, i32> {
+        let this = self.eval_context_mut();
+
+        let path_scalar = this.read_scalar(path_op)?.not_undef()?;
+        let path: PathBuf = this.read_os_str_from_c_str(path_scalar)?.into();
+
+        let metadata = match FileMetadata::from_path(this, path, follow_symlink)? {
+            Some(metadata) => metadata,
+            None => return Ok(-1),
+        };
+        this.macos_stat_write_buf(metadata, buf_op)
+    }
+
+    fn macos_stat_write_buf(
+        &mut self,
+        metadata: FileMetadata,
+        buf_op: OpTy<'tcx, Tag>,
+    ) -> InterpResult<'tcx, i32> {
+        let this = self.eval_context_mut();
+
+        let mode: u16 = metadata.mode.to_u16()?;
+
+        let (access_sec, access_nsec) = metadata.accessed.unwrap_or((0, 0));
+        let (created_sec, created_nsec) = metadata.created.unwrap_or((0, 0));
+        let (modified_sec, modified_nsec) = metadata.modified.unwrap_or((0, 0));
+
+        let dev_t_layout = this.libc_ty_layout("dev_t")?;
+        let mode_t_layout = this.libc_ty_layout("mode_t")?;
+        let nlink_t_layout = this.libc_ty_layout("nlink_t")?;
+        let ino_t_layout = this.libc_ty_layout("ino_t")?;
+        let uid_t_layout = this.libc_ty_layout("uid_t")?;
+        let gid_t_layout = this.libc_ty_layout("gid_t")?;
+        let time_t_layout = this.libc_ty_layout("time_t")?;
+        let long_layout = this.libc_ty_layout("c_long")?;
+        let off_t_layout = this.libc_ty_layout("off_t")?;
+        let blkcnt_t_layout = this.libc_ty_layout("blkcnt_t")?;
+        let blksize_t_layout = this.libc_ty_layout("blksize_t")?;
+        let uint32_t_layout = this.libc_ty_layout("uint32_t")?;
+
+        // We need to add 32 bits of padding after `st_rdev` if we are on a 64-bit platform.
+        let pad_layout = if this.tcx.sess.target.ptr_width == 64 {
+            uint32_t_layout
+        } else {
+            this.layout_of(this.tcx.mk_unit())?
+        };
+
+        let imms = [
+            immty_from_uint_checked(0u128, dev_t_layout)?, // st_dev
+            immty_from_uint_checked(mode, mode_t_layout)?, // st_mode
+            immty_from_uint_checked(0u128, nlink_t_layout)?, // st_nlink
+            immty_from_uint_checked(0u128, ino_t_layout)?, // st_ino
+            immty_from_uint_checked(0u128, uid_t_layout)?, // st_uid
+            immty_from_uint_checked(0u128, gid_t_layout)?, // st_gid
+            immty_from_uint_checked(0u128, dev_t_layout)?, // st_rdev
+            immty_from_uint_checked(0u128, pad_layout)?, // padding for 64-bit targets
+            immty_from_uint_checked(access_sec, time_t_layout)?, // st_atime
+            immty_from_uint_checked(access_nsec, long_layout)?, // st_atime_nsec
+            immty_from_uint_checked(modified_sec, time_t_layout)?, // st_mtime
+            immty_from_uint_checked(modified_nsec, long_layout)?, // st_mtime_nsec
+            immty_from_uint_checked(0u128, time_t_layout)?, // st_ctime
+            immty_from_uint_checked(0u128, long_layout)?, // st_ctime_nsec
+            immty_from_uint_checked(created_sec, time_t_layout)?, // st_birthtime
+            immty_from_uint_checked(created_nsec, long_layout)?, // st_birthtime_nsec
+            immty_from_uint_checked(metadata.size, off_t_layout)?, // st_size
+            immty_from_uint_checked(0u128, blkcnt_t_layout)?, // st_blocks
+            immty_from_uint_checked(0u128, blksize_t_layout)?, // st_blksize
+            immty_from_uint_checked(0u128, uint32_t_layout)?, // st_flags
+            immty_from_uint_checked(0u128, uint32_t_layout)?, // st_gen
+        ];
+
+        let buf = this.deref_operand(buf_op)?;
+        this.write_packed_immediates(buf, &imms)?;
+
+        Ok(0)
+    }
+
+    /// Function used when a handle is not found inside `FileHandler`. It returns `Ok(-1)`and sets
+    /// the last OS error to `libc::EBADF` (invalid file descriptor). This function uses
+    /// `T: From<i32>` instead of `i32` directly because some fs functions return different integer
+    /// types (like `read`, that returns an `i64`).
+    fn handle_not_found<T: From<i32>>(&mut self) -> InterpResult<'tcx, T> {
+        let this = self.eval_context_mut();
+        let ebadf = this.eval_libc("EBADF")?;
+        this.set_last_error(ebadf)?;
+        Ok((-1).into())
+    }
+}
+
 impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
 pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
     fn open(
@@ -432,29 +531,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             Some(metadata) => metadata,
             None => return Ok(-1),
         };
-        macos_stat_write_buf(this, metadata, buf_op)
-    }
-
-    /// Emulate `stat` or `lstat` on the `macos` platform. This function is not intended to be
-    /// called directly from `emulate_foreign_item_by_name`, so it does not check if isolation is
-    /// disabled or if the target platform is the correct one. Please use `macos_stat` or
-    /// `macos_lstat` instead.
-    fn macos_stat_or_lstat(
-        &mut self,
-        follow_symlink: bool,
-        path_op: OpTy<'tcx, Tag>,
-        buf_op: OpTy<'tcx, Tag>,
-    ) -> InterpResult<'tcx, i32> {
-        let this = self.eval_context_mut();
-
-        let path_scalar = this.read_scalar(path_op)?.not_undef()?;
-        let path: PathBuf = this.read_os_str_from_c_str(path_scalar)?.into();
-
-        let metadata = match FileMetadata::from_path(this, path, follow_symlink)? {
-            Some(metadata) => metadata,
-            None => return Ok(-1),
-        };
-        macos_stat_write_buf(this, metadata, buf_op)
+        this.macos_stat_write_buf(metadata, buf_op)
     }
 
     fn linux_statx(
@@ -620,17 +697,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         Ok(0)
     }
 
-    /// Function used when a handle is not found inside `FileHandler`. It returns `Ok(-1)`and sets
-    /// the last OS error to `libc::EBADF` (invalid file descriptor). This function uses
-    /// `T: From<i32>` instead of `i32` directly because some fs functions return different integer
-    /// types (like `read`, that returns an `i64`).
-    fn handle_not_found<T: From<i32>>(&mut self) -> InterpResult<'tcx, T> {
-        let this = self.eval_context_mut();
-        let ebadf = this.eval_libc("EBADF")?;
-        this.set_last_error(ebadf)?;
-        Ok((-1).into())
-    }
-
     fn rename(
         &mut self,
         oldpath_op: OpTy<'tcx, Tag>,
@@ -743,64 +809,3 @@ impl FileMetadata {
         Ok(Some(FileMetadata { mode, size, created, accessed, modified }))
     }
 }
-
-fn macos_stat_write_buf<'tcx, 'mir>(
-    ecx: &mut MiriEvalContext<'mir, 'tcx>,
-    metadata: FileMetadata,
-    buf_op: OpTy<'tcx, Tag>,
-) -> InterpResult<'tcx, i32> {
-    let mode: u16 = metadata.mode.to_u16()?;
-
-    let (access_sec, access_nsec) = metadata.accessed.unwrap_or((0, 0));
-    let (created_sec, created_nsec) = metadata.created.unwrap_or((0, 0));
-    let (modified_sec, modified_nsec) = metadata.modified.unwrap_or((0, 0));
-
-    let dev_t_layout = ecx.libc_ty_layout("dev_t")?;
-    let mode_t_layout = ecx.libc_ty_layout("mode_t")?;
-    let nlink_t_layout = ecx.libc_ty_layout("nlink_t")?;
-    let ino_t_layout = ecx.libc_ty_layout("ino_t")?;
-    let uid_t_layout = ecx.libc_ty_layout("uid_t")?;
-    let gid_t_layout = ecx.libc_ty_layout("gid_t")?;
-    let time_t_layout = ecx.libc_ty_layout("time_t")?;
-    let long_layout = ecx.libc_ty_layout("c_long")?;
-    let off_t_layout = ecx.libc_ty_layout("off_t")?;
-    let blkcnt_t_layout = ecx.libc_ty_layout("blkcnt_t")?;
-    let blksize_t_layout = ecx.libc_ty_layout("blksize_t")?;
-    let uint32_t_layout = ecx.libc_ty_layout("uint32_t")?;
-
-    // We need to add 32 bits of padding after `st_rdev` if we are on a 64-bit platform.
-    let pad_layout = if ecx.tcx.sess.target.ptr_width == 64 {
-        uint32_t_layout
-    } else {
-        ecx.layout_of(ecx.tcx.mk_unit())?
-    };
-
-    let imms = [
-        immty_from_uint_checked(0u128, dev_t_layout)?, // st_dev
-        immty_from_uint_checked(mode, mode_t_layout)?, // st_mode
-        immty_from_uint_checked(0u128, nlink_t_layout)?, // st_nlink
-        immty_from_uint_checked(0u128, ino_t_layout)?, // st_ino
-        immty_from_uint_checked(0u128, uid_t_layout)?, // st_uid
-        immty_from_uint_checked(0u128, gid_t_layout)?, // st_gid
-        immty_from_uint_checked(0u128, dev_t_layout)?, // st_rdev
-        immty_from_uint_checked(0u128, pad_layout)?, // padding for 64-bit targets
-        immty_from_uint_checked(access_sec, time_t_layout)?, // st_atime
-        immty_from_uint_checked(access_nsec, long_layout)?, // st_atime_nsec
-        immty_from_uint_checked(modified_sec, time_t_layout)?, // st_mtime
-        immty_from_uint_checked(modified_nsec, long_layout)?, // st_mtime_nsec
-        immty_from_uint_checked(0u128, time_t_layout)?, // st_ctime
-        immty_from_uint_checked(0u128, long_layout)?, // st_ctime_nsec
-        immty_from_uint_checked(created_sec, time_t_layout)?, // st_birthtime
-        immty_from_uint_checked(created_nsec, long_layout)?, // st_birthtime_nsec
-        immty_from_uint_checked(metadata.size, off_t_layout)?, // st_size
-        immty_from_uint_checked(0u128, blkcnt_t_layout)?, // st_blocks
-        immty_from_uint_checked(0u128, blksize_t_layout)?, // st_blksize
-        immty_from_uint_checked(0u128, uint32_t_layout)?, // st_flags
-        immty_from_uint_checked(0u128, uint32_t_layout)?, // st_gen
-    ];
-
-    let buf = ecx.deref_operand(buf_op)?;
-    ecx.write_packed_immediates(buf, &imms)?;
-
-    Ok(0)
-}

Original file line number	Diff line number	Diff line change
`@@ -455,7 +455,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx`
`455`	`455`	`this.write_scalar(Scalar::from_f64(res), dest)?;`
`456`	`456`	`}`
`457`	`457`
`458`		`- _ => match this.tcx.sess.target.target.target_os.to_lowercase().as_str() {`
	`458`	`+ _ => match this.tcx.sess.target.target.target_os.as_str() {`
`459`	`459`	`"linux" \| "macos" => return posix::EvalContextExt::emulate_foreign_item_by_name(this, link_name, args, dest, ret),`
`460`	`460`	`"windows" => return windows::EvalContextExt::emulate_foreign_item_by_name(this, link_name, args, dest, ret),`
`461`	`461`	`target => throw_unsup_format!("The {} target platform is not supported", target),`
Original file line number	Diff line number	Diff line change
`@@ -322,7 +322,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx`
`322`	`322`	`}`
`323`	`323`
`324`	`324`	`_ => {`
`325`		`- match this.tcx.sess.target.target.target_os.to_lowercase().as_str() {`
	`325`	`+ match this.tcx.sess.target.target.target_os.as_str() {`
`326`	`326`	`"linux" => return linux::EvalContextExt::emulate_foreign_item_by_name(this, link_name, args, dest, ret),`
`327`	`327`	`"macos" => return macos::EvalContextExt::emulate_foreign_item_by_name(this, link_name, args, dest, ret),`
`328`	`328`	`_ => unreachable!(),`
Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx`
`18`	`18`	`this.write_scalar(errno_place.to_ref().to_scalar()?, dest)?;`
`19`	`19`	`}`
`20`	`20`
`21`		`- // File related shims`
	`21`	`+ // File related shims (but also see "syscall" below for statx)`
`22`	`22`
`23`	`23`	// The only reason this is not in the `posix` module is because the `macos` item has a
`24`	`24`	`// different name.`
`@@ -59,8 +59,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx`
`59`	`59`	`// so skip over it.`
`60`	`60`	`getrandom(this, &args[1..], dest)?;`
`61`	`61`	`}`
`62`		- // `statx` is used by `libstd` to retrieve metadata information in `linux`
`63`		- // instead of using `stat`,`lstat` or `fstat` as in the `macos` platform.
	`62`	+ // `statx` is used by `libstd` to retrieve metadata information on `linux`
	`63`	+ // instead of using `stat`,`lstat` or `fstat` as on `macos`.
`64`	`64`	`id if id == sys_statx => {`
`65`	`65`	`// The first argument is the syscall id,`
`66`	`66`	`// so skip over it.`
`@@ -87,7 +87,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx`
`87`	`87`	`}`
`88`	`88`	`}`
`89`	`89`
`90`		`-// Shims the linux 'getrandom()' syscall.`
	`90`	+// Shims the linux `getrandom` syscall.
`91`	`91`	`fn getrandom<'tcx>(`
`92`	`92`	`this: &mut MiriEvalContext<'_, 'tcx>,`
`93`	`93`	`args: &[OpTy<'tcx, Tag>],`