Skip to content

Display function path in unsafety violations - E0133 #96294

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 26, 2022
Merged

Display function path in unsafety violations - E0133 #96294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b8f665
add `DefId` to unsafety violations and display function path in E0133
Emilgardis Apr 24, 2022
2e47271
only show a simple description in E0133 span label
Emilgardis Apr 24, 2022
f71597c
avoid fully qualifying error output of issue-43733 test
Emilgardis Apr 24, 2022
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
68 changes: 49 additions & 19 deletions compiler/rustc_middle/src/mir/query.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use rustc_index::vec::IndexVec;
use rustc_span::Span;
use rustc_target::abi::VariantIdx;
use smallvec::SmallVec;
use std::borrow::Cow;
use std::cell::Cell;
use std::fmt::{self, Debug};

Expand All @@ -28,7 +29,7 @@ pub enum UnsafetyViolationKind {

#[derive(Copy, Clone, PartialEq, TyEncodable, TyDecodable, HashStable, Debug)]
pub enum UnsafetyViolationDetails {
CallToUnsafeFunction,
CallToUnsafeFunction(Option<DefId>),
UseOfInlineAssembly,
InitializingTypeWith,
CastOfPointerToInt,
Expand All @@ -39,66 +40,95 @@ pub enum UnsafetyViolationDetails {
AccessToUnionField,
MutationOfLayoutConstrainedField,
BorrowOfLayoutConstrainedField,
CallToFunctionWith,
CallToFunctionWith(DefId),
}

impl UnsafetyViolationDetails {
pub fn description_and_note(&self) -> (&'static str, &'static str) {
pub fn simple_description(&self) -> &'static str {
use UnsafetyViolationDetails::*;

match self {
CallToUnsafeFunction(..) => "call to unsafe function",
UseOfInlineAssembly => "use of inline assembly",
InitializingTypeWith => "initializing type with `rustc_layout_scalar_valid_range` attr",
CastOfPointerToInt => "cast of pointer to int",
UseOfMutableStatic => "use of mutable static",
UseOfExternStatic => "use of extern static",
DerefOfRawPointer => "dereference of raw pointer",
AssignToDroppingUnionField => "assignment to union field that might need dropping",
AccessToUnionField => "access to union field",
MutationOfLayoutConstrainedField => "mutation of layout constrained field",
BorrowOfLayoutConstrainedField => {
"borrow of layout constrained field with interior mutability"
}
CallToFunctionWith(..) => "call to function with `#[target_feature]`",
}
}

pub fn description_and_note(&self, tcx: TyCtxt<'_>) -> (Cow<'static, str>, &'static str) {
use UnsafetyViolationDetails::*;
match self {
CallToUnsafeFunction => (
"call to unsafe function",
CallToUnsafeFunction(did) => (
if let Some(did) = did {
Cow::from(format!("call to unsafe function `{}`", tcx.def_path_str(*did)))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure I like this wording

} else {
Cow::Borrowed(self.simple_description())
},
"consult the function's documentation for information on how to avoid undefined \
behavior",
),
UseOfInlineAssembly => (
"use of inline assembly",
Cow::Borrowed(self.simple_description()),
"inline assembly is entirely unchecked and can cause undefined behavior",
),
InitializingTypeWith => (
"initializing type with `rustc_layout_scalar_valid_range` attr",
Cow::Borrowed(self.simple_description()),
"initializing a layout restricted type's field with a value outside the valid \
range is undefined behavior",
),
CastOfPointerToInt => {
("cast of pointer to int", "casting pointers to integers in constants")
}
CastOfPointerToInt => (
Cow::Borrowed(self.simple_description()),
"casting pointers to integers in constants",
),
UseOfMutableStatic => (
"use of mutable static",
Cow::Borrowed(self.simple_description()),
"mutable statics can be mutated by multiple threads: aliasing violations or data \
races will cause undefined behavior",
),
UseOfExternStatic => (
"use of extern static",
Cow::Borrowed(self.simple_description()),
"extern statics are not controlled by the Rust type system: invalid data, \
aliasing violations or data races will cause undefined behavior",
),
DerefOfRawPointer => (
"dereference of raw pointer",
Cow::Borrowed(self.simple_description()),
"raw pointers may be null, dangling or unaligned; they can violate aliasing rules \
and cause data races: all of these are undefined behavior",
),
AssignToDroppingUnionField => (
"assignment to union field that might need dropping",
Cow::Borrowed(self.simple_description()),
"the previous content of the field will be dropped, which causes undefined \
behavior if the field was not properly initialized",
),
AccessToUnionField => (
"access to union field",
Cow::Borrowed(self.simple_description()),
"the field may not be properly initialized: using uninitialized data will cause \
undefined behavior",
),
MutationOfLayoutConstrainedField => (
"mutation of layout constrained field",
Cow::Borrowed(self.simple_description()),
"mutating layout constrained fields cannot statically be checked for valid values",
),
BorrowOfLayoutConstrainedField => (
"borrow of layout constrained field with interior mutability",
Cow::Borrowed(self.simple_description()),
"references to fields of layout constrained fields lose the constraints. Coupled \
with interior mutability, the field can be changed to invalid values",
),
CallToFunctionWith => (
"call to function with `#[target_feature]`",
CallToFunctionWith(did) => (
Cow::from(format!(
"call to function `{}` with `#[target_feature]`",
tcx.def_path_str(*did)
)),
"can only be called if the required target features are available",
),
}
Expand Down
74 changes: 53 additions & 21 deletions compiler/rustc_mir_build/src/check_unsafety.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use rustc_span::def_id::{DefId, LocalDefId};
use rustc_span::symbol::Symbol;
use rustc_span::Span;

use std::borrow::Cow;
use std::ops::Bound;

struct UnsafetyVisitor<'a, 'tcx> {
Expand Down Expand Up @@ -70,7 +71,6 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
}

fn requires_unsafe(&mut self, span: Span, kind: UnsafeOpKind) {
let (description, note) = kind.description_and_note();
let unsafe_op_in_unsafe_fn_allowed = self.unsafe_op_in_unsafe_fn_allowed();
match self.safety_context {
SafetyContext::BuiltinUnsafeBlock => {}
Expand All @@ -82,6 +82,7 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
}
SafetyContext::UnsafeFn if unsafe_op_in_unsafe_fn_allowed => {}
SafetyContext::UnsafeFn => {
let (description, note) = kind.description_and_note(self.tcx);
// unsafe_op_in_unsafe_fn is disallowed
self.tcx.struct_span_lint_hir(
UNSAFE_OP_IN_UNSAFE_FN,
Expand All @@ -92,13 +93,14 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
"{} is unsafe and requires unsafe block (error E0133)",
description,
))
.span_label(span, description)
.span_label(span, kind.simple_description())
.note(note)
.emit();
},
)
}
SafetyContext::Safe => {
let (description, note) = kind.description_and_note(self.tcx);
let fn_sugg = if unsafe_op_in_unsafe_fn_allowed { " function or" } else { "" };
struct_span_err!(
self.tcx.sess,
Expand All @@ -108,7 +110,7 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
description,
fn_sugg,
)
.span_label(span, description)
.span_label(span, kind.simple_description())
.note(note)
.emit();
}
Expand Down Expand Up @@ -350,7 +352,12 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
}
ExprKind::Call { fun, ty: _, args: _, from_hir_call: _, fn_span: _ } => {
if self.thir[fun].ty.fn_sig(self.tcx).unsafety() == hir::Unsafety::Unsafe {
self.requires_unsafe(expr.span, CallToUnsafeFunction);
let func_id = if let ty::FnDef(func_id, _) = self.thir[fun].ty.kind() {
Some(*func_id)
} else {
None
};
self.requires_unsafe(expr.span, CallToUnsafeFunction(func_id));
} else if let &ty::FnDef(func_did, _) = self.thir[fun].ty.kind() {
// If the called function has target features the calling function hasn't,
// the call requires `unsafe`. Don't check this on wasm
Expand All @@ -364,7 +371,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
.iter()
.all(|feature| self.body_target_features.contains(feature))
{
self.requires_unsafe(expr.span, CallToFunctionWith);
self.requires_unsafe(expr.span, CallToFunctionWith(func_did));
}
}
}
Expand Down Expand Up @@ -523,7 +530,7 @@ impl BodyUnsafety {

#[derive(Clone, Copy, PartialEq)]
enum UnsafeOpKind {
CallToUnsafeFunction,
CallToUnsafeFunction(Option<DefId>),
UseOfInlineAssembly,
InitializingTypeWith,
UseOfMutableStatic,
Expand All @@ -533,64 +540,89 @@ enum UnsafeOpKind {
AccessToUnionField,
MutationOfLayoutConstrainedField,
BorrowOfLayoutConstrainedField,
CallToFunctionWith,
CallToFunctionWith(DefId),
}

use UnsafeOpKind::*;

impl UnsafeOpKind {
pub fn description_and_note(&self) -> (&'static str, &'static str) {
pub fn simple_description(&self) -> &'static str {
match self {
CallToUnsafeFunction => (
"call to unsafe function",
CallToUnsafeFunction(..) => "call to unsafe function",
UseOfInlineAssembly => "use of inline assembly",
InitializingTypeWith => "initializing type with `rustc_layout_scalar_valid_range` attr",
UseOfMutableStatic => "use of mutable static",
UseOfExternStatic => "use of extern static",
DerefOfRawPointer => "dereference of raw pointer",
AssignToDroppingUnionField => "assignment to union field that might need dropping",
AccessToUnionField => "access to union field",
MutationOfLayoutConstrainedField => "mutation of layout constrained field",
BorrowOfLayoutConstrainedField => {
"borrow of layout constrained field with interior mutability"
}
CallToFunctionWith(..) => "call to function with `#[target_feature]`",
}
}

pub fn description_and_note(&self, tcx: TyCtxt<'_>) -> (Cow<'static, str>, &'static str) {
match self {
CallToUnsafeFunction(did) => (
if let Some(did) = did {
Cow::from(format!("call to unsafe function `{}`", tcx.def_path_str(*did)))
} else {
Cow::Borrowed(self.simple_description())
},
"consult the function's documentation for information on how to avoid undefined \
behavior",
),
UseOfInlineAssembly => (
"use of inline assembly",
Cow::Borrowed(self.simple_description()),
"inline assembly is entirely unchecked and can cause undefined behavior",
),
InitializingTypeWith => (
"initializing type with `rustc_layout_scalar_valid_range` attr",
Cow::Borrowed(self.simple_description()),
"initializing a layout restricted type's field with a value outside the valid \
range is undefined behavior",
),
UseOfMutableStatic => (
"use of mutable static",
Cow::Borrowed(self.simple_description()),
"mutable statics can be mutated by multiple threads: aliasing violations or data \
races will cause undefined behavior",
),
UseOfExternStatic => (
"use of extern static",
Cow::Borrowed(self.simple_description()),
"extern statics are not controlled by the Rust type system: invalid data, \
aliasing violations or data races will cause undefined behavior",
),
DerefOfRawPointer => (
"dereference of raw pointer",
Cow::Borrowed(self.simple_description()),
"raw pointers may be null, dangling or unaligned; they can violate aliasing rules \
and cause data races: all of these are undefined behavior",
),
AssignToDroppingUnionField => (
"assignment to union field that might need dropping",
Cow::Borrowed(self.simple_description()),
"the previous content of the field will be dropped, which causes undefined \
behavior if the field was not properly initialized",
),
AccessToUnionField => (
"access to union field",
Cow::Borrowed(self.simple_description()),
"the field may not be properly initialized: using uninitialized data will cause \
undefined behavior",
),
MutationOfLayoutConstrainedField => (
"mutation of layout constrained field",
Cow::Borrowed(self.simple_description()),
"mutating layout constrained fields cannot statically be checked for valid values",
),
BorrowOfLayoutConstrainedField => (
"borrow of layout constrained field with interior mutability",
Cow::Borrowed(self.simple_description()),
"references to fields of layout constrained fields lose the constraints. Coupled \
with interior mutability, the field can be changed to invalid values",
),
CallToFunctionWith => (
"call to function with `#[target_feature]`",
CallToFunctionWith(did) => (
Cow::from(format!(
"call to function `{}` with `#[target_feature]`",
tcx.def_path_str(*did)
)),
"can only be called if the required target features are available",
),
}
Expand Down
13 changes: 8 additions & 5 deletions compiler/rustc_mir_transform/src/check_unsafety.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,17 @@ impl<'tcx> Visitor<'tcx> for UnsafetyChecker<'_, 'tcx> {

TerminatorKind::Call { ref func, .. } => {
let func_ty = func.ty(self.body, self.tcx);
let func_id =
if let ty::FnDef(func_id, _) = func_ty.kind() { Some(func_id) } else { None };
let sig = func_ty.fn_sig(self.tcx);
if let hir::Unsafety::Unsafe = sig.unsafety() {
self.require_unsafe(
UnsafetyViolationKind::General,
UnsafetyViolationDetails::CallToUnsafeFunction,
UnsafetyViolationDetails::CallToUnsafeFunction(func_id.copied()),
)
}

if let ty::FnDef(func_id, _) = func_ty.kind() {
if let Some(func_id) = func_id {
self.check_target_features(*func_id);
}
}
Expand Down Expand Up @@ -379,7 +381,7 @@ impl<'tcx> UnsafetyChecker<'_, 'tcx> {
if !callee_features.iter().all(|feature| self_features.contains(feature)) {
self.require_unsafe(
UnsafetyViolationKind::General,
UnsafetyViolationDetails::CallToFunctionWith,
UnsafetyViolationDetails::CallToFunctionWith(func_did),
)
}
}
Expand Down Expand Up @@ -578,7 +580,8 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) {
let UnsafetyCheckResult { violations, unused_unsafes, .. } = tcx.unsafety_check_result(def_id);

for &UnsafetyViolation { source_info, lint_root, kind, details } in violations.iter() {
let (description, note) = details.description_and_note();
let (description, note) =
ty::print::with_no_trimmed_paths!(details.description_and_note(tcx));

// Report an error.
let unsafe_fn_msg =
Expand All @@ -595,7 +598,7 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) {
description,
unsafe_fn_msg,
)
.span_label(source_info.span, description)
.span_label(source_info.span, details.simple_description())
.note(note)
.emit();
}
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/async-await/async-unsafe-fn-call-in-safe.mir.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,28 @@
error[E0133]: call to unsafe function is unsafe and requires unsafe function or block
error[E0133]: call to unsafe function `S::f` is unsafe and requires unsafe function or block
--> $DIR/async-unsafe-fn-call-in-safe.rs:14:5
|
LL | S::f();
| ^^^^^^ call to unsafe function
|
= note: consult the function's documentation for information on how to avoid undefined behavior

error[E0133]: call to unsafe function is unsafe and requires unsafe function or block
error[E0133]: call to unsafe function `f` is unsafe and requires unsafe function or block
--> $DIR/async-unsafe-fn-call-in-safe.rs:15:5
|
LL | f();
| ^^^ call to unsafe function
|
= note: consult the function's documentation for information on how to avoid undefined behavior

error[E0133]: call to unsafe function is unsafe and requires unsafe function or block
error[E0133]: call to unsafe function `S::f` is unsafe and requires unsafe function or block
--> $DIR/async-unsafe-fn-call-in-safe.rs:19:5
|
LL | S::f();
| ^^^^^^ call to unsafe function
|
= note: consult the function's documentation for information on how to avoid undefined behavior

error[E0133]: call to unsafe function is unsafe and requires unsafe function or block
error[E0133]: call to unsafe function `f` is unsafe and requires unsafe function or block
--> $DIR/async-unsafe-fn-call-in-safe.rs:20:5
|
LL | f();
Expand Down
Loading