Don't emit null pointer lint for raw ref of null deref

This commit is contained in:
Michael Goulet 2024-08-18 12:33:21 -04:00
parent f2a80a0f89
commit 367183bc0c
7 changed files with 28 additions and 41 deletions

View File

@ -2657,8 +2657,8 @@ fn ty_find_init_error<'tcx>(
/// ///
/// ### Explanation /// ### Explanation
/// ///
/// Dereferencing a null pointer causes [undefined behavior] even as a place expression, /// Dereferencing a null pointer causes [undefined behavior] if it is accessed
/// like `&*(0 as *const i32)` or `addr_of!(*(0 as *const i32))`. /// (loaded from or stored to).
/// ///
/// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
pub DEREF_NULLPTR, pub DEREF_NULLPTR,
@ -2673,14 +2673,14 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &hir::Expr<'_>) {
/// test if expression is a null ptr /// test if expression is a null ptr
fn is_null_ptr(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { fn is_null_ptr(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
match &expr.kind { match &expr.kind {
rustc_hir::ExprKind::Cast(expr, ty) => { hir::ExprKind::Cast(expr, ty) => {
if let rustc_hir::TyKind::Ptr(_) = ty.kind { if let hir::TyKind::Ptr(_) = ty.kind {
return is_zero(expr) || is_null_ptr(cx, expr); return is_zero(expr) || is_null_ptr(cx, expr);
} }
} }
// check for call to `core::ptr::null` or `core::ptr::null_mut` // check for call to `core::ptr::null` or `core::ptr::null_mut`
rustc_hir::ExprKind::Call(path, _) => { hir::ExprKind::Call(path, _) => {
if let rustc_hir::ExprKind::Path(ref qpath) = path.kind { if let hir::ExprKind::Path(ref qpath) = path.kind {
if let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id() { if let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id() {
return matches!( return matches!(
cx.tcx.get_diagnostic_name(def_id), cx.tcx.get_diagnostic_name(def_id),
@ -2697,7 +2697,7 @@ fn is_null_ptr(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
/// test if expression is the literal `0` /// test if expression is the literal `0`
fn is_zero(expr: &hir::Expr<'_>) -> bool { fn is_zero(expr: &hir::Expr<'_>) -> bool {
match &expr.kind { match &expr.kind {
rustc_hir::ExprKind::Lit(lit) => { hir::ExprKind::Lit(lit) => {
if let LitKind::Int(a, _) = lit.node { if let LitKind::Int(a, _) = lit.node {
return a == 0; return a == 0;
} }
@ -2707,8 +2707,16 @@ fn is_zero(expr: &hir::Expr<'_>) -> bool {
false false
} }
if let rustc_hir::ExprKind::Unary(rustc_hir::UnOp::Deref, expr_deref) = expr.kind { if let hir::ExprKind::Unary(hir::UnOp::Deref, expr_deref) = expr.kind
if is_null_ptr(cx, expr_deref) { && is_null_ptr(cx, expr_deref)
{
if let hir::Node::Expr(hir::Expr {
kind: hir::ExprKind::AddrOf(hir::BorrowKind::Raw, ..),
..
}) = cx.tcx.parent_hir_node(expr.hir_id)
{
// `&raw *NULL` is ok.
} else {
cx.emit_span_lint(DEREF_NULLPTR, expr.span, BuiltinDerefNullptr { cx.emit_span_lint(DEREF_NULLPTR, expr.span, BuiltinDerefNullptr {
label: expr.span, label: expr.span,
}); });

View File

@ -509,8 +509,6 @@ fn visit_expr(&mut self, expr: &'a Expr<'tcx>) {
} }
ExprKind::RawBorrow { arg, .. } => { ExprKind::RawBorrow { arg, .. } => {
if let ExprKind::Scope { value: arg, .. } = self.thir[arg].kind if let ExprKind::Scope { value: arg, .. } = self.thir[arg].kind
// THIR desugars UNSAFE_STATIC into *UNSAFE_STATIC_REF, where
// UNSAFE_STATIC_REF holds the addr of the UNSAFE_STATIC, so: take two steps
&& let ExprKind::Deref { arg } = self.thir[arg].kind && let ExprKind::Deref { arg } = self.thir[arg].kind
{ {
// Taking a raw ref to a deref place expr is always safe. // Taking a raw ref to a deref place expr is always safe.

View File

@ -27,9 +27,9 @@ fn f() {
let ub = &*ptr::null_mut::<i32>(); let ub = &*ptr::null_mut::<i32>();
//~^ ERROR dereferencing a null pointer //~^ ERROR dereferencing a null pointer
ptr::addr_of!(*ptr::null::<i32>()); ptr::addr_of!(*ptr::null::<i32>());
//~^ ERROR dereferencing a null pointer // ^^ OKAY
ptr::addr_of_mut!(*ptr::null_mut::<i32>()); ptr::addr_of_mut!(*ptr::null_mut::<i32>());
//~^ ERROR dereferencing a null pointer // ^^ OKAY
let offset = ptr::addr_of!((*ptr::null::<Struct>()).field); let offset = ptr::addr_of!((*ptr::null::<Struct>()).field);
//~^ ERROR dereferencing a null pointer //~^ ERROR dereferencing a null pointer
} }

View File

@ -46,23 +46,11 @@ error: dereferencing a null pointer
LL | let ub = &*ptr::null_mut::<i32>(); LL | let ub = &*ptr::null_mut::<i32>();
| ^^^^^^^^^^^^^^^^^^^^^^^ this code causes undefined behavior when executed | ^^^^^^^^^^^^^^^^^^^^^^^ this code causes undefined behavior when executed
error: dereferencing a null pointer
--> $DIR/lint-deref-nullptr.rs:29:23
|
LL | ptr::addr_of!(*ptr::null::<i32>());
| ^^^^^^^^^^^^^^^^^^^ this code causes undefined behavior when executed
error: dereferencing a null pointer
--> $DIR/lint-deref-nullptr.rs:31:27
|
LL | ptr::addr_of_mut!(*ptr::null_mut::<i32>());
| ^^^^^^^^^^^^^^^^^^^^^^^ this code causes undefined behavior when executed
error: dereferencing a null pointer error: dereferencing a null pointer
--> $DIR/lint-deref-nullptr.rs:33:36 --> $DIR/lint-deref-nullptr.rs:33:36
| |
LL | let offset = ptr::addr_of!((*ptr::null::<Struct>()).field); LL | let offset = ptr::addr_of!((*ptr::null::<Struct>()).field);
| ^^^^^^^^^^^^^^^^^^^^^^^^ this code causes undefined behavior when executed | ^^^^^^^^^^^^^^^^^^^^^^^^ this code causes undefined behavior when executed
error: aborting due to 10 previous errors error: aborting due to 8 previous errors

View File

@ -1,13 +1,14 @@
//@ check-pass //@ check-pass
use std::ptr; use std::ptr;
// This code should remain unsafe because of the two unsafe operations here,
// even if in a hypothetical future we deem all &raw (const|mut) *ptr exprs safe.
static mut BYTE: u8 = 0; static mut BYTE: u8 = 0;
static mut BYTE_PTR: *mut u8 = ptr::addr_of_mut!(BYTE); static mut BYTE_PTR: *mut u8 = ptr::addr_of_mut!(BYTE);
// This code should remain unsafe because reading from a static mut is *always* unsafe.
// An unsafe static's ident is a place expression in its own right, so despite the above being safe // An unsafe static's ident is a place expression in its own right, so despite the above being safe
// (it's fine to create raw refs to places!) the following derefs the ptr before creating its ref // (it's fine to create raw refs to places!) the following *reads* from the static mut place before
// derefing it explicitly with the `*` below.
static mut DEREF_BYTE_PTR: *mut u8 = unsafe { ptr::addr_of_mut!(*BYTE_PTR) }; static mut DEREF_BYTE_PTR: *mut u8 = unsafe { ptr::addr_of_mut!(*BYTE_PTR) };
fn main() { fn main() {

View File

@ -1,10 +1,10 @@
use std::ptr; use std::ptr;
// This code should remain unsafe because of the two unsafe operations here,
// even if in a hypothetical future we deem all &raw (const|mut) *ptr exprs safe.
static mut BYTE: u8 = 0; static mut BYTE: u8 = 0;
static mut BYTE_PTR: *mut u8 = ptr::addr_of_mut!(BYTE); static mut BYTE_PTR: *mut u8 = ptr::addr_of_mut!(BYTE);
// This code should remain unsafe because reading from a static mut is *always* unsafe.
// An unsafe static's ident is a place expression in its own right, so despite the above being safe // An unsafe static's ident is a place expression in its own right, so despite the above being safe
// (it's fine to create raw refs to places!) the following derefs the ptr before creating its ref! // (it's fine to create raw refs to places!) the following derefs the ptr before creating its ref!
static mut DEREF_BYTE_PTR: *mut u8 = ptr::addr_of_mut!(*BYTE_PTR); static mut DEREF_BYTE_PTR: *mut u8 = ptr::addr_of_mut!(*BYTE_PTR);

View File

@ -1,11 +1,3 @@
error[E0133]: dereference of raw pointer is unsafe and requires unsafe function or block
--> $DIR/raw-ref-deref-without-unsafe.rs:10:56
|
LL | static mut DEREF_BYTE_PTR: *mut u8 = ptr::addr_of_mut!(*BYTE_PTR);
| ^^^^^^^^^ dereference of raw pointer
|
= note: raw pointers may be null, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior
error[E0133]: use of mutable static is unsafe and requires unsafe function or block error[E0133]: use of mutable static is unsafe and requires unsafe function or block
--> $DIR/raw-ref-deref-without-unsafe.rs:10:57 --> $DIR/raw-ref-deref-without-unsafe.rs:10:57
| |