FIX: store_be(size_t(42)) would not compile on Xcode

src/lib/utils/bswap.h

@@ -2,6 +2,7 @@
 * Byte Swapping Operations
 * (C) 1999-2011,2018 Jack Lloyd
 * (C) 2007 Yves Jerschow
+* (C) 2024 René Meusel - Rohde & Schwarz Cybersecurity
 *
 * TODO: C++23: replace this entire implementation with std::byteswap
 *
@@ -16,49 +17,40 @@
 namespace Botan {
 
 /**
-* Swap a 16 bit integer
-*/
-inline constexpr uint16_t reverse_bytes(uint16_t x) {
+ * Swap the byte order of an unsigned integer
+ */
+template <std::unsigned_integral T>
+   requires(sizeof(T) == 1 || sizeof(T) == 2 || sizeof(T) == 4 || sizeof(T) == 8)
+inline constexpr T reverse_bytes(T x) {
+   if constexpr(sizeof(T) == 1) {
+      return x;
+   } else if constexpr(sizeof(T) == 2) {
 #if BOTAN_COMPILER_HAS_BUILTIN(__builtin_bswap16)
-   return __builtin_bswap16(x);
+      return static_cast<T>(__builtin_bswap16(x));
 #else
-   return static_cast<uint16_t>((x << 8) | (x >> 8));
+      return static_cast<T>((x << 8) | (x >> 8));
 #endif
-}
-
-/**
-* Swap a 32 bit integer
-*
-* We cannot use MSVC's _byteswap_ulong because it does not consider
-* the builtin to be constexpr.
-*/
-inline constexpr uint32_t reverse_bytes(uint32_t x) {
+   } else if constexpr(sizeof(T) == 4) {
 #if BOTAN_COMPILER_HAS_BUILTIN(__builtin_bswap32)
-   return __builtin_bswap32(x);
+      return static_cast<T>(__builtin_bswap32(x));
 #else
-   // MSVC at least recognizes this as a bswap
-   return ((x & 0x000000FF) << 24) | ((x & 0x0000FF00) << 8) | ((x & 0x00FF0000) >> 8) | ((x & 0xFF000000) >> 24);
+      // MSVC at least recognizes this as a bswap
+      return static_cast<T>(((x & 0x000000FF) << 24) | ((x & 0x0000FF00) << 8) | ((x & 0x00FF0000) >> 8) |
+                            ((x & 0xFF000000) >> 24));
 #endif
-}
-
-/**
-* Swap a 64 bit integer
-*
-* We cannot use MSVC's _byteswap_uint64 because it does not consider
-* the builtin to be constexpr.
-*/
-inline constexpr uint64_t reverse_bytes(uint64_t x) {
+   } else if constexpr(sizeof(T) == 8) {
 #if BOTAN_COMPILER_HAS_BUILTIN(__builtin_bswap64)
-   return __builtin_bswap64(x);
+      return static_cast<T>(__builtin_bswap64(x));
 #else
-   uint32_t hi = static_cast<uint32_t>(x >> 32);
-   uint32_t lo = static_cast<uint32_t>(x);
+      uint32_t hi = static_cast<uint32_t>(x >> 32);
+      uint32_t lo = static_cast<uint32_t>(x);
 
-   hi = reverse_bytes(hi);
-   lo = reverse_bytes(lo);
+      hi = reverse_bytes(hi);
+      lo = reverse_bytes(lo);
 
-   return (static_cast<uint64_t>(lo) << 32) | hi;
+      return (static_cast<T>(lo) << 32) | hi;
 #endif
+   }
 }
 
 }  // namespace Botan

src/tests/test_utils.cpp

@@ -44,6 +44,7 @@ class Utility_Function_Tests final : public Test {
          results.push_back(test_checked_cast());
          results.push_back(test_round_up());
          results.push_back(test_loadstore());
+         results.push_back(test_loadstore_ambiguity());
          results.push_back(test_loadstore_fallback());
          results.push_back(test_loadstore_constexpr());
          return Botan::concat(results, test_copy_out_be_le());
@@ -496,6 +497,36 @@ class Utility_Function_Tests final : public Test {
       template <size_t N>
       using a = std::array<uint8_t, N>;
 
+      static Test::Result test_loadstore_ambiguity() {
+         // This is a regression test for a (probable) compiler bug in Xcode 15
+         // where it would fail to compile the load/store functions for size_t
+         //
+         // It seems that this platform defines uint64_t as "unsigned long long"
+         // and size_t as "unsigned long". Both are 64-bits but the compiler
+         // was unable to disambiguate the two in reverse_bytes in bswap.h
+
+         const uint32_t in32 = 0x01234567;
+         const uint64_t in64 = 0x0123456789ABCDEF;
+         const size_t inszt = 0x87654321;
+
+         Test::Result result("Util load/store ambiguity");
+         const auto out_be_32 = Botan::store_be(in32);
+         const auto out_le_32 = Botan::store_le(in32);
+         const auto out_be_64 = Botan::store_be(in64);
+         const auto out_le_64 = Botan::store_le(in64);
+         const auto out_be_szt = Botan::store_be(inszt);
+         const auto out_le_szt = Botan::store_le(inszt);
+
+         result.test_is_eq<uint32_t>("be 32", Botan::load_be<uint32_t>(out_be_32), in32);
+         result.test_is_eq<uint32_t>("le 32", Botan::load_le<uint32_t>(out_le_32), in32);
+         result.test_is_eq<uint64_t>("be 64", Botan::load_be<uint64_t>(out_be_64), in64);
+         result.test_is_eq<uint64_t>("le 64", Botan::load_le<uint64_t>(out_le_64), in64);
+         result.test_is_eq<size_t>("be szt", Botan::load_be<size_t>(out_be_szt), inszt);
+         result.test_is_eq<size_t>("le szt", Botan::load_le<size_t>(out_le_szt), inszt);
+
+         return result;
+      }
+
       static Test::Result test_loadstore_fallback() {
          // The fallback implementation is only used if we don't know the
          // endianness of the target at compile time. This makes sure that the