From 828ebcff39f6fc1d10a2d28818b2297d15d52896 Mon Sep 17 00:00:00 2001 From: Dominik Maier Date: Sun, 22 May 2022 02:43:25 +0200 Subject: [PATCH] Clippy nits & fixes (#640) * release autofix * fix unused backtrace * clippy fixes * clippy * more clippy * more autofix * clippy for frida * more clippy --- libafl/src/bolts/llmp.rs | 2 +- libafl/src/bolts/minibsod.rs | 4 +- libafl_cc/build.rs | 9 ++--- libafl_frida/src/alloc.rs | 6 +-- libafl_frida/src/asan/asan_rt.rs | 69 ++++++++++++++++++-------------- libafl_frida/src/asan/errors.rs | 4 +- libafl_frida/src/cmplog_rt.rs | 1 + libafl_frida/src/coverage_rt.rs | 4 +- libafl_frida/src/helper.rs | 8 +++- libafl_frida/src/utils.rs | 12 +++--- scripts/autofix.sh | 6 ++- 11 files changed, 71 insertions(+), 54 deletions(-) diff --git a/libafl/src/bolts/llmp.rs b/libafl/src/bolts/llmp.rs index b7235bf994..e3a84abca9 100644 --- a/libafl/src/bolts/llmp.rs +++ b/libafl/src/bolts/llmp.rs @@ -82,7 +82,7 @@ use std::{ thread, }; -#[cfg(all(feature = "llmp_debug", feature = "std"))] +#[cfg(all(debug_assertions, feature = "llmp_debug", feature = "std"))] use backtrace::Backtrace; #[cfg(unix)] diff --git a/libafl/src/bolts/minibsod.rs b/libafl/src/bolts/minibsod.rs index 844bc8b5b6..231d57b0e8 100644 --- a/libafl/src/bolts/minibsod.rs +++ b/libafl/src/bolts/minibsod.rs @@ -98,12 +98,13 @@ pub fn dump_registers( /// Write the content of all important registers #[cfg(all(target_vendor = "apple", target_arch = "aarch64"))] +#[allow(clippy::similar_names)] pub fn dump_registers( writer: &mut BufWriter, ucontext: &ucontext_t, ) -> Result<(), std::io::Error> { let mcontext = unsafe { *ucontext.uc_mcontext }; - for reg in 0..29 { + for reg in 0..29_u8 { writeln!( writer, "x{:02}: 0x{:016x} ", @@ -218,6 +219,7 @@ fn write_crash( } #[cfg(all(target_vendor = "apple", target_arch = "aarch64"))] +#[allow(clippy::similar_names)] fn write_crash( writer: &mut BufWriter, signal: Signal, diff --git a/libafl_cc/build.rs b/libafl_cc/build.rs index b045d02103..5c6b306f53 100644 --- a/libafl_cc/build.rs +++ b/libafl_cc/build.rs @@ -118,13 +118,10 @@ fn main() { None => None, }; - match llvm_version { - Some(ver) => { - if ver >= 14 { - custom_flags.push("-DUSE_NEW_PM".to_string()); - } + if let Some(ver) = llvm_version { + if ver >= 14 { + custom_flags.push("-DUSE_NEW_PM".to_string()); } - None => (), } if let Ok(output) = Command::new(&llvm_config).args(&["--bindir"]).output() { diff --git a/libafl_frida/src/alloc.rs b/libafl_frida/src/alloc.rs index 6fc6232d4d..9bc02d17b7 100644 --- a/libafl_frida/src/alloc.rs +++ b/libafl_frida/src/alloc.rs @@ -125,10 +125,8 @@ impl Allocator { // On x64, if end > 2**52, then range is not in userspace #[cfg(target_arch = "aarch64")] - if end <= base.pow(52) { - if end > userspace_max { - userspace_max = end; - } + if end <= base.pow(52) && end > userspace_max { + userspace_max = end; } true diff --git a/libafl_frida/src/asan/asan_rt.rs b/libafl_frida/src/asan/asan_rt.rs index 467823768e..402ecf229c 100644 --- a/libafl_frida/src/asan/asan_rt.rs +++ b/libafl_frida/src/asan/asan_rt.rs @@ -2134,7 +2134,6 @@ impl AsanRuntime { #[must_use] #[inline] pub fn asan_is_interesting_instruction( - &self, capstone: &Capstone, _address: u64, instr: &Insn, @@ -2184,10 +2183,8 @@ impl AsanRuntime { #[cfg(all(target_arch = "x86_64", unix))] #[inline] #[must_use] - #[allow(clippy::unused_self)] #[allow(clippy::result_unit_err)] pub fn asan_is_interesting_instruction( - &self, capstone: &Capstone, _address: u64, instr: &Insn, @@ -2424,6 +2421,7 @@ impl AsanRuntime { /// Emit a shadow memory check into the instruction stream #[cfg(target_arch = "aarch64")] #[inline] + #[allow(clippy::too_many_lines, clippy::too_many_arguments)] pub fn emit_shadow_check( &mut self, _address: u64, @@ -2435,14 +2433,19 @@ impl AsanRuntime { shift: Arm64Shift, extender: Arm64Extender, ) { + debug_assert!( + i32::try_from(frida_gum_sys::GUM_RED_ZONE_SIZE).is_ok(), + "GUM_RED_ZONE_SIZE is bigger than i32::max" + ); + #[allow(clippy::cast_possible_wrap)] let redzone_size = frida_gum_sys::GUM_RED_ZONE_SIZE as i32; let writer = output.writer(); let basereg = writer_register(basereg); - let indexreg = if indexreg.0 != 0 { - Some(writer_register(indexreg)) - } else { + let indexreg = if indexreg.0 == 0 { None + } else { + Some(writer_register(indexreg)) }; if self.current_report_impl == 0 @@ -2470,7 +2473,7 @@ impl AsanRuntime { Aarch64Register::X0, Aarch64Register::X1, Aarch64Register::Sp, - -(16 + redzone_size) as i64, + i64::from(-(16 + redzone_size)), IndexMode::PreAdjust, ); @@ -2523,7 +2526,7 @@ impl AsanRuntime { Arm64Extender::ARM64_EXT_SXTH => 0b101, Arm64Extender::ARM64_EXT_SXTW => 0b110, Arm64Extender::ARM64_EXT_SXTX => 0b111, - _ => -1, + Arm64Extender::ARM64_EXT_INVALID => -1, }; let (shift_encoding, shift_amount): (i32, u32) = match shift { Arm64Shift::Lsl(amount) => (0b00, amount), @@ -2534,11 +2537,13 @@ impl AsanRuntime { if extender_encoding != -1 && shift_amount < 0b1000 { // emit add extended register: https://developer.arm.com/documentation/ddi0602/latest/Base-Instructions/ADD--extended-register---Add--extended-register-- + #[allow(clippy::cast_sign_loss)] writer.put_bytes( &(0x8b210000 | ((extender_encoding as u32) << 13) | (shift_amount << 10)) .to_le_bytes(), ); } else if shift_encoding != -1 { + #[allow(clippy::cast_sign_loss)] writer.put_bytes( &(0x8b010000 | ((shift_encoding as u32) << 22) | (shift_amount << 10)) .to_le_bytes(), @@ -2559,56 +2564,62 @@ impl AsanRuntime { #[allow(clippy::comparison_chain)] if displacement < 0 { if displacement > -4096 { + #[allow(clippy::cast_sign_loss)] + let displacement = displacement.abs() as u32; // Subtract the displacement into x0 writer.put_sub_reg_reg_imm( Aarch64Register::X0, Aarch64Register::X0, - displacement.abs() as u64, + u64::from(displacement), ); } else { - let displacement_hi = displacement.abs() / 4096; - let displacement_lo = displacement.abs() % 4096; - writer.put_bytes(&(0xd1400000u32 | ((displacement_hi as u32) << 10)).to_le_bytes()); + #[allow(clippy::cast_sign_loss)] + let displacement = displacement.abs() as u32; + let displacement_hi = displacement / 4096; + let displacement_lo = displacement % 4096; + writer.put_bytes(&(0xd1400000u32 | (displacement_hi << 10)).to_le_bytes()); writer.put_sub_reg_reg_imm( Aarch64Register::X0, Aarch64Register::X0, - displacement_lo as u64, + u64::from(displacement_lo), ); } } else if displacement > 0 { + #[allow(clippy::cast_sign_loss)] + let displacement = displacement as u32; if displacement < 4096 { // Add the displacement into x0 writer.put_add_reg_reg_imm( Aarch64Register::X0, Aarch64Register::X0, - displacement as u64, + u64::from(displacement), ); } else { let displacement_hi = displacement / 4096; let displacement_lo = displacement % 4096; - writer.put_bytes(&(0x91400000u32 | ((displacement_hi as u32) << 10)).to_le_bytes()); + writer.put_bytes(&(0x91400000u32 | (displacement_hi << 10)).to_le_bytes()); writer.put_add_reg_reg_imm( Aarch64Register::X0, Aarch64Register::X0, - displacement_lo as u64, + u64::from(displacement_lo), ); } } // Insert the check_shadow_mem code blob #[cfg(unix)] match width { - 1 => writer.put_bytes(&self.blob_check_mem_byte()), - 2 => writer.put_bytes(&self.blob_check_mem_halfword()), - 3 => writer.put_bytes(&self.blob_check_mem_3bytes()), - 4 => writer.put_bytes(&self.blob_check_mem_dword()), - 6 => writer.put_bytes(&self.blob_check_mem_6bytes()), - 8 => writer.put_bytes(&self.blob_check_mem_qword()), - 12 => writer.put_bytes(&self.blob_check_mem_12bytes()), - 16 => writer.put_bytes(&self.blob_check_mem_16bytes()), - 24 => writer.put_bytes(&self.blob_check_mem_24bytes()), - 32 => writer.put_bytes(&self.blob_check_mem_32bytes()), - 48 => writer.put_bytes(&self.blob_check_mem_48bytes()), - 64 => writer.put_bytes(&self.blob_check_mem_64bytes()), + 1 => writer.put_bytes(self.blob_check_mem_byte()), + 2 => writer.put_bytes(self.blob_check_mem_halfword()), + 3 => writer.put_bytes(self.blob_check_mem_3bytes()), + 4 => writer.put_bytes(self.blob_check_mem_dword()), + 6 => writer.put_bytes(self.blob_check_mem_6bytes()), + 8 => writer.put_bytes(self.blob_check_mem_qword()), + 12 => writer.put_bytes(self.blob_check_mem_12bytes()), + 16 => writer.put_bytes(self.blob_check_mem_16bytes()), + 24 => writer.put_bytes(self.blob_check_mem_24bytes()), + 32 => writer.put_bytes(self.blob_check_mem_32bytes()), + 48 => writer.put_bytes(self.blob_check_mem_48bytes()), + 64 => writer.put_bytes(self.blob_check_mem_64bytes()), _ => false, }; @@ -2629,7 +2640,7 @@ impl AsanRuntime { Aarch64Register::X0, Aarch64Register::X1, Aarch64Register::Sp, - 16 + redzone_size as i64, + 16 + i64::from(redzone_size), IndexMode::PostAdjust, )); } diff --git a/libafl_frida/src/asan/errors.rs b/libafl_frida/src/asan/errors.rs index efc349c77c..aeafa98474 100644 --- a/libafl_frida/src/asan/errors.rs +++ b/libafl_frida/src/asan/errors.rs @@ -444,7 +444,7 @@ impl AsanErrors { writeln!(output, "{:━^100}", " REGISTERS ").unwrap(); #[cfg(target_arch = "aarch64")] - for reg in 0..=30 { + for (reg, val) in registers.iter().enumerate().take(30 + 1) { if basereg.is_some() && reg == basereg.unwrap() as usize { output .set_color(ColorSpec::new().set_fg(Some(Color::Red))) @@ -454,7 +454,7 @@ impl AsanErrors { .set_color(ColorSpec::new().set_fg(Some(Color::Yellow))) .unwrap(); } - write!(output, "x{:02}: 0x{:016x} ", reg, registers[reg]).unwrap(); + write!(output, "x{:02}: 0x{:016x} ", reg, val).unwrap(); output.reset().unwrap(); if reg % 4 == 3 { writeln!(output).unwrap(); diff --git a/libafl_frida/src/cmplog_rt.rs b/libafl_frida/src/cmplog_rt.rs index 59adb8a934..0d3ca3666f 100644 --- a/libafl_frida/src/cmplog_rt.rs +++ b/libafl_frida/src/cmplog_rt.rs @@ -633,6 +633,7 @@ impl CmpLogRuntime { None }; + #[allow(clippy::cast_sign_loss)] let operand2 = match special_case { true => Some(CmplogOperandType::Imm(0)), false => { diff --git a/libafl_frida/src/coverage_rt.rs b/libafl_frida/src/coverage_rt.rs index 9c22a2c50d..68e67f7079 100644 --- a/libafl_frida/src/coverage_rt.rs +++ b/libafl_frida/src/coverage_rt.rs @@ -97,7 +97,7 @@ impl CoverageRuntime { ; ldr x2, >previous_loc ; ldr x4, [x2] ; eor x4, x4, x0 - ; mov x3, ((MAP_SIZE - 1) as u32) as u64 + ; mov x3, u64::from((MAP_SIZE - 1) as u32) ; and x4, x4, x3 ; ldr x3, [x1, x4] ; add x3, x3, #1 @@ -113,7 +113,7 @@ impl CoverageRuntime { ;.qword 0 ); let ops_vec = ops.finalize().unwrap(); - self.blob_maybe_log = Some(ops_vec[..ops_vec.len() - 8].to_vec().into_boxed_slice()) + self.blob_maybe_log = Some(ops_vec[..ops_vec.len() - 8].to_vec().into_boxed_slice()); } /// A minimal `maybe_log` implementation. We insert this into the transformed instruction stream diff --git a/libafl_frida/src/helper.rs b/libafl_frida/src/helper.rs index 1f56ccaaf7..2416315ca8 100644 --- a/libafl_frida/src/helper.rs +++ b/libafl_frida/src/helper.rs @@ -285,8 +285,12 @@ where } #[cfg(unix)] - let res = if let Some(rt) = helper.runtime::() { - rt.asan_is_interesting_instruction(&helper.capstone, address, instr) + let res = if let Some(_rt) = helper.runtime::() { + AsanRuntime::asan_is_interesting_instruction( + &helper.capstone, + address, + instr, + ) } else { None }; diff --git a/libafl_frida/src/utils.rs b/libafl_frida/src/utils.rs index e805ee41cc..47d88cee51 100644 --- a/libafl_frida/src/utils.rs +++ b/libafl_frida/src/utils.rs @@ -15,7 +15,8 @@ use num_traits::cast::FromPrimitive; /// Determine the width of the specified instruction #[cfg(target_arch = "aarch64")] #[inline] -pub fn instruction_width(instr: &Insn, operands: &Vec) -> u32 { +#[must_use] +pub fn instruction_width(instr: &Insn, operands: &[arch::ArchOperand]) -> u32 { use capstone::arch::arm64::Arm64Insn as I; use capstone::arch::arm64::Arm64Reg as R; use capstone::arch::arm64::Arm64Vas as V; @@ -48,7 +49,7 @@ pub fn instruction_width(instr: &Insn, operands: &Vec) -> u32 }; return match operand.vas { - V::ARM64_VAS_1B => 1 * count_byte, + V::ARM64_VAS_1B => count_byte, V::ARM64_VAS_1H => 2 * count_byte, V::ARM64_VAS_4B | V::ARM64_VAS_1S | V::ARM64_VAS_1D | V::ARM64_VAS_2H => { 4 * count_byte @@ -64,7 +65,7 @@ pub fn instruction_width(instr: &Insn, operands: &Vec) -> u32 } }; } else if let Arm64OperandType::Reg(operand) = operand.op_type { - match operand.0 as u32 { + match u32::from(operand.0) { R::ARM64_REG_W0..=R::ARM64_REG_W30 | R::ARM64_REG_WZR | R::ARM64_REG_WSP @@ -79,12 +80,13 @@ pub fn instruction_width(instr: &Insn, operands: &Vec) -> u32 8 * num_registers } -/// Convert from a capstone register id to a frida InstructionWriter register index +/// Convert from a capstone register id to a frida `InstructionWriter` register index #[cfg(target_arch = "aarch64")] +#[must_use] #[inline] pub fn writer_register(reg: capstone::RegId) -> Aarch64Register { let regint: u16 = reg.0; - Aarch64Register::from_u32(regint as u32).unwrap() + Aarch64Register::from_u32(u32::from(regint)).unwrap() } /// The writer registers diff --git a/scripts/autofix.sh b/scripts/autofix.sh index 1fb398124f..eb987a6b28 100755 --- a/scripts/autofix.sh +++ b/scripts/autofix.sh @@ -11,13 +11,13 @@ fi echo echo "[+] Fixing build" -cargo +nightly fix --workspace --all-features +cargo +nightly fix --release --workspace --all-features echo "[+] Done fixing build" echo echo 'Fixing clippy (might need a "git commit" and a rerun, if "cargo fix" changed the source)' -RUST_BACKTRACE=full cargo +nightly clippy --fix --all --all-features --tests -- -Z macro-backtrace \ +RUST_BACKTRACE=full cargo +nightly clippy --fix --release --all --all-features --tests -- -Z macro-backtrace \ -D clippy::all \ -D clippy::pedantic \ -W clippy::similar_names \ @@ -32,6 +32,8 @@ RUST_BACKTRACE=full cargo +nightly clippy --fix --all --all-features --tests -- -A clippy::module-name-repetitions \ -A clippy::unreadable-literal \ +cargo +nightly clippy --fix --tests --all-features --allow-dirty --allow-staged + echo "[+] Done fixing clippy" echo