diff --git a/CHANGELOG.md b/CHANGELOG.md index 55281f3cbec0..5affd9c506ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5714,6 +5714,7 @@ Released 2018-09-13 [`path_ends_with_ext`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_ends_with_ext [`pattern_type_mismatch`]: https://rust-lang.github.io/rust-clippy/master/index.html#pattern_type_mismatch [`permissions_set_readonly_false`]: https://rust-lang.github.io/rust-clippy/master/index.html#permissions_set_readonly_false +[`pointer_in_nomem_asm_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#pointer_in_nomem_asm_block [`positional_named_format_parameters`]: https://rust-lang.github.io/rust-clippy/master/index.html#positional_named_format_parameters [`possible_missing_comma`]: https://rust-lang.github.io/rust-clippy/master/index.html#possible_missing_comma [`precedence`]: https://rust-lang.github.io/rust-clippy/master/index.html#precedence diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index eabc67601a2f..8ef9d0cf04e7 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -600,6 +600,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::pass_by_ref_or_value::TRIVIALLY_COPY_PASS_BY_REF_INFO, crate::pattern_type_mismatch::PATTERN_TYPE_MISMATCH_INFO, crate::permissions_set_readonly_false::PERMISSIONS_SET_READONLY_FALSE_INFO, + crate::pointer_in_nomem_asm_block::POINTER_IN_NOMEM_ASM_BLOCK_INFO, crate::precedence::PRECEDENCE_INFO, crate::ptr::CMP_NULL_INFO, crate::ptr::INVALID_NULL_PTR_USAGE_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index f1a0b5c3d411..0e83e3325f9f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -289,6 +289,7 @@ mod partialeq_to_none; mod pass_by_ref_or_value; mod pattern_type_mismatch; mod permissions_set_readonly_false; +mod pointer_in_nomem_asm_block; mod precedence; mod ptr; mod ptr_offset_with_cast; @@ -1176,6 +1177,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(set_contains_or_insert::HashsetInsertAfterContains)); store.register_early_pass(|| Box::new(byte_char_slices::ByteCharSlice)); store.register_early_pass(|| Box::new(cfg_not_test::CfgNotTest)); + store.register_late_pass(|_| Box::new(pointer_in_nomem_asm_block::PointerInNomemAsmBlock)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/pointer_in_nomem_asm_block.rs b/clippy_lints/src/pointer_in_nomem_asm_block.rs new file mode 100644 index 000000000000..7e725d7c2a0a --- /dev/null +++ b/clippy_lints/src/pointer_in_nomem_asm_block.rs @@ -0,0 +1,88 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use rustc_ast::InlineAsmOptions; +use rustc_hir::{Expr, ExprKind, InlineAsm, InlineAsmOperand}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::declare_lint_pass; +use rustc_span::Span; + +declare_clippy_lint! { + /// ### What it does + /// Checks if any pointer is being passed to an asm! block with `nomem` option. + /// + /// ### Why is this bad? + /// `nomem` forbids any reads or writes to memory and passing a pointer suggests + /// that either of those will happen. + /// + /// ### Example + /// ```no_run + /// fn f(p: *mut u32) { + /// unsafe { core::arch::asm!("mov [{p}], 42", p = in(reg) p, options(nomem, nostack)); } + /// } + /// ``` + /// Use instead: + /// ```no_run + /// fn f(p: *mut u32) { + /// unsafe { core::arch::asm!("mov [{p}], 42", p = in(reg) p, options(nostack)); } + /// } + /// ``` + #[clippy::version = "1.81.0"] + pub POINTER_IN_NOMEM_ASM_BLOCK, + suspicious, + "pointer in nomem asm block" +} + +declare_lint_pass!(PointerInNomemAsmBlock => [POINTER_IN_NOMEM_ASM_BLOCK]); + +impl<'tcx> LateLintPass<'tcx> for PointerInNomemAsmBlock { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { + if let ExprKind::InlineAsm(asm) = &expr.kind { + check_asm(cx, asm); + } + } +} + +fn check_asm(cx: &LateContext<'_>, asm: &InlineAsm<'_>) { + if !asm.options.contains(InlineAsmOptions::NOMEM) { + return; + } + + let spans = asm + .operands + .iter() + .filter(|(op, _span)| has_in_operand_pointer(cx, op)) + .map(|(_op, span)| *span) + .collect::>(); + + if spans.is_empty() { + return; + } + + span_lint_and_then( + cx, + POINTER_IN_NOMEM_ASM_BLOCK, + spans, + "passing pointer to nomem asm block", + additional_notes, + ); +} + +fn has_in_operand_pointer(cx: &LateContext<'_>, asm_op: &InlineAsmOperand<'_>) -> bool { + let asm_in_expr = match asm_op { + InlineAsmOperand::SymStatic { .. } + | InlineAsmOperand::Out { .. } + | InlineAsmOperand::Const { .. } + | InlineAsmOperand::SymFn { .. } + | InlineAsmOperand::Label { .. } => return false, + InlineAsmOperand::SplitInOut { in_expr, .. } => in_expr, + InlineAsmOperand::In { expr, .. } | InlineAsmOperand::InOut { expr, .. } => expr, + }; + + // This checks for raw ptrs, refs and function pointers - the last one + // also technically counts as reading memory. + cx.typeck_results().expr_ty(asm_in_expr).is_any_ptr() +} + +fn additional_notes(diag: &mut rustc_errors::Diag<'_, ()>) { + diag.note("`nomem` means that no memory write or read happens inside them asm! block"); + diag.note("if this is intentional and no pointers are read or written to, consider allowing the lint"); +} diff --git a/tests/ui/pointer_in_nomem_asm_block.rs b/tests/ui/pointer_in_nomem_asm_block.rs new file mode 100644 index 000000000000..f0e51600b615 --- /dev/null +++ b/tests/ui/pointer_in_nomem_asm_block.rs @@ -0,0 +1,33 @@ +//@ needs-asm-support +#![warn(clippy::pointer_in_nomem_asm_block)] +#![crate_type = "lib"] +#![no_std] + +use core::arch::asm; + +unsafe fn nomem_bad(p: &i32) { + asm!( + "asdf {p1}, {p2}, {p3}", + p1 = in(reg) p, + //~^ ERROR: passing pointer to nomem asm block + p2 = in(reg) p as *const _ as usize, + p3 = in(reg) p, + options(nomem, nostack, preserves_flags) + ); +} + +unsafe fn nomem_good(p: &i32) { + asm!("asdf {p}", p = in(reg) p, options(readonly, nostack, preserves_flags)); + let p = p as *const i32 as usize; + asm!("asdf {p}", p = in(reg) p, options(nomem, nostack, preserves_flags)); +} + +unsafe fn nomem_bad2(p: &mut i32) { + asm!("asdf {p}", p = in(reg) p, options(nomem, nostack, preserves_flags)); + //~^ ERROR: passing pointer to nomem asm block +} + +unsafe fn nomem_fn(p: extern "C" fn()) { + asm!("call {p}", p = in(reg) p, options(nomem)); + //~^ ERROR: passing pointer to nomem asm block +} diff --git a/tests/ui/pointer_in_nomem_asm_block.stderr b/tests/ui/pointer_in_nomem_asm_block.stderr new file mode 100644 index 000000000000..c22e2093fb1e --- /dev/null +++ b/tests/ui/pointer_in_nomem_asm_block.stderr @@ -0,0 +1,34 @@ +error: passing pointer to nomem asm block + --> tests/ui/pointer_in_nomem_asm_block.rs:11:9 + | +LL | p1 = in(reg) p, + | ^^^^^^^^^^^^^^ +... +LL | p3 = in(reg) p, + | ^^^^^^^^^^^^^^ + | + = note: `nomem` means that no memory write or read happens inside them asm! block + = note: if this is intentional and no pointers are read or written to, consider allowing the lint + = note: `-D clippy::pointer-in-nomem-asm-block` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::pointer_in_nomem_asm_block)]` + +error: passing pointer to nomem asm block + --> tests/ui/pointer_in_nomem_asm_block.rs:26:22 + | +LL | asm!("asdf {p}", p = in(reg) p, options(nomem, nostack, preserves_flags)); + | ^^^^^^^^^^^^^ + | + = note: `nomem` means that no memory write or read happens inside them asm! block + = note: if this is intentional and no pointers are read or written to, consider allowing the lint + +error: passing pointer to nomem asm block + --> tests/ui/pointer_in_nomem_asm_block.rs:31:22 + | +LL | asm!("call {p}", p = in(reg) p, options(nomem)); + | ^^^^^^^^^^^^^ + | + = note: `nomem` means that no memory write or read happens inside them asm! block + = note: if this is intentional and no pointers are read or written to, consider allowing the lint + +error: aborting due to 3 previous errors +