Thriftpy / thriftpy2

Pure python approach of Apache Thrift.
MIT License
562 stars 89 forks source link

Build fails with strict-aliasing violations #246

Closed eli-schwartz closed 4 months ago

eli-schwartz commented 4 months ago

Here is some generated code in cybin.pyx -> cybin.c:

  /* "cybin.pyx":102
 * 
 * cdef inline int write_double(CyTransportBase buf, double val) except -1:
 *     cdef int64_t v = htobe64((<int64_t*>(&val))[0])             # <<<<<<<<<<<<<<
 *     buf.c_write(<char*>(&v), 8)
 *     return 0
 */
  __pyx_v_v = htobe64((((int64_t *)(&__pyx_v_val))[0]));

and

    /* "cybin.pyx":290
 *     elif ttype == T_DOUBLE:
 *         n = read_i64(buf)
 *         return (<double*>(&n))[0]             # <<<<<<<<<<<<<<
 * 
 *     elif ttype == T_BINARY:
 */
    __Pyx_XDECREF(__pyx_r);
    __pyx_t_2 = PyFloat_FromDouble((((double *)(&__pyx_v_n))[0])); if (unlikely(!__pyx_t_2)) __PYX_ERR(0, 290, __pyx_L1_error)
    __Pyx_GOTREF(__pyx_t_2);
    __pyx_r = __pyx_t_2;
    __pyx_t_2 = 0;
    goto __pyx_L0;

Attempting to compile thriftpy2 produces the following compiler errors:

2024-03-03 15:49:54,283 root INFO x86_64-pc-linux-gnu-gcc -Wsign-compare -DNDEBUG -march=native -fstack-protector-all -O2 -pipe -fdiagnostics-color=always -frecord-gcc-switches -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-clash-protection -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing -Wformat -Werror=format-security -Werror=implicit-function-declaration -Werror=implicit-int -Werror=int-conversion -Werror=incompatible-pointer-types -Wno-error=incompatible-pointer-types -DNDEBUG -fPIC -I/usr/include/python3.11 -c thriftpy2/protocol/cybin/cybin.c -o /var/tmp/portage/dev-python/thriftpy2-0.4.20/work/thriftpy2-0.4.20-python3_11/build/temp.linux-x86_64-cpython-311/thriftpy2/protocol/cybin/cybin.o
In file included from /usr/include/sys/types.h:176,
                 from /usr/include/stdlib.h:514,
                 from /usr/include/python3.11/Python.h:23,
                 from thriftpy2/protocol/cybin/cybin.c:34:
thriftpy2/protocol/cybin/cybin.c: In function ‘__pyx_f_5cybin_write_double’:
thriftpy2/protocol/cybin/cybin.c:4623:51: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
 4623 |   __pyx_v_v = htobe64((((int64_t *)(&__pyx_v_val))[0]));
      |                                                   ^
thriftpy2/protocol/cybin/cybin.c: In function ‘__pyx_f_5cybin_c_read_val’:
thriftpy2/protocol/cybin/cybin.c:6850:61: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
 6850 |     __pyx_t_2 = PyFloat_FromDouble((((double *)(&__pyx_v_n))[0])); if (unlikely(!__pyx_t_2)) __PYX_ERR(0, 290, __pyx_L1_error)
      |                                                             ^

I suspect there are easier ways to bitfiddle a double.

aisk commented 4 months ago

Thanks for reporting. First, I want to recreate this error to find the right way to fix it, so can you provide your GCC, cython (not python), and thriftpy2 versions?

eli-schwartz commented 4 months ago
$ gcc --version
gcc (Gentoo 13.2.1_p20240113-r1 p12) 13.2.1 20240113

$ cython --version
Cython version 3.0.8

I attempted to compile thriftpy2 0.4.20 (the latest tag, no significant changes since then).

Here's a full build log in case it helps: build.log

eli-schwartz commented 4 months ago

For a bit of additional context, this was discovered while trying to build thriftpy2 with LTO. In gentoo, we are using the following flags to uncover very likely runtime bugs with LTO (but strict-aliasing violations can cause issues even without LTO):

CFLAGS="-flto -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing"
LDFLAGS="-flto -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing"