Skip to content

Commit

Permalink
Lint against &T to &mut T and &T to &UnsafeCell<T> transmutes
Browse files Browse the repository at this point in the history
This adds the lint against `&T`->`&UnsafeCell<T>` transmutes, and also check in struct fields, and reference casts (`&*(&a as *const u8 as *const UnsafeCell<u8>)`).

The code is quite complex; I've tried my best to simplify and comment it.

This is missing one parts: array transmutes. When transmuting an array, this only consider the first element. The reason for that is that the code is already quite complex, and I didn't want to complicate it more.
This catches the most common pattern of transmuting an array into an array of the same length with type of the same size; more complex cases are likely not properly handled.
We could take a bigger sample, for example the first and last elements to increase the chance that the lint will catch mistakes, but then the runtime complexity becomes exponential with the nesting of the arrays (`[[[[[T; 2]; 2]; 2]; 2]; 2]` has complexity of O(2**5), for instance).
  • Loading branch information
ChayimFriedman2 committed Dec 7, 2024
1 parent 7e565cc commit 2ea0578
Show file tree
Hide file tree
Showing 20 changed files with 1,126 additions and 93 deletions.
6 changes: 6 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ lint_builtin_missing_doc = missing documentation for {$article} {$desc}
lint_builtin_mutable_transmutes =
transmuting &T to &mut T is undefined behavior, even if the reference is unused, consider instead using an UnsafeCell
.note = transmute from `{$from}` to `{$to}`
lint_builtin_no_mangle_fn = declaration of a `no_mangle` function
lint_builtin_no_mangle_generic = functions generic over types or consts must be mangled
Expand Down Expand Up @@ -888,6 +889,11 @@ lint_unsafe_attr_outside_unsafe = unsafe attribute used without unsafe
.label = usage of unsafe attribute
lint_unsafe_attr_outside_unsafe_suggestion = wrap the attribute in `unsafe(...)`
lint_unsafe_cell_transmutes =
transmuting &T to &UnsafeCell<T> is error-prone, rarely intentional and may cause undefined behavior
.label = transmuting happend here
.note = transmute from `{$from}` to `{$to}`
lint_unsupported_group = `{$lint_group}` lint group is not supported with ´--force-warn´
lint_untranslatable_diag = diagnostics should be created using translatable messages
Expand Down
78 changes: 5 additions & 73 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,11 @@ use crate::lints::{
BuiltinEllipsisInclusiveRangePatternsLint, BuiltinExplicitOutlives,
BuiltinExplicitOutlivesSuggestion, BuiltinFeatureIssueNote, BuiltinIncompleteFeatures,
BuiltinIncompleteFeaturesHelp, BuiltinInternalFeatures, BuiltinKeywordIdents,
BuiltinMissingCopyImpl, BuiltinMissingDebugImpl, BuiltinMissingDoc, BuiltinMutablesTransmutes,
BuiltinNoMangleGeneric, BuiltinNonShorthandFieldPatterns, BuiltinSpecialModuleNameUsed,
BuiltinTrivialBounds, BuiltinTypeAliasBounds, BuiltinUngatedAsyncFnTrackCaller,
BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub,
BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub,
BuiltinWhileTrue, InvalidAsmLabel,
BuiltinMissingCopyImpl, BuiltinMissingDebugImpl, BuiltinMissingDoc, BuiltinNoMangleGeneric,
BuiltinNonShorthandFieldPatterns, BuiltinSpecialModuleNameUsed, BuiltinTrivialBounds,
BuiltinTypeAliasBounds, BuiltinUngatedAsyncFnTrackCaller, BuiltinUnpermittedTypeInit,
BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub, BuiltinUnsafe, BuiltinUnstableFeatures,
BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub, BuiltinWhileTrue, InvalidAsmLabel,
};
use crate::nonstandard_style::{MethodLateContext, method_context};
use crate::{
Expand Down Expand Up @@ -1076,72 +1075,6 @@ impl<'tcx> LateLintPass<'tcx> for InvalidNoMangleItems {
}
}

declare_lint! {
/// The `mutable_transmutes` lint catches transmuting from `&T` to `&mut
/// T` because it is [undefined behavior].
///
/// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
///
/// ### Example
///
/// ```rust,compile_fail
/// unsafe {
/// let y = std::mem::transmute::<&i32, &mut i32>(&5);
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Certain assumptions are made about aliasing of data, and this transmute
/// violates those assumptions. Consider using [`UnsafeCell`] instead.
///
/// [`UnsafeCell`]: https://doc.rust-lang.org/std/cell/struct.UnsafeCell.html
MUTABLE_TRANSMUTES,
Deny,
"transmuting &T to &mut T is undefined behavior, even if the reference is unused"
}

declare_lint_pass!(MutableTransmutes => [MUTABLE_TRANSMUTES]);

impl<'tcx> LateLintPass<'tcx> for MutableTransmutes {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) {
if let Some((&ty::Ref(_, _, from_mutbl), &ty::Ref(_, _, to_mutbl))) =
get_transmute_from_to(cx, expr).map(|(ty1, ty2)| (ty1.kind(), ty2.kind()))
{
if from_mutbl < to_mutbl {
cx.emit_span_lint(MUTABLE_TRANSMUTES, expr.span, BuiltinMutablesTransmutes);
}
}

fn get_transmute_from_to<'tcx>(
cx: &LateContext<'tcx>,
expr: &hir::Expr<'_>,
) -> Option<(Ty<'tcx>, Ty<'tcx>)> {
let def = if let hir::ExprKind::Path(ref qpath) = expr.kind {
cx.qpath_res(qpath, expr.hir_id)
} else {
return None;
};
if let Res::Def(DefKind::Fn, did) = def {
if !def_id_is_transmute(cx, did) {
return None;
}
let sig = cx.typeck_results().node_type(expr.hir_id).fn_sig(cx.tcx);
let from = sig.inputs().skip_binder()[0];
let to = sig.output().skip_binder();
return Some((from, to));
}
None
}

fn def_id_is_transmute(cx: &LateContext<'_>, def_id: DefId) -> bool {
cx.tcx.is_intrinsic(def_id, sym::transmute)
}
}
}

declare_lint! {
/// The `unstable_features` lint detects uses of `#![feature]`.
///
Expand Down Expand Up @@ -1595,7 +1528,6 @@ declare_lint_pass!(
UNUSED_DOC_COMMENTS,
NO_MANGLE_CONST_ITEMS,
NO_MANGLE_GENERIC_ITEMS,
MUTABLE_TRANSMUTES,
UNSTABLE_FEATURES,
UNREACHABLE_PUB,
TYPE_ALIAS_BOUNDS,
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ mod lints;
mod macro_expr_fragment_specifier_2024_migration;
mod map_unit_fn;
mod multiple_supertrait_upcastable;
mod mutable_transmutes;
mod non_ascii_idents;
mod non_fmt_panic;
mod non_local_def;
Expand Down Expand Up @@ -98,6 +99,7 @@ use let_underscore::*;
use macro_expr_fragment_specifier_2024_migration::*;
use map_unit_fn::*;
use multiple_supertrait_upcastable::*;
use mutable_transmutes::*;
use non_ascii_idents::*;
use non_fmt_panic::NonPanicFmt;
use non_local_def::*;
Expand Down
35 changes: 33 additions & 2 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use std::num::NonZero;
use rustc_errors::codes::*;
use rustc_errors::{
Applicability, Diag, DiagArgValue, DiagMessage, DiagStyledString, ElidedLifetimeInPathSubdiag,
EmissionGuarantee, LintDiagnostic, MultiSpan, SubdiagMessageOp, Subdiagnostic, SuggestionStyle,
EmissionGuarantee, IntoDiagArg, LintDiagnostic, MultiSpan, SubdiagMessageOp, Subdiagnostic,
SuggestionStyle,
};
use rustc_hir::def::Namespace;
use rustc_hir::def_id::DefId;
Expand Down Expand Up @@ -224,9 +225,39 @@ pub(crate) struct BuiltinConstNoMangle {
pub suggestion: Span,
}

// This would be more convenient as `from: String` and `to: String`, but then we'll ICE for formatting a `Ty`
// when the lint is `#[allow]`ed.
pub(crate) struct TransmuteBreadcrumbs<'a> {
pub before_ty: &'static str,
pub ty: Ty<'a>,
pub after_ty: String,
}

impl IntoDiagArg for TransmuteBreadcrumbs<'_> {
fn into_diag_arg(self) -> DiagArgValue {
format!("{}{}{}", self.before_ty, self.ty, self.after_ty).into_diag_arg()
}
}

// mutable_transmutes.rs
#[derive(LintDiagnostic)]
#[diag(lint_builtin_mutable_transmutes)]
pub(crate) struct BuiltinMutablesTransmutes;
#[note]
pub(crate) struct BuiltinMutablesTransmutes<'a> {
pub from: TransmuteBreadcrumbs<'a>,
pub to: TransmuteBreadcrumbs<'a>,
}

// mutable_transmutes.rs
#[derive(LintDiagnostic)]
#[diag(lint_unsafe_cell_transmutes)]
#[note]
pub(crate) struct UnsafeCellTransmutes<'a> {
#[label]
pub orig_cast: Option<Span>,
pub from: TransmuteBreadcrumbs<'a>,
pub to: TransmuteBreadcrumbs<'a>,
}

#[derive(LintDiagnostic)]
#[diag(lint_builtin_unstable_features)]
Expand Down
Loading

0 comments on commit 2ea0578

Please sign in to comment.