Merge #635

635: More Clippy fixes r=vext01 a=ltratt



Co-authored-by: Laurence Tratt <laurie@tratt.net>

hwtracer/src/decode/ykpt/mod.rs

@@ -703,7 +703,7 @@ impl<'t> YkPTBlockIterator<'t> {
 
     /// Fetch the next packet and update iterator state.
     fn packet(&mut self) -> Result<Packet, HWTracerError> {
-        let ret = if let Some(pkt_or_err) = self.parser.next() {
+        if let Some(pkt_or_err) = self.parser.next() {
             let mut pkt = pkt_or_err?;
 
             if pkt.kind() == PacketKind::OVF {
@@ -778,9 +778,7 @@ impl<'t> YkPTBlockIterator<'t> {
             Ok(pkt)
         } else {
             Err(HWTracerError::NoMorePackets)
-        };
-
-        ret
+        }
     }
 }
 

hwtracer/src/lib.rs

@@ -1,5 +1,6 @@
-#![allow(clippy::upper_case_acronyms)]
+#![allow(clippy::len_without_is_empty)]
 #![allow(clippy::new_without_default)]
+#![allow(clippy::upper_case_acronyms)]
 #![feature(once_cell)]
 #![feature(ptr_sub_ptr)]
 

ykcapi/src/lib.rs

@@ -13,7 +13,6 @@
 #[cfg(feature = "yk_testing")]
 mod testing;
 
-use libc;
 use std::arch::asm;
 use std::convert::{TryFrom, TryInto};
 use std::ffi::{c_char, c_void, CString};
@@ -25,6 +24,7 @@ use ykrt::{HotThreshold, Location, MT};
 use yksmp::{Location as SMLocation, StackMapParser};
 
 #[no_mangle]
+#[allow(clippy::not_unsafe_ptr_arg_deref)]
 pub extern "C" fn yk_mt_new(err_msg: *mut *const c_char) -> *mut MT {
     match MT::new() {
         Ok(mt) => Box::into_raw(Box::new(mt)),
@@ -45,6 +45,7 @@ pub extern "C" fn yk_mt_new(err_msg: *mut *const c_char) -> *mut MT {
 }
 
 #[no_mangle]
+#[allow(clippy::not_unsafe_ptr_arg_deref)]
 pub extern "C" fn yk_mt_drop(mt: *mut MT) {
     unsafe { Box::from_raw(mt) };
 }
@@ -58,6 +59,7 @@ pub extern "C" fn yk_mt_control_point(_mt: *mut MT, _loc: *mut Location) {
 // The "real" control point, that is called once the interpreter has been patched by ykllvm.
 // Returns the address of a reconstructed stack or null if there wasn#t a guard failure.
 #[no_mangle]
+#[allow(clippy::not_unsafe_ptr_arg_deref)]
 pub extern "C" fn __ykrt_control_point(
     mt: *mut MT,
     loc: *mut Location,

ykfr/src/lib.rs

@@ -1,4 +1,6 @@
 #![feature(once_cell)]
+#![allow(clippy::comparison_chain)]
+#![allow(clippy::missing_safety_doc)]
 
 use llvm_sys::{core::*, target::LLVMABISizeOfType};
 use object::{Object, ObjectSection};

ykrt/src/location.rs

@@ -373,6 +373,7 @@ impl LocationInner {
     /// If this `State` is not counting, return its `HotLocation`. It is undefined behaviour to
     /// call this function if this `State` is in the counting phase and/or if this `State` is not
     /// locked.
+    #[allow(clippy::mut_from_ref)]
     pub(super) unsafe fn hot_location(&self) -> &mut HotLocation {
         debug_assert!(!self.is_counting());
         debug_assert!(self.is_locked());

ykrt/src/mt.rs

@@ -78,7 +78,7 @@ impl MT {
             max_worker_threads: AtomicUsize::new(cmp::max(1, num_cpus::get() - 1)),
             active_worker_threads: AtomicUsize::new(0),
             trace_decoder_kind: TraceDecoderKind::default_for_platform()
-                .ok_or_else(|| "Tracing is not supported on this platform")?,
+                .ok_or("Tracing is not supported on this platform")?,
             tracing_kind: TracingKind::default(),
         })
     }

yksmp/src/lib.rs

@@ -1,3 +1,4 @@
+#![allow(clippy::len_without_is_empty)]
 #![allow(clippy::new_without_default)]
 
 //! Note that LLVM currently only supports stackmaps for 64 bit architectures. Once they support

yktrace/src/hwt/mapper.rs

@@ -104,6 +104,7 @@ impl<'a> HWTMapper {
                 // function's symbol. If the cache knows that block A and B are from the same
                 // function, and a block X has a start address between blocks A and B, then X must
                 // also belong to the same function and there's no need to query the linker.
+                // FIXME: Is this `unwrap` safe?
                 let sio = vaddr_to_sym_and_obj(usize::try_from(block_vaddr).unwrap()).unwrap();
                 debug_assert_eq!(
                     obj_name.to_str().unwrap(),

yktrace/src/lib.rs

@@ -2,7 +2,9 @@
 
 #![feature(once_cell)]
 #![feature(naked_functions)]
+#![allow(clippy::len_without_is_empty)]
 #![allow(clippy::new_without_default)]
+#![allow(clippy::missing_safety_doc)]
 
 mod errors;
 use hwtracer::decode::TraceDecoderKind;
@@ -229,7 +231,7 @@ impl IRTrace {
                 di_tmpname_c,
             )
         };
-        if ret == ptr::null() {
+        if ret.is_null() {
             Err("Could not compile trace.".into())
         } else {
             Ok((ret, di_tmp))

ykutil/src/addr.rs

@@ -130,14 +130,12 @@ pub fn off_to_vaddr(containing_obj: &Path, off: u64) -> Option<usize> {
 ///
 /// This function uses `dladdr()` internally, and thus inherits the same symbol visibility rules
 /// used there. For example, this function will not find unexported symbols.
-pub fn vaddr_to_sym_and_obj(vaddr: usize) -> Result<DLInfo, ()> {
-    let info = dladdr(vaddr)?;
+pub fn vaddr_to_sym_and_obj(vaddr: usize) -> Option<DLInfo> {
     // `dladdr()` returns success if at least the virtual address could be mapped to an object
     // file, but here it is crucial that we can also find the symbol that the address belongs to.
-    if info.dli_sname().is_some() {
-        Ok(info)
-    } else {
-        Err(())
+    match dladdr(vaddr) {
+        Ok(x) if x.dli_sname().is_some() => Some(x),
+        Ok(_) | Err(()) => None,
     }
 }
 
@@ -211,13 +209,13 @@ mod tests {
     #[test]
     fn vaddr_to_sym_and_obj_cant_find_obj() {
         let func_vaddr = 1; // Obscure address unlikely to be in any loaded object.
-        assert!(vaddr_to_sym_and_obj(func_vaddr as usize).is_err());
+        assert!(vaddr_to_sym_and_obj(func_vaddr as usize).is_none());
     }
 
     #[test]
     fn vaddr_to_sym_and_obj_cant_find_sym() {
         // Address valid, but symbol not exported (test bin not built with `-Wl,--export-dynamic`).
         let func_vaddr = vaddr_to_sym_and_obj_cant_find_sym as *const fn();
-        assert!(vaddr_to_sym_and_obj(func_vaddr as usize).is_err());
+        assert!(vaddr_to_sym_and_obj(func_vaddr as usize).is_none());
     }
 }