Skip to content

Commit

Permalink
FIX: store_be(size_t(42)) would not compile on Xcode
Browse files Browse the repository at this point in the history
The affected platform defines uint64_t as "unsigned long long" and size_t
as "unsigned long int". Both are 64bit types, but the compiler failed to
call reverse_bytes() for size_t regardless. It claimed the call is
ambiguous.
  • Loading branch information
reneme committed Jun 14, 2024
1 parent 5649a10 commit 2f83991
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 32 deletions.
56 changes: 24 additions & 32 deletions src/lib/utils/bswap.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand All @@ -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
Expand Down
31 changes: 31 additions & 0 deletions src/tests/test_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 2f83991

Please sign in to comment.