Skip to content

Commit 2decadb

Browse files
ChayimFriedman2GrigorenkoPV
authored andcommitted
Lint against &T to &mut T and &T to &UnsafeCell<T> transmutes
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).
1 parent 1e1a394 commit 2decadb

20 files changed

+1134
-96
lines changed

compiler/rustc_lint/messages.ftl

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,8 @@ lint_builtin_missing_debug_impl =
114114
lint_builtin_missing_doc = missing documentation for {$article} {$desc}
115115
116116
lint_builtin_mutable_transmutes =
117-
transmuting &T to &mut T is undefined behavior, even if the reference is unused, consider instead using an UnsafeCell
117+
transmuting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell`
118+
.note = transmute from `{$from}` to `{$to}`
118119
119120
lint_builtin_no_mangle_fn = declaration of a `no_mangle` function
120121
lint_builtin_no_mangle_generic = functions generic over types or consts must be mangled
@@ -900,6 +901,11 @@ lint_unsafe_attr_outside_unsafe = unsafe attribute used without unsafe
900901
.label = usage of unsafe attribute
901902
lint_unsafe_attr_outside_unsafe_suggestion = wrap the attribute in `unsafe(...)`
902903
904+
lint_unsafe_cell_transmutes =
905+
converting `&T` to `&UnsafeCell<T>` is error-prone, rarely intentional and may cause undefined behavior
906+
.label = conversion happened here
907+
.note = conversion from `{$from}` to `{$to}`
908+
903909
lint_unsupported_group = `{$lint_group}` lint group is not supported with ´--force-warn´
904910
905911
lint_untranslatable_diag = diagnostics should be created using translatable messages

compiler/rustc_lint/src/builtin.rs

Lines changed: 5 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,11 @@ use crate::lints::{
5555
BuiltinExplicitOutlives, BuiltinExplicitOutlivesSuggestion, BuiltinFeatureIssueNote,
5656
BuiltinIncompleteFeatures, BuiltinIncompleteFeaturesHelp, BuiltinInternalFeatures,
5757
BuiltinKeywordIdents, BuiltinMissingCopyImpl, BuiltinMissingDebugImpl, BuiltinMissingDoc,
58-
BuiltinMutablesTransmutes, BuiltinNoMangleGeneric, BuiltinNonShorthandFieldPatterns,
59-
BuiltinSpecialModuleNameUsed, BuiltinTrivialBounds, BuiltinTypeAliasBounds,
60-
BuiltinUngatedAsyncFnTrackCaller, BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub,
61-
BuiltinUnreachablePub, BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment,
62-
BuiltinUnusedDocCommentSub, BuiltinWhileTrue, InvalidAsmLabel,
58+
BuiltinNoMangleGeneric, BuiltinNonShorthandFieldPatterns, BuiltinSpecialModuleNameUsed,
59+
BuiltinTrivialBounds, BuiltinTypeAliasBounds, BuiltinUngatedAsyncFnTrackCaller,
60+
BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub,
61+
BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub,
62+
BuiltinWhileTrue, InvalidAsmLabel,
6363
};
6464
use crate::{
6565
EarlyContext, EarlyLintPass, LateContext, LateLintPass, Level, LintContext,
@@ -1034,72 +1034,6 @@ impl<'tcx> LateLintPass<'tcx> for InvalidNoMangleItems {
10341034
}
10351035
}
10361036

1037-
declare_lint! {
1038-
/// The `mutable_transmutes` lint catches transmuting from `&T` to `&mut
1039-
/// T` because it is [undefined behavior].
1040-
///
1041-
/// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
1042-
///
1043-
/// ### Example
1044-
///
1045-
/// ```rust,compile_fail
1046-
/// unsafe {
1047-
/// let y = std::mem::transmute::<&i32, &mut i32>(&5);
1048-
/// }
1049-
/// ```
1050-
///
1051-
/// {{produces}}
1052-
///
1053-
/// ### Explanation
1054-
///
1055-
/// Certain assumptions are made about aliasing of data, and this transmute
1056-
/// violates those assumptions. Consider using [`UnsafeCell`] instead.
1057-
///
1058-
/// [`UnsafeCell`]: https://doc.rust-lang.org/std/cell/struct.UnsafeCell.html
1059-
MUTABLE_TRANSMUTES,
1060-
Deny,
1061-
"transmuting &T to &mut T is undefined behavior, even if the reference is unused"
1062-
}
1063-
1064-
declare_lint_pass!(MutableTransmutes => [MUTABLE_TRANSMUTES]);
1065-
1066-
impl<'tcx> LateLintPass<'tcx> for MutableTransmutes {
1067-
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) {
1068-
if let Some((&ty::Ref(_, _, from_mutbl), &ty::Ref(_, _, to_mutbl))) =
1069-
get_transmute_from_to(cx, expr).map(|(ty1, ty2)| (ty1.kind(), ty2.kind()))
1070-
{
1071-
if from_mutbl < to_mutbl {
1072-
cx.emit_span_lint(MUTABLE_TRANSMUTES, expr.span, BuiltinMutablesTransmutes);
1073-
}
1074-
}
1075-
1076-
fn get_transmute_from_to<'tcx>(
1077-
cx: &LateContext<'tcx>,
1078-
expr: &hir::Expr<'_>,
1079-
) -> Option<(Ty<'tcx>, Ty<'tcx>)> {
1080-
let def = if let hir::ExprKind::Path(ref qpath) = expr.kind {
1081-
cx.qpath_res(qpath, expr.hir_id)
1082-
} else {
1083-
return None;
1084-
};
1085-
if let Res::Def(DefKind::Fn, did) = def {
1086-
if !def_id_is_transmute(cx, did) {
1087-
return None;
1088-
}
1089-
let sig = cx.typeck_results().node_type(expr.hir_id).fn_sig(cx.tcx);
1090-
let from = sig.inputs().skip_binder()[0];
1091-
let to = sig.output().skip_binder();
1092-
return Some((from, to));
1093-
}
1094-
None
1095-
}
1096-
1097-
fn def_id_is_transmute(cx: &LateContext<'_>, def_id: DefId) -> bool {
1098-
cx.tcx.is_intrinsic(def_id, sym::transmute)
1099-
}
1100-
}
1101-
}
1102-
11031037
declare_lint! {
11041038
/// The `unstable_features` lint detects uses of `#![feature]`.
11051039
///
@@ -1625,7 +1559,6 @@ declare_lint_pass!(
16251559
UNUSED_DOC_COMMENTS,
16261560
NO_MANGLE_CONST_ITEMS,
16271561
NO_MANGLE_GENERIC_ITEMS,
1628-
MUTABLE_TRANSMUTES,
16291562
UNSTABLE_FEATURES,
16301563
UNREACHABLE_PUB,
16311564
TYPE_ALIAS_BOUNDS,

compiler/rustc_lint/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ mod lints;
6060
mod macro_expr_fragment_specifier_2024_migration;
6161
mod map_unit_fn;
6262
mod multiple_supertrait_upcastable;
63+
mod mutable_transmutes;
6364
mod non_ascii_idents;
6465
mod non_fmt_panic;
6566
mod non_local_def;
@@ -101,6 +102,7 @@ use lifetime_syntax::*;
101102
use macro_expr_fragment_specifier_2024_migration::*;
102103
use map_unit_fn::*;
103104
use multiple_supertrait_upcastable::*;
105+
use mutable_transmutes::*;
104106
use non_ascii_idents::*;
105107
use non_fmt_panic::NonPanicFmt;
106108
use non_local_def::*;

compiler/rustc_lint/src/lints.rs

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::num::NonZero;
66
use rustc_errors::codes::*;
77
use rustc_errors::{
88
Applicability, Diag, DiagArgValue, DiagMessage, DiagStyledString, ElidedLifetimeInPathSubdiag,
9-
EmissionGuarantee, LintDiagnostic, MultiSpan, Subdiagnostic, SuggestionStyle,
9+
EmissionGuarantee, IntoDiagArg, LintDiagnostic, MultiSpan, Subdiagnostic, SuggestionStyle,
1010
};
1111
use rustc_hir as hir;
1212
use rustc_hir::def_id::DefId;
@@ -233,9 +233,41 @@ pub(crate) struct BuiltinConstNoMangle {
233233
pub suggestion: Span,
234234
}
235235

236+
// This would be more convenient as `from: String` and `to: String` instead of `from` and `to`
237+
// being `ConversionBreadcrumbs`, but that'll mean we'll have to format the `Ty`s in the lint code,
238+
// and then we'll ICE when the lint is allowed, because formatting a `Ty` is expensive and so the
239+
// compiler panics if it is done when there aren't diagnostics.
240+
pub(crate) struct ConversionBreadcrumbs<'a> {
241+
pub before_ty: &'static str,
242+
pub ty: Ty<'a>,
243+
pub after_ty: String,
244+
}
245+
246+
impl IntoDiagArg for ConversionBreadcrumbs<'_> {
247+
fn into_diag_arg(self, path: &mut Option<std::path::PathBuf>) -> DiagArgValue {
248+
format!("{}{}{}", self.before_ty, self.ty, self.after_ty).into_diag_arg(path)
249+
}
250+
}
251+
252+
// mutable_transmutes.rs
236253
#[derive(LintDiagnostic)]
237254
#[diag(lint_builtin_mutable_transmutes)]
238-
pub(crate) struct BuiltinMutablesTransmutes;
255+
#[note]
256+
pub(crate) struct BuiltinMutablesTransmutes<'a> {
257+
pub from: ConversionBreadcrumbs<'a>,
258+
pub to: ConversionBreadcrumbs<'a>,
259+
}
260+
261+
// mutable_transmutes.rs
262+
#[derive(LintDiagnostic)]
263+
#[diag(lint_unsafe_cell_transmutes)]
264+
#[note]
265+
pub(crate) struct UnsafeCellTransmutes<'a> {
266+
#[label]
267+
pub orig_cast: Option<Span>,
268+
pub from: ConversionBreadcrumbs<'a>,
269+
pub to: ConversionBreadcrumbs<'a>,
270+
}
239271

240272
#[derive(LintDiagnostic)]
241273
#[diag(lint_builtin_unstable_features)]

0 commit comments

Comments
 (0)