Auto merge of #1435 - RalfJung:stdin-read-error, r=RalfJung

better error when using FDs 0-2 the wrong way

Author: bors

src/shims/foreign_items/posix.rs

@@ -62,20 +62,31 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             }
             "read" => {
                 let &[fd, buf, count] = check_arg_count(args)?;
-                let result = this.read(fd, buf, count)?;
+                let fd = this.read_scalar(fd)?.to_i32()?;
+                let buf = this.read_scalar(buf)?.not_undef()?;
+                let count = this.read_scalar(count)?.to_machine_usize(this)?;
+                let result = if fd == 0 {
+                    throw_unsup_format!("reading from stdin is not implemented")
+                } else if fd == 1 || fd == 2 {
+                    throw_unsup_format!("cannot read from stdout/stderr")
+                } else {
+                    this.read(fd, buf, count)?
+                };
                 this.write_scalar(Scalar::from_machine_isize(result, this), dest)?;
             }
             "write" => {
                 let &[fd, buf, n] = check_arg_count(args)?;
                 let fd = this.read_scalar(fd)?.to_i32()?;
                 let buf = this.read_scalar(buf)?.not_undef()?;
-                let n = this.read_scalar(n)?.to_machine_usize(this)?;
-                trace!("Called write({:?}, {:?}, {:?})", fd, buf, n);
-                let result = if fd == 1 || fd == 2 {
+                let count = this.read_scalar(n)?.to_machine_usize(this)?;
+                trace!("Called write({:?}, {:?}, {:?})", fd, buf, count);
+                let result = if fd == 0 {
+                    throw_unsup_format!("cannot write to stdin")
+                } else if fd == 1 || fd == 2 {
                     // stdout/stderr
                     use std::io::{self, Write};
 
-                    let buf_cont = this.memory.read_bytes(buf, Size::from_bytes(n))?;
+                    let buf_cont = this.memory.read_bytes(buf, Size::from_bytes(count))?;
                     // We need to flush to make sure this actually appears on the screen
                     let res = if fd == 1 {
                         // Stdout is buffered, flush to make sure it appears on the screen.
@@ -94,7 +105,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                         Err(_) => -1,
                     }
                 } else {
-                    let &[fd, buf, count] = check_arg_count(args)?;
                     this.write(fd, buf, count)?
                 };
                 // Now, `result` is the value we return back to the program.

src/shims/fs.rs

@@ -5,6 +5,8 @@ use std::io::{Read, Seek, SeekFrom, Write};
 use std::path::Path;
 use std::time::SystemTime;
 
+use log::trace;
+
 use rustc_data_structures::fx::FxHashMap;
 use rustc_target::abi::{Align, LayoutOf, Size};
 
@@ -413,17 +415,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
     fn read(
         &mut self,
-        fd_op: OpTy<'tcx, Tag>,
-        buf_op: OpTy<'tcx, Tag>,
-        count_op: OpTy<'tcx, Tag>,
+        fd: i32,
+        buf: Scalar<Tag>,
+        count: u64,
     ) -> InterpResult<'tcx, i64> {
         let this = self.eval_context_mut();
 
         this.check_no_isolation("read")?;
+        assert!(fd >= 3);
 
-        let fd = this.read_scalar(fd_op)?.to_i32()?;
-        let buf = this.read_scalar(buf_op)?.not_undef()?;
-        let count = this.read_scalar(count_op)?.to_machine_usize(&*this.tcx)?;
+        trace!("Reading from FD {}, size {}", fd, count);
 
         // Check that the *entire* buffer is actually valid memory.
         this.memory.check_ptr_access(
@@ -437,6 +438,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         let count = count.min(this.machine_isize_max() as u64).min(isize::MAX as u64);
 
         if let Some(FileHandle { file, writable: _ }) = this.machine.file_handler.handles.get_mut(&fd) {
+            trace!("read: FD mapped to {:?}", file);
             // This can never fail because `count` was capped to be smaller than
             // `isize::MAX`.
             let count = isize::try_from(count).unwrap();
@@ -461,23 +463,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 }
             }
         } else {
+            trace!("read: FD not found");
             this.handle_not_found()
         }
     }
 
     fn write(
         &mut self,
-        fd_op: OpTy<'tcx, Tag>,
-        buf_op: OpTy<'tcx, Tag>,
-        count_op: OpTy<'tcx, Tag>,
+        fd: i32,
+        buf: Scalar<Tag>,
+        count: u64,
     ) -> InterpResult<'tcx, i64> {
         let this = self.eval_context_mut();
 
         this.check_no_isolation("write")?;
-
-        let fd = this.read_scalar(fd_op)?.to_i32()?;
-        let buf = this.read_scalar(buf_op)?.not_undef()?;
-        let count = this.read_scalar(count_op)?.to_machine_usize(&*this.tcx)?;
+        assert!(fd >= 3);
 
         // Check that the *entire* buffer is actually valid memory.
         this.memory.check_ptr_access(