[libc-commits] [libc] [libc] Read known objects bytewise in the clang_vector string loads (PR #236612)

[Mike Lothian via libc-commits]

================
@@ -28,6 +28,17 @@ LIBC_INLINE constexpr cpp::simd_mask<byte_ty> shift_mask(cpp::simd_mask<char> m,
 LIBC_NO_SANITIZE_OOB_ACCESS LIBC_INLINE size_t string_length(const char *src) {
   constexpr cpp::simd<char> null_byte = cpp::splat('\0');
 
+  // The vector loads below read outside the string on purpose. That is only
+  // sound when the compiler cannot see the object, since it treats a read
+  // outside a known object as undefined. Once a call is inlined on a known
+  // array, as under LTO, the size is known, so read it bytewise.
+  if (__builtin_object_size(src, 0) != static_cast<size_t>(-1)) {
----------------
FireBurn wrote:

On the bounds: type 0 is the maximum estimate and type 2 the minimum, and the minimum is 0 whenever the compiler cannot say which object it is, so the two only agree when the size is exact. The printf case needs the exact size, so it needs both to match. Here the question is only whether the compiler can see an object the pointer may refer to, and the maximum answers that. If it is known, even as an upper bound for a pointer that may refer to either of two arrays, the vector loads may be treated as out of bounds, so the bytewise loop is the safe choice. Checking both bounds would send that two array case back to the vector loads

On using the size directly: the vector path starts at the aligned address at or below `src`, so its first load reads bytes before the object whatever the size is. Knowing how many bytes follow `src` does not make that load valid, and the string has to end inside the object anyway, so the size would not shorten the scan. That is why the fallback is the plain loop and not something sized. A pointer to a known object is mostly a small local array or a literal, where the cost is not worth a second vector path


https://github.com/llvm/llvm-project/pull/236612