Skip to content

test: avoid strict-aliasing/alignment UB in unit tests #110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
l0rinc wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

test: avoid strict-aliasing/alignment UB in unit tests #110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
74e1001
test: avoid strict-aliasing/alignment UB in unit tests
l0rinc Jan 28, 2026
f52e140
script,memusage,wallet: pass trivial scalars by value
l0rinc Jan 28, 2026
2b7b075
crypto: use unaligned SIMD pointer type for SHA-NI load/store
l0rinc Jan 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions src/crypto/sha256_x86_shani.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@

namespace {

#if (defined(__GNUC__) || defined(__clang__)) && !defined(_MSC_VER)
using m128i_u = __m128i_u;
#else
using m128i_u = __m128i;
#endif

alignas(__m128i) const uint8_t MASK[16] = {0x03, 0x02, 0x01, 0x00, 0x07, 0x06, 0x05, 0x04, 0x0b, 0x0a, 0x09, 0x08, 0x0f, 0x0e, 0x0d, 0x0c};
alignas(__m128i) const uint8_t INIT0[16] = {0x8c, 0x68, 0x05, 0x9b, 0x7f, 0x52, 0x0e, 0x51, 0x85, 0xae, 0x67, 0xbb, 0x67, 0xe6, 0x09, 0x6a};
alignas(__m128i) const uint8_t INIT1[16] = {0x19, 0xcd, 0xe0, 0x5b, 0xab, 0xd9, 0x83, 0x1f, 0x3a, 0xf5, 0x4f, 0xa5, 0x72, 0xf3, 0x6e, 0x3c};
Expand Down Expand Up @@ -68,12 +74,14 @@ void ALWAYS_INLINE Unshuffle(__m128i& s0, __m128i& s1)

__m128i ALWAYS_INLINE Load(const unsigned char* in)
{
return _mm_shuffle_epi8(_mm_loadu_si128((const __m128i*)in), _mm_load_si128((const __m128i*)MASK));
return _mm_shuffle_epi8(_mm_loadu_si128(reinterpret_cast<const m128i_u*>(in)),
_mm_load_si128(reinterpret_cast<const __m128i*>(MASK)));
}

void ALWAYS_INLINE Save(unsigned char* out, __m128i s)
{
_mm_storeu_si128((__m128i*)out, _mm_shuffle_epi8(s, _mm_load_si128((const __m128i*)MASK)));
_mm_storeu_si128(reinterpret_cast<m128i_u*>(out),
_mm_shuffle_epi8(s, _mm_load_si128(reinterpret_cast<const __m128i*>(MASK))));
}
}

Expand All @@ -83,8 +91,8 @@ void Transform(uint32_t* s, const unsigned char* chunk, size_t blocks)
__m128i m0, m1, m2, m3, s0, s1, so0, so1;

/* Load state */
s0 = _mm_loadu_si128((const __m128i*)s);
s1 = _mm_loadu_si128((const __m128i*)(s + 4));
s0 = _mm_loadu_si128(reinterpret_cast<const m128i_u*>(s));
s1 = _mm_loadu_si128(reinterpret_cast<const m128i_u*>(s + 4));
Shuffle(s0, s1);

while (blocks--) {
Expand Down Expand Up @@ -137,8 +145,8 @@ void Transform(uint32_t* s, const unsigned char* chunk, size_t blocks)
}

Unshuffle(s0, s1);
_mm_storeu_si128((__m128i*)s, s0);
_mm_storeu_si128((__m128i*)(s + 4), s1);
_mm_storeu_si128(reinterpret_cast<m128i_u*>(s), s0);
_mm_storeu_si128(reinterpret_cast<m128i_u*>(s + 4), s1);
}
}

Expand Down
24 changes: 12 additions & 12 deletions src/memusage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,18 @@ namespace memusage
static size_t MallocUsage(size_t alloc);

/** Dynamic memory usage for built-in types is zero. */
static inline size_t DynamicUsage(const int8_t& v) { return 0; }
static inline size_t DynamicUsage(const uint8_t& v) { return 0; }
static inline size_t DynamicUsage(const int16_t& v) { return 0; }
static inline size_t DynamicUsage(const uint16_t& v) { return 0; }
static inline size_t DynamicUsage(const int32_t& v) { return 0; }
static inline size_t DynamicUsage(const uint32_t& v) { return 0; }
static inline size_t DynamicUsage(const int64_t& v) { return 0; }
static inline size_t DynamicUsage(const uint64_t& v) { return 0; }
static inline size_t DynamicUsage(const float& v) { return 0; }
static inline size_t DynamicUsage(const double& v) { return 0; }
template<typename X> static inline size_t DynamicUsage(X * const &v) { return 0; }
template<typename X> static inline size_t DynamicUsage(const X * const &v) { return 0; }
static inline size_t DynamicUsage(int8_t) { return 0; }
static inline size_t DynamicUsage(uint8_t) { return 0; }
static inline size_t DynamicUsage(int16_t) { return 0; }
static inline size_t DynamicUsage(uint16_t) { return 0; }
static inline size_t DynamicUsage(int32_t) { return 0; }
static inline size_t DynamicUsage(uint32_t) { return 0; }
static inline size_t DynamicUsage(int64_t) { return 0; }
static inline size_t DynamicUsage(uint64_t) { return 0; }
static inline size_t DynamicUsage(float) { return 0; }
static inline size_t DynamicUsage(double) { return 0; }
template<typename X> static inline size_t DynamicUsage(X* const) { return 0; }
template<typename X> static inline size_t DynamicUsage(const X* const) { return 0; }

/** Compute the memory used for dynamically allocated but owned data structures.
* For generic data types, this is *not* recursive. DynamicUsage(vector<vector<int> >)
Expand Down
22 changes: 11 additions & 11 deletions src/script/script.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ class CScriptNum
*/
public:

explicit CScriptNum(const int64_t& n)
explicit CScriptNum(int64_t n)
{
m_value = n;
}
Expand Down Expand Up @@ -269,21 +269,21 @@ class CScriptNum
m_value = set_vch(vch);
}

inline bool operator==(const int64_t& rhs) const { return m_value == rhs; }
inline auto operator<=>(const int64_t& rhs) const { return m_value <=> rhs; }
inline bool operator==(int64_t rhs) const { return m_value == rhs; }
inline auto operator<=>(int64_t rhs) const { return m_value <=> rhs; }

inline bool operator==(const CScriptNum& rhs) const { return operator==(rhs.m_value); }
inline auto operator<=>(const CScriptNum& rhs) const { return operator<=>(rhs.m_value); }

inline CScriptNum operator+( const int64_t& rhs) const { return CScriptNum(m_value + rhs);}
inline CScriptNum operator-( const int64_t& rhs) const { return CScriptNum(m_value - rhs);}
inline CScriptNum operator+( int64_t rhs) const { return CScriptNum(m_value + rhs);}
inline CScriptNum operator-( int64_t rhs) const { return CScriptNum(m_value - rhs);}
inline CScriptNum operator+( const CScriptNum& rhs) const { return operator+(rhs.m_value); }
inline CScriptNum operator-( const CScriptNum& rhs) const { return operator-(rhs.m_value); }

inline CScriptNum& operator+=( const CScriptNum& rhs) { return operator+=(rhs.m_value); }
inline CScriptNum& operator-=( const CScriptNum& rhs) { return operator-=(rhs.m_value); }

inline CScriptNum operator&( const int64_t& rhs) const { return CScriptNum(m_value & rhs);}
inline CScriptNum operator&( int64_t rhs) const { return CScriptNum(m_value & rhs);}
inline CScriptNum operator&( const CScriptNum& rhs) const { return operator&(rhs.m_value); }

inline CScriptNum& operator&=( const CScriptNum& rhs) { return operator&=(rhs.m_value); }
Expand All @@ -294,29 +294,29 @@ class CScriptNum
return CScriptNum(-m_value);
}

inline CScriptNum& operator=( const int64_t& rhs)
inline CScriptNum& operator=( int64_t rhs)
{
m_value = rhs;
return *this;
}

inline CScriptNum& operator+=( const int64_t& rhs)
inline CScriptNum& operator+=( int64_t rhs)
{
assert(rhs == 0 || (rhs > 0 && m_value <= std::numeric_limits<int64_t>::max() - rhs) ||
(rhs < 0 && m_value >= std::numeric_limits<int64_t>::min() - rhs));
m_value += rhs;
return *this;
}

inline CScriptNum& operator-=( const int64_t& rhs)
inline CScriptNum& operator-=( int64_t rhs)
{
assert(rhs == 0 || (rhs > 0 && m_value >= std::numeric_limits<int64_t>::min() + rhs) ||
(rhs < 0 && m_value <= std::numeric_limits<int64_t>::max() + rhs));
m_value -= rhs;
return *this;
}

inline CScriptNum& operator&=( const int64_t& rhs)
inline CScriptNum& operator&=( int64_t rhs)
{
m_value &= rhs;
return *this;
Expand All @@ -338,7 +338,7 @@ class CScriptNum
return serialize(m_value);
}

static std::vector<unsigned char> serialize(const int64_t& value)
static std::vector<unsigned char> serialize(int64_t value)
{
if(value == 0)
return std::vector<unsigned char>();
Expand Down
9 changes: 7 additions & 2 deletions src/test/allocator_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#include <common/system.h>
#include <support/lockedpool.h>

#include <cstdint>
#include <cstring>
#include <limits>
#include <memory>
#include <stdexcept>
Expand Down Expand Up @@ -220,8 +222,11 @@ BOOST_AUTO_TEST_CASE(lockedpool_tests_live)
void *a0 = pool.alloc(16);
BOOST_CHECK(a0);
// Test reading and writing the allocated memory
*((uint32_t*)a0) = 0x1234;
BOOST_CHECK(*((uint32_t*)a0) == 0x1234);
uint32_t value = 0x1234;
std::memcpy(a0, &value, sizeof(value));
uint32_t read_value;
std::memcpy(&read_value, a0, sizeof(read_value));
BOOST_CHECK_EQUAL(read_value, value);

pool.free(a0);
try { // Test exception on double-free
Expand Down
29 changes: 17 additions & 12 deletions src/test/cuckoocache_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <cuckoocache.h>
#include <crypto/common.h>
#include <random.h>
#include <script/sigcache.h>
#include <test/util/random.h>
Expand Down Expand Up @@ -64,9 +65,10 @@ double test_cache(size_t megabytes, double load)
uint32_t n_insert = static_cast<uint32_t>(load * (bytes / sizeof(uint256)));
hashes.resize(n_insert);
for (uint32_t i = 0; i < n_insert; ++i) {
uint32_t* ptr = (uint32_t*)hashes[i].begin();
for (uint8_t j = 0; j < 8; ++j)
*(ptr++) = m_rng.rand32();
unsigned char* const data = hashes[i].begin();
for (uint8_t j = 0; j < 8; ++j) {
WriteLE32(data + j * sizeof(uint32_t), m_rng.rand32());
}
}
/** We make a copy of the hashes because future optimizations of the
* cuckoocache may overwrite the inserted element, so the test is
Expand Down Expand Up @@ -137,9 +139,10 @@ void test_cache_erase(size_t megabytes)
uint32_t n_insert = static_cast<uint32_t>(load * (bytes / sizeof(uint256)));
hashes.resize(n_insert);
for (uint32_t i = 0; i < n_insert; ++i) {
uint32_t* ptr = (uint32_t*)hashes[i].begin();
for (uint8_t j = 0; j < 8; ++j)
*(ptr++) = m_rng.rand32();
unsigned char* const data = hashes[i].begin();
for (uint8_t j = 0; j < 8; ++j) {
WriteLE32(data + j * sizeof(uint32_t), m_rng.rand32());
}
}
/** We make a copy of the hashes because future optimizations of the
* cuckoocache may overwrite the inserted element, so the test is
Expand Down Expand Up @@ -202,9 +205,10 @@ void test_cache_erase_parallel(size_t megabytes)
uint32_t n_insert = static_cast<uint32_t>(load * (bytes / sizeof(uint256)));
hashes.resize(n_insert);
for (uint32_t i = 0; i < n_insert; ++i) {
uint32_t* ptr = (uint32_t*)hashes[i].begin();
for (uint8_t j = 0; j < 8; ++j)
*(ptr++) = m_rng.rand32();
unsigned char* const data = hashes[i].begin();
for (uint8_t j = 0; j < 8; ++j) {
WriteLE32(data + j * sizeof(uint32_t), m_rng.rand32());
}
}
/** We make a copy of the hashes because future optimizations of the
* cuckoocache may overwrite the inserted element, so the test is
Expand Down Expand Up @@ -316,9 +320,10 @@ void test_cache_generations()
inserts.resize(n_insert);
reads.reserve(n_insert / 2);
for (uint32_t i = 0; i < n_insert; ++i) {
uint32_t* ptr = (uint32_t*)inserts[i].begin();
for (uint8_t j = 0; j < 8; ++j)
*(ptr++) = rng.rand32();
unsigned char* const data = inserts[i].begin();
for (uint8_t j = 0; j < 8; ++j) {
WriteLE32(data + j * sizeof(uint32_t), rng.rand32());
}
}
for (uint32_t i = 0; i < n_insert / 4; ++i)
reads.push_back(inserts[i]);
Expand Down
4 changes: 2 additions & 2 deletions src/wallet/coinselection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
}

// Set output set
for (const size_t& i : best_selection) {
for (size_t i : best_selection) {
result.AddInput(utxo_pool.at(i));
}
result.RecalculateWaste(cost_of_change, cost_of_change, CAmount{0});
Expand Down Expand Up @@ -517,7 +517,7 @@ util::Result<SelectionResult> CoinGrinder(std::vector<OutputGroup>& utxo_pool, c
return max_tx_weight_exceeded ? ErrorMaxWeightExceeded() : util::Error();
}

for (const size_t& i : best_selection) {
for (size_t i : best_selection) {
result.AddInput(utxo_pool[i]);
}

Expand Down
Loading