Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print list of missing target features when calling a function with target features outside an unsafe block #118333

Merged
merged 2 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions compiler/rustc_middle/src/mir/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub enum UnsafetyViolationKind {
UnsafeFn,
}

#[derive(Copy, Clone, PartialEq, TyEncodable, TyDecodable, HashStable, Debug)]
#[derive(Clone, PartialEq, TyEncodable, TyDecodable, HashStable, Debug)]
pub enum UnsafetyViolationDetails {
CallToUnsafeFunction,
UseOfInlineAssembly,
Expand All @@ -39,10 +39,17 @@ pub enum UnsafetyViolationDetails {
AccessToUnionField,
MutationOfLayoutConstrainedField,
BorrowOfLayoutConstrainedField,
CallToFunctionWith,
CallToFunctionWith {
/// Target features enabled in callee's `#[target_feature]` but missing in
/// caller's `#[target_feature]`.
missing: Vec<Symbol>,
/// Target features in `missing` that are enabled at compile time
/// (e.g., with `-C target-feature`).
build_enabled: Vec<Symbol>,
},
}

#[derive(Copy, Clone, PartialEq, TyEncodable, TyDecodable, HashStable, Debug)]
#[derive(Clone, PartialEq, TyEncodable, TyDecodable, HashStable, Debug)]
pub struct UnsafetyViolation {
pub source_info: SourceInfo,
pub lint_root: hir::HirId,
Expand Down
36 changes: 33 additions & 3 deletions compiler/rustc_mir_build/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,32 @@ mir_build_borrow_of_moved_value = borrow of moved value

mir_build_call_to_fn_with_requires_unsafe =
call to function `{$function}` with `#[target_feature]` is unsafe and requires unsafe block
.note = can only be called if the required target features are available
.help = in order for the call to be safe, the context requires the following additional target {$missing_target_features_count ->
[1] feature
*[count] features
}: {$missing_target_features}
.note = the {$build_target_features} target {$build_target_features_count ->
[1] feature
*[count] features
} being enabled in the build configuration does not remove the requirement to list {$build_target_features_count ->
[1] it
*[count] them
} in `#[target_feature]`
.label = call to function with `#[target_feature]`

mir_build_call_to_fn_with_requires_unsafe_unsafe_op_in_unsafe_fn_allowed =
call to function `{$function}` with `#[target_feature]` is unsafe and requires unsafe function or block
.note = can only be called if the required target features are available
.help = in order for the call to be safe, the context requires the following additional target {$missing_target_features_count ->
[1] feature
*[count] features
}: {$missing_target_features}
.note = the {$build_target_features} target {$build_target_features_count ->
[1] feature
*[count] features
} being enabled in the build configuration does not remove the requirement to list {$build_target_features_count ->
[1] it
*[count] them
} in `#[target_feature]`
.label = call to function with `#[target_feature]`

mir_build_call_to_unsafe_fn_requires_unsafe =
Expand Down Expand Up @@ -330,7 +350,17 @@ mir_build_unsafe_op_in_unsafe_fn_borrow_of_layout_constrained_field_requires_uns

mir_build_unsafe_op_in_unsafe_fn_call_to_fn_with_requires_unsafe =
call to function `{$function}` with `#[target_feature]` is unsafe and requires unsafe block (error E0133)
.note = can only be called if the required target features are available
.help = in order for the call to be safe, the context requires the following additional target {$missing_target_features_count ->
[1] feature
*[count] features
}: {$missing_target_features}
.note = the {$build_target_features} target {$build_target_features_count ->
[1] feature
*[count] features
} being enabled in the build configuration does not remove the requirement to list {$build_target_features_count ->
[1] it
*[count] them
} in `#[target_feature]`
.label = call to function with `#[target_feature]`

mir_build_unsafe_op_in_unsafe_fn_call_to_unsafe_fn_requires_unsafe =
Expand Down
80 changes: 67 additions & 13 deletions compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use std::borrow::Cow;

use crate::build::ExprCategory;
use crate::errors::*;
use rustc_middle::thir::visit::{self, Visitor};

use rustc_errors::DiagnosticArgValue;
use rustc_hir as hir;
use rustc_middle::mir::BorrowKind;
use rustc_middle::thir::*;
Expand Down Expand Up @@ -392,15 +395,29 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
// the call requires `unsafe`. Don't check this on wasm
// targets, though. For more information on wasm see the
// is_like_wasm check in hir_analysis/src/collect.rs
let callee_features = &self.tcx.codegen_fn_attrs(func_did).target_features;
if !self.tcx.sess.target.options.is_like_wasm
&& !self
.tcx
.codegen_fn_attrs(func_did)
.target_features
&& !callee_features
.iter()
.all(|feature| self.body_target_features.contains(feature))
{
self.requires_unsafe(expr.span, CallToFunctionWith(func_did));
let missing: Vec<_> = callee_features
.iter()
.copied()
.filter(|feature| !self.body_target_features.contains(feature))
.collect();
let build_enabled = self
.tcx
.sess
.target_features
.iter()
.copied()
.filter(|feature| missing.contains(feature))
.collect();
self.requires_unsafe(
expr.span,
CallToFunctionWith { function: func_did, missing, build_enabled },
);
}
}
}
Expand Down Expand Up @@ -526,7 +543,7 @@ struct UnusedUnsafeWarning {
enclosing_unsafe: Option<UnusedUnsafeEnclosing>,
}

#[derive(Clone, Copy, PartialEq)]
#[derive(Clone, PartialEq)]
enum UnsafeOpKind {
CallToUnsafeFunction(Option<DefId>),
UseOfInlineAssembly,
Expand All @@ -537,7 +554,15 @@ enum UnsafeOpKind {
AccessToUnionField,
MutationOfLayoutConstrainedField,
BorrowOfLayoutConstrainedField,
CallToFunctionWith(DefId),
CallToFunctionWith {
function: DefId,
/// Target features enabled in callee's `#[target_feature]` but missing in
/// caller's `#[target_feature]`.
missing: Vec<Symbol>,
/// Target features in `missing` that are enabled at compile time
/// (e.g., with `-C target-feature`).
build_enabled: Vec<Symbol>,
},
}

use UnsafeOpKind::*;
Expand Down Expand Up @@ -658,13 +683,22 @@ impl UnsafeOpKind {
unsafe_not_inherited_note,
},
),
CallToFunctionWith(did) => tcx.emit_spanned_lint(
CallToFunctionWith { function, missing, build_enabled } => tcx.emit_spanned_lint(
UNSAFE_OP_IN_UNSAFE_FN,
hir_id,
span,
UnsafeOpInUnsafeFnCallToFunctionWithRequiresUnsafe {
span,
function: &with_no_trimmed_paths!(tcx.def_path_str(*did)),
function: &with_no_trimmed_paths!(tcx.def_path_str(*function)),
missing_target_features: DiagnosticArgValue::StrListSepByAnd(
missing.iter().map(|feature| Cow::from(feature.as_str())).collect(),
),
missing_target_features_count: missing.len(),
note: if build_enabled.is_empty() { None } else { Some(()) },
build_target_features: DiagnosticArgValue::StrListSepByAnd(
build_enabled.iter().map(|feature| Cow::from(feature.as_str())).collect(),
),
build_target_features_count: build_enabled.len(),
unsafe_not_inherited_note,
},
),
Expand Down Expand Up @@ -821,18 +855,38 @@ impl UnsafeOpKind {
unsafe_not_inherited_note,
});
}
CallToFunctionWith(did) if unsafe_op_in_unsafe_fn_allowed => {
CallToFunctionWith { function, missing, build_enabled }
if unsafe_op_in_unsafe_fn_allowed =>
{
tcx.sess.emit_err(CallToFunctionWithRequiresUnsafeUnsafeOpInUnsafeFnAllowed {
span,
missing_target_features: DiagnosticArgValue::StrListSepByAnd(
missing.iter().map(|feature| Cow::from(feature.as_str())).collect(),
),
missing_target_features_count: missing.len(),
note: if build_enabled.is_empty() { None } else { Some(()) },
build_target_features: DiagnosticArgValue::StrListSepByAnd(
build_enabled.iter().map(|feature| Cow::from(feature.as_str())).collect(),
),
build_target_features_count: build_enabled.len(),
unsafe_not_inherited_note,
function: &tcx.def_path_str(*did),
function: &tcx.def_path_str(*function),
});
}
CallToFunctionWith(did) => {
CallToFunctionWith { function, missing, build_enabled } => {
tcx.sess.emit_err(CallToFunctionWithRequiresUnsafe {
span,
missing_target_features: DiagnosticArgValue::StrListSepByAnd(
missing.iter().map(|feature| Cow::from(feature.as_str())).collect(),
),
missing_target_features_count: missing.len(),
note: if build_enabled.is_empty() { None } else { Some(()) },
build_target_features: DiagnosticArgValue::StrListSepByAnd(
build_enabled.iter().map(|feature| Cow::from(feature.as_str())).collect(),
),
build_target_features_count: build_enabled.len(),
unsafe_not_inherited_note,
function: &tcx.def_path_str(*did),
function: &tcx.def_path_str(*function),
});
}
}
Expand Down
25 changes: 22 additions & 3 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::{
fluent_generated as fluent,
thir::pattern::{deconstruct_pat::WitnessPat, MatchCheckCtxt},
};
use rustc_errors::DiagnosticArgValue;
use rustc_errors::{
error_code, AddToDiagnostic, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed,
Handler, IntoDiagnostic, MultiSpan, SubdiagnosticMessage,
Expand Down Expand Up @@ -124,11 +125,17 @@ pub struct UnsafeOpInUnsafeFnBorrowOfLayoutConstrainedFieldRequiresUnsafe {

#[derive(LintDiagnostic)]
#[diag(mir_build_unsafe_op_in_unsafe_fn_call_to_fn_with_requires_unsafe)]
#[note]
#[help]
pub struct UnsafeOpInUnsafeFnCallToFunctionWithRequiresUnsafe<'a> {
#[label]
pub span: Span,
pub function: &'a str,
pub missing_target_features: DiagnosticArgValue<'a>,
pub missing_target_features_count: usize,
#[note]
pub note: Option<()>,
pub build_target_features: DiagnosticArgValue<'a>,
pub build_target_features_count: usize,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedLintNote>,
}
Expand Down Expand Up @@ -369,24 +376,36 @@ pub struct BorrowOfLayoutConstrainedFieldRequiresUnsafeUnsafeOpInUnsafeFnAllowed

#[derive(Diagnostic)]
#[diag(mir_build_call_to_fn_with_requires_unsafe, code = "E0133")]
#[note]
#[help]
pub struct CallToFunctionWithRequiresUnsafe<'a> {
#[primary_span]
#[label]
pub span: Span,
pub function: &'a str,
pub missing_target_features: DiagnosticArgValue<'a>,
pub missing_target_features_count: usize,
#[note]
pub note: Option<()>,
pub build_target_features: DiagnosticArgValue<'a>,
pub build_target_features_count: usize,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedNote>,
}

#[derive(Diagnostic)]
#[diag(mir_build_call_to_fn_with_requires_unsafe_unsafe_op_in_unsafe_fn_allowed, code = "E0133")]
#[note]
#[help]
pub struct CallToFunctionWithRequiresUnsafeUnsafeOpInUnsafeFnAllowed<'a> {
#[primary_span]
#[label]
pub span: Span,
pub function: &'a str,
pub missing_target_features: DiagnosticArgValue<'a>,
pub missing_target_features_count: usize,
#[note]
pub note: Option<()>,
pub build_target_features: DiagnosticArgValue<'a>,
pub build_target_features_count: usize,
#[subdiagnostic]
pub unsafe_not_inherited_note: Option<UnsafeNotInheritedNote>,
}
Expand Down
13 changes: 12 additions & 1 deletion compiler/rustc_mir_transform/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,19 @@ mir_transform_requires_unsafe = {$details} is unsafe and requires unsafe {$op_in
}
.not_inherited = items do not inherit unsafety from separate enclosing items

mir_transform_target_feature_call_help = in order for the call to be safe, the context requires the following additional target {$missing_target_features_count ->
[1] feature
*[count] features
}: {$missing_target_features}

mir_transform_target_feature_call_label = call to function with `#[target_feature]`
mir_transform_target_feature_call_note = can only be called if the required target features are available
mir_transform_target_feature_call_note = the {$build_target_features} target {$build_target_features_count ->
[1] feature
*[count] features
} being enabled in the build configuration does not remove the requirement to list {$build_target_features_count ->
[1] it
*[count] them
} in `#[target_feature]`

mir_transform_unaligned_packed_ref = reference to packed field is unaligned
.note = packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses
Expand Down
29 changes: 22 additions & 7 deletions compiler/rustc_mir_transform/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,19 +287,20 @@ impl<'tcx> UnsafetyChecker<'_, 'tcx> {
.safety;
match safety {
// `unsafe` blocks are required in safe code
Safety::Safe => violations.into_iter().for_each(|&violation| {
Safety::Safe => violations.into_iter().for_each(|violation| {
match violation.kind {
UnsafetyViolationKind::General => {}
UnsafetyViolationKind::UnsafeFn => {
bug!("`UnsafetyViolationKind::UnsafeFn` in an `Safe` context")
}
}
if !self.violations.contains(&violation) {
self.violations.push(violation)
if !self.violations.contains(violation) {
self.violations.push(violation.clone())
}
}),
// With the RFC 2585, no longer allow `unsafe` operations in `unsafe fn`s
Safety::FnUnsafe => violations.into_iter().for_each(|&(mut violation)| {
Safety::FnUnsafe => violations.into_iter().for_each(|violation| {
let mut violation = violation.clone();
violation.kind = UnsafetyViolationKind::UnsafeFn;
if !self.violations.contains(&violation) {
self.violations.push(violation)
Expand Down Expand Up @@ -367,9 +368,22 @@ impl<'tcx> UnsafetyChecker<'_, 'tcx> {

// Is `callee_features` a subset of `calling_features`?
if !callee_features.iter().all(|feature| self_features.contains(feature)) {
let missing: Vec<_> = callee_features
.iter()
.copied()
.filter(|feature| !self_features.contains(feature))
.collect();
let build_enabled = self
.tcx
.sess
.target_features
.iter()
.copied()
.filter(|feature| missing.contains(feature))
.collect();
self.require_unsafe(
UnsafetyViolationKind::General,
UnsafetyViolationDetails::CallToFunctionWith,
UnsafetyViolationDetails::CallToFunctionWith { missing, build_enabled },
)
}
}
Expand Down Expand Up @@ -528,8 +542,9 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) {
// Only suggest wrapping the entire function body in an unsafe block once
let mut suggest_unsafe_block = true;

for &UnsafetyViolation { source_info, lint_root, kind, details } in violations.iter() {
let details = errors::RequiresUnsafeDetail { violation: details, span: source_info.span };
for &UnsafetyViolation { source_info, lint_root, kind, ref details } in violations.iter() {
let details =
errors::RequiresUnsafeDetail { violation: details.clone(), span: source_info.span };

match kind {
UnsafetyViolationKind::General => {
Expand Down
Loading
Loading