diff --git a/src/include/OpenImageIO/simd.h b/src/include/OpenImageIO/simd.h index 1d00caef2b..c6853a0e1f 100644 --- a/src/include/OpenImageIO/simd.h +++ b/src/include/OpenImageIO/simd.h @@ -4735,7 +4735,8 @@ OIIO_FORCEINLINE void vint4::store (unsigned short *values) const { // high = xxxx00CD vint4 highswapped = shuffle_sse<2,3,0,1>(high); // 00CDxxxx vint4 result = low | highswapped; // ABCDxxxx - _mm_storel_pd ((double *)values, _mm_castsi128_pd(result)); + // Use storel_epi64, which allows unaligned addresses. + _mm_storel_epi64 ((__m128i *)values, result); // At this point, values[] should hold A,B,C,D #else SIMD_DO (values[i] = m_val[i]); @@ -4751,12 +4752,15 @@ OIIO_FORCEINLINE void vint4::store (unsigned char *values) const { vint4 clamped = m_simd & vint4(0xff); // A000 B000 C000 D000 simd_t val16 = _mm_packs_epi32(clamped, _mm_setzero_si128()); // A0B0 C0D0 xxxx xxxx simd_t val8 = _mm_packus_epi16(val16, _mm_setzero_si128()); // ABCD xxxx xxxx xxxx - _mm_store_ss((float*)values, _mm_castsi128_ps(val8)); + // values may be unaligned; memcpy compiles to a single movd/movss. + int32_t packed = _mm_cvtsi128_si32(val8); + memcpy(values, &packed, sizeof(packed)); #elif OIIO_SIMD_NEON vint4 clamped = m_simd & vint4(0xff); int16x8_t val16 = vcombine_s16(vqmovn_s32(clamped), vdup_n_s16(0)); uint8x16_t val8 = vcombine_u8(vqmovun_s16(val16), vdup_n_u8(0)); - vst1q_lane_u32((uint32_t*)values, vreinterpretq_u32_u8(val8), 0); + uint32_t packed = vgetq_lane_u32(vreinterpretq_u32_u8(val8), 0); + memcpy(values, &packed, sizeof(packed)); #else SIMD_DO (values[i] = m_val[i]); #endif @@ -6897,8 +6901,8 @@ OIIO_FORCEINLINE void vfloat4::store (float *values, int n) const { _mm_store_ss (values, m_simd); break; case 2: - // Trickery: store two floats as a double worth of bits - _mm_store_sd ((double*)values, _mm_castps_pd(m_simd)); + // Trickery: store two floats as 64 bits (unaligned-safe) + _mm_storel_epi64 ((__m128i*)values, _mm_castps_si128(m_simd)); break; case 3: values[0] = m_val[0]; @@ -6945,7 +6949,7 @@ OIIO_FORCEINLINE void vfloat4::store (float *values, int n) const { OIIO_FORCEINLINE void vfloat4::store (half *values) const { #if OIIO_F16C_ENABLED && OIIO_SIMD_SSE __m128i h = _mm_cvtps_ph (m_simd, _MM_FROUND_TO_NEAREST_INT); - _mm_store_sd ((double *)values, _mm_castsi128_pd(h)); + _mm_storel_epi64 ((__m128i *)values, h); #elif OIIO_SIMD_NEON float16x4_t f16 = vcvt_f16_f32(m_simd); uint16x4_t u16 = vreinterpret_u16_f16(f16); diff --git a/src/libutil/simd_test.cpp b/src/libutil/simd_test.cpp index fba977bf02..477fe728fa 100644 --- a/src/libutil/simd_test.cpp +++ b/src/libutil/simd_test.cpp @@ -562,6 +562,44 @@ void test_vint_to_uint8s () +// Partial/converting stores must work to addresses aligned only for the +// element type, not the wider type the SIMD store might use internally. +// Run under UBSan to catch misaligned typed stores (issue #5560). +void test_unaligned_stores () +{ + test_heading ("unaligned partial stores"); + alignas(16) unsigned char buf[32]; + + // uint8: offset 1, misaligned for any wider type + std::fill (std::begin(buf), std::end(buf), 0); + vint4(1, 2, 3, 4).store (buf + 1); + for (int i = 0; i < 4; ++i) + OIIO_CHECK_EQUAL (int(buf[1 + i]), i + 1); + + // uint16: offset 2, aligned for ushort but not for 8-byte stores + unsigned short us[4]; + vint4(1, 2, 3, 4).store ((unsigned short*)(buf + 2)); + memcpy (us, buf + 2, sizeof(us)); + for (int i = 0; i < 4; ++i) + OIIO_CHECK_EQUAL (int(us[i]), i + 1); + + // half: offset 2 + half h[4]; + vfloat4(1, 2, 3, 4).store ((half*)(buf + 2)); + memcpy (h, buf + 2, sizeof(h)); + for (int i = 0; i < 4; ++i) + OIIO_CHECK_EQUAL (float(h[i]), float(i + 1)); + + // 2 floats: offset 4, aligned for float but not for 8-byte stores + float f[2]; + vfloat4(1, 2, 3, 4).store ((float*)(buf + 4), 2); + memcpy (f, buf + 4, sizeof(f)); + OIIO_CHECK_EQUAL (f[0], 1.0f); + OIIO_CHECK_EQUAL (f[1], 2.0f); +} + + + template void test_masked_loadstore () { @@ -2124,6 +2162,7 @@ main(int argc, char* argv[]) test_metaprogramming(); test_matrix(); test_trivially_copyable(); + test_unaligned_stores(); std::cout << "\nTotal time: " << Strutil::timeintervalformat(timer()) << "\n";