Skip to content

Commit 0b550dd

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 1ce9c97 commit 0b550dd

20 files changed

+1130
-92
lines changed

compiler/rustc_lint/messages.ftl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ lint_builtin_missing_doc = missing documentation for {$article} {$desc}
121121
122122
lint_builtin_mutable_transmutes =
123123
transmuting &T to &mut T is undefined behavior, even if the reference is unused, consider instead using an UnsafeCell
124+
.note = transmute from `{$from}` to `{$to}`
124125
125126
lint_builtin_no_mangle_fn = declaration of a `no_mangle` function
126127
lint_builtin_no_mangle_generic = functions generic over types or consts must be mangled
@@ -934,6 +935,11 @@ lint_unsafe_attr_outside_unsafe = unsafe attribute used without unsafe
934935
.label = usage of unsafe attribute
935936
lint_unsafe_attr_outside_unsafe_suggestion = wrap the attribute in `unsafe(...)`
936937
938+
lint_unsafe_cell_transmutes =
939+
converting &T to &UnsafeCell<T> is error-prone, rarely intentional and may cause undefined behavior
940+
.label = conversion happend here
941+
.note = conversion from `{$from}` to `{$to}`
942+
937943
lint_unsupported_group = `{$lint_group}` lint group is not supported with ´--force-warn´
938944
939945
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
@@ -54,11 +54,11 @@ use crate::lints::{
5454
BuiltinExplicitOutlives, BuiltinExplicitOutlivesSuggestion, BuiltinFeatureIssueNote,
5555
BuiltinIncompleteFeatures, BuiltinIncompleteFeaturesHelp, BuiltinInternalFeatures,
5656
BuiltinKeywordIdents, BuiltinMissingCopyImpl, BuiltinMissingDebugImpl, BuiltinMissingDoc,
57-
BuiltinMutablesTransmutes, BuiltinNoMangleGeneric, BuiltinNonShorthandFieldPatterns,
58-
BuiltinSpecialModuleNameUsed, BuiltinTrivialBounds, BuiltinTypeAliasBounds,
59-
BuiltinUngatedAsyncFnTrackCaller, BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub,
60-
BuiltinUnreachablePub, BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment,
61-
BuiltinUnusedDocCommentSub, BuiltinWhileTrue, InvalidAsmLabel,
57+
BuiltinNoMangleGeneric, BuiltinNonShorthandFieldPatterns, BuiltinSpecialModuleNameUsed,
58+
BuiltinTrivialBounds, BuiltinTypeAliasBounds, BuiltinUngatedAsyncFnTrackCaller,
59+
BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub,
60+
BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub,
61+
BuiltinWhileTrue, InvalidAsmLabel,
6262
};
6363
use crate::nonstandard_style::{MethodLateContext, method_context};
6464
use crate::{
@@ -1031,72 +1031,6 @@ impl<'tcx> LateLintPass<'tcx> for InvalidNoMangleItems {
10311031
}
10321032
}
10331033

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