improve error message shown for unsafe operations: explain why undefined behavior could arise

Inspired by @gnzlbg at https://github.com/rust-lang/rust/issues/46043#issuecomment-381544673

src/librustc/ich/impls_mir.rs

@@ -32,7 +32,7 @@
 });
 impl_stable_hash_for!(struct mir::UpvarDecl { debug_name, var_hir_id, by_ref, mutability });
 impl_stable_hash_for!(struct mir::BasicBlockData<'tcx> { statements, terminator, is_cleanup });
-impl_stable_hash_for!(struct mir::UnsafetyViolation { source_info, description, kind });
+impl_stable_hash_for!(struct mir::UnsafetyViolation { source_info, description, details, kind });
 impl_stable_hash_for!(struct mir::UnsafetyCheckResult { violations, unsafe_blocks });
 
 impl<'a> HashStable<StableHashingContext<'a>>

src/librustc/mir/mod.rs

@@ -2377,6 +2377,7 @@ pub enum UnsafetyViolationKind {
 pub struct UnsafetyViolation {
     pub source_info: SourceInfo,
     pub description: InternedString,
+    pub details: InternedString,
     pub kind: UnsafetyViolationKind,
 }
 

src/librustc_mir/transform/check_unsafety.rs

@@ -85,7 +85,9 @@ fn visit_terminator(&mut self,
                 let func_ty = func.ty(self.mir, self.tcx);
                 let sig = func_ty.fn_sig(self.tcx);
                 if let hir::Unsafety::Unsafe = sig.unsafety() {
-                    self.require_unsafe("call to unsafe function")
+                    self.require_unsafe("call to unsafe function",
+                        "consult the function's documentation for information on how to avoid \
+                         undefined behavior")
                 }
             }
         }
@@ -112,7 +114,8 @@ fn visit_statement(&mut self,
             }
 
             StatementKind::InlineAsm { .. } => {
-                self.require_unsafe("use of inline assembly")
+                self.require_unsafe("use of inline assembly",
+                    "inline assembly is entirely unchecked and can cause undefined behavior")
             },
         }
         self.super_statement(block, statement, location);
@@ -151,6 +154,11 @@ fn visit_place(&mut self,
                 self.register_violations(&[UnsafetyViolation {
                     source_info,
                     description: Symbol::intern("borrow of packed field").as_interned_str(),
+                    details:
+                        Symbol::intern("fields of packed structs might be misaligned: \
+                                        dereferencing a misaligned pointer or even just creating a \
+                                        misaligned reference is undefined behavior")
+                            .as_interned_str(),
                     kind: UnsafetyViolationKind::BorrowPacked(lint_root)
                 }], &[]);
             }
@@ -172,7 +180,10 @@ fn visit_place(&mut self,
                 let base_ty = base.ty(self.mir, self.tcx).to_ty(self.tcx);
                 match base_ty.sty {
                     ty::TyRawPtr(..) => {
-                        self.require_unsafe("dereference of raw pointer")
+                        self.require_unsafe("dereference of raw pointer",
+                            "raw pointers may be NULL, dangling or unaligned; they can violate \
+                             aliasing rules and cause data races: all of these are undefined \
+                             behavior")
                     }
                     ty::TyAdt(adt, _) => {
                         if adt.is_union() {
@@ -190,12 +201,17 @@ fn visit_place(&mut self,
                                 if elem_ty.moves_by_default(self.tcx, self.param_env,
                                                             self.source_info.span) {
                                     self.require_unsafe(
-                                        "assignment to non-`Copy` union field")
+                                        "assignment to non-`Copy` union field",
+                                        "the previous content of the field may be dropped, which \
+                                         cause undefined behavior if the field was not properly \
+                                         initialized")
                                 } else {
                                     // write to non-move union, safe
                                 }
                             } else {
-                                self.require_unsafe("access to union field")
+                                self.require_unsafe("access to union field",
+                                    "the field may not be properly initialized: using \
+                                     uninitialized data will cause undefined behavior")
                             }
                         }
                     }
@@ -208,7 +224,9 @@ fn visit_place(&mut self,
             }
             &Place::Static(box Static { def_id, ty: _ }) => {
                 if self.tcx.is_static(def_id) == Some(hir::Mutability::MutMutable) {
-                    self.require_unsafe("use of mutable static");
+                    self.require_unsafe("use of mutable static",
+                        "mutable statics can be mutated by multiple threads: aliasing violations \
+                         or data races will cause undefined behavior");
                 } else if self.tcx.is_foreign_item(def_id) {
                     let source_info = self.source_info;
                     let lint_root =
@@ -216,6 +234,11 @@ fn visit_place(&mut self,
                     self.register_violations(&[UnsafetyViolation {
                         source_info,
                         description: Symbol::intern("use of extern static").as_interned_str(),
+                        details:
+                            Symbol::intern("extern statics are not controlled by the Rust type \
+                                            system: invalid data, aliasing violations or data \
+                                            races will cause undefined behavior")
+                                .as_interned_str(),
                         kind: UnsafetyViolationKind::ExternStatic(lint_root)
                     }], &[]);
                 }
@@ -227,12 +250,14 @@ fn visit_place(&mut self,
 
 impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
     fn require_unsafe(&mut self,
-                      description: &'static str)
+                      description: &'static str,
+                      details: &'static str)
     {
         let source_info = self.source_info;
         self.register_violations(&[UnsafetyViolation {
             source_info,
             description: Symbol::intern(description).as_interned_str(),
+            details: Symbol::intern(details).as_interned_str(),
             kind: UnsafetyViolationKind::General,
         }], &[]);
     }
@@ -437,33 +462,36 @@ pub fn check_unsafety<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) {
     } = tcx.unsafety_check_result(def_id);
 
     for &UnsafetyViolation {
-        source_info, description, kind
+        source_info, description, details, kind
     } in violations.iter() {
         // Report an error.
         match kind {
             UnsafetyViolationKind::General => {
                 struct_span_err!(
                     tcx.sess, source_info.span, E0133,
-                    "{} requires unsafe function or block", description)
+                    "{} is unsafe and requires unsafe function or block", description)
                     .span_label(source_info.span, &description.as_str()[..])
+                    .note(&details.as_str()[..])
                     .emit();
             }
             UnsafetyViolationKind::ExternStatic(lint_node_id) => {
-                tcx.lint_node(SAFE_EXTERN_STATICS,
+                tcx.lint_node_note(SAFE_EXTERN_STATICS,
                               lint_node_id,
                               source_info.span,
-                              &format!("{} requires unsafe function or \
-                                        block (error E0133)", &description.as_str()[..]));
+                              &format!("{} is unsafe and requires unsafe function or block \
+                                        (error E0133)", &description.as_str()[..]),
+                              &details.as_str()[..]);
             }
             UnsafetyViolationKind::BorrowPacked(lint_node_id) => {
                 if let Some(impl_def_id) = builtin_derive_def_id(tcx, def_id) {
                     tcx.unsafe_derive_on_repr_packed(impl_def_id);
                 } else {
-                    tcx.lint_node(SAFE_PACKED_BORROWS,
+                    tcx.lint_node_note(SAFE_PACKED_BORROWS,
                                   lint_node_id,
                                   source_info.span,
-                                  &format!("{} requires unsafe function or \
-                                            block (error E0133)", &description.as_str()[..]));
+                                  &format!("{} is unsafe and requires unsafe function or block \
+                                            (error E0133)", &description.as_str()[..]),
+                                  &details.as_str()[..]);
                 }
             }
         }

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

@@ -17,5 +17,5 @@ mod test {
 
 fn main() {
     test::free();
-    //~^ ERROR call to unsafe function requires unsafe function or block
+    //~^ ERROR call to unsafe function is unsafe
 }

src/test/compile-fail/forget-init-unsafe.rs → src/test/compile-fail/init-unsafe.rs

@@ -12,7 +12,7 @@
 
 use std::intrinsics::{init};
 
-// Test that the `forget` and `init` intrinsics are really unsafe
+// Test that the `init` intrinsic is really unsafe
 pub fn main() {
-    let stuff = init::<isize>(); //~ ERROR call to unsafe function requires unsafe
+    let stuff = init::<isize>(); //~ ERROR call to unsafe function is unsafe
 }

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

@@ -27,12 +27,12 @@ fn __getit() -> std::option::Option<
     &'static std::cell::UnsafeCell<
         std::option::Option<Foo>>>
 {
-    __KEY.get() //~ ERROR call to unsafe function requires unsafe
+    __KEY.get() //~ ERROR call to unsafe function is unsafe
 }
 
 static FOO: std::thread::LocalKey<Foo> =
     std::thread::LocalKey::new(__getit, Default::default);
-//~^ ERROR call to unsafe function requires unsafe
+//~^ ERROR call to unsafe function is unsafe
 
 fn main() {
     FOO.with(|foo| println!("{}", foo.borrow()));

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

@@ -11,5 +11,5 @@
 fn main() {
     return;
     *(1 as *mut u32) = 42;
-    //~^ ERROR dereference of raw pointer requires unsafe
+    //~^ ERROR dereference of raw pointer is unsafe
 }

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

@@ -13,7 +13,7 @@
 fn main() {
     let _ = || {
         *(1 as *mut u32) = 42;
-        //~^ ERROR dereference of raw pointer requires unsafe
+        //~^ ERROR dereference of raw pointer is unsafe
         yield;
     };
 }

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

@@ -19,13 +19,13 @@ fn union_field() {
     union Union { unit: (), void: Void }
     let u = Union { unit: () };
     match u.void {}
-    //~^ ERROR access to union field requires unsafe function or block
+    //~^ ERROR access to union field is unsafe
 }
 
 fn raw_ptr_deref() {
     let ptr = std::ptr::null::<Void>();
     match *ptr {}
-    //~^ ERROR dereference of raw pointer requires unsafe function or block
+    //~^ ERROR dereference of raw pointer is unsafe
 }
 
 fn main() {}

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

@@ -18,8 +18,8 @@
 }
 
 fn main() {
-    let b = B; //~ ERROR use of mutable static requires unsafe function or block
-    let rb = &B; //~ ERROR use of mutable static requires unsafe function or block
-    let xb = XB; //~ ERROR use of mutable static requires unsafe function or block
-    let xrb = &XB; //~ ERROR use of mutable static requires unsafe function or block
+    let b = B; //~ ERROR use of mutable static is unsafe
+    let rb = &B; //~ ERROR use of mutable static is unsafe
+    let xb = XB; //~ ERROR use of mutable static is unsafe
+    let xrb = &XB; //~ ERROR use of mutable static is unsafe
 }

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

@@ -20,12 +20,12 @@
 }
 
 fn main() {
-    let a = A; //~ ERROR use of extern static requires unsafe function or block
+    let a = A; //~ ERROR use of extern static is unsafe
                //~^ WARN this was previously accepted by the compiler
-    let ra = &A; //~ ERROR use of extern static requires unsafe function or block
+    let ra = &A; //~ ERROR use of extern static is unsafe
                  //~^ WARN this was previously accepted by the compiler
-    let xa = XA; //~ ERROR use of extern static requires unsafe function or block
+    let xa = XA; //~ ERROR use of extern static is unsafe
                  //~^ WARN this was previously accepted by the compiler
-    let xra = &XA; //~ ERROR use of extern static requires unsafe function or block
+    let xra = &XA; //~ ERROR use of extern static is unsafe
                    //~^ WARN this was previously accepted by the compiler
 }

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

@@ -28,7 +28,7 @@ union U4<T: Copy> {
 
 fn generic_noncopy<T: Default>() {
     let mut u3 = U3 { a: T::default() };
-    u3.a = T::default(); //~ ERROR assignment to non-`Copy` union field requires unsafe
+    u3.a = T::default(); //~ ERROR assignment to non-`Copy` union field is unsafe
 }
 
 fn generic_copy<T: Copy + Default>() {
@@ -40,16 +40,16 @@ fn generic_copy<T: Copy + Default>() {
 
 fn main() {
     let mut u1 = U1 { a: 10 }; // OK
-    let a = u1.a; //~ ERROR access to union field requires unsafe
+    let a = u1.a; //~ ERROR access to union field is unsafe
     u1.a = 11; // OK
-    let U1 { a } = u1; //~ ERROR access to union field requires unsafe
-    if let U1 { a: 12 } = u1 {} //~ ERROR access to union field requires unsafe
+    let U1 { a } = u1; //~ ERROR access to union field is unsafe
+    if let U1 { a: 12 } = u1 {} //~ ERROR access to union field is unsafe
     // let U1 { .. } = u1; // OK
 
     let mut u2 = U2 { a: String::from("old") }; // OK
-    u2.a = String::from("new"); //~ ERROR assignment to non-`Copy` union field requires unsafe
+    u2.a = String::from("new"); //~ ERROR assignment to non-`Copy` union field is unsafe
     let mut u3 = U3 { a: 0 }; // OK
     u3.a = 1; // OK
     let mut u3 = U3 { a: String::from("old") }; // OK
-    u3.a = String::from("new"); //~ ERROR assignment to non-`Copy` union field requires unsafe
+    u3.a = String::from("new"); //~ ERROR assignment to non-`Copy` union field is unsafe
 }

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

@@ -11,7 +11,7 @@
 
 
 fn f(p: *mut u8) {
-    *p = 0; //~ ERROR dereference of raw pointer requires unsafe function or block
+    *p = 0; //~ ERROR dereference of raw pointer is unsafe
     return;
 }
 

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

@@ -12,5 +12,5 @@
 unsafe fn f() { return; }
 
 fn main() {
-    f(); //~ ERROR call to unsafe function requires unsafe function or block
+    f(); //~ ERROR call to unsafe function is unsafe
 }

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

@@ -10,7 +10,7 @@
 
 
 fn f(p: *const u8) -> u8 {
-    return *p; //~ ERROR dereference of raw pointer requires unsafe function or block
+    return *p; //~ ERROR dereference of raw pointer is unsafe
 }
 
 fn main() {

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

@@ -13,5 +13,5 @@
 
 fn main() {
     let x = f;
-    x();    //~ ERROR call to unsafe function requires unsafe function or block
+    x();    //~ ERROR call to unsafe function is unsafe
 }

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

@@ -16,5 +16,5 @@
 // as unsafe.
 fn main() {
     intrinsics::move_val_init(1 as *mut u32, 1);
-    //~^ ERROR dereference of raw pointer requires unsafe function or block
+    //~^ ERROR dereference of raw pointer is unsafe
 }

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

-error[E0133]: call to unsafe function requires unsafe function or block
+error[E0133]: call to unsafe function is unsafe and requires unsafe function or block
   --> $DIR/E0133.rs:14:5
    |
 LL |     f();
    |     ^^^ call to unsafe function
+   |
+   = note: consult the function's documentation for information on how to avoid undefined behavior
 
 error: aborting due to previous error
 

src/test/compile-fail/issue-27060.rs → src/test/ui/issue-27060.rs

@@ -33,9 +33,9 @@ fn main() {
         let _ = &good.data2[0]; // ok
     }
 
-    let _ = &good.data; //~ ERROR borrow of packed field requires unsafe
+    let _ = &good.data; //~ ERROR borrow of packed field is unsafe
                         //~| hard error
-    let _ = &good.data2[0]; //~ ERROR borrow of packed field requires unsafe
+    let _ = &good.data2[0]; //~ ERROR borrow of packed field is unsafe
                             //~| hard error
     let _ = &*good.data; // ok, behind a pointer
     let _ = &good.aligned; // ok, has align 1

src/test/ui/issue-27060.stderr

+error: borrow of packed field is unsafe and requires unsafe function or block (error E0133)
+  --> $DIR/issue-27060.rs:36:13
+   |
+LL |     let _ = &good.data; //~ ERROR borrow of packed field is unsafe
+   |             ^^^^^^^^^^
+   |
+note: lint level defined here
+  --> $DIR/issue-27060.rs:23:8
+   |
+LL | #[deny(safe_packed_borrows)]
+   |        ^^^^^^^^^^^^^^^^^^^
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #46043 <https://github.com/rust-lang/rust/issues/46043>
+   = note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior
+
+error: borrow of packed field is unsafe and requires unsafe function or block (error E0133)
+  --> $DIR/issue-27060.rs:38:13
+   |
+LL |     let _ = &good.data2[0]; //~ ERROR borrow of packed field is unsafe
+   |             ^^^^^^^^^^^^^^
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #46043 <https://github.com/rust-lang/rust/issues/46043>
+   = note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior
+
+error: aborting due to 2 previous errors
+

src/test/ui/issue-28776.stderr

-error[E0133]: call to unsafe function requires unsafe function or block
+error[E0133]: call to unsafe function is unsafe and requires unsafe function or block
   --> $DIR/issue-28776.rs:14:5
    |
 LL |     (&ptr::write)(1 as *mut _, 42);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function
+   |
+   = note: consult the function's documentation for information on how to avoid undefined behavior
 
 error: aborting due to previous error
 

src/test/ui/trait-safety-fn-body.stderr

-error[E0133]: dereference of raw pointer requires unsafe function or block
+error[E0133]: dereference of raw pointer is unsafe and requires unsafe function or block
   --> $DIR/trait-safety-fn-body.rs:21:9
    |
 LL |         *self += 1;
    |         ^^^^^^^^^^ dereference of raw pointer
+   |
+   = note: raw pointers may be NULL, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior
 
 error: aborting due to previous error
 

src/test/ui/unsafe-const-fn.stderr

-error[E0133]: call to unsafe function requires unsafe function or block
+error[E0133]: call to unsafe function is unsafe and requires unsafe function or block
   --> $DIR/unsafe-const-fn.rs:19:18
    |
 LL | const VAL: u32 = dummy(0xFFFF);
    |                  ^^^^^^^^^^^^^ call to unsafe function
+   |
+   = note: consult the function's documentation for information on how to avoid undefined behavior
 
 error: aborting due to previous error