[libc-commits] [libc] d7917fd - [libc] Use cpp::byte instead of char in mem* functions
Guillaume Chatelet via libc-commits
libc-commits at lists.llvm.org
Mon Oct 24 03:30:44 PDT 2022
Author: Guillaume Chatelet
Date: 2022-10-24T10:30:32Z
New Revision: d7917fdc0fd25dd032838b781c74e15d7c17400c
URL: https://github.com/llvm/llvm-project/commit/d7917fdc0fd25dd032838b781c74e15d7c17400c
DIFF: https://github.com/llvm/llvm-project/commit/d7917fdc0fd25dd032838b781c74e15d7c17400c.diff
LOG: [libc] Use cpp::byte instead of char in mem* functions
`cpp::byte` is better than `char` which -depending on platform- can be `signed char` or `unsigned char`. This has introduced subtle arithmetic errors.
Added:
Modified:
libc/src/string/memory_utils/CMakeLists.txt
libc/src/string/memory_utils/op_x86.h
libc/src/string/memory_utils/utils.h
libc/test/src/string/memory_utils/CMakeLists.txt
libc/test/src/string/memory_utils/op_tests.cpp
Removed:
################################################################################
diff --git a/libc/src/string/memory_utils/CMakeLists.txt b/libc/src/string/memory_utils/CMakeLists.txt
index 4348fb6e85ac..72b4f1310e59 100644
--- a/libc/src/string/memory_utils/CMakeLists.txt
+++ b/libc/src/string/memory_utils/CMakeLists.txt
@@ -16,6 +16,8 @@ add_header_library(
DEPS
libc.src.__support.common
libc.src.__support.CPP.bit
+ libc.src.__support.CPP.cstddef
+ libc.src.__support.CPP.type_traits
)
add_header_library(
diff --git a/libc/src/string/memory_utils/op_x86.h b/libc/src/string/memory_utils/op_x86.h
index 004e5002f0da..a4b59a12b0b7 100644
--- a/libc/src/string/memory_utils/op_x86.h
+++ b/libc/src/string/memory_utils/op_x86.h
@@ -129,8 +129,8 @@ template <size_t Size> using Bcmp = BcmpImpl<Size, 64, bcmp64>;
static inline MemcmpReturnType char_
diff _no_zero(CPtr p1, CPtr p2,
uint64_t mask) {
const size_t
diff _index = __builtin_ctzll(mask);
- const int16_t ca = cpp::bit_cast<uint8_t>(p1[
diff _index]);
- const int16_t cb = cpp::bit_cast<uint8_t>(p2[
diff _index]);
+ const int16_t ca = cpp::to_integer<uint8_t>(p1[
diff _index]);
+ const int16_t cb = cpp::to_integer<uint8_t>(p2[
diff _index]);
return ca - cb;
}
diff --git a/libc/src/string/memory_utils/utils.h b/libc/src/string/memory_utils/utils.h
index 9d1321f99b83..683bddadd433 100644
--- a/libc/src/string/memory_utils/utils.h
+++ b/libc/src/string/memory_utils/utils.h
@@ -10,6 +10,7 @@
#define LLVM_LIBC_SRC_MEMORY_UTILS_UTILS_H
#include "src/__support/CPP/bit.h"
+#include "src/__support/CPP/cstddef.h"
#include "src/__support/CPP/type_traits.h"
#include <stddef.h> // size_t
@@ -103,8 +104,8 @@ static inline void memcpy_inline(void *__restrict dst,
#endif
}
-using Ptr = char *; // Pointer to raw data.
-using CPtr = const char *; // Const pointer to raw data.
+using Ptr = cpp::byte *; // Pointer to raw data.
+using CPtr = const cpp::byte *; // Const pointer to raw data.
// This type makes sure that we don't accidentally promote an integral type to
// another one. It is only constructible from the exact T type.
diff --git a/libc/test/src/string/memory_utils/CMakeLists.txt b/libc/test/src/string/memory_utils/CMakeLists.txt
index 75647f1d1626..1e4a5d0ed6a1 100644
--- a/libc/test/src/string/memory_utils/CMakeLists.txt
+++ b/libc/test/src/string/memory_utils/CMakeLists.txt
@@ -14,4 +14,5 @@ add_libc_unittest(
libc.src.string.memory_utils.memory_utils
libc.src.__support.CPP.array
libc.src.__support.CPP.span
+ libc.src.__support.CPP.cstddef
)
diff --git a/libc/test/src/string/memory_utils/op_tests.cpp b/libc/test/src/string/memory_utils/op_tests.cpp
index c41c1ae94450..715c61152e1b 100644
--- a/libc/test/src/string/memory_utils/op_tests.cpp
+++ b/libc/test/src/string/memory_utils/op_tests.cpp
@@ -58,6 +58,10 @@ static void Copy(cpp::span<char> dst, const cpp::span<char> src) {
dst[i] = src[i];
}
+cpp::byte *as_byte(cpp::span<char> span) {
+ return reinterpret_cast<cpp::byte *>(span.data());
+}
+
// Simple structure to allocate a buffer of a particular size.
struct PoisonedBuffer {
PoisonedBuffer(size_t size) : ptr((char *)malloc(size)) {
@@ -131,7 +135,7 @@ bool CheckMemcpy(cpp::span<char> dst, cpp::span<char> src, size_t size) {
assert(dst.size() == src.size());
assert(dst.size() == size);
Randomize(dst);
- FnImpl(dst.data(), src.data(), size);
+ FnImpl(as_byte(dst), as_byte(src), size);
for (size_t i = 0; i < size; ++i)
if (dst[i] != src[i])
return false;
@@ -216,7 +220,7 @@ using MemsetImplementations = testing::TypeList<
template <auto FnImpl>
bool CheckMemset(cpp::span<char> dst, uint8_t value, size_t size) {
Randomize(dst);
- FnImpl(dst.data(), value, size);
+ FnImpl(as_byte(dst), value, size);
for (char c : dst)
if (c != (char)value)
return false;
@@ -296,14 +300,14 @@ bool CheckBcmp(cpp::span<char> span1, cpp::span<char> span2, size_t size) {
assert(span1.size() == span2.size());
Copy(span2, span1);
// Compare equal
- if (int cmp = (int)FnImpl(span1.data(), span2.data(), size); cmp != 0)
+ if (int cmp = (int)FnImpl(as_byte(span1), as_byte(span2), size); cmp != 0)
return false;
// Compare not equal if any byte
diff ers
for (size_t i = 0; i < size; ++i) {
++span2[i];
- if (int cmp = (int)FnImpl(span1.data(), span2.data(), size); cmp == 0)
+ if (int cmp = (int)FnImpl(as_byte(span1), as_byte(span2), size); cmp == 0)
return false;
- if (int cmp = (int)FnImpl(span2.data(), span1.data(), size); cmp == 0)
+ if (int cmp = (int)FnImpl(as_byte(span2), as_byte(span1), size); cmp == 0)
return false;
--span2[i];
}
@@ -385,21 +389,21 @@ bool CheckMemcmp(cpp::span<char> span1, cpp::span<char> span2, size_t size) {
assert(span1.size() == span2.size());
Copy(span2, span1);
// Compare equal
- if (int cmp = (int)FnImpl(span1.data(), span2.data(), size); cmp != 0)
+ if (int cmp = (int)FnImpl(as_byte(span1), as_byte(span2), size); cmp != 0)
return false;
// Compare not equal if any byte
diff ers
for (size_t i = 0; i < size; ++i) {
++span2[i];
int ground_truth = __builtin_memcmp(span1.data(), span2.data(), size);
if (ground_truth > 0) {
- if (int cmp = (int)FnImpl(span1.data(), span2.data(), size); cmp <= 0)
+ if (int cmp = (int)FnImpl(as_byte(span1), as_byte(span2), size); cmp <= 0)
return false;
- if (int cmp = (int)FnImpl(span2.data(), span1.data(), size); cmp >= 0)
+ if (int cmp = (int)FnImpl(as_byte(span2), as_byte(span1), size); cmp >= 0)
return false;
} else {
- if (int cmp = (int)FnImpl(span1.data(), span2.data(), size); cmp >= 0)
+ if (int cmp = (int)FnImpl(as_byte(span1), as_byte(span2), size); cmp >= 0)
return false;
- if (int cmp = (int)FnImpl(span2.data(), span1.data(), size); cmp <= 0)
+ if (int cmp = (int)FnImpl(as_byte(span2), as_byte(span1), size); cmp <= 0)
return false;
}
--span2[i];
More information about the libc-commits
mailing list