Skip to content

Commit

Permalink
Auto merge of rust-lang#96294 - Emilgardis:def_id-in-unsafetyviolatio…
Browse files Browse the repository at this point in the history 
…ndetails, r=oli-obk

Display function path in unsafety violations - E0133

adds `DefId` to `UnsafetyViolationDetails`

this enables consumers to access the function definition that was reported to be unsafe and also changes the output for some E0133 diagnostics
  • Loading branch information
@bors
bors committed Apr 25, 2022
2 parents 055bf4c + f71597c commit ec8619d
Show file tree
Hide file tree
Showing 42 changed files with 211 additions and 142 deletions.
68 changes: 49 additions & 19 deletions compiler/rustc_middle/src/mir/query.rs
View file
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)))
} 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
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
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
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

0 comments on commit ec8619d

Please sign in to comment.