Fixed invalid cast and unaligned memory access

Although acceptible to Intel CPUs, it's still undefined behaviour according to the C++ standard.

It can be replaced with memcpy, which makes the code simpler, and it generates the same assembly code with gcc and clang with -O2 (verified with godbolt).

Also expanded the test to include other little endian CPUs by testing for __LITTLE_ENDIAN__.
pull/23734/head
Sean McBride 2 years ago
parent 5330112f05
commit 57da72d444
  1. 33
      modules/core/src/persistence.cpp

@ -295,16 +295,20 @@ int decodeSimpleFormat( const char* dt )
} }
#if defined __i386__ || defined(_M_IX86) || defined __x86_64__ || defined(_M_X64) #if defined __i386__ || defined(_M_IX86) || defined __x86_64__ || defined(_M_X64) || \
#define CV_UNALIGNED_LITTLE_ENDIAN_MEM_ACCESS 1 (defined (__LITTLE_ENDIAN__) && __LITTLE_ENDIAN__)
#define CV_LITTLE_ENDIAN_MEM_ACCESS 1
#else #else
#define CV_UNALIGNED_LITTLE_ENDIAN_MEM_ACCESS 0 #define CV_LITTLE_ENDIAN_MEM_ACCESS 0
#endif #endif
static inline int readInt(const uchar* p) static inline int readInt(const uchar* p)
{ {
#if CV_UNALIGNED_LITTLE_ENDIAN_MEM_ACCESS // On little endian CPUs, both branches produce the same result. On big endian, only the else branch does.
return *(const int*)p; #if CV_LITTLE_ENDIAN_MEM_ACCESS
int val;
memcpy(&val, p, sizeof(val));
return val;
#else #else
int val = (int)(p[0] | (p[1] << 8) | (p[2] << 16) | (p[3] << 24)); int val = (int)(p[0] | (p[1] << 8) | (p[2] << 16) | (p[3] << 24));
return val; return val;
@ -313,8 +317,11 @@ static inline int readInt(const uchar* p)
static inline double readReal(const uchar* p) static inline double readReal(const uchar* p)
{ {
#if CV_UNALIGNED_LITTLE_ENDIAN_MEM_ACCESS // On little endian CPUs, both branches produce the same result. On big endian, only the else branch does.
return *(const double*)p; #if CV_LITTLE_ENDIAN_MEM_ACCESS
double val;
memcpy(&val, p, sizeof(val));
return val;
#else #else
unsigned val0 = (unsigned)(p[0] | (p[1] << 8) | (p[2] << 16) | (p[3] << 24)); unsigned val0 = (unsigned)(p[0] | (p[1] << 8) | (p[2] << 16) | (p[3] << 24));
unsigned val1 = (unsigned)(p[4] | (p[5] << 8) | (p[6] << 16) | (p[7] << 24)); unsigned val1 = (unsigned)(p[4] | (p[5] << 8) | (p[6] << 16) | (p[7] << 24));
@ -326,9 +333,9 @@ static inline double readReal(const uchar* p)
static inline void writeInt(uchar* p, int ival) static inline void writeInt(uchar* p, int ival)
{ {
#if CV_UNALIGNED_LITTLE_ENDIAN_MEM_ACCESS // On little endian CPUs, both branches produce the same result. On big endian, only the else branch does.
int* ip = (int*)p; #if CV_LITTLE_ENDIAN_MEM_ACCESS
*ip = ival; memcpy(p, &ival, sizeof(ival));
#else #else
p[0] = (uchar)ival; p[0] = (uchar)ival;
p[1] = (uchar)(ival >> 8); p[1] = (uchar)(ival >> 8);
@ -339,9 +346,9 @@ static inline void writeInt(uchar* p, int ival)
static inline void writeReal(uchar* p, double fval) static inline void writeReal(uchar* p, double fval)
{ {
#if CV_UNALIGNED_LITTLE_ENDIAN_MEM_ACCESS // On little endian CPUs, both branches produce the same result. On big endian, only the else branch does.
double* fp = (double*)p; #if CV_LITTLE_ENDIAN_MEM_ACCESS
*fp = fval; memcpy(p, &fval, sizeof(fval));
#else #else
Cv64suf v; Cv64suf v;
v.f = fval; v.f = fval;

Loading…
Cancel
Save