|
| 1 | +use clippy_utils::consts::is_promotable; |
| 2 | +use clippy_utils::diagnostics::{span_lint_and_note, span_lint_hir_and_then}; |
| 3 | +use clippy_utils::is_from_proc_macro; |
| 4 | +use clippy_utils::mir::{location_to_node, StatementOrTerminator}; |
| 5 | +use rustc_data_structures::fx::FxHashMap; |
| 6 | +use rustc_hir::def_id::LocalDefId; |
| 7 | +use rustc_hir::intravisit::FnKind; |
| 8 | +use rustc_hir::{Body, BorrowKind, Expr, ExprKind, FnDecl, ItemKind, OwnerNode}; |
| 9 | +use rustc_lint::{LateContext, LateLintPass, LintContext}; |
| 10 | +use rustc_middle::lint::in_external_macro; |
| 11 | +use rustc_middle::mir::visit::{MutatingUseContext, PlaceContext, Visitor}; |
| 12 | +use rustc_middle::mir::{ |
| 13 | + self, BasicBlock, BasicBlockData, CallSource, Local, Location, Operand, Place, Rvalue, StatementKind, |
| 14 | + TerminatorKind, |
| 15 | +}; |
| 16 | +use rustc_middle::ty::{self, TypeVisitableExt}; |
| 17 | +use rustc_session::{declare_lint_pass, declare_tool_lint}; |
| 18 | +use rustc_span::{sym, Span}; |
| 19 | + |
| 20 | +declare_clippy_lint! { |
| 21 | + /// ### What it does |
| 22 | + /// Checks for raw pointers pointing to temporary values that will **not** be promoted to a |
| 23 | + /// constant through |
| 24 | + /// [constant promotion](https://doc.rust-lang.org/stable/reference/destructors.html#constant-promotion). |
| 25 | + /// |
| 26 | + /// ### Why is this bad? |
| 27 | + /// Usage of such a pointer will result in Undefined Behavior, as the pointer will stop |
| 28 | + /// pointing to valid stack memory once the temporary is dropped. |
| 29 | + /// |
| 30 | + /// ### Example |
| 31 | + /// ```rust,ignore |
| 32 | + /// fn returning_temp() -> *const i32 { |
| 33 | + /// let x = 0; |
| 34 | + /// &x as *const i32 |
| 35 | + /// } |
| 36 | + /// |
| 37 | + /// let px = returning_temp(); |
| 38 | + /// unsafe { *px }; // ⚠️ |
| 39 | + /// let pv = vec![].as_ptr(); |
| 40 | + /// unsafe { *pv }; // ⚠️ |
| 41 | + /// ``` |
| 42 | + #[clippy::version = "1.72.0"] |
| 43 | + pub PTR_TO_TEMPORARY, |
| 44 | + // TODO: Let's make it warn-by-default for now, and change this to deny-by-default once we know |
| 45 | + // there are no major FPs |
| 46 | + suspicious, |
| 47 | + "disallows obtaining raw pointers to temporary values" |
| 48 | +} |
| 49 | +declare_lint_pass!(PtrToTemporary => [PTR_TO_TEMPORARY]); |
| 50 | + |
| 51 | +impl<'tcx> LateLintPass<'tcx> for PtrToTemporary { |
| 52 | + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { |
| 53 | + if in_external_macro(cx.sess(), expr.span) { |
| 54 | + return; |
| 55 | + } |
| 56 | + |
| 57 | + _ = check_for_returning_raw_ptr(cx, expr) |
| 58 | + } |
| 59 | + |
| 60 | + fn check_fn( |
| 61 | + &mut self, |
| 62 | + cx: &LateContext<'tcx>, |
| 63 | + _: FnKind<'_>, |
| 64 | + _: &FnDecl<'_>, |
| 65 | + _: &Body<'_>, |
| 66 | + _: Span, |
| 67 | + def_id: LocalDefId, |
| 68 | + ) { |
| 69 | + let mir = cx.tcx.optimized_mir(def_id); |
| 70 | + |
| 71 | + // Collect all local assignments in this body. This is faster than continuously passing over the |
| 72 | + // body every time we want to get the assignments. |
| 73 | + let mut assignments = LocalAssignmentsVisitor { |
| 74 | + results: FxHashMap::default(), |
| 75 | + }; |
| 76 | + assignments.visit_body(mir); |
| 77 | + |
| 78 | + let mut v = DanglingPtrVisitor { |
| 79 | + cx, |
| 80 | + body: mir, |
| 81 | + results: vec![], |
| 82 | + local_assignments: assignments.results, |
| 83 | + }; |
| 84 | + v.visit_body(mir); |
| 85 | + |
| 86 | + for dangling_ptr_span in v.results { |
| 87 | + // TODO: We need to lint on the call in question instead, so lint attributes work fine. I'm not sure |
| 88 | + // how to though |
| 89 | + span_lint_hir_and_then( |
| 90 | + cx, |
| 91 | + PTR_TO_TEMPORARY, |
| 92 | + cx.tcx.local_def_id_to_hir_id(def_id), |
| 93 | + dangling_ptr_span, |
| 94 | + "calling `TODO` on a temporary value", |
| 95 | + |diag| { |
| 96 | + diag.note( |
| 97 | + "usage of this pointer will cause Undefined Behavior as the temporary will be deallocated at \ |
| 98 | + the end of the statement, yet the pointer will continue pointing to it, resulting in a \ |
| 99 | + dangling pointer", |
| 100 | + ); |
| 101 | + }, |
| 102 | + ); |
| 103 | + } |
| 104 | + } |
| 105 | +} |
| 106 | + |
| 107 | +/// Check for returning raw pointers to temporaries that are not promoted to a constant |
| 108 | +fn check_for_returning_raw_ptr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { |
| 109 | + // Get the final return statement if this is a return statement, or don't lint |
| 110 | + let expr = if let ExprKind::Ret(Some(expr)) = expr.kind { |
| 111 | + expr |
| 112 | + } else if let OwnerNode::Item(parent) = cx.tcx.hir().owner(cx.tcx.hir().get_parent_item(expr.hir_id)) |
| 113 | + && let ItemKind::Fn(_, _, body) = parent.kind |
| 114 | + && let block = cx.tcx.hir().body(body).value |
| 115 | + && let ExprKind::Block(block, _) = block.kind |
| 116 | + && let Some(final_block_expr) = block.expr |
| 117 | + && final_block_expr.hir_id == expr.hir_id |
| 118 | + { |
| 119 | + expr |
| 120 | + } else { |
| 121 | + return false; |
| 122 | + }; |
| 123 | + |
| 124 | + if let ExprKind::Cast(cast_expr, _) = expr.kind |
| 125 | + && let ExprKind::AddrOf(BorrowKind::Ref, _, e) = cast_expr.kind |
| 126 | + && !is_promotable(cx, e) |
| 127 | + && !is_from_proc_macro(cx, expr) |
| 128 | + { |
| 129 | + span_lint_and_note( |
| 130 | + cx, |
| 131 | + PTR_TO_TEMPORARY, |
| 132 | + expr.span, |
| 133 | + "returning a raw pointer to a temporary value that cannot be promoted to a constant", |
| 134 | + None, |
| 135 | + "usage of this pointer by callers will cause Undefined Behavior as the temporary will be deallocated at \ |
| 136 | + the end of the statement, yet the pointer will continue pointing to it, resulting in a dangling pointer", |
| 137 | + ); |
| 138 | + |
| 139 | + return true; |
| 140 | + } |
| 141 | + |
| 142 | + false |
| 143 | +} |
| 144 | + |
| 145 | +struct LocalAssignmentsVisitor { |
| 146 | + results: FxHashMap<Local, Vec<Location>>, |
| 147 | +} |
| 148 | + |
| 149 | +impl Visitor<'_> for LocalAssignmentsVisitor { |
| 150 | + fn visit_place(&mut self, place: &Place<'_>, ctxt: PlaceContext, loc: Location) { |
| 151 | + if matches!( |
| 152 | + ctxt, |
| 153 | + PlaceContext::MutatingUse( |
| 154 | + MutatingUseContext::Store | MutatingUseContext::Call | MutatingUseContext::Borrow |
| 155 | + ) |
| 156 | + ) { |
| 157 | + self.results.entry(place.local).or_insert(vec![]).push(loc); |
| 158 | + } |
| 159 | + } |
| 160 | +} |
| 161 | + |
| 162 | +struct DanglingPtrVisitor<'a, 'tcx> { |
| 163 | + cx: &'a LateContext<'tcx>, |
| 164 | + body: &'tcx mir::Body<'tcx>, |
| 165 | + local_assignments: FxHashMap<Local, Vec<Location>>, |
| 166 | + results: Vec<Span>, |
| 167 | +} |
| 168 | + |
| 169 | +impl<'tcx> Visitor<'tcx> for DanglingPtrVisitor<'_, 'tcx> { |
| 170 | + fn visit_basic_block_data(&mut self, _: BasicBlock, data: &BasicBlockData<'tcx>) { |
| 171 | + let Self { |
| 172 | + cx, |
| 173 | + body, |
| 174 | + local_assignments, |
| 175 | + results, |
| 176 | + } = self; |
| 177 | + |
| 178 | + if let Some(term) = &data.terminator |
| 179 | + && let TerminatorKind::Call { |
| 180 | + func, |
| 181 | + args, |
| 182 | + destination, |
| 183 | + target: Some(target), |
| 184 | + call_source: CallSource::Normal, |
| 185 | + .. |
| 186 | + } = &term.kind |
| 187 | + && destination.ty(&body.local_decls, cx.tcx).ty.is_unsafe_ptr() |
| 188 | + && let [recv] = args.as_slice() |
| 189 | + && let Some(recv) = recv.place() |
| 190 | + && let Some((def_id, _)) = func.const_fn_def() |
| 191 | + && let [.., last] = &*cx.tcx.def_path(def_id).data |
| 192 | + && let Some(ident) = last.data.get_opt_name() |
| 193 | + && (ident == sym::as_ptr || ident == sym!(as_mut_ptr)) |
| 194 | + { |
| 195 | + let mut actual_recv = recv; |
| 196 | + let mut done = false; |
| 197 | + let mut is_const = false; |
| 198 | + // TODO: I really don't like this, at the very least we have to extract this. |
| 199 | + while let Some(recv) = local_assignments.get(&actual_recv.local).and_then(|assignments| { |
| 200 | + if let [loc] = assignments.as_slice() { |
| 201 | + match location_to_node(body, *loc) { |
| 202 | + StatementOrTerminator::Statement(stmt) |
| 203 | + if let StatementKind::Assign(box (_, rvalue)) = &stmt.kind => { |
| 204 | + match rvalue { |
| 205 | + Rvalue::Use(op) => { |
| 206 | + if matches!(op, Operand::Constant(_)) { |
| 207 | + is_const = true; |
| 208 | + } |
| 209 | + op.place() |
| 210 | + }, |
| 211 | + Rvalue::Ref(_, _, place) => { |
| 212 | + if !place.has_deref() { |
| 213 | + done = true; |
| 214 | + } |
| 215 | + Some(*place) |
| 216 | + }, |
| 217 | + Rvalue::Cast(_, op, _) => { |
| 218 | + if matches!(op, Operand::Constant(_)) { |
| 219 | + is_const = true; |
| 220 | + } |
| 221 | + op.place() |
| 222 | + }, |
| 223 | + _ => None, |
| 224 | + } |
| 225 | + }, |
| 226 | + StatementOrTerminator::Terminator(term) if let TerminatorKind::Call { args, .. } = &term.kind |
| 227 | + && let [arg] = args.as_slice() => |
| 228 | + { |
| 229 | + arg.place() |
| 230 | + } |
| 231 | + _ => None, |
| 232 | + } |
| 233 | + } else { |
| 234 | + None |
| 235 | + } |
| 236 | + }) { |
| 237 | + if recv == actual_recv { |
| 238 | + break; |
| 239 | + } |
| 240 | + |
| 241 | + actual_recv = recv; |
| 242 | + |
| 243 | + if done { |
| 244 | + break; |
| 245 | + } |
| 246 | + } |
| 247 | + |
| 248 | + if is_const { |
| 249 | + return; |
| 250 | + } |
| 251 | + |
| 252 | + if let Some([assign]) = local_assignments.get(&actual_recv.local).map(Vec::as_slice) |
| 253 | + && let StatementOrTerminator::Terminator(term) = location_to_node(body, *assign) |
| 254 | + && let TerminatorKind::Call { |
| 255 | + func, |
| 256 | + call_source: CallSource::Normal, |
| 257 | + .. |
| 258 | + } = &term.kind |
| 259 | + && let Some((def_id, _)) = func.const_fn_def() |
| 260 | + && let ty = cx.tcx.fn_sig(def_id).subst_identity().skip_binder().output() |
| 261 | + && ((!ty.has_late_bound_vars() && ty.is_ref()) |
| 262 | + || matches!(ty.kind(), ty::Ref(region, _, _) if region.is_static())) |
| 263 | + { |
| 264 | + return; |
| 265 | + } |
| 266 | + |
| 267 | + check_for_dangling( |
| 268 | + cx, |
| 269 | + body, |
| 270 | + *target, |
| 271 | + &actual_recv, |
| 272 | + destination, |
| 273 | + term.source_info.span, |
| 274 | + results, |
| 275 | + ); |
| 276 | + } |
| 277 | + } |
| 278 | +} |
| 279 | + |
| 280 | +fn check_for_dangling<'tcx>( |
| 281 | + _cx: &LateContext<'tcx>, |
| 282 | + body: &mir::Body<'tcx>, |
| 283 | + bb: BasicBlock, |
| 284 | + recv: &Place<'tcx>, |
| 285 | + ptr: &Place<'_>, |
| 286 | + span: Span, |
| 287 | + results: &mut Vec<Span>, |
| 288 | +) { |
| 289 | + let data = &body.basic_blocks[bb]; |
| 290 | + let mut recv_dead = false; |
| 291 | + |
| 292 | + // If there's a `Drop`, we must include the statements in its target so we don't miss any |
| 293 | + // potentially important `StorageDead`s. |
| 294 | + let rest = vec![]; |
| 295 | + let rest = if let Some(term) = &data.terminator && let TerminatorKind::Drop { place, target, .. } = term.kind { |
| 296 | + debug_assert_eq!(place, *recv, "dropped place is not receiver"); |
| 297 | + |
| 298 | + &body.basic_blocks[target].statements |
| 299 | + } else { |
| 300 | + &rest |
| 301 | + }; |
| 302 | + |
| 303 | + for dead_local in data.statements.iter().chain(rest).filter_map(|stmt| { |
| 304 | + if let StatementKind::StorageDead(local) = stmt.kind { |
| 305 | + return Some(local); |
| 306 | + } |
| 307 | + |
| 308 | + None |
| 309 | + }) { |
| 310 | + match (dead_local == recv.local, dead_local == ptr.local) { |
| 311 | + (true, false) => recv_dead = true, |
| 312 | + (false, true) if recv_dead => { |
| 313 | + results.push(span); |
| 314 | + }, |
| 315 | + _ => continue, |
| 316 | + } |
| 317 | + } |
| 318 | +} |
0 commit comments