Boost C++ Libraries: Ticket #12773: WINDOWS- Boost thread 1.63.0 strict aliasing warnings

attachment set

Thu, 19 Jan 2017 13:35:38 GMT

attachment → win32_shared_mutex.patch

Path fixing the warning/potential misbehavior

attachment set

Thu, 19 Jan 2017 14:32:59 GMT

attachment → win32_shared_mutex_punning.patch

Newer patch, uses type punning union.

Thu, 19 Jan 2017 14:34:50 GMT

The win32_shared_mutex.patch didn't work with gcc 6.3.0. I came up with a type-punning union instead, but I'm unsure the anonymous union is portable. One can come up with a better idea for that case. Works for me™

summary changed

viboes — Sun, 29 Jan 2017 16:32:27 GMT

summary Boost thread 1.63.0 strict aliasing warnings → WINDOWS- Boost thread 1.63.0 strict aliasing warnings

viboes — Sun, 29 Jan 2017 16:36:14 GMT

Could you report explicitly the strict aliasing complaints?

viboes — Sun, 19 Feb 2017 11:37:50 GMT

Ok, I see from the patch where the strict aliasing problem could come from. I don't think the union should solve the issue as we introduce then UB.

The safe way is to check each one of the bit fields.

Please, add here the report you got.

alexandre.nunes@… — Sun, 19 Feb 2017 13:29:04 GMT

You're absolutely right that type-punning leads to UB, and so does current implementation. It seems to me that other than checking member-by-member, the other option would be to - providing the structure is sufficiently sane - use std::memcmp() to check the data. Most compilers can optimize the call away, when that's safe.

Here's the compiler report:

    gcc.compile.c++ bin.v2/libs/type_erasure/build/gcc-mingw-6.3/release/link-static/runtime-link-static/target-os-windows/threadapi-win32/threading-multi/dynamic_binding.o
    In file included from ./boost/thread/shared_mutex.hpp:18:0,
                     from libs/type_erasure/src/dynamic_binding.cpp:14:
    ./boost/thread/win32/shared_mutex.hpp: In instantiation of 'T boost::shared_mutex::interlocked_compare_exchange(T*, T, T) [with T = boost::shared_mutex::state_data]':
    ./boost/thread/win32/shared_mutex.hpp:130:103:   required from here
    ./boost/thread/win32/shared_mutex.hpp:51:63: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
                                                               *reinterpret_cast<long*>(&new_value),
                                                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    ./boost/thread/win32/shared_mutex.hpp:52:63: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
                                                               *reinterpret_cast<long*>(&comparand));
                                                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    ./boost/thread/win32/shared_mutex.hpp:53:20: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
             return *reinterpret_cast<T const*>(&res);
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    ./boost/thread/win32/shared_mutex.hpp:53:52: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
             return *reinterpret_cast<T const*>(&res);
                                                    ^
    cc1plus: some warnings being treated as errors

anonymous — Sun, 19 Feb 2017 13:34:41 GMT

IIRC there was also other similar reports in this code (line 39):

friend bool operator==(state_data const& lhs,state_data const& rhs)
            {
                return *reinterpret_cast<unsigned const*>(&lhs)==*reinterpret_cast<unsigned const*>(&rhs);
            }

But these happened further down the road, after "fixing" the other errors.

viboes — Fri, 24 Feb 2017 14:32:25 GMT

Thanks for the reported errors.

owner, status changed

viboes — Tue, 14 Nov 2017 20:54:51 GMT

owner changed from Anthony Williams to viboes
status new → assigned