Skip to content

Commit 850ca5a

Browse files
committed
Fix strict aliasing violation in swap function
There might be platform where addressing the user provided array of unknown alignment as `uint32_t` would be incorrect. Generally the previous code violates strict aliasing and thus invokes UB. By using the "magic" properties of `memcpy` we can sidestep the issue by letting it do the load and store for us. In practice this means on platforms where addressing the memory as 4 byte regions is safe and efficient to do - which is most widely used platforms - the compiler can insert the appropriate `mov` instructions inline and if a platform does not support this operation it will likely insert a call to the `memcpy` function.
1 parent aecca52 commit 850ca5a

File tree

1 file changed

+14
-19
lines changed

1 file changed

+14
-19
lines changed

libc/src/stdlib/qsort_data.h

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@
1111

1212
#include "src/__support/CPP/cstddef.h"
1313
#include "src/__support/macros/config.h"
14-
#include "src/string/memory_utils/inline_memcpy.h"
15-
#include "src/string/memory_utils/inline_memmove.h"
1614

1715
#include <stdint.h>
1816

@@ -48,26 +46,23 @@ class ArrayGenericSize {
4846
using block_t = uint32_t;
4947
constexpr size_t BLOCK_SIZE = sizeof(block_t);
5048

49+
alignas(block_t) cpp::byte tmp_block[BLOCK_SIZE];
50+
5151
cpp::byte *elem_i = get_internal(i);
5252
cpp::byte *elem_j = get_internal(j);
5353

5454
const size_t elem_size_rem = elem_size % BLOCK_SIZE;
55-
const block_t *elem_i_block_end =
56-
reinterpret_cast<block_t *>(elem_i + (elem_size - elem_size_rem));
57-
58-
block_t *elem_i_block = reinterpret_cast<block_t *>(elem_i);
59-
block_t *elem_j_block = reinterpret_cast<block_t *>(elem_j);
60-
61-
while (elem_i_block != elem_i_block_end) {
62-
block_t tmp = *elem_i_block;
63-
*elem_i_block = *elem_j_block;
64-
*elem_j_block = tmp;
65-
elem_i_block += 1;
66-
elem_j_block += 1;
55+
const cpp::byte *elem_i_block_end = elem_i + (elem_size - elem_size_rem);
56+
57+
while (elem_i != elem_i_block_end) {
58+
__builtin_memcpy(tmp_block, elem_i, BLOCK_SIZE);
59+
__builtin_memcpy(elem_i, elem_j, BLOCK_SIZE);
60+
__builtin_memcpy(elem_j, tmp_block, BLOCK_SIZE);
61+
62+
elem_i += BLOCK_SIZE;
63+
elem_j += BLOCK_SIZE;
6764
}
6865

69-
elem_i = reinterpret_cast<cpp::byte *>(elem_i_block);
70-
elem_j = reinterpret_cast<cpp::byte *>(elem_j_block);
7166
for (size_t n = 0; n < elem_size_rem; ++n) {
7267
cpp::byte tmp = elem_i[n];
7368
elem_i[n] = elem_j[n];
@@ -118,9 +113,9 @@ template <size_t ELEM_SIZE> class ArrayFixedSize {
118113
cpp::byte *elem_i = get_internal(i);
119114
cpp::byte *elem_j = get_internal(j);
120115

121-
inline_memcpy(tmp, elem_i, ELEM_SIZE);
122-
inline_memmove(elem_i, elem_j, ELEM_SIZE);
123-
inline_memcpy(elem_j, tmp, ELEM_SIZE);
116+
__builtin_memcpy(tmp, elem_i, ELEM_SIZE);
117+
__builtin_memmove(elem_i, elem_j, ELEM_SIZE);
118+
__builtin_memcpy(elem_j, tmp, ELEM_SIZE);
124119
}
125120

126121
LIBC_INLINE size_t len() const { return array_len; }

0 commit comments

Comments
 (0)