Skip to content

Commit 4e3d1fe

Browse files
committed
Address comments.
1 parent f46d197 commit 4e3d1fe

File tree

7 files changed

+22
-26
lines changed

7 files changed

+22
-26
lines changed

src/helpers.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,7 @@ pub fn check_arg_count<'a, 'tcx, const N: usize>(args: &'a [OpTy<'tcx, Tag>]) ->
477477
if let Ok(ops) = args.try_into() {
478478
return Ok(ops);
479479
}
480-
throw_ub_format!("incorrect number of arguments, got {}, needed {}", args.len(), N)
480+
throw_ub_format!("incorrect number of arguments: got {}, expected {}", args.len(), N)
481481
}
482482

483483
pub fn immty_from_int_checked<'tcx>(

src/shims/foreign_items.rs

+1
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
454454

455455
// Architecture-specific shims
456456
"llvm.x86.sse2.pause" if this.tcx.sess.target.target.arch == "x86" || this.tcx.sess.target.target.arch == "x86_64" => {
457+
let &[] = check_arg_count(args)?;
457458
this.sched_yield()?;
458459
}
459460

src/shims/foreign_items/posix/linux.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
102102
.to_machine_usize(this)?;
103103

104104
if args.is_empty() {
105-
throw_ub_format!("incorrect number of arguments for syscall, needed at least 1");
105+
throw_ub_format!("incorrect number of arguments for syscall: got 0, expected at least 1");
106106
}
107107
match this.read_scalar(args[0])?.to_machine_usize(this)? {
108108
// `libc::syscall(NR_GETRANDOM, buf.as_mut_ptr(), buf.len(), GRND_NONBLOCK)`
@@ -143,6 +143,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
143143
// Incomplete shims that we "stub out" just to get pre-main initialization code to work.
144144
// These shims are enabled only when the caller is in the standard library.
145145
"pthread_getattr_np" if this.frame().instance.to_string().starts_with("std::sys::unix::") => {
146+
let &[_thread, _attr] = check_arg_count(args)?;
146147
this.write_null(dest)?;
147148
}
148149

src/shims/foreign_items/posix/macos.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
118118

119119
// Threading
120120
"pthread_setname_np" => {
121-
let ptr = this.read_scalar(args[0])?.not_undef()?;
122-
this.pthread_setname_np(ptr)?;
121+
let &[name] = check_arg_count(args)?;
122+
let name = this.read_scalar(name)?.not_undef()?;
123+
this.pthread_setname_np(name)?;
123124
}
124125

125126
// Incomplete shims that we "stub out" just to get pre-main initialization code to work.

src/shims/foreign_items/windows.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -106,17 +106,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
106106
this.write_scalar(res, dest)?;
107107
}
108108
"HeapFree" => {
109-
let &[handle, _flags, ptr] = check_arg_count(args)?;
109+
let &[handle, flags, ptr] = check_arg_count(args)?;
110110
this.read_scalar(handle)?.to_machine_isize(this)?;
111-
let _flags = this.read_scalar(_flags)?.to_u32()?;
111+
this.read_scalar(flags)?.to_u32()?;
112112
let ptr = this.read_scalar(ptr)?.not_undef()?;
113113
this.free(ptr, MiriMemoryKind::WinHeap)?;
114114
this.write_scalar(Scalar::from_i32(1), dest)?;
115115
}
116116
"HeapReAlloc" => {
117-
let &[handle, _flags, ptr, size] = check_arg_count(args)?;
117+
let &[handle, flags, ptr, size] = check_arg_count(args)?;
118118
this.read_scalar(handle)?.to_machine_isize(this)?;
119-
let _flags = this.read_scalar(_flags)?.to_u32()?;
119+
this.read_scalar(flags)?.to_u32()?;
120120
let ptr = this.read_scalar(ptr)?.not_undef()?;
121121
let size = this.read_scalar(size)?.to_machine_usize(this)?;
122122
let res = this.realloc(ptr, size, MiriMemoryKind::WinHeap)?;

src/shims/fs.rs

+9-16
Original file line numberDiff line numberDiff line change
@@ -328,22 +328,18 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
328328

329329
this.check_no_isolation("fcntl")?;
330330

331-
let (fd, cmd, start) = if args.len() == 2 {
332-
let &[fd, cmd] = check_arg_count(args)?;
333-
(fd, cmd, None)
334-
} else {
335-
// If args.len() isn't 2 or 3 this will error appropriately.
336-
let &[fd, cmd, start] = check_arg_count(args)?;
337-
(fd, cmd, Some(start))
338-
};
339-
let fd = this.read_scalar(fd)?.to_i32()?;
340-
let cmd = this.read_scalar(cmd)?.to_i32()?;
331+
if args.len() < 2 {
332+
throw_ub_format!("incorrect number of arguments for fcntl: got {}, expected at least 2", args.len());
333+
}
334+
let cmd = this.read_scalar(args[1])?.to_i32()?;
341335
// We only support getting the flags for a descriptor.
342336
if cmd == this.eval_libc_i32("F_GETFD")? {
343337
// Currently this is the only flag that `F_GETFD` returns. It is OK to just return the
344338
// `FD_CLOEXEC` value without checking if the flag is set for the file because `std`
345339
// always sets this flag when opening a file. However we still need to check that the
346340
// file itself is open.
341+
let &[fd, _] = check_arg_count(args)?;
342+
let fd = this.read_scalar(fd)?.to_i32()?;
347343
if this.machine.file_handler.handles.contains_key(&fd) {
348344
Ok(this.eval_libc_i32("FD_CLOEXEC")?)
349345
} else {
@@ -356,15 +352,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
356352
// because exec() isn't supported. The F_DUPFD and F_DUPFD_CLOEXEC commands only
357353
// differ in whether the FD_CLOEXEC flag is pre-set on the new file descriptor,
358354
// thus they can share the same implementation here.
355+
let &[fd, _, start] = check_arg_count(args)?;
356+
let fd = this.read_scalar(fd)?.to_i32()?;
357+
let start = this.read_scalar(start)?.to_i32()?;
359358
if fd < MIN_NORMAL_FILE_FD {
360359
throw_unsup_format!("duplicating file descriptors for stdin, stdout, or stderr is not supported")
361360
}
362-
let start = start.ok_or_else(|| {
363-
err_unsup_format!(
364-
"fcntl with command F_DUPFD or F_DUPFD_CLOEXEC requires a third argument"
365-
)
366-
})?;
367-
let start = this.read_scalar(start)?.to_i32()?;
368361
let fh = &mut this.machine.file_handler;
369362
let (file_result, writable) = match fh.handles.get(&fd) {
370363
Some(FileHandle { file, writable }) => (file.try_clone(), *writable),

src/shims/thread.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -121,12 +121,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
121121

122122
fn pthread_setname_np(
123123
&mut self,
124-
ptr: Scalar<Tag>,
124+
name: Scalar<Tag>,
125125
) -> InterpResult<'tcx> {
126126
let this = self.eval_context_mut();
127127
this.assert_target_os("macos", "pthread_setname_np");
128128

129-
let name = this.memory.read_c_str(ptr)?.to_owned();
129+
let name = this.memory.read_c_str(name)?.to_owned();
130130
this.set_active_thread_name(name)?;
131131

132132
Ok(())

0 commit comments

Comments
 (0)