[libc] -Warray-bounds= in libc/src/string/memory_utils/op_builtin.h

[nickdesaulniers]

building with GCC produces the following diagnostic:

In static member function ‘static void __llvm_libc_18_0_0_git::builtin::Memcpy<Size>::block_offset(__llvm_libc_18_0_0_git::Ptr, __llvm_libc_18_0_0_git::CPtr, size_t) [with long unsigned int Size = 32]’,
    inlined from ‘static void __llvm_libc_18_0_0_git::builtin::Memcpy<Size>::tail(__llvm_libc_18_0_0_git::Ptr, __llvm_libc_18_0_0_git::CPtr, size_t) [with long unsigned int Size = 32]’ at /android0/llvm-project/libc/src/string/memory_utils/op_builtin.h:44:17,
    inlined from ‘static void __llvm_libc_18_0_0_git::builtin::Memcpy<Size>::loop_and_tail_offset(__llvm_libc_18_0_0_git::Ptr, __llvm_libc_18_0_0_git::CPtr, size_t, size_t) [with long unsigned int Size = 32]’ at /android0/llvm-project/libc/src/string/memory_utils/op_builtin.h:61:9,
    inlined from ‘static void __llvm_libc_18_0_0_git::builtin::Memcpy<Size>::loop_and_tail(__llvm_libc_18_0_0_git::Ptr, __llvm_libc_18_0_0_git::CPtr, size_t) [with long unsigned int Size = 32]’ at /android0/llvm-project/libc/src/string/memory_utils/op_builtin.h:66:32,
    inlined from ‘void __llvm_libc_18_0_0_git::inline_memcpy_x86_sse2_ge64(Ptr, CPtr, size_t)’ at /android0/llvm-project/libc/src/string/memory_utils/x86_64/inline_memcpy.h:57:44,
    inlined from ‘void __llvm_libc_18_0_0_git::inline_memcpy_x86(Ptr, CPtr, size_t)’ at /android0/llvm-project/libc/src/string/memory_utils/x86_64/inline_memcpy.h:193:41,
    inlined from ‘void __llvm_libc_18_0_0_git::inline_memcpy_x86_maybe_interpose_repmovsb(Ptr, CPtr, size_t)’ at /android0/llvm-project/libc/src/string/memory_utils/x86_64/inline_memcpy.h:204:29,
    inlined from ‘void __llvm_libc_18_0_0_git::inline_memcpy(void*, const void*, size_t)’ at /android0/llvm-project/libc/src/string/memory_utils/inline_memcpy.h:45:38,
    inlined from ‘int __llvm_libc_18_0_0_git::printf_core::Writer::write(__llvm_libc_18_0_0_git::cpp::string_view)’ at /android0/llvm-project/libc/src/stdio/printf_core/writer.h:98:20,
    inlined from ‘int __llvm_libc_18_0_0_git::printf_core::convert_inf_nan(Writer*, const FormatSection&)’ at /android0/llvm-project/libc/src/stdio/printf_core/float_inf_nan_converter.h:68:5:
/android0/llvm-project/libc/src/string/memory_utils/op_builtin.h:34:39: warning: array subscript [66, 9223372036854775807] is outside array bounds of ‘const char [4]’ [-Warray-bounds=]
   34 |       dst[i + offset] = src[i + offset];
      |                         ~~~~~~~~~~~~~~^

FWICT, Memcpy::block_offset falls back to a element by element copy if LLVM_LIBC_HAS_BUILTIN_MEMCPY_INLINE is not defined (hence this only being reproducible with GCC).

IIUC, it looks like inline_memcpy_x86 is generating a 32 element Memcpy (Size = 32), which indeed would read outside of src for a string literal like is done in convert_inf_nan

RET_IF_RESULT_NEGATIVE(writer->write(a == 'a' ? "inf" : "INF"));

in that case, I don't see why

RET_IF_RESULT_NEGATIVE(writer->write(sign_char));

doesn't warn since sign_char is a char.

---

I'm not sure yet if this can/should be fixed in convert_inf_nan, Memcpy, or somewhere else. But we need to fix this to re-enable -Werror in #74506

cc @gchatelet @michaelrj-google

[michaelrj-google]

The reason it's not erroring on writing a single char is because the write function is templated, see https://github.com/llvm/llvm-project/blob/main/libc/src/stdio/printf_core/writer.h#L122

I'm not sure how it's ending up with 32 as the size for the array, the version of write that's being called is the string_view version, and the const string is being implicitly converted to a string_view with this constructor: https://github.com/llvm/llvm-project/blob/main/libc/src/__support/CPP/string_view.h#L70
From this I'd guess that somehow the length function is getting messed up but I'm not sure how.

[gchatelet]

This looks like another instance of

llvm-project/libc/src/string/memory_utils/utils.h

Lines 82 to 99 in 79e6231

	// Performs a constant count copy.
	template <size_t Size>
	LIBC_INLINE void memcpy_inline(void *__restrict dst,
	const void *__restrict src) {
	#ifdef LLVM_LIBC_HAS_BUILTIN_MEMCPY_INLINE
	__builtin_memcpy_inline(dst, src, Size);
	#else
	// In memory functions `memcpy_inline` is instantiated several times with
	// different value of the Size parameter. This doesn't play well with GCC's
	// Value Range Analysis that wrongly detects out of bounds accesses. We
	// disable the 'array-bounds' warning for the purpose of this function.
	#pragma GCC diagnostic push
	#pragma GCC diagnostic ignored "-Warray-bounds"
	for (size_t i = 0; i < Size; ++i)
	static_cast<char *>(dst)[i] = static_cast<const char *>(src)[i];
	#pragma GCC diagnostic pop
	#endif
	}

static void __llvm_libc_18_0_0_git::builtin::Memcpy<Size>::block_offset should be implemented in terms of memcpy_inline.