Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new check for passing pointers to an asm! block with nomem option #13247

Merged
merged 1 commit into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5758,6 +5758,7 @@ Released 2018-09-13
[`pathbuf_init_then_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#pathbuf_init_then_push
[`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
[`pointers_in_nomem_asm_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#pointers_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 static LINTS: &[&crate::LintInfo] = &[
crate::pathbuf_init_then_push::PATHBUF_INIT_THEN_PUSH_INFO,
crate::pattern_type_mismatch::PATTERN_TYPE_MISMATCH_INFO,
crate::permissions_set_readonly_false::PERMISSIONS_SET_READONLY_FALSE_INFO,
crate::pointers_in_nomem_asm_block::POINTERS_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 @@ -287,6 +287,7 @@ mod pass_by_ref_or_value;
mod pathbuf_init_then_push;
mod pattern_type_mismatch;
mod permissions_set_readonly_false;
mod pointers_in_nomem_asm_block;
mod precedence;
mod ptr;
mod ptr_offset_with_cast;
Expand Down Expand Up @@ -935,5 +936,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
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(zombie_processes::ZombieProcesses));
store.register_late_pass(|_| Box::new(pointers_in_nomem_asm_block::PointersInNomemAsmBlock));
// add lints here, do not remove this comment, it's used in `new_lint`
}
88 changes: 88 additions & 0 deletions clippy_lints/src/pointers_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 POINTERS_IN_NOMEM_ASM_BLOCK,
suspicious,
"pointers in nomem asm block"
}

declare_lint_pass!(PointersInNomemAsmBlock => [POINTERS_IN_NOMEM_ASM_BLOCK]);

impl<'tcx> LateLintPass<'tcx> for PointersInNomemAsmBlock {
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,
POINTERS_IN_NOMEM_ASM_BLOCK,
spans,
"passing pointers 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 the 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/pointers_in_nomem_asm_block.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//@ needs-asm-support
#![warn(clippy::pointers_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 pointers 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 pointers to nomem asm block
}

unsafe fn nomem_fn(p: extern "C" fn()) {
asm!("call {p}", p = in(reg) p, options(nomem));
//~^ ERROR: passing pointers to nomem asm block
}
34 changes: 34 additions & 0 deletions tests/ui/pointers_in_nomem_asm_block.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
error: passing pointers to nomem asm block
--> tests/ui/pointers_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 the asm! block
= note: if this is intentional and no pointers are read or written to, consider allowing the lint
= note: `-D clippy::pointers-in-nomem-asm-block` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::pointers_in_nomem_asm_block)]`

error: passing pointers to nomem asm block
--> tests/ui/pointers_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 the asm! block
= note: if this is intentional and no pointers are read or written to, consider allowing the lint

error: passing pointers to nomem asm block
--> tests/ui/pointers_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 the 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

Loading