Add safety comments and use consistent format for existing ones. (goo…

…gle#1981)

We should have safety comments on all `unsafe` blocks, to set a good
example.

src/android/interoperability/with-c.md

@@ -12,6 +12,7 @@ extern "C" {
 
 fn main() {
     let x = -42;
+    // SAFETY: `abs` doesn't have any safety requirements.
     let abs_x = unsafe { abs(x) };
     println!("{x}, {abs_x}");
 }

src/android/interoperability/with-c/bindgen/main.rs

@@ -20,7 +20,10 @@ use birthday_bindgen::{card, print_card};
 fn main() {
     let name = std::ffi::CString::new("Peter").unwrap();
     let card = card { name: name.as_ptr(), years: 42 };
-    // SAFETY: `print_card` is safe to call with a valid `card` pointer.
+    // SAFETY: The pointer we pass is valid because it came from a Rust
+    // reference, and the `name` it contains refers to `name` above which also
+    // remains valid. `print_card` doesn't store either pointer to use later
+    // after it returns.
     unsafe {
         print_card(&card as *const card);
     }

src/android/interoperability/with-c/hand-written.md

@@ -9,6 +9,7 @@ extern "C" {
 
 fn main() {
     let x = -42;
+    // SAFETY: `abs` doesn't have any safety requirements.
     let abs_x = unsafe { abs(x) };
     println!("{x}, {abs_x}");
 }

src/bare-metal/alloc-example/src/main.rs

@@ -29,7 +29,7 @@ static HEAP_ALLOCATOR: LockedHeap<32> = LockedHeap::<32>::new();
 static mut HEAP: [u8; 65536] = [0; 65536];
 
 pub fn entry() {
-    // Safe because `HEAP` is only used here and `entry` is only called once.
+    // SAFETY: `HEAP` is only used here and `entry` is only called once.
     unsafe {
         // Give the allocator some memory to allocate.
         HEAP_ALLOCATOR.lock().init(HEAP.as_mut_ptr() as usize, HEAP.len());

src/bare-metal/aps/examples/src/main_improved.rs

@@ -31,8 +31,8 @@ const PL011_BASE_ADDRESS: *mut u32 = 0x900_0000 as _;
 
 #[no_mangle]
 extern "C" fn main(x0: u64, x1: u64, x2: u64, x3: u64) {
-    // Safe because `PL011_BASE_ADDRESS` is the base address of a PL011 device,
-    // and nothing else accesses that address range.
+    // SAFETY: `PL011_BASE_ADDRESS` is the base address of a PL011 device, and
+    // nothing else accesses that address range.
     let mut uart = unsafe { Uart::new(PL011_BASE_ADDRESS) };
 
     writeln!(uart, "main({x0:#x}, {x1:#x}, {x2:#x}, {x3:#x})").unwrap();

src/bare-metal/aps/examples/src/main_logger.rs

@@ -31,8 +31,8 @@ const PL011_BASE_ADDRESS: *mut u32 = 0x900_0000 as _;
 
 #[no_mangle]
 extern "C" fn main(x0: u64, x1: u64, x2: u64, x3: u64) {
-    // Safe because `PL011_BASE_ADDRESS` is the base address of a PL011 device,
-    // and nothing else accesses that address range.
+    // SAFETY: `PL011_BASE_ADDRESS` is the base address of a PL011 device, and
+    // nothing else accesses that address range.
     let uart = unsafe { Uart::new(PL011_BASE_ADDRESS) };
     logger::init(uart, LevelFilter::Trace).unwrap();
 

src/bare-metal/aps/examples/src/main_minimal.rs

@@ -31,8 +31,8 @@ const PL011_BASE_ADDRESS: *mut u8 = 0x900_0000 as _;
 
 #[no_mangle]
 extern "C" fn main(x0: u64, x1: u64, x2: u64, x3: u64) {
-    // Safe because `PL011_BASE_ADDRESS` is the base address of a PL011 device,
-    // and nothing else accesses that address range.
+    // SAFETY: `PL011_BASE_ADDRESS` is the base address of a PL011 device, and
+    // nothing else accesses that address range.
     let mut uart = unsafe { Uart::new(PL011_BASE_ADDRESS) };
 
     writeln!(uart, "main({:#x}, {:#x}, {:#x}, {:#x})", x0, x1, x2, x3).unwrap();

src/bare-metal/aps/examples/src/main_psci.rs

@@ -25,8 +25,8 @@ const PSCI_SYSTEM_OFF: u32 = 0x84000008;
 
 #[no_mangle]
 extern "C" fn main(_x0: u64, _x1: u64, _x2: u64, _x3: u64) {
-    // Safe because this only uses the declared registers and doesn't do
-    // anything with memory.
+    // SAFETY: this only uses the declared registers and doesn't do anything
+    // with memory.
     unsafe {
         asm!("hvc #0",
             inout("w0") PSCI_SYSTEM_OFF => _,

src/bare-metal/aps/examples/src/pl011.rs

@@ -120,8 +120,8 @@ impl Uart {
         // Wait until there is room in the TX buffer.
         while self.read_flag_register().contains(Flags::TXFF) {}
 
-        // Safe because we know that self.registers points to the control
-        // registers of a PL011 device which is appropriately mapped.
+        // SAFETY: We know that self.registers points to the control registers
+        // of a PL011 device which is appropriately mapped.
         unsafe {
             // Write to the TX buffer.
             addr_of_mut!((*self.registers).dr).write_volatile(byte.into());
@@ -137,15 +137,17 @@ impl Uart {
         if self.read_flag_register().contains(Flags::RXFE) {
             None
         } else {
+            // SAFETY: We know that self.registers points to the control
+            // registers of a PL011 device which is appropriately mapped.
             let data = unsafe { addr_of!((*self.registers).dr).read_volatile() };
             // TODO: Check for error conditions in bits 8-11.
             Some(data as u8)
         }
     }
 
     fn read_flag_register(&self) -> Flags {
-        // Safe because we know that self.registers points to the control
-        // registers of a PL011 device which is appropriately mapped.
+        // SAFETY: We know that self.registers points to the control registers
+        // of a PL011 device which is appropriately mapped.
         unsafe { addr_of!((*self.registers).fr).read_volatile() }
     }
 }

src/bare-metal/aps/examples/src/pl011_minimal.rs

@@ -41,7 +41,7 @@ impl Uart {
         // Wait until there is room in the TX buffer.
         while self.read_flag_register() & FR_TXFF != 0 {}
 
-        // Safe because we know that the base address points to the control
+        // SAFETY: We know that the base address points to the control
         // registers of a PL011 device which is appropriately mapped.
         unsafe {
             // Write to the TX buffer.
@@ -53,7 +53,7 @@ impl Uart {
     }
 
     fn read_flag_register(&self) -> u8 {
-        // Safe because we know that the base address points to the control
+        // SAFETY: We know that the base address points to the control
         // registers of a PL011 device which is appropriately mapped.
         unsafe { self.base_address.add(FLAG_REGISTER_OFFSET).read_volatile() }
     }
@@ -72,7 +72,7 @@ impl Write for Uart {
     }
 }
 
-// Safe because it just contains a pointer to device memory, which can be
+// SAFETY: `Uart` just contains a pointer to device memory, which can be
 // accessed from any context.
 unsafe impl Send for Uart {}
 // ANCHOR_END: Traits

src/bare-metal/microcontrollers/examples/src/bin/mmio.rs

@@ -43,8 +43,8 @@ fn main() -> ! {
     // Configure GPIO 0 pins 21 and 28 as push-pull outputs.
     let pin_cnf_21 = (GPIO_P0 + PIN_CNF + 21 * size_of::<u32>()) as *mut u32;
     let pin_cnf_28 = (GPIO_P0 + PIN_CNF + 28 * size_of::<u32>()) as *mut u32;
-    // Safe because the pointers are to valid peripheral control registers, and
-    // no aliases exist.
+    // SAFETY: The pointers are to valid peripheral control registers, and no
+    // aliases exist.
     unsafe {
         pin_cnf_21.write_volatile(
             DIR_OUTPUT
@@ -65,8 +65,8 @@ fn main() -> ! {
     // Set pin 28 low and pin 21 high to turn the LED on.
     let gpio0_outset = (GPIO_P0 + OUTSET) as *mut u32;
     let gpio0_outclr = (GPIO_P0 + OUTCLR) as *mut u32;
-    // Safe because the pointers are to valid peripheral control registers, and
-    // no aliases exist.
+    // SAFETY: The pointers are to valid peripheral control registers, and no
+    // aliases exist.
     unsafe {
         gpio0_outclr.write_volatile(1 << 28);
         gpio0_outset.write_volatile(1 << 21);

src/exercises/bare-metal/rtc/src/main.rs

@@ -52,22 +52,22 @@ const PL031_IRQ: IntId = IntId::spi(2);
 // ANCHOR: main
 #[no_mangle]
 extern "C" fn main(x0: u64, x1: u64, x2: u64, x3: u64) {
-    // Safe because `PL011_BASE_ADDRESS` is the base address of a PL011 device,
-    // and nothing else accesses that address range.
+    // SAFETY: `PL011_BASE_ADDRESS` is the base address of a PL011 device, and
+    // nothing else accesses that address range.
     let uart = unsafe { Uart::new(PL011_BASE_ADDRESS) };
     logger::init(uart, LevelFilter::Trace).unwrap();
 
     info!("main({:#x}, {:#x}, {:#x}, {:#x})", x0, x1, x2, x3);
 
-    // Safe because `GICD_BASE_ADDRESS` and `GICR_BASE_ADDRESS` are the base
+    // SAFETY: `GICD_BASE_ADDRESS` and `GICR_BASE_ADDRESS` are the base
     // addresses of a GICv3 distributor and redistributor respectively, and
     // nothing else accesses those address ranges.
     let mut gic = unsafe { GicV3::new(GICD_BASE_ADDRESS, GICR_BASE_ADDRESS) };
     gic.setup();
     // ANCHOR_END: main
 
-    // Safe because `PL031_BASE_ADDRESS` is the base address of a PL031 device,
-    // and nothing else accesses that address range.
+    // SAFETY: `PL031_BASE_ADDRESS` is the base address of a PL031 device, and
+    // nothing else accesses that address range.
     let mut rtc = unsafe { Rtc::new(PL031_BASE_ADDRESS) };
     let timestamp = rtc.read();
     let time = Utc.timestamp_opt(timestamp.into(), 0).unwrap();

src/exercises/bare-metal/rtc/src/pl011.rs

@@ -122,8 +122,8 @@ impl Uart {
         // Wait until there is room in the TX buffer.
         while self.read_flag_register().contains(Flags::TXFF) {}
 
-        // Safe because we know that self.registers points to the control
-        // registers of a PL011 device which is appropriately mapped.
+        // SAFETY: We know that self.registers points to the control registers
+        // of a PL011 device which is appropriately mapped.
         unsafe {
             // Write to the TX buffer.
             addr_of_mut!((*self.registers).dr).write_volatile(byte.into());
@@ -139,15 +139,17 @@ impl Uart {
         if self.read_flag_register().contains(Flags::RXFE) {
             None
         } else {
+            // SAFETY: We know that self.registers points to the control
+            // registers of a PL011 device which is appropriately mapped.
             let data = unsafe { addr_of!((*self.registers).dr).read_volatile() };
             // TODO: Check for error conditions in bits 8-11.
             Some(data as u8)
         }
     }
 
     fn read_flag_register(&self) -> Flags {
-        // Safe because we know that self.registers points to the control
-        // registers of a PL011 device which is appropriately mapped.
+        // SAFETY: We know that self.registers points to the control registers
+        // of a PL011 device which is appropriately mapped.
         unsafe { addr_of!((*self.registers).fr).read_volatile() }
     }
 }

src/exercises/bare-metal/rtc/src/pl031.rs

@@ -61,24 +61,24 @@ impl Rtc {
 
     /// Reads the current RTC value.
     pub fn read(&self) -> u32 {
-        // Safe because we know that self.registers points to the control
-        // registers of a PL031 device which is appropriately mapped.
+        // SAFETY: We know that self.registers points to the control registers
+        // of a PL031 device which is appropriately mapped.
         unsafe { addr_of!((*self.registers).dr).read_volatile() }
     }
 
     /// Writes a match value. When the RTC value matches this then an interrupt
     /// will be generated (if it is enabled).
     pub fn set_match(&mut self, value: u32) {
-        // Safe because we know that self.registers points to the control
-        // registers of a PL031 device which is appropriately mapped.
+        // SAFETY: We know that self.registers points to the control registers
+        // of a PL031 device which is appropriately mapped.
         unsafe { addr_of_mut!((*self.registers).mr).write_volatile(value) }
     }
 
     /// Returns whether the match register matches the RTC value, whether or not
     /// the interrupt is enabled.
     pub fn matched(&self) -> bool {
-        // Safe because we know that self.registers points to the control
-        // registers of a PL031 device which is appropriately mapped.
+        // SAFETY: We know that self.registers points to the control registers
+        // of a PL031 device which is appropriately mapped.
         let ris = unsafe { addr_of!((*self.registers).ris).read_volatile() };
         (ris & 0x01) != 0
     }
@@ -88,8 +88,8 @@ impl Rtc {
     /// This should be true if and only if `matched` returns true and the
     /// interrupt is masked.
     pub fn interrupt_pending(&self) -> bool {
-        // Safe because we know that self.registers points to the control
-        // registers of a PL031 device which is appropriately mapped.
+        // SAFETY: We know that self.registers points to the control registers
+        // of a PL031 device which is appropriately mapped.
         let ris = unsafe { addr_of!((*self.registers).mis).read_volatile() };
         (ris & 0x01) != 0
     }
@@ -100,19 +100,19 @@ impl Rtc {
     /// interrupt is disabled.
     pub fn enable_interrupt(&mut self, mask: bool) {
         let imsc = if mask { 0x01 } else { 0x00 };
-        // Safe because we know that self.registers points to the control
-        // registers of a PL031 device which is appropriately mapped.
+        // SAFETY: We know that self.registers points to the control registers
+        // of a PL031 device which is appropriately mapped.
         unsafe { addr_of_mut!((*self.registers).imsc).write_volatile(imsc) }
     }
 
     /// Clears a pending interrupt, if any.
     pub fn clear_interrupt(&mut self) {
-        // Safe because we know that self.registers points to the control
-        // registers of a PL031 device which is appropriately mapped.
+        // SAFETY: We know that self.registers points to the control registers
+        // of a PL031 device which is appropriately mapped.
         unsafe { addr_of_mut!((*self.registers).icr).write_volatile(0x01) }
     }
 }
 
-// Safe because it just contains a pointer to device memory, which can be
+// SAFETY: `Rtc` just contains a pointer to device memory, which can be
 // accessed from any context.
 unsafe impl Send for Rtc {}

src/unsafe-rust/dereferencing.md

@@ -13,11 +13,11 @@ fn main() {
     let r1 = &mut s as *mut String;
     let r2 = r1 as *const String;
 
-    // Safe because r1 and r2 were obtained from references and so are
-    // guaranteed to be non-null and properly aligned, the objects underlying
-    // the references from which they were obtained are live throughout the
-    // whole unsafe block, and they are not accessed either through the
-    // references or concurrently through any other pointers.
+    // SAFETY: r1 and r2 were obtained from references and so are guaranteed to
+    // be non-null and properly aligned, the objects underlying the references
+    // from which they were obtained are live throughout the whole unsafe
+    // block, and they are not accessed either through the references or
+    // concurrently through any other pointers.
     unsafe {
         println!("r1 is: {}", *r1);
         *r1 = String::from("uhoh");

src/unsafe-rust/mutable-static.md

@@ -21,6 +21,7 @@ static variables:
 static mut COUNTER: u32 = 0;
 
 fn add_to_counter(inc: u32) {
+    // SAFETY: There are no other threads which could be accessing `COUNTER`.
     unsafe {
         COUNTER += inc;
     }
@@ -29,6 +30,7 @@ fn add_to_counter(inc: u32) {
 fn main() {
     add_to_counter(42);
 
+    // SAFETY: There are no other threads which could be accessing `COUNTER`.
     unsafe {
         println!("COUNTER: {COUNTER}");
     }

src/unsafe-rust/unsafe-functions.md

@@ -17,8 +17,8 @@ extern "C" {
 fn main() {
     let emojis = "🗻∈🌏";
 
-    // Safe because the indices are in the correct order, within the bounds of
-    // the string slice, and lie on UTF-8 sequence boundaries.
+    // SAFETY: The indices are in the correct order, within the bounds of the
+    // string slice, and lie on UTF-8 sequence boundaries.
     unsafe {
         println!("emoji: {}", emojis.get_unchecked(0..4));
         println!("emoji: {}", emojis.get_unchecked(4..7));
@@ -27,8 +27,9 @@ fn main() {
 
     println!("char count: {}", count_chars(unsafe { emojis.get_unchecked(0..7) }));
 
+    // SAFETY: `abs` doesn't deal with pointers and doesn't have any safety
+    // requirements.
     unsafe {
-        // Undefined behavior if abs misbehaves.
         println!("Absolute value of -3 according to C: {}", abs(-3));
     }
 
@@ -64,7 +65,7 @@ fn main() {
     let mut a = 42;
     let mut b = 66;
 
-    // Safe because ...
+    // SAFETY: ...
     unsafe {
         swap(&mut a, &mut b);
     }

src/unsafe-rust/unsafe-traits.md

@@ -28,7 +28,7 @@ pub unsafe trait AsBytes {
     }
 }
 
-// Safe because u32 has a defined representation and no padding.
+// SAFETY: `u32` has a defined representation and no padding.
 unsafe impl AsBytes for u32 {}
 ```