Skip to content

Commit f511c5e

Browse files
RalfJungRalfJung
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
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@ extern {
2020
}
2121

2222
fn main() {
23-
let a = A; //~ ERROR use of extern static requires unsafe function or block
23+
let a = A; //~ ERROR use of extern static is unsafe
2424
//~^ WARN this was previously accepted by the compiler
25-
let ra = &A; //~ ERROR use of extern static requires unsafe function or block
25+
let ra = &A; //~ ERROR use of extern static is unsafe
2626
//~^ WARN this was previously accepted by the compiler
27-
let xa = XA; //~ ERROR use of extern static requires unsafe function or block
27+
let xa = XA; //~ ERROR use of extern static is unsafe
2828
//~^ WARN this was previously accepted by the compiler
29-
let xra = &XA; //~ ERROR use of extern static requires unsafe function or block
29+
let xra = &XA; //~ ERROR use of extern static is unsafe
3030
//~^ WARN this was previously accepted by the compiler
3131
}

‎src/test/compile-fail/union/union-unsafe.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ union U4<T: Copy> {
2828

2929
fn generic_noncopy<T: Default>() {
3030
let mut u3 = U3 { a: T::default() };
31-
u3.a = T::default(); //~ ERROR assignment to non-`Copy` union field requires unsafe
31+
u3.a = T::default(); //~ ERROR assignment to non-`Copy` union field is unsafe
3232
}
3333

3434
fn generic_copy<T: Copy + Default>() {
@@ -40,16 +40,16 @@ fn generic_copy<T: Copy + Default>() {
4040

4141
fn main() {
4242
let mut u1 = U1 { a: 10 }; // OK
43-
let a = u1.a; //~ ERROR access to union field requires unsafe
43+
let a = u1.a; //~ ERROR access to union field is unsafe
4444
u1.a = 11; // OK
45-
let U1 { a } = u1; //~ ERROR access to union field requires unsafe
46-
if let U1 { a: 12 } = u1 {} //~ ERROR access to union field requires unsafe
45+
let U1 { a } = u1; //~ ERROR access to union field is unsafe
46+
if let U1 { a: 12 } = u1 {} //~ ERROR access to union field is unsafe
4747
// let U1 { .. } = u1; // OK
4848

4949
let mut u2 = U2 { a: String::from("old") }; // OK
50-
u2.a = String::from("new"); //~ ERROR assignment to non-`Copy` union field requires unsafe
50+
u2.a = String::from("new"); //~ ERROR assignment to non-`Copy` union field is unsafe
5151
let mut u3 = U3 { a: 0 }; // OK
5252
u3.a = 1; // OK
5353
let mut u3 = U3 { a: String::from("old") }; // OK
54-
u3.a = String::from("new"); //~ ERROR assignment to non-`Copy` union field requires unsafe
54+
u3.a = String::from("new"); //~ ERROR assignment to non-`Copy` union field is unsafe
5555
}

‎src/test/compile-fail/unsafe-fn-assign-deref-ptr.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212

1313
fn f(p: *mut u8) {
14-
*p = 0; //~ ERROR dereference of raw pointer requires unsafe function or block
14+
*p = 0; //~ ERROR dereference of raw pointer is unsafe
1515
return;
1616
}
1717

‎src/test/compile-fail/unsafe-fn-called-from-safe.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,5 @@
1212
unsafe fn f() { return; }
1313

1414
fn main() {
15-
f(); //~ ERROR call to unsafe function requires unsafe function or block
15+
f(); //~ ERROR call to unsafe function is unsafe
1616
}

‎src/test/compile-fail/unsafe-fn-deref-ptr.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111

1212
fn f(p: *const u8) -> u8 {
13-
return *p; //~ ERROR dereference of raw pointer requires unsafe function or block
13+
return *p; //~ ERROR dereference of raw pointer is unsafe
1414
}
1515

1616
fn main() {

‎src/test/compile-fail/unsafe-fn-used-as-value.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,5 @@ unsafe fn f() { return; }
1313

1414
fn main() {
1515
let x = f;
16-
x(); //~ ERROR call to unsafe function requires unsafe function or block
16+
x(); //~ ERROR call to unsafe function is unsafe
1717
}

‎src/test/compile-fail/unsafe-move-val-init.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,5 @@ use std::intrinsics;
1616
// as unsafe.
1717
fn main() {
1818
intrinsics::move_val_init(1 as *mut u32, 1);
19-
//~^ ERROR dereference of raw pointer requires unsafe function or block
19+
//~^ ERROR dereference of raw pointer is unsafe
2020
}

‎src/test/ui/error-codes/E0133.stderr

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1-
error[E0133]: call to unsafe function requires unsafe function or block
1+
error[E0133]: call to unsafe function is unsafe and requires unsafe function or block
22
--> $DIR/E0133.rs:14:5
33
|
44
LL | f();
55
| ^^^ call to unsafe function
6+
|
7+
= note: consult the function's documentation for information on how to avoid undefined behavior
68

79
error: aborting due to previous error
810

‎src/test/compile-fail/issue-27060.rs renamed to ‎src/test/ui/issue-27060.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ fn main() {
3333
let _ = &good.data2[0]; // ok
3434
}
3535

36-
let _ = &good.data; //~ ERROR borrow of packed field requires unsafe
36+
let _ = &good.data; //~ ERROR borrow of packed field is unsafe
3737
//~| hard error
38-
let _ = &good.data2[0]; //~ ERROR borrow of packed field requires unsafe
38+
let _ = &good.data2[0]; //~ ERROR borrow of packed field is unsafe
3939
//~| hard error
4040
let _ = &*good.data; // ok, behind a pointer
4141
let _ = &good.aligned; // ok, has align 1

‎src/test/ui/issue-27060.stderr

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
error: borrow of packed field is unsafe and requires unsafe function or block (error E0133)
2+
--> $DIR/issue-27060.rs:36:13
3+
|
4+
LL | let _ = &good.data; //~ ERROR borrow of packed field is unsafe
5+
| ^^^^^^^^^^
6+
|
7+
note: lint level defined here
8+
--> $DIR/issue-27060.rs:23:8
9+
|
10+
LL | #[deny(safe_packed_borrows)]
11+
| ^^^^^^^^^^^^^^^^^^^
12+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
13+
= note: for more information, see issue #46043 <https://github.com/rust-lang/rust/issues/46043>
14+
= note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior
15+
16+
error: borrow of packed field is unsafe and requires unsafe function or block (error E0133)
17+
--> $DIR/issue-27060.rs:38:13
18+
|
19+
LL | let _ = &good.data2[0]; //~ ERROR borrow of packed field is unsafe
20+
| ^^^^^^^^^^^^^^
21+
|
22+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
23+
= note: for more information, see issue #46043 <https://github.com/rust-lang/rust/issues/46043>
24+
= note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior
25+
26+
error: aborting due to 2 previous errors
27+

‎src/test/ui/issue-28776.stderr

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1-
error[E0133]: call to unsafe function requires unsafe function or block
1+
error[E0133]: call to unsafe function is unsafe and requires unsafe function or block
22
--> $DIR/issue-28776.rs:14:5
33
|
44
LL | (&ptr::write)(1 as *mut _, 42);
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function
6+
|
7+
= note: consult the function's documentation for information on how to avoid undefined behavior
68

79
error: aborting due to previous error
810

0 commit comments

Comments
 (0)