From f492da1e30175a221f6ff39c6edfa25b818e1053 Mon Sep 17 00:00:00 2001 From: Jason White Date: Mon, 10 Jul 2023 16:46:43 -0700 Subject: [PATCH] ci: Fix clippy (#20) Summary: The clippy action has been in a broken state since 1438ff0f2e0b432a2e4da2b320fff8f6f58e6bbe. This fixes the GitHub action and all of the clippy warnings. Pull Request resolved: https://github.com/facebookexperimental/reverie/pull/20 Reviewed By: VladimirMakaev Differential Revision: D47322336 Pulled By: jasonwhite fbshipit-source-id: 88f8d16cc81269448c2425d8b56bcc3623e04b31 --- .github/workflows/ci.yml | 2 +- reverie-examples/chaos.rs | 6 ++--- reverie-examples/counter2.rs | 1 - reverie-memory/src/lib.rs | 10 ++----- .../src/gdbstub/commands/base/_vFile.rs | 2 +- reverie-ptrace/src/gdbstub/server.rs | 2 +- reverie-ptrace/src/gdbstub/session.rs | 26 +++++++++---------- reverie-ptrace/src/stack.rs | 4 +-- reverie-ptrace/src/task.rs | 23 ++++++++-------- reverie-ptrace/src/timer.rs | 18 ++++++++++--- reverie-syscalls/src/args/mod.rs | 4 +-- reverie-syscalls/src/raw.rs | 10 +++---- reverie-syscalls/src/syscalls/mod.rs | 2 +- reverie/src/backtrace/mod.rs | 4 +-- reverie/src/regs.rs | 6 ++--- safeptrace/src/lib.rs | 10 +++---- safeptrace/src/waitid.rs | 4 ++- 17 files changed, 69 insertions(+), 65 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7bf4d8a..f637d64 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -66,4 +66,4 @@ jobs: with: components: clippy - name: Run cargo clippy - run: cargo clippy all-features -- -Dclippy::all + run: cargo clippy --all-features -- -Dclippy::all diff --git a/reverie-examples/chaos.rs b/reverie-examples/chaos.rs index 6b08ed6..1ce3425 100644 --- a/reverie-examples/chaos.rs +++ b/reverie-examples/chaos.rs @@ -123,17 +123,17 @@ impl Tool for ChaosTool { *guest.thread_state_mut() = true; // XXX: inject a signal like SIGINT? - let ret = Err(Errno::ERESTARTSYS); + let err = Errno::ERESTARTSYS; eprintln!( "[pid={}, n={}] {} = {}", guest.pid(), count, syscall.display(&memory), - ret.unwrap_or_else(|errno| -errno.into_raw() as i64) + -err.into_raw() as i64 ); - return Ok(ret?); + return Ok(Err(err)?); } else if !config.no_read { // Reduce read length to 1 byte at most. Syscall::Read(read.with_len(1.min(read.len()))) diff --git a/reverie-examples/counter2.rs b/reverie-examples/counter2.rs index 68e8e6b..1fe9afb 100644 --- a/reverie-examples/counter2.rs +++ b/reverie-examples/counter2.rs @@ -139,7 +139,6 @@ impl Tool for CounterLocal { ) -> Result<(), Error> { let count = self.proc_syscalls.load(Ordering::SeqCst); let threads = self.exited_threads.load(Ordering::SeqCst); - drop(self); debug!( "At ExitProc (pid {}), contributing {} to global count.", pid, count diff --git a/reverie-memory/src/lib.rs b/reverie-memory/src/lib.rs index 564b77f..64c67c0 100644 --- a/reverie-memory/src/lib.rs +++ b/reverie-memory/src/lib.rs @@ -151,10 +151,7 @@ pub trait MemoryAccess { T: Sized, { let buf = unsafe { - ::core::slice::from_raw_parts_mut( - buf.as_mut_ptr() as *mut u8, - buf.len() * mem::size_of::(), - ) + ::core::slice::from_raw_parts_mut(buf.as_mut_ptr() as *mut u8, mem::size_of_val(buf)) }; self.read_exact(addr.cast::(), buf) @@ -167,10 +164,7 @@ pub trait MemoryAccess { T: Sized, { let buf = unsafe { - ::core::slice::from_raw_parts( - buf.as_ptr() as *const u8, - buf.len() * mem::size_of::(), - ) + ::core::slice::from_raw_parts(buf.as_ptr() as *const u8, mem::size_of_val(buf)) }; self.write_exact(addr.cast::(), buf) diff --git a/reverie-ptrace/src/gdbstub/commands/base/_vFile.rs b/reverie-ptrace/src/gdbstub/commands/base/_vFile.rs index 5802b36..dc509a1 100644 --- a/reverie-ptrace/src/gdbstub/commands/base/_vFile.rs +++ b/reverie-ptrace/src/gdbstub/commands/base/_vFile.rs @@ -44,7 +44,7 @@ impl From for HostioStat { st_dev: stat.st_dev as u32, st_ino: stat.st_ino as u32, st_nlink: stat.st_nlink as u32, - st_mode: stat.st_mode as u32, + st_mode: stat.st_mode, st_uid: stat.st_uid, st_gid: stat.st_gid, st_rdev: stat.st_rdev as u32, diff --git a/reverie-ptrace/src/gdbstub/server.rs b/reverie-ptrace/src/gdbstub/server.rs index d6b1a65..02a8e2b 100644 --- a/reverie-ptrace/src/gdbstub/server.rs +++ b/reverie-ptrace/src/gdbstub/server.rs @@ -254,7 +254,7 @@ impl GdbServerImpl { // could get timeout. Some requests such as `g' needs IPC with a // gdb session, which only becomes ready later. if let Some(server_rx) = self.server_rx.take() { - let _ = server_rx.await.map_err(|_| Error::GdbServerNotStarted)?; + server_rx.await.map_err(|_| Error::GdbServerNotStarted)?; let mut session = self.session.take().ok_or(Error::SessionNotStarted)?; let run_session = session.run(); let run_loop = self.relay_gdb_packets(); diff --git a/reverie-ptrace/src/gdbstub/session.rs b/reverie-ptrace/src/gdbstub/session.rs index 3366f5f..3a7df3b 100644 --- a/reverie-ptrace/src/gdbstub/session.rs +++ b/reverie-ptrace/src/gdbstub/session.rs @@ -733,12 +733,12 @@ impl Session { }, reply_tx, ); - let _ = request_tx + request_tx .send(request) .await .map_err(|_| Error::GdbRequestSendError)?; - let reply = reply_rx.await.map_err(|_| Error::GdbRequestRecvError)??; - Ok(reply) + reply_rx.await.map_err(|_| Error::GdbRequestRecvError)??; + Ok(()) }) .await } @@ -762,9 +762,9 @@ impl Session { .send(request) .await .map_err(|_| Error::GdbRequestSendError)?; - let reply = reply_rx.await.map_err(|_| Error::GdbRequestRecvError)??; + reply_rx.await.map_err(|_| Error::GdbRequestRecvError)??; - Ok(reply) + Ok(()) }) .await } @@ -777,7 +777,7 @@ impl Session { .ok_or(Error::SessionNotStarted)?; let (reply_tx, reply_rx) = oneshot::channel(); let request = GdbRequest::ReadInferiorMemory(addr, size, reply_tx); - let _ = request_tx + request_tx .send(request) .await .map_err(|_| Error::GdbRequestSendError)?; @@ -801,12 +801,12 @@ impl Session { .ok_or(Error::SessionNotStarted)?; let (reply_tx, reply_rx) = oneshot::channel(); let request = GdbRequest::WriteInferiorMemory(addr, size, data, reply_tx); - let _ = request_tx + request_tx .send(request) .await .map_err(|_| Error::GdbRequestSendError)?; - let reply = reply_rx.await.map_err(|_| Error::GdbRequestRecvError)??; - Ok(reply) + reply_rx.await.map_err(|_| Error::GdbRequestRecvError)??; + Ok(()) }) .await } @@ -819,7 +819,7 @@ impl Session { .ok_or(Error::SessionNotStarted)?; let (reply_tx, reply_rx) = oneshot::channel(); let request = GdbRequest::ReadRegisters(reply_tx); - let _ = request_tx + request_tx .send(request) .await .map_err(|_| Error::GdbRequestSendError)?; @@ -840,12 +840,12 @@ impl Session { let core_regs: CoreRegs = bincode::deserialize(regs).map_err(|_| CommandParseError::MalformedRegisters)?; let request = GdbRequest::WriteRegisters(core_regs, reply_tx); - let _ = request_tx + request_tx .send(request) .await .map_err(|_| Error::GdbRequestSendError)?; - let reply = reply_rx.await.map_err(|_| Error::GdbRequestRecvError)??; - Ok(reply) + reply_rx.await.map_err(|_| Error::GdbRequestRecvError)??; + Ok(()) }) .await } diff --git a/reverie-ptrace/src/stack.rs b/reverie-ptrace/src/stack.rs index 0383c31..f34258e 100644 --- a/reverie-ptrace/src/stack.rs +++ b/reverie-ptrace/src/stack.rs @@ -52,7 +52,7 @@ impl GuestStack { } let task = Stopped::new_unchecked(pid); let rsp = task.getregs()?.stack_ptr() as usize; - let top = rsp - REDZONE_SIZE as usize; + let top = rsp - REDZONE_SIZE; Ok(GuestStack { top, sp: top, @@ -106,7 +106,7 @@ impl Stack for GuestStack { type StackGuard = StackGuard; fn size(&self) -> usize { - (self.top - self.sp) as usize + self.top - self.sp } fn capacity(&self) -> usize { self.capacity diff --git a/reverie-ptrace/src/task.rs b/reverie-ptrace/src/task.rs index acef72a..f081282 100644 --- a/reverie-ptrace/src/task.rs +++ b/reverie-ptrace/src/task.rs @@ -656,7 +656,7 @@ impl TracedTask { // NOTE: This point in the code assumes that a specific instruction // sequence "SYSCALL; INT3", has been patched into the guest, and // that RIP points to the syscall. - let mut regs = saved_regs.clone(); + let mut regs = *saved_regs; let page_addr = cp::PRIVATE_PAGE_OFFSET; @@ -710,7 +710,7 @@ impl TracedTask { page_addr ); - cp::populate_mmap_page(task.pid().into(), page_addr).map_err(|err| err)?; + cp::populate_mmap_page(task.pid().into(), page_addr)?; // Restore our saved registers, including our instruction pointer. task.setregs(saved_regs)?; @@ -1070,7 +1070,7 @@ impl TracedTask { // loaded (due to execve). Otherwise gdb may try to manipulate // old process' address space. if let Some(attach_tx) = self.gdb_stop_tx.as_ref() { - let _ = attach_tx.send(stopped).await.unwrap(); + attach_tx.send(stopped).await.unwrap(); } let running = self .await_gdb_resume(task, ExpectedGdbResume::Resume) @@ -1252,9 +1252,8 @@ impl TracedTask { // NB: We could potentially hit a breakpoint after above resume, // make sure we don't miss the breakpoint and await for gdb // resume (once again). This is possible because result of - // handle_new_task in from_task_state is ignored, while it could - // be a valid state like SIGTRAP, which could be a breakpoint is - // hit. + // handle_new_task in status_to_result is ignored, while it could be + // a valid state like SIGTRAP, which could be a breakpoint is hit. running .next_state() .and_then(|wait| self.check_swbreak(wait)) @@ -1265,7 +1264,7 @@ impl TracedTask { } async fn handle_vfork_done_event(&mut self, stopped: Stopped) -> Result { - Ok(stopped.resume(None)?.next_state().await?) + stopped.resume(None)?.next_state().await } async fn handle_exit_event(task: Stopped) -> Result { @@ -1601,7 +1600,7 @@ impl TracedTask { let wait = task.step(None)?.next_state().await?; // Get the result of the syscall to return to the caller. - self.from_task_state(wait, Some(oldregs)).await + self.status_to_result(wait, Some(oldregs)).await } // Helper function @@ -1616,7 +1615,7 @@ impl TracedTask { self.untraced_syscall(task, nr, args).await } - async fn from_task_state( + async fn status_to_result( &mut self, wait_status: Wait, context: Option, @@ -1696,7 +1695,7 @@ impl TracedTask { // If we're reinjecting the same syscall with the same arguments, // then we can just let the tracee continue and stop at sysexit. let wait = task.syscall(None)?.next_state().await?; - self.from_task_state(wait, None).await + self.status_to_result(wait, None).await } else { self.private_inject(task, nr, args).await } @@ -1760,7 +1759,7 @@ impl TracedTask { request_tx: request_tx.unwrap(), resume_tx: resume_tx.unwrap(), }; - let _ = stop_tx.send(stop).await.unwrap(); + stop_tx.send(stop).await.unwrap(); } Ok(()) } @@ -1884,7 +1883,7 @@ impl TracedTask { // a different task, need to notify this task is fully stopped. self.suspended.store(true, Ordering::SeqCst); if let Some((suspended_flag, stop_tx)) = self.get_stop_tx().await { - let _ = stop_tx + stop_tx .send(( self.tid(), Suspended { diff --git a/reverie-ptrace/src/timer.rs b/reverie-ptrace/src/timer.rs index 35649e0..93719d9 100644 --- a/reverie-ptrace/src/timer.rs +++ b/reverie-ptrace/src/timer.rs @@ -356,10 +356,20 @@ impl ActiveEvent { ActiveEvent::Precise { clock_target, offset: _, - } => (clock_target.saturating_sub(curr_clock) > MAX_SINGLE_STEP_COUNT) - .then(|| TimerEventRequest::Precise(*clock_target - curr_clock)), - ActiveEvent::Imprecise { clock_min } => (*clock_min > curr_clock) - .then(|| TimerEventRequest::Imprecise(*clock_min - curr_clock)), + } => { + if clock_target.saturating_sub(curr_clock) > MAX_SINGLE_STEP_COUNT { + Some(TimerEventRequest::Precise(*clock_target - curr_clock)) + } else { + None + } + } + ActiveEvent::Imprecise { clock_min } => { + if *clock_min > curr_clock { + Some(TimerEventRequest::Imprecise(*clock_min - curr_clock)) + } else { + None + } + } } } } diff --git a/reverie-syscalls/src/args/mod.rs b/reverie-syscalls/src/args/mod.rs index 28a398f..d6a69c4 100644 --- a/reverie-syscalls/src/args/mod.rs +++ b/reverie-syscalls/src/args/mod.rs @@ -287,7 +287,7 @@ impl<'a> StatPtr<'a> { /// Creates the `StatPtr` from a raw pointer. Returns `None` if the given /// pointer is NULL. pub fn from_ptr(r: *const libc::stat) -> Option { - AddrMut::from_ptr(r as *const libc::stat).map(StatPtr) + AddrMut::from_ptr(r).map(StatPtr) } } @@ -351,7 +351,7 @@ impl<'a> StatxPtr<'a> { /// Creates the `StatxPtr` from a raw pointer. Returns `None` if the given /// pointer is NULL. pub fn from_ptr(r: *const libc::statx) -> Option { - AddrMut::from_ptr(r as *const libc::statx).map(StatxPtr) + AddrMut::from_ptr(r).map(StatxPtr) } } diff --git a/reverie-syscalls/src/raw.rs b/reverie-syscalls/src/raw.rs index 408c1e7..a445798 100644 --- a/reverie-syscalls/src/raw.rs +++ b/reverie-syscalls/src/raw.rs @@ -94,21 +94,21 @@ impl FromToRaw for i64 { impl<'a, T> FromToRaw for Option> { fn from_raw(raw: usize) -> Self { - Addr::from_raw(raw as usize) + Addr::from_raw(raw) } fn into_raw(self) -> usize { - self.map_or(0, |addr| addr.as_raw() as usize) + self.map_or(0, |addr| addr.as_raw()) } } impl<'a, T> FromToRaw for Option> { fn from_raw(raw: usize) -> Self { - AddrMut::from_raw(raw as usize) + AddrMut::from_raw(raw) } fn into_raw(self) -> usize { - self.map_or(0, |addr| addr.as_raw() as usize) + self.map_or(0, |addr| addr.as_raw()) } } @@ -176,7 +176,7 @@ where T: FromToRaw, { fn from_raw(raw: usize) -> Self { - Errno::from_ret(raw).map(|x| T::from_raw(x as usize)) + Errno::from_ret(raw).map(|x| T::from_raw(x)) } fn into_raw(self) -> usize { diff --git a/reverie-syscalls/src/syscalls/mod.rs b/reverie-syscalls/src/syscalls/mod.rs index 767dbaf..4bc5b73 100644 --- a/reverie-syscalls/src/syscalls/mod.rs +++ b/reverie-syscalls/src/syscalls/mod.rs @@ -1005,7 +1005,7 @@ typed_syscall! { } } -#[cfg(any(target_arch = "x86_64"))] +#[cfg(target_arch = "x86_64")] typed_syscall! { pub struct Clone { flags: CloneFlags, diff --git a/reverie/src/backtrace/mod.rs b/reverie/src/backtrace/mod.rs index 1ce3d4f..1b50d7a 100644 --- a/reverie/src/backtrace/mod.rs +++ b/reverie/src/backtrace/mod.rs @@ -278,7 +278,7 @@ impl From for Vec { impl fmt::Display for Backtrace { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let thread_name = self.thread_name(); - let thread_name = thread_name.as_ref().map(String::as_str); + let thread_name = thread_name.as_deref(); let thread_name = thread_name.unwrap_or(""); writeln!( f, @@ -298,7 +298,7 @@ impl fmt::Display for Backtrace { impl fmt::Display for PrettyBacktrace { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let thread_name = self.thread_name(); - let thread_name = thread_name.as_ref().map(String::as_str); + let thread_name = thread_name.as_deref(); let thread_name = thread_name.unwrap_or(""); writeln!( f, diff --git a/reverie/src/regs.rs b/reverie/src/regs.rs index 2e7350f..181b9f3 100644 --- a/reverie/src/regs.rs +++ b/reverie/src/regs.rs @@ -11,17 +11,17 @@ /// Trait providing reusable display formatting for registers pub trait RegDisplay { /// Returns a display object associated with a trait implementor - fn display<'a>(&'a self) -> Display<'a> { + fn display(&self) -> Display<'_> { self.display_with_options(Default::default()) } /// Return a display object associated with a trait implementor. /// Additionally specifis ['RegDisplayOptions'] structure to adjust formatting - fn display_with_options<'a>(&'a self, options: RegDisplayOptions) -> Display<'a>; + fn display_with_options(&self, options: RegDisplayOptions) -> Display<'_>; } impl RegDisplay for libc::user_regs_struct { - fn display_with_options<'a>(&'a self, options: RegDisplayOptions) -> Display<'a> { + fn display_with_options(&self, options: RegDisplayOptions) -> Display<'_> { Display { options, regs: self, diff --git a/safeptrace/src/lib.rs b/safeptrace/src/lib.rs index c6c6902..d304085 100644 --- a/safeptrace/src/lib.rs +++ b/safeptrace/src/lib.rs @@ -242,7 +242,7 @@ impl TryWait { pub fn assume_stopped(self) -> (Stopped, Event) { match self { Self::Wait(Wait::Stopped(stopped, event)) => (stopped, event), - status => Err(InvalidState(status)).unwrap(), + status => panic!("{:?}", InvalidState(status)), } } @@ -250,7 +250,7 @@ impl TryWait { pub fn assume_running(self) -> Running { match self { Self::Running(running) => running, - status => Err(InvalidState(status)).unwrap(), + status => panic!("{:?}", InvalidState(status)), } } @@ -258,7 +258,7 @@ impl TryWait { pub fn assume_exited(self) -> (Pid, ExitStatus) { match self { Self::Wait(Wait::Exited(pid, exit_status)) => (pid, exit_status), - status => Err(InvalidState(status)).unwrap(), + status => panic!("{:?}", InvalidState(status)), } } } @@ -314,7 +314,7 @@ impl Wait { pub fn assume_stopped(self) -> (Stopped, Event) { match self { Self::Stopped(stopped, event) => (stopped, event), - state => Err(InvalidState(state.into())).unwrap(), + state => panic!("{:?}", InvalidState(state.into())), } } @@ -322,7 +322,7 @@ impl Wait { pub fn assume_exited(self) -> (Pid, ExitStatus) { match self { Self::Exited(pid, exit_status) => (pid, exit_status), - state => Err(InvalidState(state.into())).unwrap(), + state => panic!("{:?}", InvalidState(state.into())), } } diff --git a/safeptrace/src/waitid.rs b/safeptrace/src/waitid.rs index 66112c1..e051d45 100644 --- a/safeptrace/src/waitid.rs +++ b/safeptrace/src/waitid.rs @@ -42,7 +42,7 @@ fn si_status_signal(info: &libc::siginfo_t) -> Signal { #[inline] fn si_status_event(info: &libc::siginfo_t) -> i32 { - (unsafe { info.si_status() } >> 8) as i32 + (unsafe { info.si_status() }) >> 8 } /// Returns the raw siginfo from a waitid call. @@ -70,6 +70,7 @@ fn waitid_si(waitid_type: IdType, flags: WaitPidFlag) -> Result Result, Errno> { let si = waitid_si(IdType::Pid(pid), flags)?; @@ -82,6 +83,7 @@ pub fn waitpid(pid: Pid, flags: WaitPidFlag) -> Result, Errno> { } // Converts a siginfo to a more compact status code. +#[cfg(feature = "notifier")] fn siginfo_to_status(si: libc::siginfo_t) -> i32 { let si_status = unsafe { si.si_status() };