Skip to content

Commit

Permalink
add lint for suspicious asm! usage
Browse files Browse the repository at this point in the history
  • Loading branch information
Soveu committed Aug 11, 2024
1 parent 86d348d commit 5f44d9e
Show file tree
Hide file tree
Showing 6 changed files with 159 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}

Expand Down
88 changes: 88 additions & 0 deletions clippy_lints/src/pointer_in_nomem_asm_block.rs
Original file line number Diff line number Diff line change
@@ -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::<Vec<Span>>();

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");
}
33 changes: 33 additions & 0 deletions tests/ui/pointer_in_nomem_asm_block.rs
Original file line number Diff line number Diff line change
@@ -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
}
34 changes: 34 additions & 0 deletions tests/ui/pointer_in_nomem_asm_block.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 5f44d9e

Please sign in to comment.