Implement requested changes

Author: 1c3t3a

compiler/rustc_mir_transform/src/check_null.rs

@@ -47,7 +47,7 @@ fn insert_null_check<'tcx>(
         kind: StatementKind::Assign(Box::new((sizeof_pointee, rvalue))),
     });
 
-    // Check that the pointee is a ZST.
+    // Check that the pointee is not a ZST.
     let zero = Operand::Constant(Box::new(ConstOperand {
         span: source_info.span,
         user_ty: None,
@@ -59,11 +59,11 @@ fn insert_null_check<'tcx>(
         source_info,
         kind: StatementKind::Assign(Box::new((
             is_pointee_no_zst,
-            Rvalue::BinaryOp(BinOp::Eq, Box::new((Operand::Copy(sizeof_pointee), zero.clone()))),
+            Rvalue::BinaryOp(BinOp::Ne, Box::new((Operand::Copy(sizeof_pointee), zero.clone()))),
         ))),
     });
 
-    // Check if the pointer is null.
+    // Check whether the pointer is null.
     let is_null = local_decls.push(LocalDecl::with_source_info(tcx.types.bool, source_info)).into();
     stmts.push(Statement {
         source_info,
@@ -73,7 +73,7 @@ fn insert_null_check<'tcx>(
         ))),
     });
 
-    // Check if the pointer is null or points to a ZST.
+    // We want to throw an exception if the pointer is null and doesn't point to a ZST.
     let should_throw_exception =
         local_decls.push(LocalDecl::with_source_info(tcx.types.bool, source_info)).into();
     stmts.push(Statement {
@@ -87,10 +87,20 @@ fn insert_null_check<'tcx>(
         ))),
     });
 
+    // The final condition whether this pointer usage is ok or not.
+    let is_ok = local_decls.push(LocalDecl::with_source_info(tcx.types.bool, source_info)).into();
+    stmts.push(Statement {
+        source_info,
+        kind: StatementKind::Assign(Box::new((
+            is_ok,
+            Rvalue::UnaryOp(UnOp::Not, Operand::Copy(should_throw_exception)),
+        ))),
+    });
+
     // Emit a PointerCheck that asserts on the condition and otherwise triggers
     // a AssertKind::NullPointerDereference.
     PointerCheck {
-        cond: Operand::Copy(should_throw_exception),
+        cond: Operand::Copy(is_ok),
         assert_kind: Box::new(AssertKind::NullPointerDereference),
     }
 }

compiler/rustc_mir_transform/src/check_pointers.rs

@@ -12,17 +12,22 @@ pub(crate) struct PointerCheck<'tcx> {
     pub(crate) assert_kind: Box<AssertKind<Operand<'tcx>>>,
 }
 
-/// Utility for adding a check for read/write on every sized, unsafe pointer.
+/// Utility for adding a check for read/write on every sized, raw pointer.
 ///
-/// Visits every read/write access to a [Sized], unsafe pointer and inserts a
-/// new basic block directly before the pointer access. Then calls `on_finding`
-/// to insert the actual logic for a pointer check (e.g. check for alignment).
+/// Visits every read/write access to a [Sized], raw pointer and inserts a
+/// new basic block directly before the pointer access. (Read/write accesses
+/// are determined by the `PlaceContext` of the MIR visitor. In particular,
+/// uses of pointers in borrow expressions are *not* visited). Then calls
+/// `on_finding` to insert the actual logic for a pointer check (e.g. check for
+/// alignment).
 /// This utility takes care of the right order of blocks, the only thing a
 /// caller must do in `on_finding` is:
 /// - Append [Statement]s to `stmts`.
 /// - Append [LocalDecl]s to `local_decls`.
 /// - Return a [PointerCheck] that contains the condition and an [AssertKind].
-///   The AssertKind must be a panic with `#[rustc_nounwind]`.
+///   The AssertKind must be a panic with `#[rustc_nounwind]`. The condition
+///   should always return the boolean `is_ok`, so evaluate to true in case of
+///   success and fail the check otherwise.
 /// This utility will insert a terminator block that asserts on the condition
 /// and panics on failure.
 pub(crate) fn check_pointers<'a, 'tcx, F>(
@@ -151,17 +156,17 @@ impl<'a, 'tcx> Visitor<'tcx> for PointerFinder<'a, 'tcx> {
         let pointer = Place::from(place.local);
         let pointer_ty = self.local_decls[place.local].ty;
 
-        // We only want to check places based on unsafe pointers
+        // We only want to check places based on raw pointers
         if !pointer_ty.is_unsafe_ptr() {
-            trace!("Indirect, but not based on an unsafe ptr, not checking {:?}", place);
+            trace!("Indirect, but not based on an raw ptr, not checking {:?}", place);
             return;
         }
 
         let pointee_ty =
-            pointer_ty.builtin_deref(true).expect("no builtin_deref for an unsafe pointer");
+            pointer_ty.builtin_deref(true).expect("no builtin_deref for an raw pointer");
         // Ideally we'd support this in the future, but for now we are limited to sized types.
         if !pointee_ty.is_sized(self.tcx, self.typing_env) {
-            debug!("Unsafe pointer, but pointee is not known to be sized: {:?}", pointer_ty);
+            trace!("Raw pointer, but pointee is not known to be sized: {:?}", pointer_ty);
             return;
         }
 
@@ -171,7 +176,7 @@ impl<'a, 'tcx> Visitor<'tcx> for PointerFinder<'a, 'tcx> {
             _ => pointee_ty,
         };
         if self.excluded_pointees.contains(&element_ty) {
-            debug!("Skipping pointer for type: {:?}", pointee_ty);
+            trace!("Skipping pointer for type: {:?}", pointee_ty);
             return;
         }