Skip to content

Commit f511c5e

Browse files
committed
improve error message shown for unsafe operations: explain why undefined behavior could arise
Inspired by @gnzlbg at #46043 (comment)
1 parent c30acc7 commit f511c5e

23 files changed

+114
-50
lines changed

src/librustc/ich/impls_mir.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ impl_stable_hash_for!(struct mir::LocalDecl<'tcx> {
3232
});
3333
impl_stable_hash_for!(struct mir::UpvarDecl { debug_name, var_hir_id, by_ref, mutability });
3434
impl_stable_hash_for!(struct mir::BasicBlockData<'tcx> { statements, terminator, is_cleanup });
35-
impl_stable_hash_for!(struct mir::UnsafetyViolation { source_info, description, kind });
35+
impl_stable_hash_for!(struct mir::UnsafetyViolation { source_info, description, details, kind });
3636
impl_stable_hash_for!(struct mir::UnsafetyCheckResult { violations, unsafe_blocks });
3737

3838
impl<'a> HashStable<StableHashingContext<'a>>

src/librustc/mir/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2377,6 +2377,7 @@ pub enum UnsafetyViolationKind {
23772377
pub struct UnsafetyViolation {
23782378
pub source_info: SourceInfo,
23792379
pub description: InternedString,
2380+
pub details: InternedString,
23802381
pub kind: UnsafetyViolationKind,
23812382
}
23822383

src/librustc_mir/transform/check_unsafety.rs

Lines changed: 43 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,9 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
8585
let func_ty = func.ty(self.mir, self.tcx);
8686
let sig = func_ty.fn_sig(self.tcx);
8787
if let hir::Unsafety::Unsafe = sig.unsafety() {
88-
self.require_unsafe("call to unsafe function")
88+
self.require_unsafe("call to unsafe function",
89+
"consult the function's documentation for information on how to avoid \
90+
undefined behavior")
8991
}
9092
}
9193
}
@@ -112,7 +114,8 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
112114
}
113115

114116
StatementKind::InlineAsm { .. } => {
115-
self.require_unsafe("use of inline assembly")
117+
self.require_unsafe("use of inline assembly",
118+
"inline assembly is entirely unchecked and can cause undefined behavior")
116119
},
117120
}
118121
self.super_statement(block, statement, location);
@@ -151,6 +154,11 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
151154
self.register_violations(&[UnsafetyViolation {
152155
source_info,
153156
description: Symbol::intern("borrow of packed field").as_interned_str(),
157+
details:
158+
Symbol::intern("fields of packed structs might be misaligned: \
159+
dereferencing a misaligned pointer or even just creating a \
160+
misaligned reference is undefined behavior")
161+
.as_interned_str(),
154162
kind: UnsafetyViolationKind::BorrowPacked(lint_root)
155163
}], &[]);
156164
}
@@ -172,7 +180,10 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
172180
let base_ty = base.ty(self.mir, self.tcx).to_ty(self.tcx);
173181
match base_ty.sty {
174182
ty::TyRawPtr(..) => {
175-
self.require_unsafe("dereference of raw pointer")
183+
self.require_unsafe("dereference of raw pointer",
184+
"raw pointers may be NULL, dangling or unaligned; they can violate \
185+
aliasing rules and cause data races: all of these are undefined \
186+
behavior")
176187
}
177188
ty::TyAdt(adt, _) => {
178189
if adt.is_union() {
@@ -190,12 +201,17 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
190201
if elem_ty.moves_by_default(self.tcx, self.param_env,
191202
self.source_info.span) {
192203
self.require_unsafe(
193-
"assignment to non-`Copy` union field")
204+
"assignment to non-`Copy` union field",
205+
"the previous content of the field may be dropped, which \
206+
cause undefined behavior if the field was not properly \
207+
initialized")
194208
} else {
195209
// write to non-move union, safe
196210
}
197211
} else {
198-
self.require_unsafe("access to union field")
212+
self.require_unsafe("access to union field",
213+
"the field may not be properly initialized: using \
214+
uninitialized data will cause undefined behavior")
199215
}
200216
}
201217
}
@@ -208,14 +224,21 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
208224
}
209225
&Place::Static(box Static { def_id, ty: _ }) => {
210226
if self.tcx.is_static(def_id) == Some(hir::Mutability::MutMutable) {
211-
self.require_unsafe("use of mutable static");
227+
self.require_unsafe("use of mutable static",
228+
"mutable statics can be mutated by multiple threads: aliasing violations \
229+
or data races will cause undefined behavior");
212230
} else if self.tcx.is_foreign_item(def_id) {
213231
let source_info = self.source_info;
214232
let lint_root =
215233
self.source_scope_local_data[source_info.scope].lint_root;
216234
self.register_violations(&[UnsafetyViolation {
217235
source_info,
218236
description: Symbol::intern("use of extern static").as_interned_str(),
237+
details:
238+
Symbol::intern("extern statics are not controlled by the Rust type \
239+
system: invalid data, aliasing violations or data \
240+
races will cause undefined behavior")
241+
.as_interned_str(),
219242
kind: UnsafetyViolationKind::ExternStatic(lint_root)
220243
}], &[]);
221244
}
@@ -227,12 +250,14 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
227250

228251
impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
229252
fn require_unsafe(&mut self,
230-
description: &'static str)
253+
description: &'static str,
254+
details: &'static str)
231255
{
232256
let source_info = self.source_info;
233257
self.register_violations(&[UnsafetyViolation {
234258
source_info,
235259
description: Symbol::intern(description).as_interned_str(),
260+
details: Symbol::intern(details).as_interned_str(),
236261
kind: UnsafetyViolationKind::General,
237262
}], &[]);
238263
}
@@ -437,33 +462,36 @@ pub fn check_unsafety<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) {
437462
} = tcx.unsafety_check_result(def_id);
438463

439464
for &UnsafetyViolation {
440-
source_info, description, kind
465+
source_info, description, details, kind
441466
} in violations.iter() {
442467
// Report an error.
443468
match kind {
444469
UnsafetyViolationKind::General => {
445470
struct_span_err!(
446471
tcx.sess, source_info.span, E0133,
447-
"{} requires unsafe function or block", description)
472+
"{} is unsafe and requires unsafe function or block", description)
448473
.span_label(source_info.span, &description.as_str()[..])
474+
.note(&details.as_str()[..])
449475
.emit();
450476
}
451477
UnsafetyViolationKind::ExternStatic(lint_node_id) => {
452-
tcx.lint_node(SAFE_EXTERN_STATICS,
478+
tcx.lint_node_note(SAFE_EXTERN_STATICS,
453479
lint_node_id,
454480
source_info.span,
455-
&format!("{} requires unsafe function or \
456-
block (error E0133)", &description.as_str()[..]));
481+
&format!("{} is unsafe and requires unsafe function or block \
482+
(error E0133)", &description.as_str()[..]),
483+
&details.as_str()[..]);
457484
}
458485
UnsafetyViolationKind::BorrowPacked(lint_node_id) => {
459486
if let Some(impl_def_id) = builtin_derive_def_id(tcx, def_id) {
460487
tcx.unsafe_derive_on_repr_packed(impl_def_id);
461488
} else {
462-
tcx.lint_node(SAFE_PACKED_BORROWS,
489+
tcx.lint_node_note(SAFE_PACKED_BORROWS,
463490
lint_node_id,
464491
source_info.span,
465-
&format!("{} requires unsafe function or \
466-
block (error E0133)", &description.as_str()[..]));
492+
&format!("{} is unsafe and requires unsafe function or block \
493+
(error E0133)", &description.as_str()[..]),
494+
&details.as_str()[..]);
467495
}
468496
}
469497
}

src/test/compile-fail/foreign-unsafe-fn-called.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,5 @@ mod test {
1717

1818
fn main() {
1919
test::free();
20-
//~^ ERROR call to unsafe function requires unsafe function or block
20+
//~^ ERROR call to unsafe function is unsafe
2121
}

src/test/compile-fail/forget-init-unsafe.rs renamed to src/test/compile-fail/init-unsafe.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
use std::intrinsics::{init};
1414

15-
// Test that the `forget` and `init` intrinsics are really unsafe
15+
// Test that the `init` intrinsic is really unsafe
1616
pub fn main() {
17-
let stuff = init::<isize>(); //~ ERROR call to unsafe function requires unsafe
17+
let stuff = init::<isize>(); //~ ERROR call to unsafe function is unsafe
1818
}

src/test/compile-fail/issue-43733.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@ fn __getit() -> std::option::Option<
2727
&'static std::cell::UnsafeCell<
2828
std::option::Option<Foo>>>
2929
{
30-
__KEY.get() //~ ERROR call to unsafe function requires unsafe
30+
__KEY.get() //~ ERROR call to unsafe function is unsafe
3131
}
3232

3333
static FOO: std::thread::LocalKey<Foo> =
3434
std::thread::LocalKey::new(__getit, Default::default);
35-
//~^ ERROR call to unsafe function requires unsafe
35+
//~^ ERROR call to unsafe function is unsafe
3636

3737
fn main() {
3838
FOO.with(|foo| println!("{}", foo.borrow()));

src/test/compile-fail/issue-45087-unreachable-unsafe.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,5 @@
1111
fn main() {
1212
return;
1313
*(1 as *mut u32) = 42;
14-
//~^ ERROR dereference of raw pointer requires unsafe
14+
//~^ ERROR dereference of raw pointer is unsafe
1515
}

src/test/compile-fail/issue-45729-unsafe-in-generator.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
fn main() {
1414
let _ = || {
1515
*(1 as *mut u32) = 42;
16-
//~^ ERROR dereference of raw pointer requires unsafe
16+
//~^ ERROR dereference of raw pointer is unsafe
1717
yield;
1818
};
1919
}

src/test/compile-fail/issue-47412.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,13 @@ fn union_field() {
1919
union Union { unit: (), void: Void }
2020
let u = Union { unit: () };
2121
match u.void {}
22-
//~^ ERROR access to union field requires unsafe function or block
22+
//~^ ERROR access to union field is unsafe
2323
}
2424

2525
fn raw_ptr_deref() {
2626
let ptr = std::ptr::null::<Void>();
2727
match *ptr {}
28-
//~^ ERROR dereference of raw pointer requires unsafe function or block
28+
//~^ ERROR dereference of raw pointer is unsafe
2929
}
3030

3131
fn main() {}

src/test/compile-fail/safe-extern-statics-mut.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ extern {
1818
}
1919

2020
fn main() {
21-
let b = B; //~ ERROR use of mutable static requires unsafe function or block
22-
let rb = &B; //~ ERROR use of mutable static requires unsafe function or block
23-
let xb = XB; //~ ERROR use of mutable static requires unsafe function or block
24-
let xrb = &XB; //~ ERROR use of mutable static requires unsafe function or block
21+
let b = B; //~ ERROR use of mutable static is unsafe
22+
let rb = &B; //~ ERROR use of mutable static is unsafe
23+
let xb = XB; //~ ERROR use of mutable static is unsafe
24+
let xrb = &XB; //~ ERROR use of mutable static is unsafe
2525
}

0 commit comments

Comments
 (0)