From 3d3d11b3610a7d202995137acb82e29c17cda8e7 Mon Sep 17 00:00:00 2001 From: Lukasz Anforowicz Date: Fri, 25 Oct 2024 16:18:38 +0000 Subject: [PATCH] Add safety comments to more granular `unsafe` blocks. --- src/byteset/scalar.rs | 137 ++++++++++++++++++++++++------------------ 1 file changed, 78 insertions(+), 59 deletions(-) diff --git a/src/byteset/scalar.rs b/src/byteset/scalar.rs index fe5ab63..627e8bf 100644 --- a/src/byteset/scalar.rs +++ b/src/byteset/scalar.rs @@ -21,44 +21,53 @@ pub fn inv_memchr(n1: u8, haystack: &[u8]) -> Option { let vn1 = repeat_byte(n1); let confirm = |byte| byte != n1; let start_ptr = haystack.as_ptr(); + let mut ptr = start_ptr; - unsafe { - let end_ptr = haystack.as_ptr().add(haystack.len()); - let mut ptr = start_ptr; + // SAFETY: 2nd safety requirement of `sub` either 1) doesn't apply when `haystack.len()` is + // zero or 2) is in-bounds (i.e. one-past allocation) of the same allocated object. + let end_ptr = unsafe { start_ptr.add(haystack.len()) }; - if haystack.len() < USIZE_BYTES { - return forward_search(start_ptr, end_ptr, ptr, confirm); - } + if haystack.len() < USIZE_BYTES { + return unsafe { forward_search(start_ptr, end_ptr, ptr, confirm) }; + } - let chunk = read_unaligned_usize(ptr); - if (chunk ^ vn1) != 0 { - return forward_search(start_ptr, end_ptr, ptr, confirm); - } + let chunk = unsafe { read_unaligned_usize(ptr) }; + if (chunk ^ vn1) != 0 { + return unsafe { forward_search(start_ptr, end_ptr, ptr, confirm) }; + } - ptr = ptr.add(USIZE_BYTES - (start_ptr as usize & ALIGN_MASK)); - debug_assert!(ptr > start_ptr); - debug_assert!(end_ptr.sub(USIZE_BYTES) >= start_ptr); - - if haystack.len() >= LOOP_SIZE { - // The `if` condition guarantees that `end_ptr.sub(LOOP_SIZE)` (in the loop condition) - // meets the safety requrement that the result must be in bounds of the same allocated - // object. - while ptr <= end_ptr.sub(LOOP_SIZE) { - debug_assert_eq!(0, (ptr as usize) % USIZE_BYTES); - - let a = *(ptr as *const usize); - let b = *(ptr.add(USIZE_BYTES) as *const usize); - let eqa = (a ^ vn1) != 0; - let eqb = (b ^ vn1) != 0; - if eqa || eqb { - break; - } - ptr = ptr.add(LOOP_SIZE); + // SAFETY: Adding `1..=USIZE_BYTES`. One of the `if`s above means that `haystack.len() >= + // USIZE_BYTES`. So the result of `add` is in-bounds of the same allocated object. + ptr = unsafe { ptr.add(USIZE_BYTES - (start_ptr as usize & ALIGN_MASK)) }; + debug_assert!(ptr > start_ptr); + // SAFETY: One of the `if`s above means that `haystack.len() >= USIZE_BYTES`. So the result of + // `sub` is in-bounds of the same allocated object. + debug_assert!(unsafe { end_ptr.sub(USIZE_BYTES) } >= start_ptr); + + if haystack.len() >= LOOP_SIZE { + // SAFETY: The `if` condition above guarantees that `end_ptr.sub(LOOP_SIZE)` will + // stay in bounds of the same allocated object. + while ptr <= unsafe { end_ptr.sub(LOOP_SIZE) } { + debug_assert_eq!(0, (ptr as usize) % USIZE_BYTES); + + // SAFETY: Loop condition (and the fact that `LOOP_SIZE` is twice the size of + // `USIZE_BYTES` together guarantee that dereferences and `add` have their + // safety requirements met. + let a = unsafe { *(ptr as *const usize) }; + let b = unsafe { *(ptr.add(USIZE_BYTES) as *const usize) }; + let eqa = (a ^ vn1) != 0; + let eqb = (b ^ vn1) != 0; + if eqa || eqb { + break; } - } - forward_search(start_ptr, end_ptr, ptr, confirm) + // SAFETY: The loop condition guarantees that `add` will stay in bounds of the same + // allocated object. + ptr = unsafe { ptr.add(LOOP_SIZE) }; + } } + + unsafe { forward_search(start_ptr, end_ptr, ptr, confirm) } } /// Return the last index not matching the byte `x` in `text`. @@ -67,40 +76,50 @@ pub fn inv_memrchr(n1: u8, haystack: &[u8]) -> Option { let confirm = |byte| byte != n1; let start_ptr = haystack.as_ptr(); - unsafe { - let end_ptr = haystack.as_ptr().add(haystack.len()); - let mut ptr = end_ptr; + // SAFETY: 2nd safety requirement of `add` either 1) doesn't apply when `haystack.len()` is + // zero or 2) is in-bounds (i.e. one-past allocation) of the same allocated object. + let end_ptr = unsafe { start_ptr.add(haystack.len()) }; + let mut ptr = end_ptr; - if haystack.len() < USIZE_BYTES { - return reverse_search(start_ptr, end_ptr, ptr, confirm); - } + if haystack.len() < USIZE_BYTES { + return unsafe { reverse_search(start_ptr, end_ptr, ptr, confirm) }; + } - let chunk = read_unaligned_usize(ptr.sub(USIZE_BYTES)); - if (chunk ^ vn1) != 0 { - return reverse_search(start_ptr, end_ptr, ptr, confirm); - } + let chunk = unsafe { read_unaligned_usize(ptr.sub(USIZE_BYTES)) }; + if (chunk ^ vn1) != 0 { + return unsafe { reverse_search(start_ptr, end_ptr, ptr, confirm) }; + } - ptr = ptr.sub(end_ptr as usize & ALIGN_MASK); - debug_assert!(start_ptr <= ptr && ptr <= end_ptr); - if haystack.len() >= LOOP_SIZE { - // The `if` condition guarantees that `start_ptr.add(LOOP_SIZE)` (in the loop - // condition) meets the safety requrement that the result must be in bounds of the same - // allocated object. - while ptr >= start_ptr.add(LOOP_SIZE) { - debug_assert_eq!(0, (ptr as usize) % USIZE_BYTES); - - let a = *(ptr.sub(2 * USIZE_BYTES) as *const usize); - let b = *(ptr.sub(1 * USIZE_BYTES) as *const usize); - let eqa = (a ^ vn1) != 0; - let eqb = (b ^ vn1) != 0; - if eqa || eqb { - break; - } - ptr = ptr.sub(LOOP_SIZE); + // SAFETY: Subtracting `1..=USIZE_BYTES`. One of the `if`s above means that `haystack.len() >= + // USIZE_BYTES`. So the result of `sub` is in-bounds of the same allocated object. + ptr = unsafe { ptr.sub(end_ptr as usize & ALIGN_MASK) }; + debug_assert!(start_ptr <= ptr && ptr <= end_ptr); + + if haystack.len() >= LOOP_SIZE { + // SAFETY: The `if` condition above guarantees that `start_ptr.add(LOOP_SIZE)` will + // stay in bounds of the same allocated object. + while ptr >= unsafe { start_ptr.add(LOOP_SIZE) } { + debug_assert_eq!(0, (ptr as usize) % USIZE_BYTES); + + // SAFETY: Loop condition (and the fact that `LOOP_SIZE` is twice the size of + // `USIZE_BYTES` together guarantee that dereferences and `sub`s have their + // safety requirements met. + let a = unsafe { *(ptr.sub(2 * USIZE_BYTES) as *const usize) }; + let b = unsafe { *(ptr.sub(1 * USIZE_BYTES) as *const usize) }; + + let eqa = (a ^ vn1) != 0; + let eqb = (b ^ vn1) != 0; + if eqa || eqb { + break; } + + // SAFETY: The loop condition guarantees that `sub` will stay in bounds of the same + // allocated object. + ptr = unsafe { ptr.sub(LOOP_SIZE) }; } - reverse_search(start_ptr, end_ptr, ptr, confirm) } + + unsafe { reverse_search(start_ptr, end_ptr, ptr, confirm) } } #[inline(always)]