[libc-commits] [libc] 3179726 - [libc] Fix load_aligned big-endian handling. (#185937)

[via libc-commits]

Author: Simon Tatham
Date: 2026-03-13T08:59:55Z
New Revision: 317972681c79fcbecbeaca94189247256b3a513a

URL: https://github.com/llvm/llvm-project/commit/317972681c79fcbecbeaca94189247256b3a513a
DIFF: https://github.com/llvm/llvm-project/commit/317972681c79fcbecbeaca94189247256b3a513a.diff

LOG: [libc] Fix load_aligned big-endian handling. (#185937)

The variadic template helper `load_aligned` performs a specific case of
an unaligned integer load, by loading a sequence of integers from memory
at addresses expected to be aligned, and glues the results back together
with shifts and ORs into an output.

The implementation works by performing the first load, recursing on a
shorter parameter type list for the rest, and recombining via

  first | (rest << size_of_first)           // if little-endian
  (first << size_of_first) | rest           // if big-endian

But the big-endian case is wrong: it should shift left by the size of
the _rest_ of the types, not the size of the first. In the case where
you load 8, 16 and 8 bits from an odd address, you want

  (first_byte << 24) | (middle_halfword << 8) | (last_byte)

but in fact we were calculating

  (first_byte << 8) | (middle_halfword << 16) | (last_byte)

leading to three out of every four bytes being permuted in a memcpy from
an odd to an even address.

Added: 
    

Modified: 
    libc/src/string/memory_utils/utils.h
    libc/test/src/string/memory_utils/utils_test.cpp

Removed: 
    


################################################################################
diff  --git a/libc/src/string/memory_utils/utils.h b/libc/src/string/memory_utils/utils.h
index 86ff4f12e8c26..b0cfd29ca968c 100644
--- a/libc/src/string/memory_utils/utils.h
+++ b/libc/src/string/memory_utils/utils.h
@@ -233,13 +233,18 @@ LIBC_INLINE ValueType load_aligned(CPtr src) {
   static_assert(sizeof(ValueType) >= (sizeof(T) + ... + sizeof(TS)));
   const ValueType value = load<T>(assume_aligned<sizeof(T)>(src));
   if constexpr (sizeof...(TS) > 0) {
-    constexpr size_t SHIFT = sizeof(T) * 8;
     const ValueType next = load_aligned<ValueType, TS...>(src + sizeof(T));
-    if constexpr (Endian::IS_LITTLE)
+    if constexpr (Endian::IS_LITTLE) {
+      // T goes at the bottom of the output, so shift up everything
+      // else by the number of bits in T.
+      constexpr size_t SHIFT = sizeof(T) * 8;
       return value | (next << SHIFT);
-    else if constexpr (Endian::IS_BIG)
+    } else if constexpr (Endian::IS_BIG) {
+      // T goes at the top of the output, so shift it up by the number
+      // of bits in everything else that goes below it.
+      constexpr size_t SHIFT = (0 + ... + sizeof(TS)) * 8;
       return (value << SHIFT) | next;
-    else
+    } else
       static_assert(cpp::always_false<T>, "Invalid endianness");
   } else {
     return value;

diff  --git a/libc/test/src/string/memory_utils/utils_test.cpp b/libc/test/src/string/memory_utils/utils_test.cpp
index 2ab2e8d3ad867..8ad0dce5062c6 100644
--- a/libc/test/src/string/memory_utils/utils_test.cpp
+++ b/libc/test/src/string/memory_utils/utils_test.cpp
@@ -138,4 +138,22 @@ TEST(LlvmLibcUtilsTest, LoadStoreAligned) {
   }
 }
 
+TEST(LlvmLibcUtilsTest, LoadStoreAlignedOddAddress) {
+  const uint8_t inbuf[8] = {0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF};
+  uint8_t outbuf[8];
+
+  const uint32_t loaded = load_aligned<uint32_t, uint8_t, uint16_t, uint8_t>(
+      reinterpret_cast<CPtr>(inbuf + 1));
+  if constexpr (Endian::IS_LITTLE) {
+    EXPECT_EQ(loaded, uint32_t(0x89674523));
+  } else if constexpr (Endian::IS_BIG) {
+    EXPECT_EQ(loaded, uint32_t(0x23456789));
+  }
+  store_aligned<uint32_t, uint8_t, uint16_t, uint8_t>(
+      loaded, reinterpret_cast<Ptr>(outbuf + 1));
+
+  for (size_t i = 1; i < 5; ++i)
+    EXPECT_EQ(outbuf[i], inbuf[i]);
+}
+
 } // namespace LIBC_NAMESPACE_DECL