blob: aea3d26e18788007bad6dc510f900425ed895ee9 [file] [log] [blame]
use std::collections::btree_map::Entry;
use std::collections::BTreeMap;
use clippy_utils::diagnostics::span_lint_hir_and_then;
use clippy_utils::is_lint_allowed;
use itertools::Itertools;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::intravisit::{walk_block, walk_expr, walk_stmt, Visitor};
use rustc_hir::{BlockCheckMode, Expr, ExprKind, HirId, Stmt, UnsafeSource};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::impl_lint_pass;
use rustc_span::{sym, Span, SyntaxContext};
declare_clippy_lint! {
/// ### What it does
/// Looks for macros that expand metavariables in an unsafe block.
///
/// ### Why is this bad?
/// This hides an unsafe block and allows the user of the macro to write unsafe code without an explicit
/// unsafe block at callsite, making it possible to perform unsafe operations in seemingly safe code.
///
/// The macro should be restructured so that these metavariables are referenced outside of unsafe blocks
/// and that the usual unsafety checks apply to the macro argument.
///
/// This is usually done by binding it to a variable outside of the unsafe block
/// and then using that variable inside of the block as shown in the example, or by referencing it a second time
/// in a safe context, e.g. `if false { $expr }`.
///
/// ### Known limitations
/// Due to how macros are represented in the compiler at the time Clippy runs its lints,
/// it's not possible to look for metavariables in macro definitions directly.
///
/// Instead, this lint looks at expansions of macros.
/// This leads to false negatives for macros that are never actually invoked.
///
/// By default, this lint is rather conservative and will only emit warnings on publicly-exported
/// macros from the same crate, because oftentimes private internal macros are one-off macros where
/// this lint would just be noise (e.g. macros that generate `impl` blocks).
/// The default behavior should help with preventing a high number of such false positives,
/// however it can be configured to also emit warnings in private macros if desired.
///
/// ### Example
/// ```no_run
/// /// Gets the first element of a slice
/// macro_rules! first {
/// ($slice:expr) => {
/// unsafe {
/// let slice = $slice; // ⚠️ expansion inside of `unsafe {}`
///
/// assert!(!slice.is_empty());
/// // SAFETY: slice is checked to have at least one element
/// slice.first().unwrap_unchecked()
/// }
/// }
/// }
///
/// assert_eq!(*first!(&[1i32]), 1);
///
/// // This will compile as a consequence (note the lack of `unsafe {}`)
/// assert_eq!(*first!(std::hint::unreachable_unchecked() as &[i32]), 1);
/// ```
/// Use instead:
/// ```compile_fail
/// macro_rules! first {
/// ($slice:expr) => {{
/// let slice = $slice; // ✅ outside of `unsafe {}`
/// unsafe {
/// assert!(!slice.is_empty());
/// // SAFETY: slice is checked to have at least one element
/// slice.first().unwrap_unchecked()
/// }
/// }}
/// }
///
/// assert_eq!(*first!(&[1]), 1);
///
/// // This won't compile:
/// assert_eq!(*first!(std::hint::unreachable_unchecked() as &[i32]), 1);
/// ```
#[clippy::version = "1.80.0"]
pub MACRO_METAVARS_IN_UNSAFE,
suspicious,
"expanding macro metavariables in an unsafe block"
}
impl_lint_pass!(ExprMetavarsInUnsafe => [MACRO_METAVARS_IN_UNSAFE]);
#[derive(Clone, Debug)]
pub enum MetavarState {
ReferencedInUnsafe { unsafe_blocks: Vec<HirId> },
ReferencedInSafe,
}
#[derive(Default)]
pub struct ExprMetavarsInUnsafe {
pub warn_unsafe_macro_metavars_in_private_macros: bool,
/// A metavariable can be expanded more than once, potentially across multiple bodies, so it
/// requires some state kept across HIR nodes to make it possible to delay a warning
/// and later undo:
///
/// ```ignore
/// macro_rules! x {
/// ($v:expr) => {
/// unsafe { $v; } // unsafe context, it might be possible to emit a warning here, so add it to the map
///
/// $v; // `$v` expanded another time but in a safe context, set to ReferencedInSafe to suppress
/// }
/// }
/// ```
pub metavar_expns: BTreeMap<Span, MetavarState>,
}
struct BodyVisitor<'a, 'tcx> {
/// Stack of unsafe blocks -- the top item always represents the last seen unsafe block from
/// within a relevant macro.
macro_unsafe_blocks: Vec<HirId>,
/// When this is >0, it means that the node currently being visited is "within" a
/// macro definition. This is not necessary for correctness, it merely helps reduce the number
/// of spans we need to insert into the map, since only spans from macros are relevant.
expn_depth: u32,
cx: &'a LateContext<'tcx>,
lint: &'a mut ExprMetavarsInUnsafe,
}
fn is_public_macro(cx: &LateContext<'_>, def_id: LocalDefId) -> bool {
(cx.effective_visibilities.is_exported(def_id) || cx.tcx.has_attr(def_id, sym::macro_export))
&& !cx.tcx.is_doc_hidden(def_id)
}
impl<'a, 'tcx> Visitor<'tcx> for BodyVisitor<'a, 'tcx> {
fn visit_stmt(&mut self, s: &'tcx Stmt<'tcx>) {
let from_expn = s.span.from_expansion();
if from_expn {
self.expn_depth += 1;
}
walk_stmt(self, s);
if from_expn {
self.expn_depth -= 1;
}
}
fn visit_expr(&mut self, e: &'tcx Expr<'tcx>) {
let ctxt = e.span.ctxt();
if let ExprKind::Block(block, _) = e.kind
&& let BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) = block.rules
&& !ctxt.is_root()
&& let Some(macro_def_id) = ctxt.outer_expn_data().macro_def_id
&& let Some(macro_def_id) = macro_def_id.as_local()
&& (self.lint.warn_unsafe_macro_metavars_in_private_macros || is_public_macro(self.cx, macro_def_id))
{
self.macro_unsafe_blocks.push(block.hir_id);
walk_block(self, block);
self.macro_unsafe_blocks.pop();
} else if ctxt.is_root() && self.expn_depth > 0 {
let unsafe_block = self.macro_unsafe_blocks.last().copied();
match (self.lint.metavar_expns.entry(e.span), unsafe_block) {
(Entry::Vacant(e), None) => {
e.insert(MetavarState::ReferencedInSafe);
},
(Entry::Vacant(e), Some(unsafe_block)) => {
e.insert(MetavarState::ReferencedInUnsafe {
unsafe_blocks: vec![unsafe_block],
});
},
(Entry::Occupied(mut e), None) => {
if let MetavarState::ReferencedInUnsafe { .. } = *e.get() {
e.insert(MetavarState::ReferencedInSafe);
}
},
(Entry::Occupied(mut e), Some(unsafe_block)) => {
if let MetavarState::ReferencedInUnsafe { unsafe_blocks } = e.get_mut()
&& !unsafe_blocks.contains(&unsafe_block)
{
unsafe_blocks.push(unsafe_block);
}
},
}
// NB: No need to visit descendant nodes. They're guaranteed to represent the same
// metavariable
} else {
walk_expr(self, e);
}
}
}
impl<'tcx> LateLintPass<'tcx> for ExprMetavarsInUnsafe {
fn check_body(&mut self, cx: &LateContext<'tcx>, body: &'tcx rustc_hir::Body<'tcx>) {
if is_lint_allowed(cx, MACRO_METAVARS_IN_UNSAFE, body.value.hir_id) {
return;
}
// This BodyVisitor is separate and not part of the lint pass because there is no
// `check_stmt_post` on `(Late)LintPass`, which we'd need to detect when we're leaving a macro span
let mut vis = BodyVisitor {
#[expect(clippy::bool_to_int_with_if)] // obfuscates the meaning
expn_depth: if body.value.span.from_expansion() { 1 } else { 0 },
macro_unsafe_blocks: Vec::new(),
lint: self,
cx
};
vis.visit_body(body);
}
fn check_crate_post(&mut self, cx: &LateContext<'tcx>) {
// Aggregate all unsafe blocks from all spans:
// ```
// macro_rules! x {
// ($w:expr, $x:expr, $y:expr) => { $w; unsafe { $w; $x; }; unsafe { $x; $y; }; }
// }
// $w: [] (unsafe#0 is never added because it was referenced in a safe context)
// $x: [unsafe#0, unsafe#1]
// $y: [unsafe#1]
// ```
// We want to lint unsafe blocks #0 and #1
let bad_unsafe_blocks = self
.metavar_expns
.iter()
.filter_map(|(_, state)| match state {
MetavarState::ReferencedInUnsafe { unsafe_blocks } => Some(unsafe_blocks.as_slice()),
MetavarState::ReferencedInSafe => None,
})
.flatten()
.copied()
.map(|id| {
// Remove the syntax context to hide "in this macro invocation" in the diagnostic.
// The invocation doesn't matter. Also we want to dedupe by the unsafe block and not by anything
// related to the callsite.
let span = cx.tcx.hir().span(id);
(id, Span::new(span.lo(), span.hi(), SyntaxContext::root(), None))
})
.dedup_by(|&(_, a), &(_, b)| a == b);
for (id, span) in bad_unsafe_blocks {
span_lint_hir_and_then(
cx,
MACRO_METAVARS_IN_UNSAFE,
id,
span,
"this macro expands metavariables in an unsafe block",
|diag| {
diag.note("this allows the user of the macro to write unsafe code outside of an unsafe block");
diag.help(
"consider expanding any metavariables outside of this block, e.g. by storing them in a variable",
);
diag.help(
"... or also expand referenced metavariables in a safe context to require an unsafe block at callsite",
);
},
);
}
}
}